On Wed, Feb 28, 2007 at 11:10:54AM +0100, Eric Dumazet (dada1@cosmosbay.com) wrote:
> On Wednesday 28 February 2007 10:02, Evgeniy Polyakov wrote:
> > Attached patch detects in run-time things like:
> > skb = alloc_skb();
> > kfree(skb);
> >
> > where provided to kfree pointer does not belong to kmalloc caches.
> > It is turned on when slab debug config option is enabled.
> >
> > When problem is detected, following warning is printed with hint to
> > what cache/function should be used instead:
> 
> It would be less expensive to add a flag 
> #define SLAB_KFREE_NOWARNING 0x00200000UL
> 
> And OR this flags into cs->flags of all standard caches created by 
> kmem_cache_init() from malloc_sizes[]/cache_names[]
> 
> kfree() would then just test this flag.

That does not work - my x86_64 test machine fails badly with following
patch applied:

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 1ef822e..acc3cfb 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -32,6 +32,7 @@ typedef struct kmem_cache kmem_cache_t __deprecated;
 #define SLAB_PANIC		0x00040000UL	/* Panic if kmem_cache_create() fails */
 #define SLAB_DESTROY_BY_RCU	0x00080000UL	/* Defer freeing slabs to RCU */
 #define SLAB_MEM_SPREAD		0x00100000UL	/* Spread some memory over cpuset */
+#define SLAB_KFREE_NOWARNING 	0x00200000UL	/* Do not warn if object belongs to this cache and is freed via kfree */
 
 /* Flags passed to a constructor functions */
 #define SLAB_CTOR_CONSTRUCTOR	0x001UL		/* If not set, then deconstructor */
diff --git a/mm/slab.c b/mm/slab.c
index 8fdaffa..313014e 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -177,7 +177,8 @@
 			 SLAB_CACHE_DMA | \
 			 SLAB_MUST_HWCACHE_ALIGN | SLAB_STORE_USER | \
 			 SLAB_RECLAIM_ACCOUNT | SLAB_PANIC | \
-			 SLAB_DESTROY_BY_RCU | SLAB_MEM_SPREAD)
+			 SLAB_DESTROY_BY_RCU | SLAB_MEM_SPREAD | \
+			 SLAB_KFREE_NOWARNING )
 #else
 # define CREATE_MASK	(SLAB_HWCACHE_ALIGN | \
 			 SLAB_CACHE_DMA | SLAB_MUST_HWCACHE_ALIGN | \
@@ -814,7 +815,7 @@ static size_t slab_mgmt_size(size_t nr_objs, size_t align)
  * Calculate the number of objects and left-over bytes for a given buffer size.
  */
 static void cache_estimate(unsigned long gfporder, size_t buffer_size,
-			   size_t align, int flags, size_t *left_over,
+			   size_t align, unsigned long flags, size_t *left_over,
 			   unsigned int *num)
 {
 	int nr_objs;
@@ -1466,7 +1467,8 @@ void __init kmem_cache_init(void)
 	sizes[INDEX_AC].cs_cachep = kmem_cache_create(names[INDEX_AC].name,
 					sizes[INDEX_AC].cs_size,
 					ARCH_KMALLOC_MINALIGN,
-					ARCH_KMALLOC_FLAGS|SLAB_PANIC,
+					ARCH_KMALLOC_FLAGS|SLAB_PANIC|
+						SLAB_KFREE_NOWARNING,
 					NULL, NULL);
 
 	if (INDEX_AC != INDEX_L3) {
@@ -1474,7 +1476,8 @@ void __init kmem_cache_init(void)
 			kmem_cache_create(names[INDEX_L3].name,
 				sizes[INDEX_L3].cs_size,
 				ARCH_KMALLOC_MINALIGN,
-				ARCH_KMALLOC_FLAGS|SLAB_PANIC,
+				ARCH_KMALLOC_FLAGS|SLAB_PANIC|
+					SLAB_KFREE_NOWARNING,
 				NULL, NULL);
 	}
 
@@ -1492,7 +1495,8 @@ void __init kmem_cache_init(void)
 			sizes->cs_cachep = kmem_cache_create(names->name,
 					sizes->cs_size,
 					ARCH_KMALLOC_MINALIGN,
-					ARCH_KMALLOC_FLAGS|SLAB_PANIC,
+					ARCH_KMALLOC_FLAGS|SLAB_PANIC|
+						SLAB_KFREE_NOWARNING,
 					NULL, NULL);
 		}
 #ifdef CONFIG_ZONE_DMA
@@ -1501,7 +1505,7 @@ void __init kmem_cache_init(void)
 					sizes->cs_size,
 					ARCH_KMALLOC_MINALIGN,
 					ARCH_KMALLOC_FLAGS|SLAB_CACHE_DMA|
-						SLAB_PANIC,
+						SLAB_PANIC|SLAB_KFREE_NOWARNING,
 					NULL, NULL);
 #endif
 		sizes++;
@@ -2827,6 +2831,16 @@ static void kfree_debugcheck(const void *objp)
 	}
 }
 
+static void kfree_debug_cache_pointer(struct kmem_cache *cachep, const void *objp)
+{
+	if (!(cachep->flags & SLAB_KFREE_NOWARNING)) {
+		printk(KERN_ERR "kfree debug: obj: %p, likely you want to use something with "
+				"'%s' in name instead of kfree().\n",
+				objp, cachep->name);
+		WARN_ON(1);
+	}
+}
+
 static inline void verify_redzone_free(struct kmem_cache *cache, void *obj)
 {
 	unsigned long redzone1, redzone2;
@@ -2938,6 +2952,7 @@ bad:
 }
 #else
 #define kfree_debugcheck(x) do { } while(0)
+#define kfree_debug_cache_pointer(x, y) do { } while(0)
 #define cache_free_debugcheck(x,objp,z) (objp)
 #define check_slabp(x,y) do { } while(0)
 #endif
@@ -3776,6 +3791,7 @@ void kfree(const void *objp)
 	local_irq_save(flags);
 	kfree_debugcheck(objp);
 	c = virt_to_cache(objp);
+	kfree_debug_cache_pointer(c, objp);
 	debug_check_no_locks_freed(objp, obj_size(c));
 	__cache_free(c, (void *)objp);
 	local_irq_restore(flags);


Even I add a simple printk into kmalloc cache initialization path (only one)
system fails to find apic timer. The same happens if I use different bit
from higher byte (except the last one which is used).

The latest message in dmesg is:

..MP-BIOS bug: 8254 timer not connected to IO-APIC
IO-APIC+timer do not work 

(or something like that).

System boots ok with 'noapic' kernel parameter. dmesg of working system 
is attached.

I've added Andy Kleen to Cc so he would shed some light on it.

To cure (workaround) the bug, I added following patch:

diff --git a/arch/x86_64/kernel/io_apic.c b/arch/x86_64/kernel/io_apic.c
index 950682f..765278a 100644
--- a/arch/x86_64/kernel/io_apic.c
+++ b/arch/x86_64/kernel/io_apic.c
@@ -1315,7 +1315,7 @@ static int __init timer_irq_works(void)
 
 	local_irq_enable();
 	/* Let ten ticks pass... */
-	mdelay((10 * 1000) / HZ);
+	mdelay((10 * 10000) / HZ);
 
 	/*
 	 * Expect a few ticks at least, to be sure some possible


After applied system boots fine, but above message 
	..MP-BIOS bug: 8254 timer not connected to IO-APIC
still exist.
-- 
	Evgeniy Polyakov