Re: [PATCH] slab: remove colouroff from struct slab

Previous thread: [RFC/PATCH] slab: free pages in a batch in drain_freelist by Pekka J Enberg on Thursday, February 22, 2007 - 5:39 am. (2 messages)

Next thread: Re: [patch 00/13] Syslets, "Threadlets", generic AIO support, v3 by linux on Thursday, February 22, 2007 - 5:27 am. (3 messages)
From: Pekka J Enberg
Date: Thursday, February 22, 2007 - 5:37 am

From: Pekka Enberg <penberg@cs.helsinki.fi>

As the color offset is always within the first page of the slab,
virt_to_page() works just fine without slabp->colouroff.

Cc: William Lee Irwin III <wli@holomorphy.com>
Cc: Christoph Lameter <clameter@sgi.com>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---
 mm/slab.c |    8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Index: 2.6/mm/slab.c
===================================================================
--- 2.6.orig/mm/slab.c	2007-02-22 13:51:41.000000000 +0200
+++ 2.6/mm/slab.c	2007-02-22 13:57:39.000000000 +0200
@@ -219,7 +219,6 @@
  */
 struct slab {
 	struct list_head list;
-	unsigned long colouroff;
 	void *s_mem;		/* including colour offset */
 	unsigned int inuse;	/* num of objs active in slab */
 	kmem_bufctl_t free;
@@ -1908,18 +1907,16 @@
  */
 static void slab_destroy(struct kmem_cache *cachep, struct slab *slabp)
 {
-	void *addr = slabp->s_mem - slabp->colouroff;
-
 	slab_destroy_objs(cachep, slabp);
 	if (unlikely(cachep->flags & SLAB_DESTROY_BY_RCU)) {
 		struct slab_rcu *slab_rcu;
 
 		slab_rcu = (struct slab_rcu *)slabp;
 		slab_rcu->cachep = cachep;
-		slab_rcu->addr = addr;
+		slab_rcu->addr = slabp->s_mem;
 		call_rcu(&slab_rcu->head, kmem_rcu_free);
 	} else {
-		kmem_freepages(cachep, addr);
+		kmem_freepages(cachep, slabp->s_mem);
 		if (OFF_SLAB(cachep))
 			kmem_cache_free(cachep->slabp_cache, slabp);
 	}
@@ -2585,7 +2582,6 @@
 		colour_off += cachep->slab_size;
 	}
 	slabp->inuse = 0;
-	slabp->colouroff = colour_off;
 	slabp->s_mem = objp + colour_off;
 	slabp->nodeid = nodeid;
 	return slabp;
-

From: Christoph Lameter
Date: Friday, February 23, 2007 - 7:00 am

True but then we pass an address to kmem_freepages that is not the start 
of the page. kmem_freepages will then in turn call free_pages() with an 
address that is not the start of a page. free_pages() will then do another 
virt_to_page() which ignored the offset into the page again. And so the 
approach works but an uneasy feeling remains since the address we got from 
kmem_getpages() is different from what we pass to kmem_freepages().

Could you think about a way to do this in a cleaner way? Maybe use a 
struct page * in both kmem_get/freepages? Or add some comment explaining 
the situation?
-

From: Andrew Morton
Date: Friday, March 2, 2007 - 12:14 am

kernel BUG at mm/slab.c:1658!
invalid opcode: 0000 [#1]
SMP 
last sysfs file: /block/hdc/range
Modules linked in:
CPU:    1
EIP:    0060:[<c0173788>]    Not tainted VLI
EFLAGS: 00010246   (2.6.21-rc2-mm1 #7)
EIP is at kmem_freepages+0xc8/0xd0
eax: 40000000   ebx: c106e730   ecx: 00000000   edx: 00000000
esi: 00000001   edi: c21fcbe0   ebp: c2231e9c   esp: c2231e8c
ds: 007b   es: 007b   fs: 00d8  gs: 0000  ss: 0068
Process swapper (pid: 0, ti=c2230000 task=c222cac0 task.ti=c2230000)
Stack: c21fcbe0 c21fcbe0 f6252020 00000002 c2231eac c017409c f62b7020 c1b74f80 
       c2231ec0 c013078a c1b74ffc 00000000 c05502c0 c2231ec8 c0130901 c2231ee0 
       c012495a c05519d0 00000003 c04faf68 c0551a20 c2231efc c01242d7 0000000a 
Call Trace:
 [<c0103e7a>] show_trace_log_lvl+0x1a/0x30
 [<c0103f39>] show_stack_log_lvl+0xa9/0xd0
 [<c0104149>] show_registers+0x1e9/0x2f0
 [<c010436a>] die+0x11a/0x250
 [<c0104531>] do_trap+0x91/0xc0
 [<c0104e67>] do_invalid_op+0x97/0xb0
 [<c03d7e84>] error_code+0x7c/0x84
 [<c017409c>] kmem_rcu_free+0x1c/0x50
 [<c013078a>] __rcu_process_callbacks+0x6a/0x1c0
 [<c0130901>] rcu_process_callbacks+0x21/0x50
 [<c012495a>] tasklet_action+0x5a/0xe0
 [<c01242d7>] __do_softirq+0x87/0x100
 [<c01243a7>] do_softirq+0x57/0x60
 [<c01247e7>] irq_exit+0x47/0x50
 [<c0110975>] smp_apic_timer_interrupt+0x55/0x90
 [<c0103933>] apic_timer_interrupt+0x33/0x38
 [<c010102f>] cpu_idle+0x7f/0xe0
 [<c010fa91>] start_secondary+0x281/0x3c0
 [<00000000>] 0x0
 =======================
Code: fe ff 58 5b 5e 5f 5d c3 8b 03 89 f1 ba 09 00 00 00 f7 d9 c1 e8 1e 8d 04 40 8d 04 c0 c1 e0 05 05 40 f1 4c c0 e8 9a dc fe ff eb 95 <0f> 0b eb fe 8d 74 26 00 55 b9 6c dd 79 c0 89 e5 ba 52 fc 46 c0 
EIP: [<c0173788>] kmem_freepages+0xc8/0xd0 SS:ESP 0068:c2231e8c


#
# Automatically generated make config: don't edit
# Linux kernel version: 2.6.21-rc2-mm1
# Thu Mar  1 23:05:37 ...
Previous thread: [RFC/PATCH] slab: free pages in a batch in drain_freelist by Pekka J Enberg on Thursday, February 22, 2007 - 5:39 am. (2 messages)

Next thread: Re: [patch 00/13] Syslets, "Threadlets", generic AIO support, v3 by linux on Thursday, February 22, 2007 - 5:27 am. (3 messages)