[SRU Focal/Groovy] UBUNTU: SAUCE: Revert "mm: memcg/slab: fix memory leak at non-root kmem_cache destroy"

[Thadeu Lima de Souza Cascardo]

On Wed, Sep 16, 2020 at 02:51:17PM +0200, Kleber Souza wrote:
> On 16.09.20 12:46, Thadeu Lima de Souza Cascardo wrote:
> > BugLink: https://bugs.launchpad.net/bugs/1894780
> > 
> > This reverts commit 79ffe7107b13042c69c4a06394175362121b06b5. This is
> > commit d38a2b7a9c939e6d7329ab92b96559ccebf7b135 upstream.
> > 
> > Said commit causes same-sized kmemcaches to become unmergeable, and when a
> > new kmemcache is created, it will fail creating the sysfs entry, making the
> > kmemcache creation to fail.
> > 
> > Considering the original commit fix a leak but causes a different leak and
> > failures to create kmemcaches, the revert is preferable until a proper fix
> > is developed.
> > 
> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
> 
> Hi Thadeu,
> 
> How does this interact with the other commit we reverted on Focal
> (dde3c6b72a16 mm/slub: fix a memory leak in sysfs_slab_add())?
> Should we keep that revert in Focal and also revert it in Groovy
> or add this commit back to Focal?
> 

We should keep the revert and fix it some other way upstream. But I'd rather
have a leak than the crash caused by the double free.

Cascardo.

> 
> Kleber
> 
> > ---
> >  mm/slab_common.c | 35 +++++++----------------------------
> >  1 file changed, 7 insertions(+), 28 deletions(-)
> > 
> > diff --git a/mm/slab_common.c b/mm/slab_common.c
> > index e36dd36c7076..8c1ffbf7de45 100644
> > --- a/mm/slab_common.c
> > +++ b/mm/slab_common.c
> > @@ -326,14 +326,6 @@ int slab_unmergeable(struct kmem_cache *s)
> >  	if (s->refcount < 0)
> >  		return 1;
> >  
> > -#ifdef CONFIG_MEMCG_KMEM
> > -	/*
> > -	 * Skip the dying kmem_cache.
> > -	 */
> > -	if (s->memcg_params.dying)
> > -		return 1;
> > -#endif
> > -
> >  	return 0;
> >  }
> >  
> > @@ -894,15 +886,12 @@ static int shutdown_memcg_caches(struct kmem_cache *s)
> >  	return 0;
> >  }
> >  
> > -static void memcg_set_kmem_cache_dying(struct kmem_cache *s)
> > +static void flush_memcg_workqueue(struct kmem_cache *s)
> >  {
> >  	spin_lock_irq(&memcg_kmem_wq_lock);
> >  	s->memcg_params.dying = true;
> >  	spin_unlock_irq(&memcg_kmem_wq_lock);
> > -}
> >  
> > -static void flush_memcg_workqueue(struct kmem_cache *s)
> > -{
> >  	/*
> >  	 * SLAB and SLUB deactivate the kmem_caches through call_rcu. Make
> >  	 * sure all registered rcu callbacks have been invoked.
> > @@ -934,6 +923,10 @@ static inline int shutdown_memcg_caches(struct kmem_cache *s)
> >  {
> >  	return 0;
> >  }
> > +
> > +static inline void flush_memcg_workqueue(struct kmem_cache *s)
> > +{
> > +}
> >  #endif /* CONFIG_MEMCG_KMEM */
> >  
> >  void slab_kmem_cache_release(struct kmem_cache *s)
> > @@ -951,6 +944,8 @@ void kmem_cache_destroy(struct kmem_cache *s)
> >  	if (unlikely(!s))
> >  		return;
> >  
> > +	flush_memcg_workqueue(s);
> > +
> >  	get_online_cpus();
> >  	get_online_mems();
> >  
> > @@ -960,22 +955,6 @@ void kmem_cache_destroy(struct kmem_cache *s)
> >  	if (s->refcount)
> >  		goto out_unlock;
> >  
> > -#ifdef CONFIG_MEMCG_KMEM
> > -	memcg_set_kmem_cache_dying(s);
> > -
> > -	mutex_unlock(&slab_mutex);
> > -
> > -	put_online_mems();
> > -	put_online_cpus();
> > -
> > -	flush_memcg_workqueue(s);
> > -
> > -	get_online_cpus();
> > -	get_online_mems();
> > -
> > -	mutex_lock(&slab_mutex);
> > -#endif
> > -
> >  	err = shutdown_memcg_caches(s);
> >  	if (!err)
> >  		err = shutdown_cache(s);
> > 
>