[PATCH] mm: use unique zsmalloc caches names

[PATCH] mm: use unique zsmalloc caches names

[Sergey Senozhatsky]

Each zsmalloc pool maintains several named kmem-caches for
zs_handle-s and  zspage-s.  On a system with multiple zsmalloc
pools and CONFIG_DEBUG_VM this triggers kmem_cache_sanity_check():

  kmem_cache of name 'zspage' already exists
  WARNING: at mm/slab_common.c:108 do_kmem_cache_create_usercopy+0xb5/0x310
  ...

  kmem_cache of name 'zs_handle' already exists
  WARNING: at mm/slab_common.c:108 do_kmem_cache_create_usercopy+0xb5/0x310
  ...

We provide zram device name when init its zsmalloc pool, so we can
use that same name for zsmalloc caches and, hence, create unique
names that can easily be linked to zram device that has created
them.

So instead of having this

cat /proc/slabinfo
slabinfo - version: 2.1
zspage                46     46    ...
zs_handle            128    128    ...
zspage             34270  34270    ...
zs_handle          34816  34816    ...
zspage                 0      0    ...
zs_handle              0      0    ...

We now have this

cat /proc/slabinfo
slabinfo - version: 2.1
zspage-zram2          46     46    ...
zs_handle-zram2      128    128    ...
zspage-zram0       34270  34270    ...
zs_handle-zram0    34816  34816    ...
zspage-zram1           0      0    ...
zs_handle-zram1        0      0    ...

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 mm/zsmalloc.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 73a3ec5b21ad..896ca02ed75a 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -293,13 +293,17 @@ static void SetZsPageMovable(struct zs_pool *pool, struct zspage *zspage) {}
 
 static int create_cache(struct zs_pool *pool)
 {
-	pool->handle_cachep = kmem_cache_create("zs_handle", ZS_HANDLE_SIZE,
-					0, 0, NULL);
+	char name[32];
+
+	snprintf(name, sizeof(name), "zs_handle-%s", pool->name);
+	pool->handle_cachep = kmem_cache_create(name, ZS_HANDLE_SIZE,
+						0, 0, NULL);
 	if (!pool->handle_cachep)
 		return 1;
 
-	pool->zspage_cachep = kmem_cache_create("zspage", sizeof(struct zspage),
-					0, 0, NULL);
+	snprintf(name, sizeof(name), "zspage-%s", pool->name);
+	pool->zspage_cachep = kmem_cache_create(name, sizeof(struct zspage),
+						0, 0, NULL);
 	if (!pool->zspage_cachep) {
 		kmem_cache_destroy(pool->handle_cachep);
 		pool->handle_cachep = NULL;
-- 
2.46.0.469.g59c65b2a67-goog

Re: [PATCH] mm: use unique zsmalloc caches names

[Andrew Morton]

On Thu,  5 Sep 2024 15:47:23 +0900 Sergey Senozhatsky <senozhatsky@chromium.org> wrote:

> Each zsmalloc pool maintains several named kmem-caches for
> zs_handle-s and  zspage-s.  On a system with multiple zsmalloc
> pools and CONFIG_DEBUG_VM this triggers kmem_cache_sanity_check():
> 
>   kmem_cache of name 'zspage' already exists
>   WARNING: at mm/slab_common.c:108 do_kmem_cache_create_usercopy+0xb5/0x310
>   ...
> 
>   kmem_cache of name 'zs_handle' already exists
>   WARNING: at mm/slab_common.c:108 do_kmem_cache_create_usercopy+0xb5/0x310
>   ...

This is old code.  Did something recently change to trigger this warning?

> We provide zram device name when init its zsmalloc pool, so we can
> use that same name for zsmalloc caches and, hence, create unique
> names that can easily be linked to zram device that has created
> them.
> 
> So instead of having this
> 
> cat /proc/slabinfo
> slabinfo - version: 2.1
> zspage                46     46    ...
> zs_handle            128    128    ...
> zspage             34270  34270    ...
> zs_handle          34816  34816    ...
> zspage                 0      0    ...
> zs_handle              0      0    ...
> 
> We now have this
> 
> cat /proc/slabinfo
> slabinfo - version: 2.1
> zspage-zram2          46     46    ...
> zs_handle-zram2      128    128    ...
> zspage-zram0       34270  34270    ...
> zs_handle-zram0    34816  34816    ...
> zspage-zram1           0      0    ...
> zs_handle-zram1        0      0    ...
> 
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -293,13 +293,17 @@ static void SetZsPageMovable(struct zs_pool *pool, struct zspage *zspage) {}
>  
>  static int create_cache(struct zs_pool *pool)
>  {
> -	pool->handle_cachep = kmem_cache_create("zs_handle", ZS_HANDLE_SIZE,
> -					0, 0, NULL);
> +	char name[32];
> +
> +	snprintf(name, sizeof(name), "zs_handle-%s", pool->name);

Always scary seeing code making such assumptions about it arguments in
this fashion.  Can we use kasprintf() and sleep well at night?


> +	pool->handle_cachep = kmem_cache_create(name, ZS_HANDLE_SIZE,
> +						0, 0, NULL);
>  	if (!pool->handle_cachep)
>  		return 1;
>  
> -	pool->zspage_cachep = kmem_cache_create("zspage", sizeof(struct zspage),
> -					0, 0, NULL);
> +	snprintf(name, sizeof(name), "zspage-%s", pool->name);
> +	pool->zspage_cachep = kmem_cache_create(name, sizeof(struct zspage),
> +						0, 0, NULL);
>  	if (!pool->zspage_cachep) {
>  		kmem_cache_destroy(pool->handle_cachep);
>  		pool->handle_cachep = NULL;

I guess we want to backport this into earlier kernels?  If so, what
would be a suitable Fixes:?

Re: [PATCH] mm: use unique zsmalloc caches names

[Sergey Senozhatsky]

On (24/09/05 14:52), Andrew Morton wrote:
> > Each zsmalloc pool maintains several named kmem-caches for
> > zs_handle-s and  zspage-s.  On a system with multiple zsmalloc
> > pools and CONFIG_DEBUG_VM this triggers kmem_cache_sanity_check():
> > 
> >   kmem_cache of name 'zspage' already exists
> >   WARNING: at mm/slab_common.c:108 do_kmem_cache_create_usercopy+0xb5/0x310
> >   ...
> > 
> >   kmem_cache of name 'zs_handle' already exists
> >   WARNING: at mm/slab_common.c:108 do_kmem_cache_create_usercopy+0xb5/0x310
> >   ...
> 
> This is old code.  Did something recently change to trigger this warning?

The kmem_cache WARN_ON() seems to be a new thing 4c39529663b93
and I think for the past week or so my test box has been running
with DEBUG_VM disabled.

[..]
> >  static int create_cache(struct zs_pool *pool)
> >  {
> > -	pool->handle_cachep = kmem_cache_create("zs_handle", ZS_HANDLE_SIZE,
> > -					0, 0, NULL);
> > +	char name[32];
> > +
> > +	snprintf(name, sizeof(name), "zs_handle-%s", pool->name);
> 
> Always scary seeing code making such assumptions about it arguments in
> this fashion.  Can we use kasprintf() and sleep well at night?

Sure, I'll switch to kasprintf() "pillow" in v2.

[..]
> >  	if (!pool->zspage_cachep) {
> >  		kmem_cache_destroy(pool->handle_cachep);
> >  		pool->handle_cachep = NULL;
> 
> I guess we want to backport this into earlier kernels?  If so, what
> would be a suitable Fixes:?

So this doesn't affect zsmalloc, it's only some user-space tools that
can get confused.  The code in question has been around since forever.
The first kmem-cache has been introduced by 2e40e163a25a in 2015.

I'll add Fixes: 2e40e163a25af3 in v2, but I'm not certain if we are
in urge to backport anything.