Re: [BUG] CONFIG_DEBUG_SLAB broken on SMP

Re: [BUG] CONFIG_DEBUG_SLAB broken on SMP

[Manfred Spraul]

> The problem appears to be in the SMP version of __kmem_cache_alloc()
> and __kmem_cache_free(), where it simply sticks the obj in the per-CPU
> list without doing the poison or redzone stuff that is done inside
> kmem_cache_free_one_tail().
> 
The simplest solution is to skip the code that enables the per-cpu 
caches if debugging is enabled:

Search for enable_all_cpucaches in mm/slab.c, and skip the call if DEBUG 
is enabled.

2.5.41-mm1 contains a partially rewritten slab, which performs the 
poisoning before adding an object into the cpu caches. Additionally, 
even caches with constructors are not poisoned - ctor and dtor calls are 
performed in kmem_cache_alloc/free.

--
	Manfred

Re: [BUG] CONFIG_DEBUG_SLAB broken on SMP

[William Lee Irwin III]

On Wed, Oct 09, 2002 at 09:50:13PM +0200, Manfred Spraul wrote:
> 2.5.41-mm1 contains a partially rewritten slab, which performs the 
> poisoning before adding an object into the cpu caches. Additionally, 
> even caches with constructors are not poisoned - ctor and dtor calls are 
> performed in kmem_cache_alloc/free.


On an unrelated note, I have a very tiny but useful (for me) feature
request for the slab rewrite:

Can you make ctor return a success/failure code?


Thanks,
Bill

[BUG] CONFIG_DEBUG_SLAB broken on SMP

[Andreas Dilger]

We were tracking down a strange bug in our code that only appeared on
SMP and not UP, and we thought that CONFIG_DEBUG_SLAB (and the ensuing
FORCED_DEBUG which enables SLAB_POISON and SLAB_REDZONE) was going to
catch problems with slab objects, so we were very very confused when a
test like:

	struct foo *obj;

	cache = kmem_cache_create("test_cache", sizeof(struct foo))
	obj = kmem_cache_alloc(cache, GFP_KERNEL);
	kmem_cache_free(cache, obj);
	// print out contents of obj

was not poisoning obj, or setting the redzone fields on obj to "free".

The problem appears to be in the SMP version of __kmem_cache_alloc()
and __kmem_cache_free(), where it simply sticks the obj in the per-CPU
list without doing the poison or redzone stuff that is done inside
kmem_cache_free_one_tail().

Granted, there are per-slab caches for performance reasons, but putting
the object poisoning and redzoning inside the per-CPU operations does
not cause any additional overhead when it is not enabled, and also helps
to find SMP bugs, especially race conditions between referencing an
object in one thread and freeing it in another (which are much more
prevelant on SMP systems as well).

I'm working on a patch now, if people are interested in this.

Cheers, Andreas
--
Andreas Dilger
http://www-mddsp.enel.ucalgary.ca/People/adilger/
http://sourceforge.net/projects/ext2resize/

Re: [BUG] CONFIG_DEBUG_SLAB broken on SMP

[Andrew Morton]

Andreas Dilger wrote:
> 
> We were tracking down a strange bug in our code that only appeared on
> SMP and not UP, and we thought that CONFIG_DEBUG_SLAB (and the ensuing
> FORCED_DEBUG which enables SLAB_POISON and SLAB_REDZONE) was going to
> catch problems with slab objects, so we were very very confused when a
> test like:
> 
>         struct foo *obj;
> 
>         cache = kmem_cache_create("test_cache", sizeof(struct foo))
>         obj = kmem_cache_alloc(cache, GFP_KERNEL);
>         kmem_cache_free(cache, obj);
>         // print out contents of obj
> 
> was not poisoning obj, or setting the redzone fields on obj to "free".
> 

Linus recently changed the 2.5 slab allocator to stamp that out.

--- mm/slab.c	18 Sep 2002 03:48:34 -0000	1.28
+++ mm/slab.c	20 Sep 2002 16:22:53 -0000	1.29
@@ -1727,8 +1728,13 @@
 	return 0;
 }
 
+/* 
+ * If slab debugging is enabled, don't batch slabs
+ * on the per-cpu lists by defaults.
+ */
 static void enable_cpucache (kmem_cache_t *cachep)
 {
+#ifndef CONFIG_DEBUG_SLAB
 	int err;
 	int limit;
 
@@ -1746,6 +1752,7 @@
 	if (err)
 		printk(KERN_ERR "enable_cpucache failed for %s, error %d.\n",
 					cachep->name, -err);
+#endif
 }
 
 static void enable_all_cpucaches (void)