| Message ID | 20240905064736.2250735-1-senozhatsky@chromium.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: use unique zsmalloc caches names | expand |
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:?
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.
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;
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(-)