linux-kernel - Re: [REF PATCH v3 2/2] mm/slab: decouple the SLAB_OBJ

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f0959ae0-64a1-496e-811f-5412d497c9e2@gmail.com>
Date: Wed, 10 Jul 2024 14:13:09 +0800
From: Alex Shi <seakeel@...il.com>
To: alexs@...nel.org, Andrew Morton <akpm@...ux-foundation.org>,
 Christoph Lameter <cl@...ux.com>, Pekka Enberg <penberg@...nel.org>,
 David Rientjes <rientjes@...gle.com>, Joonsoo Kim <iamjoonsoo.kim@....com>,
 Vlastimil Babka <vbabka@...e.cz>, Roman Gushchin <roman.gushchin@...ux.dev>,
 Hyeonggon Yoo <42.hyeyoo@...il.com>, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org
Cc: Randy Dunlap <rdunlap@...radead.org>, Yoann Congal
 <yoann.congal@...le.fr>, Masahiro Yamada <masahiroy@...nel.org>,
 Petr Mladek <pmladek@...e.com>, Suren Baghdasaryan <surenb@...gle.com>
Subject: Re: [REF PATCH v3 2/2] mm/slab: decouple the SLAB_OBJ_EXT from MEMCG



On 7/10/24 1:43 PM, alexs@...nel.org wrote:
> From: "Alex Shi (Tencent)" <alexs@...nel.org>
> 
> commit 21c690a349ba ("mm: introduce slabobj_ext to support slab object
> extensions") selected SLAB_OBJ_EXT on MEMCG just for SLAB_MATCH
> memcg_data, that included SLAB_OBJ_EXT for MEMCG. In fact, I didn't see
> the necessary to enable SLAB_OBJ_EXT for SLAB_OBJ_EXT.
> 
> Let's decouple the SLAB_OBJ_EXT from MEMCG and move out
> alloc_slab_obj_exts() definition from SLAB_OBJ_EXT only. To alignment
> the alloc_slab_obj_exts() return 0 for good. change its return value to
> '-1' for always failed with !SLAB_OBJ_EXT. Now we could save unnecessary
> code from MEMCG but !SLAB_OBJ_EXT.

remove SLAB_OBJ_EXT from MEMCG, it introduce a new path in 
__memcg_slab_post_alloc_hook()

        for (i = 0; i < size; i++) {
                slab = virt_to_slab(p[i]);

                if (!slab_obj_exts(slab) &&
>>>                 alloc_slab_obj_exts(slab, s, flags, false)) {
                        obj_cgroup_uncharge(objcg, obj_full_size(s));
                        continue;
                }

Here alloc_slab_obj_exts() return 0 for good, that lead to 
"slab_obj_exts(slab)[off].objcg = objcg;" failed on !SLAB_OBJ_EXT.

so change the return value could fix this. and its the only new path
for this function.
Another usage of alloc_slab_obj_exts() in prepare_slab_obj_exts_hook() doesn't
exist on !SLAB_OBJ_EXT.

Thanks
Alex
> 
> Signed-off-by: Alex Shi (Tencent) <alexs@...nel.org>
> Cc: Randy Dunlap <rdunlap@...radead.org>
> Cc: Yoann Congal <yoann.congal@...le.fr>
> Cc: Masahiro Yamada <masahiroy@...nel.org>
> Cc: Petr Mladek <pmladek@...e.com>
> ---
>  init/Kconfig | 1 -
>  mm/slab.h    | 6 +++---
>  mm/slub.c    | 6 +++---
>  3 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index 26bf8bb0a7ce..61e43ac9fe75 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -965,7 +965,6 @@ config MEMCG
>  	bool "Memory controller"
>  	select PAGE_COUNTER
>  	select EVENTFD
> -	select SLAB_OBJ_EXT
>  	help
>  	  Provides control over the memory footprint of tasks in a cgroup.
>  
> diff --git a/mm/slab.h b/mm/slab.h
> index 8ffdd4f315f8..6c727ecc1068 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -559,9 +559,6 @@ static inline struct slabobj_ext *slab_obj_exts(struct slab *slab)
>  	return (struct slabobj_ext *)(obj_exts & ~OBJEXTS_FLAGS_MASK);
>  }
>  
> -int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
> -                        gfp_t gfp, bool new_slab);
> -
>  #else /* CONFIG_SLAB_OBJ_EXT */
>  
>  static inline struct slabobj_ext *slab_obj_exts(struct slab *slab)
> @@ -571,6 +568,9 @@ static inline struct slabobj_ext *slab_obj_exts(struct slab *slab)
>  
>  #endif /* CONFIG_SLAB_OBJ_EXT */
>  
> +int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
> +			gfp_t gfp, bool new_slab);
> +
>  static inline enum node_stat_item cache_vmstat_idx(struct kmem_cache *s)
>  {
>  	return (s->flags & SLAB_RECLAIM_ACCOUNT) ?
> diff --git a/mm/slub.c b/mm/slub.c
> index cc11f3869cc6..f531c2d67238 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2075,10 +2075,10 @@ alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p,
>  
>  #else /* CONFIG_SLAB_OBJ_EXT */
>  
> -static int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
> -			       gfp_t gfp, bool new_slab)
> +int alloc_slab_obj_exts(struct slab *slab, struct kmem_cache *s,
> +			gfp_t gfp, bool new_slab)
>  {
> -	return 0;
> +	return -1;
>  }
>  
>  static inline void free_slab_obj_exts(struct slab *slab)