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

[Kleber Souza]

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?


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);
>