* [PATCH 1/7] zram: drop stale comment in write_incompressible_page()
2026-11-05 4:47 [PATCH 0/7] zsmalloc: redesign object alloc API Sergey Senozhatsky
@ 2026-11-05 4:47 ` Sergey Senozhatsky
2026-11-05 4:47 ` [PATCH 2/7] zram: drop get_slot_handle() and set_slot_handle() Sergey Senozhatsky
` (6 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Sergey Senozhatsky @ 2026-11-05 4:47 UTC (permalink / raw)
To: Andrew Morton
Cc: Nhat Pham, Yosry Ahmed, Johannes Weiner, Brian Geffon,
Minchan Kim, Richard Chang, linux-kernel, linux-block, linux-mm,
Sergey Senozhatsky
zram doesn't do fast/slow-path handle allocation anymore, so
the comment in write_incompressible_page() is stale. Drop it.
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
drivers/block/zram/zram_drv.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 632045b65bdb..380615b73446 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -2271,11 +2271,6 @@ static int write_incompressible_page(struct zram *zram, struct page *page,
unsigned long handle;
void *src;
- /*
- * This function is called from preemptible context so we don't need
- * to do optimistic and fallback to pessimistic handle allocation,
- * like we do for compressible pages.
- */
handle = zs_malloc(zram->mem_pool, PAGE_SIZE,
GFP_NOIO | __GFP_NOWARN |
__GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
--
2.56.0.398.g882cbad275-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/7] zram: drop get_slot_handle() and set_slot_handle()
2026-11-05 4:47 [PATCH 0/7] zsmalloc: redesign object alloc API Sergey Senozhatsky
2026-11-05 4:47 ` [PATCH 1/7] zram: drop stale comment in write_incompressible_page() Sergey Senozhatsky
@ 2026-11-05 4:47 ` Sergey Senozhatsky
2026-11-05 4:47 ` [PATCH 3/7] zsmalloc: switch handle type to struct zs_handle Sergey Senozhatsky
` (5 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Sergey Senozhatsky @ 2026-11-05 4:47 UTC (permalink / raw)
To: Andrew Morton
Cc: Nhat Pham, Yosry Ahmed, Johannes Weiner, Brian Geffon,
Minchan Kim, Richard Chang, linux-kernel, linux-block, linux-mm,
Sergey Senozhatsky
zram_table_entry ->handle is overloaded to store zsmalloc handle,
same-filled page pattern and writeback block index, so calling
set_slot_handle() or get_slot_handle() for blk_idx and same-filled
pattern is confusing.
Turn ->handle into a union of handle, element and blk_idx, drop
get_slot_handle() and set_slot_handle(), and open-code field
accesses. No functional change.
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
drivers/block/zram/zram_drv.c | 43 +++++++++++++----------------------
drivers/block/zram/zram_drv.h | 6 ++++-
2 files changed, 21 insertions(+), 28 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 380615b73446..5c13addd22f6 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -111,17 +111,6 @@ static inline struct zram *dev_to_zram(struct device *dev)
return (struct zram *)dev_to_disk(dev)->private_data;
}
-static unsigned long get_slot_handle(struct zram *zram, unsigned long index)
-{
- return zram->table[index].handle;
-}
-
-static void set_slot_handle(struct zram *zram, unsigned long index,
- unsigned long handle)
-{
- zram->table[index].handle = handle;
-}
-
static bool test_slot_flag(struct zram *zram, unsigned long index,
enum zram_pageflags flag)
{
@@ -962,8 +951,8 @@ static int zram_writeback_complete(struct zram *zram, struct zram_wb_req *req)
if (test_slot_flag(zram, index, ZRAM_HUGE))
atomic64_dec(&zram->stats.huge_pages);
atomic64_sub(get_slot_size(zram, index), &zram->stats.compr_data_size);
- zs_free(zram->mem_pool, get_slot_handle(zram, index));
- set_slot_handle(zram, index, req->blk_idx);
+ zs_free(zram->mem_pool, zram->table[index].handle);
+ zram->table[index].blk_idx = req->blk_idx;
set_slot_flag(zram, index, ZRAM_WB);
out:
@@ -2064,7 +2053,7 @@ static void slot_free(struct zram *zram, unsigned long index)
if (test_slot_flag(zram, index, ZRAM_WB)) {
clear_slot_flag(zram, index, ZRAM_WB);
- zram_release_bdev_block(zram, get_slot_handle(zram, index));
+ zram_release_bdev_block(zram, zram->table[index].blk_idx);
goto out;
}
@@ -2078,7 +2067,7 @@ static void slot_free(struct zram *zram, unsigned long index)
goto out;
}
- handle = get_slot_handle(zram, index);
+ handle = zram->table[index].handle;
if (!handle)
return;
@@ -2088,7 +2077,7 @@ static void slot_free(struct zram *zram, unsigned long index)
&zram->stats.compr_data_size);
out:
atomic64_dec(&zram->stats.pages_stored);
- set_slot_handle(zram, index, 0);
+ zram->table[index].element = 0;
set_slot_size(zram, index, 0);
}
@@ -2098,7 +2087,7 @@ static int read_same_filled_page(struct zram *zram, struct page *page,
void *mem;
mem = kmap_local_page(page);
- zram_fill_page(mem, PAGE_SIZE, get_slot_handle(zram, index));
+ zram_fill_page(mem, PAGE_SIZE, zram->table[index].element);
kunmap_local(mem);
return 0;
}
@@ -2110,7 +2099,7 @@ static int read_incompressible_page(struct zram *zram, struct page *page,
unsigned long handle;
void *src, *dst;
- handle = get_slot_handle(zram, index);
+ handle = zram->table[index].handle;
zs_obj_read_sg_begin(zram->mem_pool, handle, sg, PAGE_SIZE);
/* an incompressible object never spans two pages */
src = kmap_local_page(sg_page(sg));
@@ -2134,7 +2123,7 @@ static int read_compressed_page(struct zram *zram, struct page *page,
void *dst;
int ret, prio;
- handle = get_slot_handle(zram, index);
+ handle = zram->table[index].handle;
size = get_slot_size(zram, index);
prio = get_slot_comp_priority(zram, index);
@@ -2161,7 +2150,7 @@ static int read_from_zspool_raw(struct zram *zram, struct page *page,
unsigned int size;
void *dst;
- handle = get_slot_handle(zram, index);
+ handle = zram->table[index].handle;
size = get_slot_size(zram, index);
/*
@@ -2188,7 +2177,7 @@ static int read_from_zspool(struct zram *zram, struct page *page,
unsigned long index)
{
if (test_slot_flag(zram, index, ZRAM_SAME) ||
- !get_slot_handle(zram, index))
+ !zram->table[index].handle)
return read_same_filled_page(zram, page, index);
if (!test_slot_flag(zram, index, ZRAM_HUGE))
@@ -2209,7 +2198,7 @@ static int zram_read_page(struct zram *zram, struct page *page,
ret = read_from_zspool(zram, page, index);
slot_unlock(zram, index);
} else {
- unsigned long blk_idx = get_slot_handle(zram, index);
+ unsigned long blk_idx = zram->table[index].blk_idx;
/*
* The slot should be unlocked before reading from the backing
@@ -2255,7 +2244,7 @@ static int write_same_filled_page(struct zram *zram, unsigned long fill,
slot_lock(zram, index);
slot_free(zram, index);
set_slot_flag(zram, index, ZRAM_SAME);
- set_slot_handle(zram, index, fill);
+ zram->table[index].element = fill;
mark_slot_accessed(zram, index);
slot_unlock(zram, index);
@@ -2289,7 +2278,7 @@ static int write_incompressible_page(struct zram *zram, struct page *page,
slot_lock(zram, index);
slot_free(zram, index);
set_slot_flag(zram, index, ZRAM_HUGE);
- set_slot_handle(zram, index, handle);
+ zram->table[index].handle = handle;
set_slot_size(zram, index, PAGE_SIZE);
mark_slot_accessed(zram, index);
slot_unlock(zram, index);
@@ -2355,7 +2344,7 @@ static int zram_write_page(struct zram *zram, struct page *page,
slot_lock(zram, index);
slot_free(zram, index);
- set_slot_handle(zram, index, handle);
+ zram->table[index].handle = handle;
set_slot_size(zram, index, comp_len);
mark_slot_accessed(zram, index);
slot_unlock(zram, index);
@@ -2471,7 +2460,7 @@ static int recompress_slot(struct zram *zram, unsigned long index,
void *src;
int ret = 0;
- handle_old = get_slot_handle(zram, index);
+ handle_old = zram->table[index].handle;
if (!handle_old)
return -EINVAL;
@@ -2554,7 +2543,7 @@ static int recompress_slot(struct zram *zram, unsigned long index,
zcomp_cstrm_put(zstrm);
slot_free(zram, index);
- set_slot_handle(zram, index, handle_new);
+ zram->table[index].handle = handle_new;
set_slot_size(zram, index, comp_len_new);
set_slot_comp_priority(zram, index, prio);
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index c563a7c13a49..5416e25e64e8 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -78,7 +78,11 @@ enum zram_pageflags {
* of each entry and hence of the meta table.
*/
struct zram_table_entry {
- unsigned long handle;
+ union {
+ unsigned long handle;
+ unsigned long element;
+ unsigned long blk_idx;
+ };
union {
unsigned long __lock;
struct attr {
--
2.56.0.398.g882cbad275-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/7] zsmalloc: switch handle type to struct zs_handle
2026-11-05 4:47 [PATCH 0/7] zsmalloc: redesign object alloc API Sergey Senozhatsky
2026-11-05 4:47 ` [PATCH 1/7] zram: drop stale comment in write_incompressible_page() Sergey Senozhatsky
2026-11-05 4:47 ` [PATCH 2/7] zram: drop get_slot_handle() and set_slot_handle() Sergey Senozhatsky
@ 2026-11-05 4:47 ` Sergey Senozhatsky
2026-11-05 17:03 ` Yosry Ahmed
2026-11-05 4:47 ` [PATCH 4/7] zram: allocate zsmalloc object under slot lock Sergey Senozhatsky
` (4 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Sergey Senozhatsky @ 2026-11-05 4:47 UTC (permalink / raw)
To: Andrew Morton
Cc: Nhat Pham, Yosry Ahmed, Johannes Weiner, Brian Geffon,
Minchan Kim, Richard Chang, linux-kernel, linux-block, linux-mm,
Sergey Senozhatsky
zsmalloc handle is an unsigned long that holds an address of a
handle_cachep object, so zsmalloc and its users have to cast it
back and forth. Introduce struct zs_handle and switch zsmalloc,
zram and zswap to struct zs_handle pointers.
This is a preparation for the next patches, which embed struct
zs_handle into zram and zswap entries. No functional change.
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
drivers/block/zram/zram_drv.c | 28 ++++++------
drivers/block/zram/zram_drv.h | 2 +-
include/linux/zsmalloc.h | 13 +++---
mm/zsmalloc.c | 83 +++++++++++++++++++----------------
mm/zswap.c | 8 ++--
5 files changed, 72 insertions(+), 62 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 5c13addd22f6..2c2e11b2b663 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -2030,7 +2030,7 @@ static bool zram_meta_alloc(struct zram *zram, u64 disksize)
static void slot_free(struct zram *zram, unsigned long index)
{
- unsigned long handle;
+ struct zs_handle *handle;
#ifdef CONFIG_ZRAM_TRACK_ENTRY_ACTIME
zram->table[index].attr.ac_time = 0;
@@ -2096,7 +2096,7 @@ static int read_incompressible_page(struct zram *zram, struct page *page,
unsigned long index)
{
struct scatterlist sg[2];
- unsigned long handle;
+ struct zs_handle *handle;
void *src, *dst;
handle = zram->table[index].handle;
@@ -2118,7 +2118,7 @@ static int read_compressed_page(struct zram *zram, struct page *page,
enum zcomp_dstrm_mode mode = ZCOMP_DSTRM_EXCLUSIVE;
struct zcomp_dstrm *zstrm;
struct scatterlist sg[2];
- unsigned long handle;
+ struct zs_handle *handle;
unsigned int size;
void *dst;
int ret, prio;
@@ -2146,7 +2146,7 @@ static int read_from_zspool_raw(struct zram *zram, struct page *page,
unsigned long index)
{
struct scatterlist sg[2];
- unsigned long handle;
+ struct zs_handle *handle;
unsigned int size;
void *dst;
@@ -2257,14 +2257,14 @@ static int write_same_filled_page(struct zram *zram, unsigned long fill,
static int write_incompressible_page(struct zram *zram, struct page *page,
unsigned long index)
{
- unsigned long handle;
+ struct zs_handle *handle;
void *src;
handle = zs_malloc(zram->mem_pool, PAGE_SIZE,
GFP_NOIO | __GFP_NOWARN |
__GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
- if (IS_ERR_VALUE(handle))
- return PTR_ERR((void *)handle);
+ if (IS_ERR(handle))
+ return PTR_ERR(handle);
if (!zram_can_store_page(zram)) {
zs_free(zram->mem_pool, handle);
@@ -2295,7 +2295,7 @@ static int zram_write_page(struct zram *zram, struct page *page,
unsigned long index)
{
int ret = 0;
- unsigned long handle;
+ struct zs_handle *handle;
unsigned int comp_len;
void *mem;
struct zcomp_cstrm *zstrm;
@@ -2328,9 +2328,9 @@ static int zram_write_page(struct zram *zram, struct page *page,
handle = zs_malloc(zram->mem_pool, comp_len,
GFP_NOIO | __GFP_NOWARN |
__GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
- if (IS_ERR_VALUE(handle)) {
+ if (IS_ERR(handle)) {
zcomp_cstrm_put(zstrm);
- return PTR_ERR((void *)handle);
+ return PTR_ERR(handle);
}
if (!zram_can_store_page(zram)) {
@@ -2451,8 +2451,8 @@ static int recompress_slot(struct zram *zram, unsigned long index,
u32 threshold, u32 prio)
{
struct zcomp_cstrm *zstrm = NULL;
- unsigned long handle_old;
- unsigned long handle_new;
+ struct zs_handle *handle_old;
+ struct zs_handle *handle_new;
unsigned int comp_len_old;
unsigned int comp_len_new;
unsigned int class_index_old;
@@ -2534,9 +2534,9 @@ static int recompress_slot(struct zram *zram, unsigned long index,
GFP_NOIO | __GFP_NOWARN |
__GFP_HIGHMEM | __GFP_MOVABLE,
page_to_nid(page));
- if (IS_ERR_VALUE(handle_new)) {
+ if (IS_ERR(handle_new)) {
zcomp_cstrm_put(zstrm);
- return PTR_ERR((void *)handle_new);
+ return PTR_ERR(handle_new);
}
zs_obj_write(zram->mem_pool, handle_new, zstrm->buffer, comp_len_new);
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index 5416e25e64e8..5b211a9f314c 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -79,7 +79,7 @@ enum zram_pageflags {
*/
struct zram_table_entry {
union {
- unsigned long handle;
+ struct zs_handle *handle;
unsigned long element;
unsigned long blk_idx;
};
diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
index 5b7298a5026b..693c9484b638 100644
--- a/include/linux/zsmalloc.h
+++ b/include/linux/zsmalloc.h
@@ -22,14 +22,15 @@ struct zs_pool_stats {
};
struct zs_pool;
+struct zs_handle;
struct scatterlist;
struct zs_pool *zs_create_pool(const char *name);
void zs_destroy_pool(struct zs_pool *pool);
-unsigned long zs_malloc(struct zs_pool *pool, size_t size, gfp_t flags,
- const int nid);
-void zs_free(struct zs_pool *pool, unsigned long obj);
+struct zs_handle *zs_malloc(struct zs_pool *pool, size_t size, gfp_t flags,
+ const int nid);
+void zs_free(struct zs_pool *pool, struct zs_handle *handle);
size_t zs_huge_class_size(struct zs_pool *pool);
@@ -40,10 +41,10 @@ unsigned int zs_lookup_class_index(struct zs_pool *pool, unsigned int size);
void zs_pool_stats(struct zs_pool *pool, struct zs_pool_stats *stats);
-void zs_obj_read_sg_begin(struct zs_pool *pool, unsigned long handle,
+void zs_obj_read_sg_begin(struct zs_pool *pool, struct zs_handle *handle,
struct scatterlist *sg, size_t mem_len);
-void zs_obj_read_sg_end(struct zs_pool *pool, unsigned long handle);
-void zs_obj_write(struct zs_pool *pool, unsigned long handle,
+void zs_obj_read_sg_end(struct zs_pool *pool, struct zs_handle *handle);
+void zs_obj_write(struct zs_pool *pool, struct zs_handle *handle,
void *handle_mem, size_t mem_len);
extern const struct movable_operations zsmalloc_mops;
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 7ef80e0da626..7d8aab56f5cd 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -264,6 +264,10 @@ struct link_free {
};
};
+struct zs_handle {
+ unsigned long __private obj;
+};
+
static struct kmem_cache *handle_cachep;
static struct kmem_cache *zspage_cachep;
@@ -438,16 +442,16 @@ static void init_deferred_free(struct zs_pool *pool) {}
static void SetZsPageMovable(struct zs_pool *pool, struct zspage *zspage) {}
#endif
-static unsigned long cache_alloc_handle(gfp_t gfp)
+static struct zs_handle *cache_alloc_handle(gfp_t gfp)
{
gfp = gfp & ~(__GFP_HIGHMEM | __GFP_MOVABLE);
- return (unsigned long)kmem_cache_alloc(handle_cachep, gfp);
+ return kmem_cache_alloc(handle_cachep, gfp);
}
-static void cache_free_handle(unsigned long handle)
+static void cache_free_handle(struct zs_handle *handle)
{
- kmem_cache_free(handle_cachep, (void *)handle);
+ kmem_cache_free(handle_cachep, handle);
}
static struct zspage *cache_alloc_zspage(gfp_t gfp)
@@ -466,9 +470,9 @@ static void cache_free_zspage(struct zspage *zspage)
* Pairs with READ_ONCE() in handle_to_obj(): zs_free() may read the
* handle locklessly, so prevent store tearing here.
*/
-static void record_obj(unsigned long handle, unsigned long obj)
+static void record_obj(struct zs_handle *handle, unsigned long obj)
{
- WRITE_ONCE(*(unsigned long *)handle, obj);
+ WRITE_ONCE(ACCESS_PRIVATE(handle, obj), obj);
}
/* Protected by class->lock */
@@ -817,13 +821,13 @@ static unsigned long location_to_obj(struct zpdesc *zpdesc, unsigned int obj_idx
return obj;
}
-static unsigned long handle_to_obj(unsigned long handle)
+static unsigned long handle_to_obj(struct zs_handle *handle)
{
- return READ_ONCE(*(unsigned long *)handle);
+ return READ_ONCE(ACCESS_PRIVATE(handle, obj));
}
static inline bool obj_allocated(struct zpdesc *zpdesc, void *obj,
- unsigned long *phandle)
+ struct zs_handle **phandle)
{
unsigned long handle;
struct zspage *zspage = get_zspage(zpdesc);
@@ -839,7 +843,7 @@ static inline bool obj_allocated(struct zpdesc *zpdesc, void *obj,
return false;
/* Clear all tags before returning the handle */
- *phandle = handle & ~OBJ_TAG_MASK;
+ *phandle = (struct zs_handle *)(handle & ~OBJ_TAG_MASK);
return true;
}
@@ -1122,7 +1126,7 @@ unsigned long zs_get_total_pages(struct zs_pool *pool)
}
EXPORT_SYMBOL_GPL(zs_get_total_pages);
-void zs_obj_read_sg_begin(struct zs_pool *pool, unsigned long handle,
+void zs_obj_read_sg_begin(struct zs_pool *pool, struct zs_handle *handle,
struct scatterlist *sg, size_t mem_len)
{
struct zspage *zspage;
@@ -1169,7 +1173,7 @@ void zs_obj_read_sg_begin(struct zs_pool *pool, unsigned long handle,
}
EXPORT_SYMBOL_GPL(zs_obj_read_sg_begin);
-void zs_obj_read_sg_end(struct zs_pool *pool, unsigned long handle)
+void zs_obj_read_sg_end(struct zs_pool *pool, struct zs_handle *handle)
{
struct zspage *zspage;
struct zpdesc *zpdesc;
@@ -1184,7 +1188,7 @@ void zs_obj_read_sg_end(struct zs_pool *pool, unsigned long handle)
}
EXPORT_SYMBOL_GPL(zs_obj_read_sg_end);
-void zs_obj_write(struct zs_pool *pool, unsigned long handle,
+void zs_obj_write(struct zs_pool *pool, struct zs_handle *handle,
void *handle_mem, size_t mem_len)
{
struct zspage *zspage;
@@ -1253,7 +1257,7 @@ size_t zs_huge_class_size(struct zs_pool *pool)
EXPORT_SYMBOL_GPL(zs_huge_class_size);
static unsigned long obj_malloc(struct zs_pool *pool,
- struct zspage *zspage, unsigned long handle)
+ struct zspage *zspage, struct zs_handle *handle)
{
int i, nr_zpdesc, offset;
unsigned long obj;
@@ -1280,9 +1284,10 @@ static unsigned long obj_malloc(struct zs_pool *pool,
set_freeobj(zspage, link->next >> OBJ_TAG_BITS);
if (likely(!ZsHugePage(zspage)))
/* record handle in the header of allocated chunk */
- link->handle = handle | OBJ_ALLOCATED_TAG;
+ link->handle = (unsigned long)handle | OBJ_ALLOCATED_TAG;
else
- zspage->first_zpdesc->handle = handle | OBJ_ALLOCATED_TAG;
+ zspage->first_zpdesc->handle =
+ (unsigned long)handle | OBJ_ALLOCATED_TAG;
kunmap_local(vaddr);
mod_zspage_inuse(zspage, 1);
@@ -1305,23 +1310,23 @@ static unsigned long obj_malloc(struct zs_pool *pool,
* otherwise an ERR_PTR().
* Allocation requests with size > ZS_MAX_ALLOC_SIZE will fail.
*/
-unsigned long zs_malloc(struct zs_pool *pool, size_t size, gfp_t gfp,
- const int nid)
+struct zs_handle *zs_malloc(struct zs_pool *pool, size_t size, gfp_t gfp,
+ const int nid)
{
- unsigned long handle;
+ struct zs_handle *handle;
struct size_class *class;
int newfg;
struct zspage *zspage;
if (unlikely(!size))
- return (unsigned long)ERR_PTR(-EINVAL);
+ return ERR_PTR(-EINVAL);
if (unlikely(size > ZS_MAX_ALLOC_SIZE))
- return (unsigned long)ERR_PTR(-ENOSPC);
+ return ERR_PTR(-ENOSPC);
handle = cache_alloc_handle(gfp);
if (!handle)
- return (unsigned long)ERR_PTR(-ENOMEM);
+ return ERR_PTR(-ENOMEM);
class = lookup_size_class(pool, size);
@@ -1342,7 +1347,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(handle);
- return (unsigned long)ERR_PTR(-ENOMEM);
+ return ERR_PTR(-ENOMEM);
}
spin_lock(&class->lock);
@@ -1410,9 +1415,11 @@ static unsigned int obj_to_class_idx(unsigned long obj)
* encoding is disabled), fall back to pool->lock for the lookup.
*/
#if ZS_OBJ_CLASS_BITS > 0
-static inline void obj_class_get_and_lock(struct zs_pool *pool, unsigned long handle,
- unsigned long *objp, struct zspage **zspagep,
- struct size_class **classp)
+static inline void obj_class_get_and_lock(struct zs_pool *pool,
+ struct zs_handle *handle,
+ unsigned long *objp,
+ struct zspage **zspagep,
+ struct size_class **classp)
__acquires(&(*classp)->lock)
{
struct zpdesc *f_zpdesc;
@@ -1428,9 +1435,11 @@ static inline void obj_class_get_and_lock(struct zs_pool *pool, unsigned long ha
*objp = obj;
}
#else
-static inline void obj_class_get_and_lock(struct zs_pool *pool, unsigned long handle,
- unsigned long *objp, struct zspage **zspagep,
- struct size_class **classp)
+static inline void obj_class_get_and_lock(struct zs_pool *pool,
+ struct zs_handle *handle,
+ unsigned long *objp,
+ struct zspage **zspagep,
+ struct size_class **classp)
__acquires(&(*classp)->lock)
{
struct zpdesc *f_zpdesc;
@@ -1447,7 +1456,7 @@ static inline void obj_class_get_and_lock(struct zs_pool *pool, unsigned long ha
}
#endif
-void zs_free(struct zs_pool *pool, unsigned long handle)
+void zs_free(struct zs_pool *pool, struct zs_handle *handle)
{
struct zspage *zspage;
unsigned long obj;
@@ -1455,7 +1464,7 @@ void zs_free(struct zs_pool *pool, unsigned long handle)
int fullness;
struct zspage *zspage_to_free = NULL;
- if (IS_ERR_OR_NULL((void *)handle))
+ if (IS_ERR_OR_NULL(handle))
return;
obj_class_get_and_lock(pool, handle, &obj, &zspage, &class);
@@ -1559,12 +1568,12 @@ static void zs_object_copy(struct size_class *class, unsigned long dst,
* Find alloced object in zspage from index object and
* return handle.
*/
-static unsigned long find_alloced_obj(struct size_class *class,
- struct zpdesc *zpdesc, int *obj_idx)
+static struct zs_handle *find_alloced_obj(struct size_class *class,
+ struct zpdesc *zpdesc, int *obj_idx)
{
unsigned int offset;
int index = *obj_idx;
- unsigned long handle = 0;
+ struct zs_handle *handle = NULL;
void *addr = kmap_local_zpdesc(zpdesc);
offset = get_first_obj_offset(zpdesc);
@@ -1589,7 +1598,7 @@ static void migrate_zspage(struct zs_pool *pool, struct zspage *src_zspage,
struct zspage *dst_zspage)
{
unsigned long used_obj, free_obj;
- unsigned long handle;
+ struct zs_handle *handle;
int obj_idx = 0;
struct zpdesc *s_zpdesc = get_first_zpdesc(src_zspage);
struct size_class *class = pool->size_class[src_zspage->class];
@@ -1760,7 +1769,7 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
struct zpdesc *zpdesc = page_zpdesc(page);
void *s_addr, *d_addr, *addr;
unsigned int offset;
- unsigned long handle;
+ struct zs_handle *handle;
unsigned long old_obj, new_obj;
unsigned int obj_idx;
@@ -2291,7 +2300,7 @@ static void zs_destroy_caches(void)
static int __init zs_init_caches(void)
{
- handle_cachep = kmem_cache_create("zs_handle", ZS_HANDLE_SIZE,
+ handle_cachep = kmem_cache_create("zs_handle", sizeof(struct zs_handle),
0, 0, NULL);
zspage_cachep = kmem_cache_create("zspage", sizeof(struct zspage),
0, 0, NULL);
diff --git a/mm/zswap.c b/mm/zswap.c
index ad01c691a263..397b06c9f33b 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -215,7 +215,7 @@ struct zswap_entry {
unsigned int length;
bool referenced;
u8 pool_idx;
- unsigned long handle;
+ struct zs_handle *handle;
struct obj_cgroup *objcg;
struct list_head lru;
};
@@ -888,7 +888,7 @@ static bool zswap_compress(struct folio *folio, long index,
struct scatterlist input, output;
int comp_ret = 0, alloc_ret = 0;
unsigned int dlen = PAGE_SIZE;
- unsigned long handle;
+ struct zs_handle *handle;
gfp_t gfp;
u8 *dst;
bool mapped = false;
@@ -949,8 +949,8 @@ static bool zswap_compress(struct folio *folio, long index,
gfp = GFP_NOWAIT | __GFP_NORETRY | __GFP_HIGHMEM | __GFP_MOVABLE;
handle = zs_malloc(pool->zs_pool, dlen, gfp, folio_nid(folio));
- if (IS_ERR_VALUE(handle)) {
- alloc_ret = PTR_ERR((void *)handle);
+ if (IS_ERR(handle)) {
+ alloc_ret = PTR_ERR(handle);
goto unlock;
}
--
2.56.0.398.g882cbad275-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 3/7] zsmalloc: switch handle type to struct zs_handle
2026-11-05 4:47 ` [PATCH 3/7] zsmalloc: switch handle type to struct zs_handle Sergey Senozhatsky
@ 2026-11-05 17:03 ` Yosry Ahmed
0 siblings, 0 replies; 15+ messages in thread
From: Yosry Ahmed @ 2026-11-05 17:03 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Andrew Morton, Nhat Pham, Yosry Ahmed, Johannes Weiner,
Brian Geffon, Minchan Kim, Richard Chang, linux-kernel,
linux-block, linux-mm
[..]
> diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
> index 5b7298a5026b..693c9484b638 100644
> --- a/include/linux/zsmalloc.h
> +++ b/include/linux/zsmalloc.h
> @@ -22,14 +22,15 @@ struct zs_pool_stats {
> };
>
> struct zs_pool;
> +struct zs_handle;
> struct scatterlist;
>
> struct zs_pool *zs_create_pool(const char *name);
> void zs_destroy_pool(struct zs_pool *pool);
>
> -unsigned long zs_malloc(struct zs_pool *pool, size_t size, gfp_t flags,
> - const int nid);
> -void zs_free(struct zs_pool *pool, unsigned long obj);
> +struct zs_handle *zs_malloc(struct zs_pool *pool, size_t size, gfp_t flags,
> + const int nid);
> +void zs_free(struct zs_pool *pool, struct zs_handle *handle);
>
> size_t zs_huge_class_size(struct zs_pool *pool);
>
> @@ -40,10 +41,10 @@ unsigned int zs_lookup_class_index(struct zs_pool *pool, unsigned int size);
>
> void zs_pool_stats(struct zs_pool *pool, struct zs_pool_stats *stats);
>
> -void zs_obj_read_sg_begin(struct zs_pool *pool, unsigned long handle,
> +void zs_obj_read_sg_begin(struct zs_pool *pool, struct zs_handle *handle,
> struct scatterlist *sg, size_t mem_len);
> -void zs_obj_read_sg_end(struct zs_pool *pool, unsigned long handle);
> -void zs_obj_write(struct zs_pool *pool, unsigned long handle,
> +void zs_obj_read_sg_end(struct zs_pool *pool, struct zs_handle *handle);
> +void zs_obj_write(struct zs_pool *pool, struct zs_handle *handle,
> void *handle_mem, size_t mem_len);
I would squash patch 7's constification here.
>
> extern const struct movable_operations zsmalloc_mops;
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 7ef80e0da626..7d8aab56f5cd 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -264,6 +264,10 @@ struct link_free {
> };
> };
>
> +struct zs_handle {
> + unsigned long __private obj;
> +};
> +
> static struct kmem_cache *handle_cachep;
> static struct kmem_cache *zspage_cachep;
>
> @@ -438,16 +442,16 @@ static void init_deferred_free(struct zs_pool *pool) {}
> static void SetZsPageMovable(struct zs_pool *pool, struct zspage *zspage) {}
> #endif
>
> -static unsigned long cache_alloc_handle(gfp_t gfp)
> +static struct zs_handle *cache_alloc_handle(gfp_t gfp)
> {
> gfp = gfp & ~(__GFP_HIGHMEM | __GFP_MOVABLE);
>
> - return (unsigned long)kmem_cache_alloc(handle_cachep, gfp);
> + return kmem_cache_alloc(handle_cachep, gfp);
> }
>
> -static void cache_free_handle(unsigned long handle)
> +static void cache_free_handle(struct zs_handle *handle)
> {
> - kmem_cache_free(handle_cachep, (void *)handle);
> + kmem_cache_free(handle_cachep, handle);
> }
>
> static struct zspage *cache_alloc_zspage(gfp_t gfp)
> @@ -466,9 +470,9 @@ static void cache_free_zspage(struct zspage *zspage)
> * Pairs with READ_ONCE() in handle_to_obj(): zs_free() may read the
> * handle locklessly, so prevent store tearing here.
> */
> -static void record_obj(unsigned long handle, unsigned long obj)
> +static void record_obj(struct zs_handle *handle, unsigned long obj)
> {
> - WRITE_ONCE(*(unsigned long *)handle, obj);
> + WRITE_ONCE(ACCESS_PRIVATE(handle, obj), obj);
> }
>
> /* Protected by class->lock */
> @@ -817,13 +821,13 @@ static unsigned long location_to_obj(struct zpdesc *zpdesc, unsigned int obj_idx
> return obj;
> }
>
> -static unsigned long handle_to_obj(unsigned long handle)
> +static unsigned long handle_to_obj(struct zs_handle *handle)
> {
> - return READ_ONCE(*(unsigned long *)handle);
> + return READ_ONCE(ACCESS_PRIVATE(handle, obj));
> }
>
> static inline bool obj_allocated(struct zpdesc *zpdesc, void *obj,
> - unsigned long *phandle)
> + struct zs_handle **phandle)
> {
> unsigned long handle;
> struct zspage *zspage = get_zspage(zpdesc);
> @@ -839,7 +843,7 @@ static inline bool obj_allocated(struct zpdesc *zpdesc, void *obj,
> return false;
>
> /* Clear all tags before returning the handle */
> - *phandle = handle & ~OBJ_TAG_MASK;
> + *phandle = (struct zs_handle *)(handle & ~OBJ_TAG_MASK);
> return true;
> }
Would it be cleaerer if obj_allocated() just returned the handle here? I
believe this can be done separately in a prep patch.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/7] zram: allocate zsmalloc object under slot lock
2026-11-05 4:47 [PATCH 0/7] zsmalloc: redesign object alloc API Sergey Senozhatsky
` (2 preceding siblings ...)
2026-11-05 4:47 ` [PATCH 3/7] zsmalloc: switch handle type to struct zs_handle Sergey Senozhatsky
@ 2026-11-05 4:47 ` Sergey Senozhatsky
2026-11-05 4:47 ` [PATCH 5/7] zsmalloc: embed struct zs_handle into zram and zswap entries Sergey Senozhatsky
` (3 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Sergey Senozhatsky @ 2026-11-05 4:47 UTC (permalink / raw)
To: Andrew Morton
Cc: Nhat Pham, Yosry Ahmed, Johannes Weiner, Brian Geffon,
Minchan Kim, Richard Chang, linux-kernel, linux-block, linux-mm,
Sergey Senozhatsky
zram_write_page() and write_incompressible_page() allocate and
write zsmalloc object first and take slot lock only to free the
old slot data and set the new handle. The next patch makes
zsmalloc store object location directly in zram table entry, so
we need to free the slot before we allocate a new object for it.
Free the slot and allocate the object under slot lock.
recompress_slot() already allocates objects under it, and
GFP_NOIO reclaim cannot re-enter zram write path.
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
drivers/block/zram/zram_drv.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 2c2e11b2b663..e2710ce35b1f 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -2260,14 +2260,19 @@ static int write_incompressible_page(struct zram *zram, struct page *page,
struct zs_handle *handle;
void *src;
+ slot_lock(zram, index);
+ slot_free(zram, index);
handle = zs_malloc(zram->mem_pool, PAGE_SIZE,
GFP_NOIO | __GFP_NOWARN |
__GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
- if (IS_ERR(handle))
+ if (IS_ERR(handle)) {
+ slot_unlock(zram, index);
return PTR_ERR(handle);
+ }
if (!zram_can_store_page(zram)) {
zs_free(zram->mem_pool, handle);
+ slot_unlock(zram, index);
return -ENOMEM;
}
@@ -2275,8 +2280,6 @@ static int write_incompressible_page(struct zram *zram, struct page *page,
zs_obj_write(zram->mem_pool, handle, src, PAGE_SIZE);
kunmap_local(src);
- slot_lock(zram, index);
- slot_free(zram, index);
set_slot_flag(zram, index, ZRAM_HUGE);
zram->table[index].handle = handle;
set_slot_size(zram, index, PAGE_SIZE);
@@ -2325,25 +2328,27 @@ static int zram_write_page(struct zram *zram, struct page *page,
return write_incompressible_page(zram, page, index);
}
+ slot_lock(zram, index);
+ slot_free(zram, index);
handle = zs_malloc(zram->mem_pool, comp_len,
GFP_NOIO | __GFP_NOWARN |
__GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
if (IS_ERR(handle)) {
zcomp_cstrm_put(zstrm);
+ slot_unlock(zram, index);
return PTR_ERR(handle);
}
if (!zram_can_store_page(zram)) {
zcomp_cstrm_put(zstrm);
zs_free(zram->mem_pool, handle);
+ slot_unlock(zram, index);
return -ENOMEM;
}
zs_obj_write(zram->mem_pool, handle, zstrm->buffer, comp_len);
zcomp_cstrm_put(zstrm);
- slot_lock(zram, index);
- slot_free(zram, index);
zram->table[index].handle = handle;
set_slot_size(zram, index, comp_len);
mark_slot_accessed(zram, index);
--
2.56.0.398.g882cbad275-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 5/7] zsmalloc: embed struct zs_handle into zram and zswap entries
2026-11-05 4:47 [PATCH 0/7] zsmalloc: redesign object alloc API Sergey Senozhatsky
` (3 preceding siblings ...)
2026-11-05 4:47 ` [PATCH 4/7] zram: allocate zsmalloc object under slot lock Sergey Senozhatsky
@ 2026-11-05 4:47 ` Sergey Senozhatsky
2026-11-05 17:19 ` Yosry Ahmed
2026-11-05 17:30 ` Yosry Ahmed
2026-11-05 4:47 ` [PATCH 6/7] zsmalloc: fold zs_obj_write() into zs_store() Sergey Senozhatsky
` (2 subsequent siblings)
7 siblings, 2 replies; 15+ messages in thread
From: Sergey Senozhatsky @ 2026-11-05 4:47 UTC (permalink / raw)
To: Andrew Morton
Cc: Nhat Pham, Yosry Ahmed, Johannes Weiner, Brian Geffon,
Minchan Kim, Richard Chang, linux-kernel, linux-block, linux-mm,
Sergey Senozhatsky
zsmalloc allocates an object from handle_cachep for every stored
object, just to have a stable address which compaction and page
migration can update when the object moves. Both zram and zswap
already have per-entry structures that hold the handle, so we basically
keep a pointer to a pointer, which costs 8 bytes per stored object,
slab alloc/free on every store and free, and one more memory load on
every access.
Make struct zs_handle public and embed it into zram_table_entry and
zswap_entry (their sizes don't change), so that zsmalloc writes object
location directly there, and remove handle_cachep. zs_malloc() now
takes a handle pointer and returns an error code, zs_free() resets the
handle.
Since compaction and migration update the handle in place, it must not
be on the stack (zs_malloc() warns about that) and must not be copied.
zram recompression allocates a new object before it frees the old one,
so it uses zram->recomp_handle, protected by ->dev_lock, and
zs_rebind_handle() moves the new object to the slot handle. zram
recompression is the only user of zs_rebind_handle().
This saves 8 bytes per stored page, e.g. 2MB per 1GB of swapped out
4K pages. Since handle is per-entry the memory savings scale.
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
Documentation/admin-guide/mm/slab.rst | 2 +-
Documentation/mm/zsmalloc.rst | 4 +-
drivers/block/zram/Kconfig | 1 +
drivers/block/zram/zram_drv.c | 74 ++++++++---------
drivers/block/zram/zram_drv.h | 6 +-
include/linux/zsmalloc.h | 30 ++++++-
mm/Kconfig | 3 +
mm/zsmalloc.c | 110 ++++++++++++++++----------
mm/zswap.c | 22 +++---
9 files changed, 154 insertions(+), 98 deletions(-)
diff --git a/Documentation/admin-guide/mm/slab.rst b/Documentation/admin-guide/mm/slab.rst
index 14429ab90611..57e4a118a391 100644
--- a/Documentation/admin-guide/mm/slab.rst
+++ b/Documentation/admin-guide/mm/slab.rst
@@ -96,7 +96,7 @@ except some that are deemed too performance critical and don't need to be
debugged by specifying global debug options followed by a list of slab names
with "-" as options::
- slab_debug=FZ;-,zs_handle,zspage
+ slab_debug=FZ;-,zspage
The state of each debug option for a slab can be found in the respective files
under::
diff --git a/Documentation/mm/zsmalloc.rst b/Documentation/mm/zsmalloc.rst
index d2bbecd78e14..dbb5c1a8dcc3 100644
--- a/Documentation/mm/zsmalloc.rst
+++ b/Documentation/mm/zsmalloc.rst
@@ -23,8 +23,8 @@ uncompressed form). For allocation requests larger than this size, failure
is returned (see zs_malloc).
Additionally, zs_malloc() does not return a dereferenceable pointer.
-Instead, it returns an opaque handle (unsigned long) which encodes actual
-location of the allocated object. The reason for this indirection is that
+Instead, it stores the encoded location of the allocated object in the
+provided ``struct zs_handle``. The reason for this indirection is that
zsmalloc does not keep zspages permanently mapped since that would cause
issues on 32-bit systems where the VA region for kernel space mappings
is very small. So, using the allocated memory should be done through the
diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index 402b7b175863..18da3a0333f4 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -140,6 +140,7 @@ config ZRAM_MEMORY_TRACKING
config ZRAM_MULTI_COMP
bool "Enable multiple compression streams"
depends on ZRAM
+ select ZSMALLOC_REBIND_HANDLE
help
This will enable multi-compression streams, so that ZRAM can
re-compress pages using a potentially slower but more effective
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index e2710ce35b1f..7c80db51b29a 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -951,7 +951,7 @@ static int zram_writeback_complete(struct zram *zram, struct zram_wb_req *req)
if (test_slot_flag(zram, index, ZRAM_HUGE))
atomic64_dec(&zram->stats.huge_pages);
atomic64_sub(get_slot_size(zram, index), &zram->stats.compr_data_size);
- zs_free(zram->mem_pool, zram->table[index].handle);
+ zs_free(zram->mem_pool, &zram->table[index].handle);
zram->table[index].blk_idx = req->blk_idx;
set_slot_flag(zram, index, ZRAM_WB);
@@ -2067,8 +2067,8 @@ static void slot_free(struct zram *zram, unsigned long index)
goto out;
}
- handle = zram->table[index].handle;
- if (!handle)
+ handle = &zram->table[index].handle;
+ if (!zs_handle_is_allocated(handle))
return;
zs_free(zram->mem_pool, handle);
@@ -2099,7 +2099,7 @@ static int read_incompressible_page(struct zram *zram, struct page *page,
struct zs_handle *handle;
void *src, *dst;
- handle = zram->table[index].handle;
+ handle = &zram->table[index].handle;
zs_obj_read_sg_begin(zram->mem_pool, handle, sg, PAGE_SIZE);
/* an incompressible object never spans two pages */
src = kmap_local_page(sg_page(sg));
@@ -2123,7 +2123,7 @@ static int read_compressed_page(struct zram *zram, struct page *page,
void *dst;
int ret, prio;
- handle = zram->table[index].handle;
+ handle = &zram->table[index].handle;
size = get_slot_size(zram, index);
prio = get_slot_comp_priority(zram, index);
@@ -2150,7 +2150,7 @@ static int read_from_zspool_raw(struct zram *zram, struct page *page,
unsigned int size;
void *dst;
- handle = zram->table[index].handle;
+ handle = &zram->table[index].handle;
size = get_slot_size(zram, index);
/*
@@ -2177,7 +2177,7 @@ static int read_from_zspool(struct zram *zram, struct page *page,
unsigned long index)
{
if (test_slot_flag(zram, index, ZRAM_SAME) ||
- !zram->table[index].handle)
+ !zs_handle_is_allocated(&zram->table[index].handle))
return read_same_filled_page(zram, page, index);
if (!test_slot_flag(zram, index, ZRAM_HUGE))
@@ -2257,31 +2257,31 @@ static int write_same_filled_page(struct zram *zram, unsigned long fill,
static int write_incompressible_page(struct zram *zram, struct page *page,
unsigned long index)
{
- struct zs_handle *handle;
void *src;
+ int ret;
slot_lock(zram, index);
slot_free(zram, index);
- handle = zs_malloc(zram->mem_pool, PAGE_SIZE,
- GFP_NOIO | __GFP_NOWARN |
- __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
- if (IS_ERR(handle)) {
+ ret = zs_malloc(zram->mem_pool, &zram->table[index].handle, PAGE_SIZE,
+ GFP_NOIO | __GFP_NOWARN |
+ __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
+ if (ret) {
slot_unlock(zram, index);
- return PTR_ERR(handle);
+ return ret;
}
if (!zram_can_store_page(zram)) {
- zs_free(zram->mem_pool, handle);
+ zs_free(zram->mem_pool, &zram->table[index].handle);
slot_unlock(zram, index);
return -ENOMEM;
}
src = kmap_local_page(page);
- zs_obj_write(zram->mem_pool, handle, src, PAGE_SIZE);
+ zs_obj_write(zram->mem_pool, &zram->table[index].handle, src,
+ PAGE_SIZE);
kunmap_local(src);
set_slot_flag(zram, index, ZRAM_HUGE);
- zram->table[index].handle = handle;
set_slot_size(zram, index, PAGE_SIZE);
mark_slot_accessed(zram, index);
slot_unlock(zram, index);
@@ -2298,7 +2298,6 @@ static int zram_write_page(struct zram *zram, struct page *page,
unsigned long index)
{
int ret = 0;
- struct zs_handle *handle;
unsigned int comp_len;
void *mem;
struct zcomp_cstrm *zstrm;
@@ -2330,26 +2329,26 @@ static int zram_write_page(struct zram *zram, struct page *page,
slot_lock(zram, index);
slot_free(zram, index);
- handle = zs_malloc(zram->mem_pool, comp_len,
- GFP_NOIO | __GFP_NOWARN |
- __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
- if (IS_ERR(handle)) {
+ ret = zs_malloc(zram->mem_pool, &zram->table[index].handle, comp_len,
+ GFP_NOIO | __GFP_NOWARN |
+ __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
+ if (ret) {
zcomp_cstrm_put(zstrm);
slot_unlock(zram, index);
- return PTR_ERR(handle);
+ return ret;
}
if (!zram_can_store_page(zram)) {
zcomp_cstrm_put(zstrm);
- zs_free(zram->mem_pool, handle);
+ zs_free(zram->mem_pool, &zram->table[index].handle);
slot_unlock(zram, index);
return -ENOMEM;
}
- zs_obj_write(zram->mem_pool, handle, zstrm->buffer, comp_len);
+ zs_obj_write(zram->mem_pool, &zram->table[index].handle, zstrm->buffer,
+ comp_len);
zcomp_cstrm_put(zstrm);
- zram->table[index].handle = handle;
set_slot_size(zram, index, comp_len);
mark_slot_accessed(zram, index);
slot_unlock(zram, index);
@@ -2456,8 +2455,6 @@ static int recompress_slot(struct zram *zram, unsigned long index,
u32 threshold, u32 prio)
{
struct zcomp_cstrm *zstrm = NULL;
- struct zs_handle *handle_old;
- struct zs_handle *handle_new;
unsigned int comp_len_old;
unsigned int comp_len_new;
unsigned int class_index_old;
@@ -2465,8 +2462,9 @@ static int recompress_slot(struct zram *zram, unsigned long index,
void *src;
int ret = 0;
- handle_old = zram->table[index].handle;
- if (!handle_old)
+ lockdep_assert_held_write(&zram->dev_lock);
+
+ if (!zs_handle_is_allocated(&zram->table[index].handle))
return -EINVAL;
comp_len_old = get_slot_size(zram, index);
@@ -2535,20 +2533,22 @@ static int recompress_slot(struct zram *zram, unsigned long index,
* modify zsmalloc API to return this information. For now, we will
* make do with the node of the page allocated for recompression.
*/
- handle_new = zs_malloc(zram->mem_pool, comp_len_new,
- GFP_NOIO | __GFP_NOWARN |
- __GFP_HIGHMEM | __GFP_MOVABLE,
- page_to_nid(page));
- if (IS_ERR(handle_new)) {
+ ret = zs_malloc(zram->mem_pool, &zram->recomp_handle, comp_len_new,
+ GFP_NOIO | __GFP_NOWARN |
+ __GFP_HIGHMEM | __GFP_MOVABLE,
+ page_to_nid(page));
+ if (ret) {
zcomp_cstrm_put(zstrm);
- return PTR_ERR(handle_new);
+ return ret;
}
- zs_obj_write(zram->mem_pool, handle_new, zstrm->buffer, comp_len_new);
+ zs_obj_write(zram->mem_pool, &zram->recomp_handle, zstrm->buffer,
+ comp_len_new);
zcomp_cstrm_put(zstrm);
slot_free(zram, index);
- zram->table[index].handle = handle_new;
+ zs_rebind_handle(zram->mem_pool, &zram->table[index].handle,
+ &zram->recomp_handle);
set_slot_size(zram, index, comp_len_new);
set_slot_comp_priority(zram, index, prio);
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index 5b211a9f314c..d64c99b74214 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -79,7 +79,7 @@ enum zram_pageflags {
*/
struct zram_table_entry {
union {
- struct zs_handle *handle;
+ struct zs_handle handle;
unsigned long element;
unsigned long blk_idx;
};
@@ -159,6 +159,10 @@ struct zram {
unsigned long bitmap_cursor;
unsigned long nr_pages;
#endif
+#ifdef CONFIG_ZRAM_MULTI_COMP
+ /* Pseudo slot for recompression, protected by ->dev_lock */
+ struct zs_handle recomp_handle;
+#endif
#ifdef CONFIG_ZRAM_MEMORY_TRACKING
struct dentry *debugfs_dir;
#endif
diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
index 693c9484b638..6c76fd10344e 100644
--- a/include/linux/zsmalloc.h
+++ b/include/linux/zsmalloc.h
@@ -14,6 +14,7 @@
#ifndef _ZS_MALLOC_H_
#define _ZS_MALLOC_H_
+#include <linux/compiler.h>
#include <linux/types.h>
struct zs_pool_stats {
@@ -21,16 +22,39 @@ struct zs_pool_stats {
atomic_long_t pages_compacted;
};
+/*
+ * zsmalloc writes object location into the handle and updates it when
+ * the object moves (compaction, page migration), so the handle must not
+ * be on the stack and must not be copied. Use zs_rebind_handle() to
+ * move an object to another handle.
+ */
+struct zs_handle {
+ unsigned long __private obj;
+} __aligned(sizeof(unsigned long));
+
+static inline void zs_handle_init(struct zs_handle *handle)
+{
+ ACCESS_PRIVATE(handle, obj) = 0;
+}
+
+static inline bool zs_handle_is_allocated(const struct zs_handle *handle)
+{
+ return READ_ONCE(ACCESS_PRIVATE(handle, obj)) != 0;
+}
+
struct zs_pool;
-struct zs_handle;
struct scatterlist;
struct zs_pool *zs_create_pool(const char *name);
void zs_destroy_pool(struct zs_pool *pool);
-struct zs_handle *zs_malloc(struct zs_pool *pool, size_t size, gfp_t flags,
- const int nid);
+int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size,
+ gfp_t flags, const int nid);
void zs_free(struct zs_pool *pool, struct zs_handle *handle);
+#ifdef CONFIG_ZSMALLOC_REBIND_HANDLE
+void zs_rebind_handle(struct zs_pool *pool, struct zs_handle *dst,
+ struct zs_handle *src);
+#endif
size_t zs_huge_class_size(struct zs_pool *pool);
diff --git a/mm/Kconfig b/mm/Kconfig
index afff1b25b543..2525fc40f2de 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -125,6 +125,9 @@ config ZSWAP_COMPRESSOR_DEFAULT
config ZSMALLOC
tristate
+config ZSMALLOC_REBIND_HANDLE
+ bool
+
menu "Zsmalloc allocator options"
depends on ZSMALLOC
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 7d8aab56f5cd..71e6dcb1fb3f 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -30,6 +30,7 @@
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/sched.h>
+#include <linux/sched/task_stack.h>
#include <linux/errno.h>
#include <linux/highmem.h>
#include <linux/string.h>
@@ -264,11 +265,6 @@ struct link_free {
};
};
-struct zs_handle {
- unsigned long __private obj;
-};
-
-static struct kmem_cache *handle_cachep;
static struct kmem_cache *zspage_cachep;
struct zs_pool {
@@ -442,18 +438,6 @@ static void init_deferred_free(struct zs_pool *pool) {}
static void SetZsPageMovable(struct zs_pool *pool, struct zspage *zspage) {}
#endif
-static struct zs_handle *cache_alloc_handle(gfp_t gfp)
-{
- gfp = gfp & ~(__GFP_HIGHMEM | __GFP_MOVABLE);
-
- return kmem_cache_alloc(handle_cachep, gfp);
-}
-
-static void cache_free_handle(struct zs_handle *handle)
-{
- kmem_cache_free(handle_cachep, handle);
-}
-
static struct zspage *cache_alloc_zspage(gfp_t gfp)
{
gfp = gfp & ~(__GFP_HIGHMEM | __GFP_MOVABLE);
@@ -1302,31 +1286,31 @@ static unsigned long obj_malloc(struct zs_pool *pool,
/**
* zs_malloc - Allocate block of given size from pool.
* @pool: pool to allocate from
+ * @handle: handle to store allocated object in (must not be on the stack)
* @size: size of block to allocate
* @gfp: gfp flags when allocating object
* @nid: The preferred node id to allocate new zspage (if needed)
*
- * On success, handle to the allocated object is returned,
- * otherwise an ERR_PTR().
* Allocation requests with size > ZS_MAX_ALLOC_SIZE will fail.
+ *
+ * Return: 0 on success, negative error code otherwise.
*/
-struct zs_handle *zs_malloc(struct zs_pool *pool, size_t size, gfp_t gfp,
- const int nid)
+int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size,
+ gfp_t gfp, const int nid)
{
- struct zs_handle *handle;
struct size_class *class;
int newfg;
struct zspage *zspage;
+ if (WARN_ON_ONCE(object_is_on_stack(handle) ||
+ zs_handle_is_allocated(handle)))
+ return -EINVAL;
+
if (unlikely(!size))
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
if (unlikely(size > ZS_MAX_ALLOC_SIZE))
- return ERR_PTR(-ENOSPC);
-
- handle = cache_alloc_handle(gfp);
- if (!handle)
- return ERR_PTR(-ENOMEM);
+ return -ENOSPC;
class = lookup_size_class(pool, size);
@@ -1345,10 +1329,8 @@ struct zs_handle *zs_malloc(struct zs_pool *pool, size_t size, gfp_t gfp,
spin_unlock(&class->lock);
zspage = alloc_zspage(pool, class, gfp, nid);
- if (!zspage) {
- cache_free_handle(handle);
- return ERR_PTR(-ENOMEM);
- }
+ if (!zspage)
+ return -ENOMEM;
spin_lock(&class->lock);
obj_malloc(pool, zspage, handle);
@@ -1363,7 +1345,7 @@ struct zs_handle *zs_malloc(struct zs_pool *pool, size_t size, gfp_t gfp,
out:
spin_unlock(&class->lock);
- return handle;
+ return 0;
}
EXPORT_SYMBOL_GPL(zs_malloc);
@@ -1464,13 +1446,14 @@ void zs_free(struct zs_pool *pool, struct zs_handle *handle)
int fullness;
struct zspage *zspage_to_free = NULL;
- if (IS_ERR_OR_NULL(handle))
+ if (!zs_handle_is_allocated(handle))
return;
obj_class_get_and_lock(pool, handle, &obj, &zspage, &class);
class_stat_sub(class, ZS_OBJS_INUSE, 1);
obj_free(class->size, obj);
+ record_obj(handle, 0);
fullness = fix_fullness_group(class, zspage);
if (fullness == ZS_INUSE_RATIO_0) {
@@ -1490,10 +1473,59 @@ void zs_free(struct zs_pool *pool, struct zs_handle *handle)
__free_zspage_lockless(zspage_to_free);
atomic_long_sub(class->pages_per_zspage, &pool->pages_allocated);
}
- cache_free_handle(handle);
}
EXPORT_SYMBOL_GPL(zs_free);
+#ifdef CONFIG_ZSMALLOC_REBIND_HANDLE
+/**
+ * zs_rebind_handle() - Rebind an allocated object to a new handle.
+ * @pool: pool the object belongs to
+ * @dst: unallocated destination handle (must not be on the stack)
+ * @src: source handle that currently owns the object
+ *
+ * Rebinds the object from @src to @dst, updating the object header so
+ * compaction and page migration update @dst instead of @src, and resets
+ * @src.
+ */
+void zs_rebind_handle(struct zs_pool *pool, struct zs_handle *dst,
+ struct zs_handle *src)
+{
+ struct size_class *class;
+ struct zspage *zspage;
+ unsigned long obj;
+
+ if (WARN_ON_ONCE(object_is_on_stack(dst) ||
+ zs_handle_is_allocated(dst) ||
+ !zs_handle_is_allocated(src)))
+ return;
+
+ obj_class_get_and_lock(pool, src, &obj, &zspage, &class);
+
+ if (likely(!ZsHugePage(zspage))) {
+ struct zpdesc *zpdesc;
+ struct link_free *link;
+ unsigned int obj_idx;
+ unsigned long off;
+ void *vaddr;
+
+ obj_to_location(obj, &zpdesc, &obj_idx);
+ off = offset_in_page(class->size * obj_idx);
+ vaddr = kmap_local_zpdesc(zpdesc);
+ link = (struct link_free *)(vaddr + off);
+ link->handle = (unsigned long)dst | OBJ_ALLOCATED_TAG;
+ kunmap_local(vaddr);
+ } else {
+ zspage->first_zpdesc->handle =
+ (unsigned long)dst | OBJ_ALLOCATED_TAG;
+ }
+
+ record_obj(dst, obj);
+ record_obj(src, 0);
+ spin_unlock(&class->lock);
+}
+EXPORT_SYMBOL_GPL(zs_rebind_handle);
+#endif
+
static void zs_object_copy(struct size_class *class, unsigned long dst,
unsigned long src)
{
@@ -2292,23 +2324,17 @@ EXPORT_SYMBOL_GPL(zs_destroy_pool);
static void zs_destroy_caches(void)
{
- kmem_cache_destroy(handle_cachep);
- handle_cachep = NULL;
kmem_cache_destroy(zspage_cachep);
zspage_cachep = NULL;
}
static int __init zs_init_caches(void)
{
- handle_cachep = kmem_cache_create("zs_handle", sizeof(struct zs_handle),
- 0, 0, NULL);
zspage_cachep = kmem_cache_create("zspage", sizeof(struct zspage),
0, 0, NULL);
- if (!handle_cachep || !zspage_cachep) {
- zs_destroy_caches();
+ if (!zspage_cachep)
return -ENOMEM;
- }
return 0;
}
diff --git a/mm/zswap.c b/mm/zswap.c
index 397b06c9f33b..f28a46cae1e1 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -215,7 +215,7 @@ struct zswap_entry {
unsigned int length;
bool referenced;
u8 pool_idx;
- struct zs_handle *handle;
+ struct zs_handle handle;
struct obj_cgroup *objcg;
struct list_head lru;
};
@@ -769,6 +769,7 @@ static struct zswap_entry *zswap_entry_cache_alloc(gfp_t gfp, int nid)
entry = kmem_cache_alloc_node(zswap_entry_cache, gfp, nid);
if (!entry)
return NULL;
+ zs_handle_init(&entry->handle);
return entry;
}
@@ -787,7 +788,7 @@ static void zswap_entry_free(struct zswap_entry *entry)
zswap_lru_del(entry);
if (!WARN_ON_ONCE(!pool)) {
- zs_free(pool->zs_pool, entry->handle);
+ zs_free(pool->zs_pool, &entry->handle);
zswap_pool_put(pool);
}
if (entry->objcg) {
@@ -888,7 +889,6 @@ static bool zswap_compress(struct folio *folio, long index,
struct scatterlist input, output;
int comp_ret = 0, alloc_ret = 0;
unsigned int dlen = PAGE_SIZE;
- struct zs_handle *handle;
gfp_t gfp;
u8 *dst;
bool mapped = false;
@@ -948,14 +948,12 @@ static bool zswap_compress(struct folio *folio, long index,
}
gfp = GFP_NOWAIT | __GFP_NORETRY | __GFP_HIGHMEM | __GFP_MOVABLE;
- handle = zs_malloc(pool->zs_pool, dlen, gfp, folio_nid(folio));
- if (IS_ERR(handle)) {
- alloc_ret = PTR_ERR(handle);
+ alloc_ret = zs_malloc(pool->zs_pool, &entry->handle, dlen, gfp,
+ folio_nid(folio));
+ if (alloc_ret)
goto unlock;
- }
- zs_obj_write(pool->zs_pool, handle, dst, dlen);
- entry->handle = handle;
+ zs_obj_write(pool->zs_pool, &entry->handle, dst, dlen);
entry->length = dlen;
unlock:
@@ -980,7 +978,7 @@ static bool __zswap_decompress(struct zswap_entry *entry,
struct scatterlist output;
int ret = 0, dlen;
- zs_obj_read_sg_begin(pool->zs_pool, entry->handle, input, entry->length);
+ zs_obj_read_sg_begin(pool->zs_pool, &entry->handle, input, entry->length);
/* zswap entries of length PAGE_SIZE are not compressed. */
if (entry->length == PAGE_SIZE) {
@@ -1002,7 +1000,7 @@ static bool __zswap_decompress(struct zswap_entry *entry,
dlen = req->dlen;
}
- zs_obj_read_sg_end(pool->zs_pool, entry->handle);
+ zs_obj_read_sg_end(pool->zs_pool, &entry->handle);
if (!ret && dlen == PAGE_SIZE)
return true;
@@ -1575,7 +1573,7 @@ static bool zswap_store_page(struct folio *folio, long index,
return true;
store_failed:
- zs_free(pool->zs_pool, entry->handle);
+ zs_free(pool->zs_pool, &entry->handle);
compress_failed:
zswap_entry_cache_free(entry);
return false;
--
2.56.0.398.g882cbad275-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 5/7] zsmalloc: embed struct zs_handle into zram and zswap entries
2026-11-05 4:47 ` [PATCH 5/7] zsmalloc: embed struct zs_handle into zram and zswap entries Sergey Senozhatsky
@ 2026-11-05 17:19 ` Yosry Ahmed
2026-11-05 17:30 ` Yosry Ahmed
1 sibling, 0 replies; 15+ messages in thread
From: Yosry Ahmed @ 2026-11-05 17:19 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Andrew Morton, Nhat Pham, Yosry Ahmed, Johannes Weiner,
Brian Geffon, Minchan Kim, Richard Chang, linux-kernel,
linux-block, linux-mm
On Thu, Nov 05, 2026 at 01:47:35PM +0900, Sergey Senozhatsky wrote:
> zsmalloc allocates an object from handle_cachep for every stored
> object, just to have a stable address which compaction and page
> migration can update when the object moves. Both zram and zswap
> already have per-entry structures that hold the handle, so we basically
> keep a pointer to a pointer, which costs 8 bytes per stored object,
> slab alloc/free on every store and free, and one more memory load on
> every access.
>
> Make struct zs_handle public and embed it into zram_table_entry and
> zswap_entry (their sizes don't change), so that zsmalloc writes object
> location directly there, and remove handle_cachep. zs_malloc() now
> takes a handle pointer and returns an error code, zs_free() resets the
> handle.
>
> Since compaction and migration update the handle in place, it must not
> be on the stack (zs_malloc() warns about that) and must not be copied.
> zram recompression allocates a new object before it frees the old one,
> so it uses zram->recomp_handle, protected by ->dev_lock, and
> zs_rebind_handle() moves the new object to the slot handle. zram
> recompression is the only user of zs_rebind_handle().
>
> This saves 8 bytes per stored page, e.g. 2MB per 1GB of swapped out
> 4K pages. Since handle is per-entry the memory savings scale.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
> Documentation/admin-guide/mm/slab.rst | 2 +-
> Documentation/mm/zsmalloc.rst | 4 +-
> drivers/block/zram/Kconfig | 1 +
> drivers/block/zram/zram_drv.c | 74 ++++++++---------
> drivers/block/zram/zram_drv.h | 6 +-
> include/linux/zsmalloc.h | 30 ++++++-
> mm/Kconfig | 3 +
> mm/zsmalloc.c | 110 ++++++++++++++++----------
> mm/zswap.c | 22 +++---
> 9 files changed, 154 insertions(+), 98 deletions(-)
>
> diff --git a/Documentation/admin-guide/mm/slab.rst b/Documentation/admin-guide/mm/slab.rst
> index 14429ab90611..57e4a118a391 100644
> --- a/Documentation/admin-guide/mm/slab.rst
> +++ b/Documentation/admin-guide/mm/slab.rst
> @@ -96,7 +96,7 @@ except some that are deemed too performance critical and don't need to be
> debugged by specifying global debug options followed by a list of slab names
> with "-" as options::
>
> - slab_debug=FZ;-,zs_handle,zspage
> + slab_debug=FZ;-,zspage
>
> The state of each debug option for a slab can be found in the respective files
> under::
> diff --git a/Documentation/mm/zsmalloc.rst b/Documentation/mm/zsmalloc.rst
> index d2bbecd78e14..dbb5c1a8dcc3 100644
> --- a/Documentation/mm/zsmalloc.rst
> +++ b/Documentation/mm/zsmalloc.rst
> @@ -23,8 +23,8 @@ uncompressed form). For allocation requests larger than this size, failure
> is returned (see zs_malloc).
>
> Additionally, zs_malloc() does not return a dereferenceable pointer.
> -Instead, it returns an opaque handle (unsigned long) which encodes actual
> -location of the allocated object. The reason for this indirection is that
> +Instead, it stores the encoded location of the allocated object in the
> +provided ``struct zs_handle``. The reason for this indirection is that
Should we explicitly call out that zsmalloc will keep a back pointer to
the handle and update it asynchronously, so the owner shouldn't modify
it, and should keep it allocated until zs_free() (or rebind)?
Probably somewhere in the code comments too?
> zsmalloc does not keep zspages permanently mapped since that would cause
> issues on 32-bit systems where the VA region for kernel space mappings
> is very small. So, using the allocated memory should be done through the
> diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
> index 693c9484b638..6c76fd10344e 100644
> --- a/include/linux/zsmalloc.h
> +++ b/include/linux/zsmalloc.h
> @@ -14,6 +14,7 @@
> #ifndef _ZS_MALLOC_H_
> #define _ZS_MALLOC_H_
>
> +#include <linux/compiler.h>
> #include <linux/types.h>
>
> struct zs_pool_stats {
> @@ -21,16 +22,39 @@ struct zs_pool_stats {
> atomic_long_t pages_compacted;
> };
>
> +/*
> + * zsmalloc writes object location into the handle and updates it when
> + * the object moves (compaction, page migration), so the handle must not
> + * be on the stack and must not be copied. Use zs_rebind_handle() to
> + * move an object to another handle.
> + */
> +struct zs_handle {
> + unsigned long __private obj;
> +} __aligned(sizeof(unsigned long));
Is alignment guaranteed even if this is embedded in a packed struct? I
wonder if we should have an explicit alignment check in
zs_malloc()/zs_store(), and probably rebind too?
> +
> +static inline void zs_handle_init(struct zs_handle *handle)
> +{
> + ACCESS_PRIVATE(handle, obj) = 0;
> +}
> +
> +static inline bool zs_handle_is_allocated(const struct zs_handle *handle)
> +{
> + return READ_ONCE(ACCESS_PRIVATE(handle, obj)) != 0;
> +}
Can this ever give a false negative? I believe that's only possible with
pfn=0, I don't know if this can be the case on any architecture. Should
we use something like OBJ_ALLOCATED_TAG?
> +
> struct zs_pool;
> -struct zs_handle;
> struct scatterlist;
>
> struct zs_pool *zs_create_pool(const char *name);
> void zs_destroy_pool(struct zs_pool *pool);
>
> -struct zs_handle *zs_malloc(struct zs_pool *pool, size_t size, gfp_t flags,
> - const int nid);
> +int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size,
> + gfp_t flags, const int nid);
> void zs_free(struct zs_pool *pool, struct zs_handle *handle);
> +#ifdef CONFIG_ZSMALLOC_REBIND_HANDLE
FWIW, I don't think this is buying as much. It's one function that is
only called by zram anyway.
> +void zs_rebind_handle(struct zs_pool *pool, struct zs_handle *dst,
> + struct zs_handle *src);
A few comments in zsmalloc.c mention zs_free() reading the handle
locklessly, do these apply to rebinding as well?
> +#endif
>
> size_t zs_huge_class_size(struct zs_pool *pool);
[..]
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 397b06c9f33b..f28a46cae1e1 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -215,7 +215,7 @@ struct zswap_entry {
> unsigned int length;
> bool referenced;
> u8 pool_idx;
> - struct zs_handle *handle;
> + struct zs_handle handle;
> struct obj_cgroup *objcg;
> struct list_head lru;
> };
> @@ -769,6 +769,7 @@ static struct zswap_entry *zswap_entry_cache_alloc(gfp_t gfp, int nid)
> entry = kmem_cache_alloc_node(zswap_entry_cache, gfp, nid);
> if (!entry)
> return NULL;
> + zs_handle_init(&entry->handle);
> return entry;
> }
>
> @@ -787,7 +788,7 @@ static void zswap_entry_free(struct zswap_entry *entry)
>
> zswap_lru_del(entry);
> if (!WARN_ON_ONCE(!pool)) {
This check is mostly defensive, and if the pool was ever NULL it
prevented a NULL deref, and potentially leaked the underlying zsmalloc
object, which is mostly fine as the pool being NULL points to a larger
problem (e.g. corruption).
However, if that happens now, the handle pointer in zsmalloc would point
at the zswap entry after it's freed by zswap_entry_cache_free() below,
potentially causing a UAF. It's arguably fine as it's just a cascade of
corruptions, but I wonder if we should just bail early and skip all
cleanups below if the pool is NULL.
We'll leak the cgroup charge, stats, and the zswap entry itself -- but
that's arguably the safer option if we already have a corruption
problem.
We can also just delete the NULL check, but it's cheap to keep and I'd
rather have it tbh.
> - zs_free(pool->zs_pool, entry->handle);
> + zs_free(pool->zs_pool, &entry->handle);
> zswap_pool_put(pool);
> }
> if (entry->objcg) {
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 5/7] zsmalloc: embed struct zs_handle into zram and zswap entries
2026-11-05 4:47 ` [PATCH 5/7] zsmalloc: embed struct zs_handle into zram and zswap entries Sergey Senozhatsky
2026-11-05 17:19 ` Yosry Ahmed
@ 2026-11-05 17:30 ` Yosry Ahmed
1 sibling, 0 replies; 15+ messages in thread
From: Yosry Ahmed @ 2026-11-05 17:30 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Andrew Morton, Nhat Pham, Yosry Ahmed, Johannes Weiner,
Brian Geffon, Minchan Kim, Richard Chang, linux-kernel,
linux-block, linux-mm
> +#ifdef CONFIG_ZSMALLOC_REBIND_HANDLE
> +/**
> + * zs_rebind_handle() - Rebind an allocated object to a new handle.
> + * @pool: pool the object belongs to
> + * @dst: unallocated destination handle (must not be on the stack)
> + * @src: source handle that currently owns the object
> + *
> + * Rebinds the object from @src to @dst, updating the object header so
> + * compaction and page migration update @dst instead of @src, and resets
> + * @src.
> + */
> +void zs_rebind_handle(struct zs_pool *pool, struct zs_handle *dst,
> + struct zs_handle *src)
> +{
> + struct size_class *class;
> + struct zspage *zspage;
> + unsigned long obj;
> +
> + if (WARN_ON_ONCE(object_is_on_stack(dst) ||
> + zs_handle_is_allocated(dst) ||
> + !zs_handle_is_allocated(src)))
> + return;
> +
> + obj_class_get_and_lock(pool, src, &obj, &zspage, &class);
> +
> + if (likely(!ZsHugePage(zspage))) {
> + struct zpdesc *zpdesc;
> + struct link_free *link;
> + unsigned int obj_idx;
> + unsigned long off;
> + void *vaddr;
I think this would be easier to read if you move the declarations to the
top of the function, so that the different logic in the branches is more
obvious.
> +
> + obj_to_location(obj, &zpdesc, &obj_idx);
> + off = offset_in_page(class->size * obj_idx);
> + vaddr = kmap_local_zpdesc(zpdesc);
> + link = (struct link_free *)(vaddr + off);
> + link->handle = (unsigned long)dst | OBJ_ALLOCATED_TAG;
> + kunmap_local(vaddr);
> + } else {
> + zspage->first_zpdesc->handle =
> + (unsigned long)dst | OBJ_ALLOCATED_TAG;
> + }
> +
> + record_obj(dst, obj);
> + record_obj(src, 0);
> + spin_unlock(&class->lock);
> +}
> +EXPORT_SYMBOL_GPL(zs_rebind_handle);
> +#endif
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 6/7] zsmalloc: fold zs_obj_write() into zs_store()
2026-11-05 4:47 [PATCH 0/7] zsmalloc: redesign object alloc API Sergey Senozhatsky
` (4 preceding siblings ...)
2026-11-05 4:47 ` [PATCH 5/7] zsmalloc: embed struct zs_handle into zram and zswap entries Sergey Senozhatsky
@ 2026-11-05 4:47 ` Sergey Senozhatsky
2026-11-05 17:29 ` Yosry Ahmed
2026-11-05 4:47 ` [PATCH 7/7] zsmalloc: constify struct zs_handle pointer in read-only functions Sergey Senozhatsky
2026-11-05 17:31 ` [PATCH 0/7] zsmalloc: redesign object alloc API Yosry Ahmed
7 siblings, 1 reply; 15+ messages in thread
From: Sergey Senozhatsky @ 2026-11-05 4:47 UTC (permalink / raw)
To: Andrew Morton
Cc: Nhat Pham, Yosry Ahmed, Johannes Weiner, Brian Geffon,
Minchan Kim, Richard Chang, linux-kernel, linux-block, linux-mm,
Sergey Senozhatsky
zram and zswap always call zs_obj_write() right after zs_malloc(), and
zs_obj_write() has to take pool->lock and look up object location
again, which zs_malloc() just did.
Replace zs_malloc() and zs_obj_write() with one single zs_store()
call.
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
Documentation/core-api/memory-allocation.rst | 2 +-
Documentation/mm/zsmalloc.rst | 4 +-
drivers/block/zram/zram_drv.c | 43 ++++++-----------
include/linux/zsmalloc.h | 6 +--
mm/zsmalloc.c | 51 +++++++++-----------
mm/zswap.c | 5 +-
6 files changed, 46 insertions(+), 65 deletions(-)
diff --git a/Documentation/core-api/memory-allocation.rst b/Documentation/core-api/memory-allocation.rst
index 648222b6c857..5069ac72b857 100644
--- a/Documentation/core-api/memory-allocation.rst
+++ b/Documentation/core-api/memory-allocation.rst
@@ -9,7 +9,7 @@ allocate small chunks using `kmalloc` or `kmem_cache_alloc` families,
large virtually contiguous areas using `vmalloc` and its derivatives,
or you can directly request pages from the page allocator with
`alloc_pages`. It is also possible to use more specialized allocators,
-for instance `cma_alloc` or `zs_malloc`.
+for instance `cma_alloc` or `zs_store`.
Most of the memory allocation APIs use GFP flags to express how that
memory should be allocated. The GFP acronym stands for "get free
diff --git a/Documentation/mm/zsmalloc.rst b/Documentation/mm/zsmalloc.rst
index dbb5c1a8dcc3..52eaba8f97e9 100644
--- a/Documentation/mm/zsmalloc.rst
+++ b/Documentation/mm/zsmalloc.rst
@@ -20,9 +20,9 @@ For simplicity, zsmalloc can only allocate objects of size up to PAGE_SIZE
since this satisfies the requirements of all its current users (in the
worst case, page is incompressible and is thus stored "as-is" i.e. in
uncompressed form). For allocation requests larger than this size, failure
-is returned (see zs_malloc).
+is returned (see zs_store).
-Additionally, zs_malloc() does not return a dereferenceable pointer.
+Additionally, zs_store() does not return a dereferenceable pointer.
Instead, it stores the encoded location of the allocated object in the
provided ``struct zs_handle``. The reason for this indirection is that
zsmalloc does not keep zspages permanently mapped since that would cause
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 7c80db51b29a..223e4151fc9d 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -2262,9 +2262,11 @@ static int write_incompressible_page(struct zram *zram, struct page *page,
slot_lock(zram, index);
slot_free(zram, index);
- ret = zs_malloc(zram->mem_pool, &zram->table[index].handle, PAGE_SIZE,
- GFP_NOIO | __GFP_NOWARN |
- __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
+ src = kmap_local_page(page);
+ ret = zs_store(zram->mem_pool, &zram->table[index].handle, src,
+ PAGE_SIZE, GFP_NOIO | __GFP_NOWARN |
+ __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
+ kunmap_local(src);
if (ret) {
slot_unlock(zram, index);
return ret;
@@ -2276,11 +2278,6 @@ static int write_incompressible_page(struct zram *zram, struct page *page,
return -ENOMEM;
}
- src = kmap_local_page(page);
- zs_obj_write(zram->mem_pool, &zram->table[index].handle, src,
- PAGE_SIZE);
- kunmap_local(src);
-
set_slot_flag(zram, index, ZRAM_HUGE);
set_slot_size(zram, index, PAGE_SIZE);
mark_slot_accessed(zram, index);
@@ -2329,26 +2326,22 @@ static int zram_write_page(struct zram *zram, struct page *page,
slot_lock(zram, index);
slot_free(zram, index);
- ret = zs_malloc(zram->mem_pool, &zram->table[index].handle, comp_len,
- GFP_NOIO | __GFP_NOWARN |
- __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
+ ret = zs_store(zram->mem_pool, &zram->table[index].handle,
+ zstrm->buffer, comp_len,
+ GFP_NOIO | __GFP_NOWARN |
+ __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
+ zcomp_cstrm_put(zstrm);
if (ret) {
- zcomp_cstrm_put(zstrm);
slot_unlock(zram, index);
return ret;
}
if (!zram_can_store_page(zram)) {
- zcomp_cstrm_put(zstrm);
zs_free(zram->mem_pool, &zram->table[index].handle);
slot_unlock(zram, index);
return -ENOMEM;
}
- zs_obj_write(zram->mem_pool, &zram->table[index].handle, zstrm->buffer,
- comp_len);
- zcomp_cstrm_put(zstrm);
-
set_slot_size(zram, index, comp_len);
mark_slot_accessed(zram, index);
slot_unlock(zram, index);
@@ -2533,18 +2526,12 @@ static int recompress_slot(struct zram *zram, unsigned long index,
* modify zsmalloc API to return this information. For now, we will
* make do with the node of the page allocated for recompression.
*/
- ret = zs_malloc(zram->mem_pool, &zram->recomp_handle, comp_len_new,
- GFP_NOIO | __GFP_NOWARN |
- __GFP_HIGHMEM | __GFP_MOVABLE,
- page_to_nid(page));
- if (ret) {
- zcomp_cstrm_put(zstrm);
- return ret;
- }
-
- zs_obj_write(zram->mem_pool, &zram->recomp_handle, zstrm->buffer,
- comp_len_new);
+ ret = zs_store(zram->mem_pool, &zram->recomp_handle, zstrm->buffer,
+ comp_len_new, GFP_NOIO | __GFP_NOWARN |
+ __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));
zcomp_cstrm_put(zstrm);
+ if (ret)
+ return ret;
slot_free(zram, index);
zs_rebind_handle(zram->mem_pool, &zram->table[index].handle,
diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
index 6c76fd10344e..674358f39191 100644
--- a/include/linux/zsmalloc.h
+++ b/include/linux/zsmalloc.h
@@ -48,8 +48,8 @@ struct scatterlist;
struct zs_pool *zs_create_pool(const char *name);
void zs_destroy_pool(struct zs_pool *pool);
-int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size,
- gfp_t flags, const int nid);
+int zs_store(struct zs_pool *pool, struct zs_handle *handle, const void *src,
+ size_t size, gfp_t flags, const int nid);
void zs_free(struct zs_pool *pool, struct zs_handle *handle);
#ifdef CONFIG_ZSMALLOC_REBIND_HANDLE
void zs_rebind_handle(struct zs_pool *pool, struct zs_handle *dst,
@@ -68,8 +68,6 @@ void zs_pool_stats(struct zs_pool *pool, struct zs_pool_stats *stats);
void zs_obj_read_sg_begin(struct zs_pool *pool, struct zs_handle *handle,
struct scatterlist *sg, size_t mem_len);
void zs_obj_read_sg_end(struct zs_pool *pool, struct zs_handle *handle);
-void zs_obj_write(struct zs_pool *pool, struct zs_handle *handle,
- void *handle_mem, size_t mem_len);
extern const struct movable_operations zsmalloc_mops;
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 71e6dcb1fb3f..ff1cde4c5dc2 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1172,26 +1172,15 @@ void zs_obj_read_sg_end(struct zs_pool *pool, struct zs_handle *handle)
}
EXPORT_SYMBOL_GPL(zs_obj_read_sg_end);
-void zs_obj_write(struct zs_pool *pool, struct zs_handle *handle,
- void *handle_mem, size_t mem_len)
+static void obj_write(struct size_class *class, struct zspage *zspage,
+ unsigned long obj, const void *handle_mem,
+ size_t mem_len)
{
- struct zspage *zspage;
struct zpdesc *zpdesc;
- unsigned long obj, off;
+ unsigned long off;
unsigned int obj_idx;
- struct size_class *class;
- /* Guarantee we can get zspage from handle safely */
- read_lock(&pool->lock);
- obj = handle_to_obj(handle);
obj_to_location(obj, &zpdesc, &obj_idx);
- zspage = get_zspage(zpdesc);
-
- /* Make sure migration doesn't move any pages in this zspage */
- zspage_read_lock(zspage);
- read_unlock(&pool->lock);
-
- class = zspage_class(pool, zspage);
off = offset_in_page(class->size * obj_idx);
if (!ZsHugePage(zspage))
@@ -1216,10 +1205,7 @@ void zs_obj_write(struct zs_pool *pool, struct zs_handle *handle,
memcpy_to_page(zpdesc_page(zpdesc), 0,
handle_mem + sizes[0], sizes[1]);
}
-
- zspage_read_unlock(zspage);
}
-EXPORT_SYMBOL_GPL(zs_obj_write);
/**
* zs_huge_class_size() - Returns the size (in bytes) of the first huge
@@ -1284,9 +1270,10 @@ static unsigned long obj_malloc(struct zs_pool *pool,
/**
- * zs_malloc - Allocate block of given size from pool.
+ * zs_store() - Allocate block of given size from pool and copy data into it.
* @pool: pool to allocate from
* @handle: handle to store allocated object in (must not be on the stack)
+ * @src: data to copy into the allocated object
* @size: size of block to allocate
* @gfp: gfp flags when allocating object
* @nid: The preferred node id to allocate new zspage (if needed)
@@ -1295,10 +1282,11 @@ static unsigned long obj_malloc(struct zs_pool *pool,
*
* Return: 0 on success, negative error code otherwise.
*/
-int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size,
- gfp_t gfp, const int nid)
+int zs_store(struct zs_pool *pool, struct zs_handle *handle, const void *src,
+ size_t size, gfp_t gfp, const int nid)
{
struct size_class *class;
+ unsigned long obj;
int newfg;
struct zspage *zspage;
@@ -1318,12 +1306,16 @@ int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size,
spin_lock(&class->lock);
zspage = find_get_zspage(class);
if (likely(zspage)) {
- obj_malloc(pool, zspage, handle);
+ obj = obj_malloc(pool, zspage, handle);
/* Now move the zspage to another fullness group, if required */
fix_fullness_group(class, zspage);
class_stat_add(class, ZS_OBJS_INUSE, 1);
+ zspage_read_lock(zspage);
+ spin_unlock(&class->lock);
- goto out;
+ obj_write(class, zspage, obj, src, size);
+ zspage_read_unlock(zspage);
+ return 0;
}
spin_unlock(&class->lock);
@@ -1332,8 +1324,14 @@ int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size,
if (!zspage)
return -ENOMEM;
+ /*
+ * New zspage is not in class->fullness_list or movable yet, so we
+ * can allocate the object and copy data before taking class->lock.
+ */
+ obj = obj_malloc(pool, zspage, handle);
+ obj_write(class, zspage, obj, src, size);
+
spin_lock(&class->lock);
- obj_malloc(pool, zspage, handle);
newfg = get_fullness_group(class, zspage);
insert_zspage(class, zspage, newfg);
atomic_long_add(class->pages_per_zspage, &pool->pages_allocated);
@@ -1342,12 +1340,11 @@ int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size,
/* We completely set up zspage so mark them as movable */
SetZsPageMovable(pool, zspage);
-out:
spin_unlock(&class->lock);
return 0;
}
-EXPORT_SYMBOL_GPL(zs_malloc);
+EXPORT_SYMBOL_GPL(zs_store);
static void obj_free(int class_size, unsigned long obj)
{
@@ -2224,7 +2221,7 @@ struct zs_pool *zs_create_pool(const char *name)
huge_class_size = size;
/*
* The object uses ZS_HANDLE_SIZE bytes to store the
- * handle. We need to subtract it, because zs_malloc()
+ * handle. We need to subtract it, because zs_store()
* unconditionally adds handle size before it performs
* size class search - so object may be smaller than
* huge class size, yet it still can end up in the huge
diff --git a/mm/zswap.c b/mm/zswap.c
index f28a46cae1e1..f4cfc552a6fc 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -948,12 +948,11 @@ static bool zswap_compress(struct folio *folio, long index,
}
gfp = GFP_NOWAIT | __GFP_NORETRY | __GFP_HIGHMEM | __GFP_MOVABLE;
- alloc_ret = zs_malloc(pool->zs_pool, &entry->handle, dlen, gfp,
- folio_nid(folio));
+ alloc_ret = zs_store(pool->zs_pool, &entry->handle, dst, dlen, gfp,
+ folio_nid(folio));
if (alloc_ret)
goto unlock;
- zs_obj_write(pool->zs_pool, &entry->handle, dst, dlen);
entry->length = dlen;
unlock:
--
2.56.0.398.g882cbad275-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 6/7] zsmalloc: fold zs_obj_write() into zs_store()
2026-11-05 4:47 ` [PATCH 6/7] zsmalloc: fold zs_obj_write() into zs_store() Sergey Senozhatsky
@ 2026-11-05 17:29 ` Yosry Ahmed
0 siblings, 0 replies; 15+ messages in thread
From: Yosry Ahmed @ 2026-11-05 17:29 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Andrew Morton, Nhat Pham, Yosry Ahmed, Johannes Weiner,
Brian Geffon, Minchan Kim, Richard Chang, linux-kernel,
linux-block, linux-mm
On Thu, Nov 05, 2026 at 01:47:36PM +0900, Sergey Senozhatsky wrote:
> zram and zswap always call zs_obj_write() right after zs_malloc(), and
> zs_obj_write() has to take pool->lock and look up object location
> again, which zs_malloc() just did.
>
> Replace zs_malloc() and zs_obj_write() with one single zs_store()
> call.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> ---
> Documentation/core-api/memory-allocation.rst | 2 +-
> Documentation/mm/zsmalloc.rst | 4 +-
> drivers/block/zram/zram_drv.c | 43 ++++++-----------
> include/linux/zsmalloc.h | 6 +--
> mm/zsmalloc.c | 51 +++++++++-----------
> mm/zswap.c | 5 +-
> 6 files changed, 46 insertions(+), 65 deletions(-)
>
> diff --git a/Documentation/core-api/memory-allocation.rst b/Documentation/core-api/memory-allocation.rst
> index 648222b6c857..5069ac72b857 100644
> --- a/Documentation/core-api/memory-allocation.rst
> +++ b/Documentation/core-api/memory-allocation.rst
> @@ -9,7 +9,7 @@ allocate small chunks using `kmalloc` or `kmem_cache_alloc` families,
> large virtually contiguous areas using `vmalloc` and its derivatives,
> or you can directly request pages from the page allocator with
> `alloc_pages`. It is also possible to use more specialized allocators,
> -for instance `cma_alloc` or `zs_malloc`.
> +for instance `cma_alloc` or `zs_store`.
>
> Most of the memory allocation APIs use GFP flags to express how that
> memory should be allocated. The GFP acronym stands for "get free
> diff --git a/Documentation/mm/zsmalloc.rst b/Documentation/mm/zsmalloc.rst
> index dbb5c1a8dcc3..52eaba8f97e9 100644
> --- a/Documentation/mm/zsmalloc.rst
> +++ b/Documentation/mm/zsmalloc.rst
> @@ -20,9 +20,9 @@ For simplicity, zsmalloc can only allocate objects of size up to PAGE_SIZE
> since this satisfies the requirements of all its current users (in the
> worst case, page is incompressible and is thus stored "as-is" i.e. in
> uncompressed form). For allocation requests larger than this size, failure
> -is returned (see zs_malloc).
> +is returned (see zs_store).
>
> -Additionally, zs_malloc() does not return a dereferenceable pointer.
> +Additionally, zs_store() does not return a dereferenceable pointer.
Well, it doesn't return a pointer at all now.
> Instead, it stores the encoded location of the allocated object in the
> provided ``struct zs_handle``. The reason for this indirection is that
> zsmalloc does not keep zspages permanently mapped since that would cause
[..]
> diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
> index 6c76fd10344e..674358f39191 100644
> --- a/include/linux/zsmalloc.h
> +++ b/include/linux/zsmalloc.h
> @@ -48,8 +48,8 @@ struct scatterlist;
> struct zs_pool *zs_create_pool(const char *name);
> void zs_destroy_pool(struct zs_pool *pool);
>
> -int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size,
> - gfp_t flags, const int nid);
> +int zs_store(struct zs_pool *pool, struct zs_handle *handle, const void *src,
> + size_t size, gfp_t flags, const int nid);
> void zs_free(struct zs_pool *pool, struct zs_handle *handle);
> #ifdef CONFIG_ZSMALLOC_REBIND_HANDLE
> void zs_rebind_handle(struct zs_pool *pool, struct zs_handle *dst,
> @@ -68,8 +68,6 @@ void zs_pool_stats(struct zs_pool *pool, struct zs_pool_stats *stats);
> void zs_obj_read_sg_begin(struct zs_pool *pool, struct zs_handle *handle,
> struct scatterlist *sg, size_t mem_len);
> void zs_obj_read_sg_end(struct zs_pool *pool, struct zs_handle *handle);
> -void zs_obj_write(struct zs_pool *pool, struct zs_handle *handle,
> - void *handle_mem, size_t mem_len);
>
> extern const struct movable_operations zsmalloc_mops;
>
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 71e6dcb1fb3f..ff1cde4c5dc2 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -1172,26 +1172,15 @@ void zs_obj_read_sg_end(struct zs_pool *pool, struct zs_handle *handle)
> }
> EXPORT_SYMBOL_GPL(zs_obj_read_sg_end);
>
> -void zs_obj_write(struct zs_pool *pool, struct zs_handle *handle,
> - void *handle_mem, size_t mem_len)
> +static void obj_write(struct size_class *class, struct zspage *zspage,
> + unsigned long obj, const void *handle_mem,
> + size_t mem_len)
Should handle_mem and mem_len be renamed now?
Also, there's a comment in zs_page_migrate() referring to zs_obj_read/write().
> {
> - struct zspage *zspage;
> struct zpdesc *zpdesc;
> - unsigned long obj, off;
> + unsigned long off;
> unsigned int obj_idx;
> - struct size_class *class;
>
> - /* Guarantee we can get zspage from handle safely */
> - read_lock(&pool->lock);
> - obj = handle_to_obj(handle);
> obj_to_location(obj, &zpdesc, &obj_idx);
> - zspage = get_zspage(zpdesc);
> -
> - /* Make sure migration doesn't move any pages in this zspage */
> - zspage_read_lock(zspage);
> - read_unlock(&pool->lock);
> -
> - class = zspage_class(pool, zspage);
> off = offset_in_page(class->size * obj_idx);
>
> if (!ZsHugePage(zspage))
[..]
> @@ -1318,12 +1306,16 @@ int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size,
> spin_lock(&class->lock);
> zspage = find_get_zspage(class);
> if (likely(zspage)) {
> - obj_malloc(pool, zspage, handle);
> + obj = obj_malloc(pool, zspage, handle);
> /* Now move the zspage to another fullness group, if required */
> fix_fullness_group(class, zspage);
> class_stat_add(class, ZS_OBJS_INUSE, 1);
> + zspage_read_lock(zspage);
The zspage lock is now acquired under the class lock instead of the pool
lock, this is probably fine but worth calling out explicitly in the
changelog.
> + spin_unlock(&class->lock);
>
> - goto out;
> + obj_write(class, zspage, obj, src, size);
> + zspage_read_unlock(zspage);
> + return 0;
> }
>
> spin_unlock(&class->lock);
> @@ -1332,8 +1324,14 @@ int zs_malloc(struct zs_pool *pool, struct zs_handle *handle, size_t size,
> if (!zspage)
> return -ENOMEM;
>
> + /*
> + * New zspage is not in class->fullness_list or movable yet, so we
> + * can allocate the object and copy data before taking class->lock.
> + */
> + obj = obj_malloc(pool, zspage, handle);
> + obj_write(class, zspage, obj, src, size);
> +
> spin_lock(&class->lock);
> - obj_malloc(pool, zspage, handle);
Should moving obj_malloc() and obj_write() outside class->lock (and
skipping zspage_read_lock()) for new zspages be split into a separate
patch on top? AFAICT, it's a separate change from merging the
interfaces.
> newfg = get_fullness_group(class, zspage);
> insert_zspage(class, zspage, newfg);
> atomic_long_add(class->pages_per_zspage, &pool->pages_allocated);
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 7/7] zsmalloc: constify struct zs_handle pointer in read-only functions
2026-11-05 4:47 [PATCH 0/7] zsmalloc: redesign object alloc API Sergey Senozhatsky
` (5 preceding siblings ...)
2026-11-05 4:47 ` [PATCH 6/7] zsmalloc: fold zs_obj_write() into zs_store() Sergey Senozhatsky
@ 2026-11-05 4:47 ` Sergey Senozhatsky
2026-11-05 17:31 ` [PATCH 0/7] zsmalloc: redesign object alloc API Yosry Ahmed
7 siblings, 0 replies; 15+ messages in thread
From: Sergey Senozhatsky @ 2026-11-05 4:47 UTC (permalink / raw)
To: Andrew Morton
Cc: Nhat Pham, Yosry Ahmed, Johannes Weiner, Brian Geffon,
Minchan Kim, Richard Chang, linux-kernel, linux-block, linux-mm,
Sergey Senozhatsky
Constify struct zs_handle * in functions that don't modify the handle.
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---
include/linux/zsmalloc.h | 4 ++--
mm/zsmalloc.c | 10 +++++-----
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
index 674358f39191..793fc887015c 100644
--- a/include/linux/zsmalloc.h
+++ b/include/linux/zsmalloc.h
@@ -65,9 +65,9 @@ unsigned int zs_lookup_class_index(struct zs_pool *pool, unsigned int size);
void zs_pool_stats(struct zs_pool *pool, struct zs_pool_stats *stats);
-void zs_obj_read_sg_begin(struct zs_pool *pool, struct zs_handle *handle,
+void zs_obj_read_sg_begin(struct zs_pool *pool, const struct zs_handle *handle,
struct scatterlist *sg, size_t mem_len);
-void zs_obj_read_sg_end(struct zs_pool *pool, struct zs_handle *handle);
+void zs_obj_read_sg_end(struct zs_pool *pool, const struct zs_handle *handle);
extern const struct movable_operations zsmalloc_mops;
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index ff1cde4c5dc2..e0f89a3b77f1 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -805,7 +805,7 @@ static unsigned long location_to_obj(struct zpdesc *zpdesc, unsigned int obj_idx
return obj;
}
-static unsigned long handle_to_obj(struct zs_handle *handle)
+static unsigned long handle_to_obj(const struct zs_handle *handle)
{
return READ_ONCE(ACCESS_PRIVATE(handle, obj));
}
@@ -1110,7 +1110,7 @@ unsigned long zs_get_total_pages(struct zs_pool *pool)
}
EXPORT_SYMBOL_GPL(zs_get_total_pages);
-void zs_obj_read_sg_begin(struct zs_pool *pool, struct zs_handle *handle,
+void zs_obj_read_sg_begin(struct zs_pool *pool, const struct zs_handle *handle,
struct scatterlist *sg, size_t mem_len)
{
struct zspage *zspage;
@@ -1157,7 +1157,7 @@ void zs_obj_read_sg_begin(struct zs_pool *pool, struct zs_handle *handle,
}
EXPORT_SYMBOL_GPL(zs_obj_read_sg_begin);
-void zs_obj_read_sg_end(struct zs_pool *pool, struct zs_handle *handle)
+void zs_obj_read_sg_end(struct zs_pool *pool, const struct zs_handle *handle)
{
struct zspage *zspage;
struct zpdesc *zpdesc;
@@ -1395,7 +1395,7 @@ static unsigned int obj_to_class_idx(unsigned long obj)
*/
#if ZS_OBJ_CLASS_BITS > 0
static inline void obj_class_get_and_lock(struct zs_pool *pool,
- struct zs_handle *handle,
+ const struct zs_handle *handle,
unsigned long *objp,
struct zspage **zspagep,
struct size_class **classp)
@@ -1415,7 +1415,7 @@ static inline void obj_class_get_and_lock(struct zs_pool *pool,
}
#else
static inline void obj_class_get_and_lock(struct zs_pool *pool,
- struct zs_handle *handle,
+ const struct zs_handle *handle,
unsigned long *objp,
struct zspage **zspagep,
struct size_class **classp)
--
2.56.0.398.g882cbad275-goog
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 0/7] zsmalloc: redesign object alloc API
2026-11-05 4:47 [PATCH 0/7] zsmalloc: redesign object alloc API Sergey Senozhatsky
` (6 preceding siblings ...)
2026-11-05 4:47 ` [PATCH 7/7] zsmalloc: constify struct zs_handle pointer in read-only functions Sergey Senozhatsky
@ 2026-11-05 17:31 ` Yosry Ahmed
2026-11-05 17:36 ` Nhat Pham
7 siblings, 1 reply; 15+ messages in thread
From: Yosry Ahmed @ 2026-11-05 17:31 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Andrew Morton, Nhat Pham, Yosry Ahmed, Johannes Weiner,
Brian Geffon, Minchan Kim, Richard Chang, linux-kernel,
linux-block, linux-mm
On Thu, Nov 05, 2026 at 01:47:30PM +0900, Sergey Senozhatsky wrote:
> Both zram and zswap already have per-entry structures that store zsmalloc
> handles, so allocating a separate handle_cachep object per stored page is
> redundant. This series embeds struct zs_handle directly into zram and
> zswap entries and drops handle_cachep, saving 8 bytes and a slab
> alloc/free per stored object (2MB per 1GB of swapped out 4K pages).
I always hated that extra layer of indirection, thanks for doing this!
>
> It also folds zs_malloc() and zs_obj_write() into zs_store() so we don't
> take pool->lock and look up object location again right after allocation.
>
> A simple zram test on a 24-vCPU VM
> ==================================
> 165 fewer kernel instructions and 0.7-1.8% fewer kernel cycles per write;
> lz4 single writer +2.3% IOPS / -2.6% average latency;
> 24 writers +0.9-+1.0% IOPS; no regressions.
> Memory: 8 bytes less per stored page.
>
> Sergey Senozhatsky (7):
> zram: drop stale comment in write_incompressible_page()
> zram: drop get_slot_handle() and set_slot_handle()
> zsmalloc: switch handle type to struct zs_handle
> zram: allocate zsmalloc object under slot lock
> zsmalloc: embed struct zs_handle into zram and zswap entries
> zsmalloc: fold zs_obj_write() into zs_store()
> zsmalloc: constify struct zs_handle pointer in read-only functions
>
> Documentation/admin-guide/mm/slab.rst | 2 +-
> Documentation/core-api/memory-allocation.rst | 2 +-
> Documentation/mm/zsmalloc.rst | 8 +-
> drivers/block/zram/Kconfig | 1 +
> drivers/block/zram/zram_drv.c | 130 +++++-------
> drivers/block/zram/zram_drv.h | 10 +-
> include/linux/zsmalloc.h | 37 +++-
> mm/Kconfig | 3 +
> mm/zsmalloc.c | 204 +++++++++++--------
> mm/zswap.c | 21 +-
> 10 files changed, 229 insertions(+), 189 deletions(-)
>
> --
> 2.56.0.398.g882cbad275-goog
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 0/7] zsmalloc: redesign object alloc API
2026-11-05 17:31 ` [PATCH 0/7] zsmalloc: redesign object alloc API Yosry Ahmed
@ 2026-11-05 17:36 ` Nhat Pham
2026-11-05 17:39 ` Yosry Ahmed
0 siblings, 1 reply; 15+ messages in thread
From: Nhat Pham @ 2026-11-05 17:36 UTC (permalink / raw)
To: Yosry Ahmed
Cc: Sergey Senozhatsky, Andrew Morton, Yosry Ahmed, Johannes Weiner,
Brian Geffon, Minchan Kim, Richard Chang, linux-kernel,
linux-block, linux-mm
On Thu, Nov 5, 2026 at 10:31 AM Yosry Ahmed <yosry@kernel.org> wrote:
>
> On Thu, Nov 05, 2026 at 01:47:30PM +0900, Sergey Senozhatsky wrote:
> > Both zram and zswap already have per-entry structures that store zsmalloc
> > handles, so allocating a separate handle_cachep object per stored page is
> > redundant. This series embeds struct zs_handle directly into zram and
> > zswap entries and drops handle_cachep, saving 8 bytes and a slab
> > alloc/free per stored object (2MB per 1GB of swapped out 4K pages).
>
> I always hated that extra layer of indirection, thanks for doing this!
+1.
The mental load saving excites me more than the memory saving :)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/7] zsmalloc: redesign object alloc API
2026-11-05 17:36 ` Nhat Pham
@ 2026-11-05 17:39 ` Yosry Ahmed
0 siblings, 0 replies; 15+ messages in thread
From: Yosry Ahmed @ 2026-11-05 17:39 UTC (permalink / raw)
To: Nhat Pham
Cc: Sergey Senozhatsky, Andrew Morton, Yosry Ahmed, Johannes Weiner,
Brian Geffon, Minchan Kim, Richard Chang, linux-kernel,
linux-block, linux-mm
On Thu, Nov 5, 2026 at 9:37 AM Nhat Pham <nphamcs@gmail.com> wrote:
>
> On Thu, Nov 5, 2026 at 10:31 AM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > On Thu, Nov 05, 2026 at 01:47:30PM +0900, Sergey Senozhatsky wrote:
> > > Both zram and zswap already have per-entry structures that store zsmalloc
> > > handles, so allocating a separate handle_cachep object per stored page is
> > > redundant. This series embeds struct zs_handle directly into zram and
> > > zswap entries and drops handle_cachep, saving 8 bytes and a slab
> > > alloc/free per stored object (2MB per 1GB of swapped out 4K pages).
> >
> > I always hated that extra layer of indirection, thanks for doing this!
>
> +1.
>
> The mental load saving excites me more than the memory saving :)
Oh I am 100% supporting this just for my own mental wellbeing, the
memory saving is the icing on the cake.
^ permalink raw reply [flat|nested] 15+ messages in thread