Re: [LOCKDEP BUG] from slub: separate out sysfs_slab_release() from sysfs_slab_remove()

From: Steven Rostedt <rostedt@goodmis.org>
To: Tejun Heo <tj@kernel.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Vladimir Davydov <vdavydov.dev@gmail.com>,
	Christoph Lameter <cl@linux.com>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [LOCKDEP BUG] from slub: separate out sysfs_slab_release() from sysfs_slab_remove()
Date: Mon, 19 Jun 2017 17:27:50 -0400	[thread overview]
Message-ID: <20170619172750.6890df32@gandalf.local.home> (raw)
In-Reply-To: <20170619203538.GN12062@htj.duckdns.org>

On Mon, 19 Jun 2017 16:35:38 -0400
Tejun Heo <tj@kernel.org> wrote:

> Hello, Steven.
> 
> Can you please see whether the following patch makes the lockdep
> warning go away?
> 

Added the patch and the lockdep splat goes away. Removed it, and it
comes back.

Reported-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Tested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

> diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
> index 07ef550c6627..93315d6b21a8 100644
> --- a/include/linux/slub_def.h
> +++ b/include/linux/slub_def.h
> @@ -84,6 +84,7 @@ struct kmem_cache {
>  	int red_left_pad;	/* Left redzone padding size */
>  #ifdef CONFIG_SYSFS
>  	struct kobject kobj;	/* For sysfs */
> +	struct work_struct kobj_remove_work;
>  #endif
>  #ifdef CONFIG_MEMCG
>  	struct memcg_cache_params memcg_params;
> diff --git a/mm/slub.c b/mm/slub.c
> index 7449593fca72..8addc535bcdc 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -5625,6 +5625,28 @@ static char *create_unique_id(struct kmem_cache *s)
>  	return name;
>  }
>  
> +static void sysfs_slab_remove_workfn(struct work_struct *work)
> +{
> +	struct kmem_cache *s =
> +		container_of(work, struct kmem_cache, kobj_remove_work);
> +
> +	if (!s->kobj.state_in_sysfs)
> +		/*
> +		 * For a memcg cache, this may be called during
> +		 * deactivation and again on shutdown.  Remove only once.
> +		 * A cache is never shut down before deactivation is
> +		 * complete, so no need to worry about synchronization.
> +		 */
> +		return;
> +
> +#ifdef CONFIG_MEMCG
> +	kset_unregister(s->memcg_kset);
> +#endif
> +	kobject_uevent(&s->kobj, KOBJ_REMOVE);
> +	kobject_del(&s->kobj);
> +	kobject_put(&s->kobj);
> +}
> +
>  static int sysfs_slab_add(struct kmem_cache *s)
>  {
>  	int err;
> @@ -5632,6 +5654,8 @@ static int sysfs_slab_add(struct kmem_cache *s)
>  	struct kset *kset = cache_kset(s);
>  	int unmergeable = slab_unmergeable(s);
>  
> +	INIT_WORK(&s->kobj_remove_work, sysfs_slab_remove_workfn);
> +
>  	if (!kset) {
>  		kobject_init(&s->kobj, &slab_ktype);
>  		return 0;
> @@ -5695,20 +5719,8 @@ static void sysfs_slab_remove(struct kmem_cache *s)
>  		 */
>  		return;
>  
> -	if (!s->kobj.state_in_sysfs)
> -		/*
> -		 * For a memcg cache, this may be called during
> -		 * deactivation and again on shutdown.  Remove only once.
> -		 * A cache is never shut down before deactivation is
> -		 * complete, so no need to worry about synchronization.
> -		 */
> -		return;
> -
> -#ifdef CONFIG_MEMCG
> -	kset_unregister(s->memcg_kset);
> -#endif
> -	kobject_uevent(&s->kobj, KOBJ_REMOVE);
> -	kobject_del(&s->kobj);
> +	kobject_get(&s->kobj);
> +	schedule_work(&s->kobj_remove_work);
>  }
>  
>  void sysfs_slab_release(struct kmem_cache *s)