Re: [BUG] Lockdep recursive locking in kmem_cache_free

[Thomas Gleixner <tglx@linutronix.de>]

On Wed, 2006-08-02 at 12:10 -0700, Ravikiran G Thirumalai wrote:
> Here's an attempt to educate lockdep about alien cache lock. tglx, can you
> confirm if this fixes the false positive?  This is just an extension of the
> l3 lock lesson :).
> 
> Note: With this approach, lockdep forgets its education for alien caches
> if all cpus of a node go down and come back up.  But taking care of 
> that scenario will make things uglier....not sure if it is worth it.
> 
> Thanks,
> Kiran

Sorry, I did not come around to test it earlier. With this patch applied
the lockdep message is gone.

	tglx

> Place the alien array cache locks of on slab malloc slab caches on a seperate 
> lockdep class.  This avoids false positives from lockdep
> 
> Signed-off-by: Ravikiran Thirumalai <kiran@scalex86.org>
> Signed-off-by: Shai Fultheim <shai@scalex86.org>
> 
> Index: linux-2.6.18-rc3-x460/mm/slab.c
> ===================================================================
> --- linux-2.6.18-rc3-x460.orig/mm/slab.c	2006-07-30 21:27:28.000000000 -0700
> +++ linux-2.6.18-rc3-x460/mm/slab.c	2006-08-01 18:01:51.000000000 -0700
> @@ -682,23 +682,43 @@
>   * The locking for this is tricky in that it nests within the locks
>   * of all other slabs in a few places; to deal with this special
>   * locking we put on-slab caches into a separate lock-class.
> + *
> + * We set lock class for alien array caches which are up during init.
> + * The lock annotation will be lost if all cpus of a node goes down and 
> + * then comes back up during hotplug
>   */
> -static struct lock_class_key on_slab_key;
> +static struct lock_class_key on_slab_l3_key;
> +static struct lock_class_key on_slab_alc_key;
> +
> +static inline void init_lock_keys(void)
>  
> -static inline void init_lock_keys(struct cache_sizes *s)
>  {
>  	int q;
> +	struct cache_sizes *s = malloc_sizes;
>  
> -	for (q = 0; q < MAX_NUMNODES; q++) {
> -		if (!s->cs_cachep->nodelists[q] || OFF_SLAB(s->cs_cachep))
> -			continue;
> -		lockdep_set_class(&s->cs_cachep->nodelists[q]->list_lock,
> -				  &on_slab_key);
> +	while (s->cs_size != ULONG_MAX) {
> +		for_each_node(q) {
> +			struct array_cache **alc;
> +			int r;
> +			struct kmem_list3 *l3 = s->cs_cachep->nodelists[q];
> +			if (!l3 || OFF_SLAB(s->cs_cachep))
> +				continue;
> +			lockdep_set_class(&l3->list_lock, &on_slab_l3_key);
> +			alc = l3->alien;
> +			if (!alc)
> +				continue;
> +			for_each_node(r) {
> +				if (alc[r])
> +					lockdep_set_class(&alc[r]->lock,
> +					     &on_slab_alc_key);
> +			}
> +		}
> +		s++;
>  	}
>  }
>  
>  #else
> -static inline void init_lock_keys(struct cache_sizes *s)
> +static inline void init_lock_keys()
>  {
>  }
>  #endif
> @@ -1422,7 +1442,6 @@
>  					ARCH_KMALLOC_FLAGS|SLAB_PANIC,
>  					NULL, NULL);
>  		}
> -		init_lock_keys(sizes);
>  
>  		sizes->cs_dmacachep = kmem_cache_create(names->name_dma,
>  					sizes->cs_size,
> @@ -1495,6 +1514,10 @@
>  		mutex_unlock(&cache_chain_mutex);
>  	}
>  
> +	/* Annotate slab for lockdep -- annotate the malloc caches */
> +	init_lock_keys();
> +	
> +
>  	/* Done! */
>  	g_cpucache_up = FULL;
>