Re: [PATCH v2] zsmalloc: prefer the the original page's node for compressed data

Sergey Senozhatsky
2025-04-03T03:55:53Z

On (25/04/02 13:44), Nhat Pham wrote:
[..]
> @@ -1981,10 +1981,15 @@ static int recompress_slot(struct zram *zram, u32 index, struct page *page,
> * We are holding per-CPU stream mutex and entry lock so better
> * avoid direct reclaim. Allocation error is not fatal since
> * we still have the old object in the mem_pool.
> + *
> + * XXX: technically, the node we really want here is the node that holds
> + * the original compressed data. But that would require us to 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);
> + __GFP_HIGHMEM | __GFP_MOVABLE, page_to_nid(page));

This works for me. I saw in other threads (and sorry, I'm on a sick
leave now, can't reply fast) that "zram people need to fix it"/etc.
I guess I'm one of those zram people and I don't think I run any
multi-node NUMA systems, so that problem probably has never occurred
to me. We can add a simple API extension to lookup node-id by
zsmalloc handle, if anyone really needs it, but I'm okay with the
status quo (and XXX) for the time being.

[..]
> -unsigned long zs_malloc(struct zs_pool *pool, size_t size, gfp_t gfp)
> +unsigned long zs_malloc(struct zs_pool *pool, size_t size, gfp_t gfp,
> + const int nid)
> {

I'm not the zspool maintainer, but if I may ask, for new zsmalloc code my
preference is to follow the kernel styles (yes, the old code doesn't follow
any at all, whenever I touch the old code I fix it).

Do you mind if we do something like below? (at least for zsmalloc)

With this
Acked-by: Sergey Senozhatsky <senozhatsky@chromium.org> #zram and zsmalloc

---

diff --git a/include/linux/zpool.h b/include/linux/zpool.h
index 697525cb00bd..369ef068fad8 100644
--- a/include/linux/zpool.h
+++ b/include/linux/zpool.h
@@ -22,7 +22,7 @@ const char *zpool_get_type(struct zpool *pool);
void zpool_destroy_pool(struct zpool *pool);

int zpool_malloc(struct zpool *pool, size_t size, gfp_t gfp,
- unsigned long *handle, const int nid);
+ unsigned long *handle, const int nid);

void zpool_free(struct zpool *pool, unsigned long handle);

@@ -64,7 +64,7 @@ struct zpool_driver {
void (*destroy)(void *pool);

int (*malloc)(void *pool, size_t size, gfp_t gfp,
- unsigned long *handle, const int nid);
+ unsigned long *handle, const int nid);
void (*free)(void *pool, unsigned long handle);

void *(*obj_read_begin)(void *pool, unsigned long handle,
diff --git a/mm/zpool.c b/mm/zpool.c
index b99a7c03e735..0a71d03369f1 100644
--- a/mm/zpool.c
+++ b/mm/zpool.c
@@ -239,7 +239,7 @@ const char *zpool_get_type(struct zpool *zpool)
* Returns: 0 on success, negative value on error.
*/
int zpool_malloc(struct zpool *zpool, size_t size, gfp_t gfp,
- unsigned long *handle, const int nid)
+ unsigned long *handle, const int nid)
{
return zpool->driver->malloc(zpool->pool, size, gfp, handle, nid);
}
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index daa16ce4cf07..70406ac94bbd 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -452,7 +452,7 @@ static void zs_zpool_destroy(void *pool)
}

static int zs_zpool_malloc(void *pool, size_t size, gfp_t gfp,
- unsigned long *handle, const int nid)
+ unsigned long *handle, const int nid)
{
*handle = zs_malloc(pool, size, gfp, nid);

@@ -1033,8 +1033,8 @@ static void create_page_chain(struct size_class *class, struct zspage *zspage,
* Allocate a zspage for the given size class
*/
static struct zspage *alloc_zspage(struct zs_pool *pool,
- struct size_class *class,
- gfp_t gfp, const int nid)
+ struct size_class *class,
+ gfp_t gfp, const int nid)
{
int i;
struct zpdesc *zpdescs[ZS_MAX_PAGES_PER_ZSPAGE];
@@ -1333,7 +1333,7 @@ static unsigned long obj_malloc(struct zs_pool *pool,
* 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)
+ const int nid)
{
unsigned long handle;
struct size_class *class;