[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <z52lw2spqxmvgn6zo7b77nvxyjtxez5nxd6gsn6s3mwqnkvynq@gtd7kpedbtfr>
Date: Fri, 16 Jan 2026 20:49:26 +0000
From: Yosry Ahmed <yosry.ahmed@...ux.dev>
To: Sergey Senozhatsky <senozhatsky@...omium.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Minchan Kim <minchan@...nel.org>, Nhat Pham <nphamcs@...il.com>,
Johannes Weiner <hannes@...xchg.org>, Brian Geffon <bgeffon@...gle.com>, linux-kernel@...r.kernel.org,
linux-mm@...ck.org
Subject: Re: [RFC PATCH] zsmalloc: make common caches global
On Fri, Jan 16, 2026 at 01:48:41PM +0900, Sergey Senozhatsky wrote:
> Currently, zsmalloc creates kmem_cache of handles and zspages
> for each pool, which may be suboptimal from the memory usage
> point of view (extra internal fragmentation per pool). Systems
> that create multiple zsmalloc pools may benefit from shared
> common zsmalloc caches.
I had a similar patch internally when we had 32 zsmalloc pools with
zswap.
You can calculate the savings by using /proc/slabinfo. The unused memory
is (num_objs-active_objs)*objsize. You can sum this across all caches
when you have multiple pools, and compare it to the unused memory with a
single cache.
>
> Make handles and zspages kmem caches global.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@...omium.org>
> ---
> mm/zsmalloc.c | 95 ++++++++++++++++++++++-----------------------------
> 1 file changed, 40 insertions(+), 55 deletions(-)
>
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 5abb8bc0956a..05ed3539aa1e 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -198,12 +198,13 @@ struct link_free {
> };
> };
>
> +static struct kmem_cache *handle_cachep;
> +static struct kmem_cache *zspage_cachep;
> +
> struct zs_pool {
> const char *name;
>
> struct size_class *size_class[ZS_SIZE_CLASSES];
> - struct kmem_cache *handle_cachep;
> - struct kmem_cache *zspage_cachep;
>
> atomic_long_t pages_allocated;
>
> @@ -376,60 +377,28 @@ static void init_deferred_free(struct zs_pool *pool) {}
> static void SetZsPageMovable(struct zs_pool *pool, struct zspage *zspage) {}
> #endif
>
> -static int create_cache(struct zs_pool *pool)
> +static unsigned long cache_alloc_handle(gfp_t gfp)
> {
> - char *name;
> -
> - name = kasprintf(GFP_KERNEL, "zs_handle-%s", pool->name);
> - if (!name)
> - return -ENOMEM;
> - pool->handle_cachep = kmem_cache_create(name, ZS_HANDLE_SIZE,
> - 0, 0, NULL);
> - kfree(name);
> - if (!pool->handle_cachep)
> - return -EINVAL;
> -
> - name = kasprintf(GFP_KERNEL, "zspage-%s", pool->name);
> - if (!name)
> - return -ENOMEM;
> - pool->zspage_cachep = kmem_cache_create(name, sizeof(struct zspage),
> - 0, 0, NULL);
> - kfree(name);
> - if (!pool->zspage_cachep) {
> - kmem_cache_destroy(pool->handle_cachep);
> - pool->handle_cachep = NULL;
> - return -EINVAL;
> - }
> -
> - return 0;
> -}
> + gfp = gfp & ~(__GFP_HIGHMEM | __GFP_MOVABLE);
>
> -static void destroy_cache(struct zs_pool *pool)
> -{
> - kmem_cache_destroy(pool->handle_cachep);
> - kmem_cache_destroy(pool->zspage_cachep);
> + return (unsigned long)kmem_cache_alloc(handle_cachep, gfp);
> }
>
> -static unsigned long cache_alloc_handle(struct zs_pool *pool, gfp_t gfp)
> +static void cache_free_handle(unsigned long handle)
> {
> - return (unsigned long)kmem_cache_alloc(pool->handle_cachep,
> - gfp & ~(__GFP_HIGHMEM|__GFP_MOVABLE));
> + kmem_cache_free(handle_cachep, (void *)handle);
> }
>
> -static void cache_free_handle(struct zs_pool *pool, unsigned long handle)
> +static struct zspage *cache_alloc_zspage(gfp_t gfp)
> {
> - kmem_cache_free(pool->handle_cachep, (void *)handle);
> -}
> + gfp = gfp & ~(__GFP_HIGHMEM | __GFP_MOVABLE);
>
> -static struct zspage *cache_alloc_zspage(struct zs_pool *pool, gfp_t flags)
> -{
> - return kmem_cache_zalloc(pool->zspage_cachep,
> - flags & ~(__GFP_HIGHMEM|__GFP_MOVABLE));
> + return kmem_cache_zalloc(zspage_cachep, gfp);
> }
>
> -static void cache_free_zspage(struct zs_pool *pool, struct zspage *zspage)
> +static void cache_free_zspage(struct zspage *zspage)
> {
> - kmem_cache_free(pool->zspage_cachep, zspage);
> + kmem_cache_free(zspage_cachep, zspage);
> }
>
> /* class->lock(which owns the handle) synchronizes races */
> @@ -858,7 +827,7 @@ static void __free_zspage(struct zs_pool *pool, struct size_class *class,
> zpdesc = next;
> } while (zpdesc != NULL);
>
> - cache_free_zspage(pool, zspage);
> + cache_free_zspage(zspage);
>
> class_stat_sub(class, ZS_OBJS_ALLOCATED, class->objs_per_zspage);
> atomic_long_sub(class->pages_per_zspage, &pool->pages_allocated);
> @@ -971,7 +940,7 @@ static struct zspage *alloc_zspage(struct zs_pool *pool,
> {
> int i;
> struct zpdesc *zpdescs[ZS_MAX_PAGES_PER_ZSPAGE];
> - struct zspage *zspage = cache_alloc_zspage(pool, gfp);
> + struct zspage *zspage = cache_alloc_zspage(gfp);
>
> if (!zspage)
> return NULL;
> @@ -993,7 +962,7 @@ static struct zspage *alloc_zspage(struct zs_pool *pool,
> zpdesc_dec_zone_page_state(zpdescs[i]);
> free_zpdesc(zpdescs[i]);
> }
> - cache_free_zspage(pool, zspage);
> + cache_free_zspage(zspage);
> return NULL;
> }
> __zpdesc_set_zsmalloc(zpdesc);
> @@ -1346,7 +1315,7 @@ unsigned long zs_malloc(struct zs_pool *pool, size_t size, gfp_t gfp,
> if (unlikely(size > ZS_MAX_ALLOC_SIZE))
> return (unsigned long)ERR_PTR(-ENOSPC);
>
> - handle = cache_alloc_handle(pool, gfp);
> + handle = cache_alloc_handle(gfp);
> if (!handle)
> return (unsigned long)ERR_PTR(-ENOMEM);
>
> @@ -1370,7 +1339,7 @@ unsigned long zs_malloc(struct zs_pool *pool, size_t size, gfp_t gfp,
>
> zspage = alloc_zspage(pool, class, gfp, nid);
> if (!zspage) {
> - cache_free_handle(pool, handle);
> + cache_free_handle(handle);
> return (unsigned long)ERR_PTR(-ENOMEM);
> }
>
> @@ -1450,7 +1419,7 @@ void zs_free(struct zs_pool *pool, unsigned long handle)
> free_zspage(pool, class, zspage);
>
> spin_unlock(&class->lock);
> - cache_free_handle(pool, handle);
> + cache_free_handle(handle);
> }
> EXPORT_SYMBOL_GPL(zs_free);
>
> @@ -2112,9 +2081,6 @@ struct zs_pool *zs_create_pool(const char *name)
> if (!pool->name)
> goto err;
>
> - if (create_cache(pool))
> - goto err;
> -
> /*
> * Iterate reversely, because, size of size_class that we want to use
> * for merging should be larger or equal to current size.
> @@ -2236,7 +2202,6 @@ void zs_destroy_pool(struct zs_pool *pool)
> kfree(class);
> }
>
> - destroy_cache(pool);
> kfree(pool->name);
> kfree(pool);
> }
> @@ -2246,10 +2211,28 @@ static int __init zs_init(void)
> {
> int rc __maybe_unused;
>
> + handle_cachep = kmem_cache_create("zs_handle", ZS_HANDLE_SIZE, 0, 0,
> + NULL);
> + if (!handle_cachep)
> + return -ENOMEM;
> +
> + zspage_cachep = kmem_cache_create("zspage", sizeof(struct zspage), 0,
> + 0, NULL);
> + if (!zspage_cachep) {
> + kmem_cache_destroy(handle_cachep);
> + handle_cachep = NULL;
> + return -ENOMEM;
> + }
> +
> #ifdef CONFIG_COMPACTION
> rc = set_movable_ops(&zsmalloc_mops, PGTY_zsmalloc);
> - if (rc)
> + if (rc) {
> + kmem_cache_destroy(zspage_cachep);
> + kmem_cache_destroy(handle_cachep);
> + zspage_cachep = NULL;
> + handle_cachep = NULL;
> return rc;
> + }
> #endif
> zs_stat_init();
> return 0;
> @@ -2261,6 +2244,8 @@ static void __exit zs_exit(void)
> set_movable_ops(NULL, PGTY_zsmalloc);
> #endif
> zs_stat_exit();
> + kmem_cache_destroy(zspage_cachep);
> + kmem_cache_destroy(handle_cachep);
> }
Hmm instead of the repeated kmem_cache_destroy() calls, can we do sth
like this:
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index dccb88d52c07..86e2ca95ac4c 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -2235,14 +2235,43 @@ void zs_destroy_pool(struct zs_pool *pool)
}
EXPORT_SYMBOL_GPL(zs_destroy_pool);
+static void __init zs_destroy_caches(void)
+{
+ kmem_cache_destroy(zs_handle_cache);
+ zs_handle_cache = NULL;
+ kmem_cache_destroy(zspage_cache);
+ zspage_cache = NULL;
+}
+
+static int __init zs_init_caches(void)
+{
+ zs_handle_cache = kmem_cache_create("zs_handle", ZS_HANDLE_SIZE,
+ 0, 0, NULL);
+ zspage_cache = kmem_cache_create("zspage", sizeof(struct zspage),
+ 0, 0, NULL);
+
+ if (!zs_handle_cache || !zspage_cache) {
+ zs_destroy_caches();
+ return -ENOMEM;
+ }
+ return 0;
+}
+
+
static int __init zs_init(void)
{
- int rc __maybe_unused;
+ int rc;
+
+ rc = zs_init_caches();
+ if (rc)
+ return rc;
#ifdef CONFIG_COMPACTION
rc = set_movable_ops(&zsmalloc_mops, PGTY_zsmalloc);
- if (rc)
+ if (rc) {
+ zs_destroy_caches();
return rc;
+ }
#endif
zs_stat_init();
return 0;
>
> module_init(zs_init);
> --
> 2.52.0.457.g6b5491de43-goog
>
Powered by blists - more mailing lists