linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2 1/2] zsmalloc: use actual object size to detect spans
@ 2026-01-07  5:21 Sergey Senozhatsky
  2026-01-07  5:21 ` [PATCHv2 2/2] zsmalloc: simplify read begin/end logic Sergey Senozhatsky
  2026-01-07 18:57 ` [PATCHv2 1/2] zsmalloc: use actual object size to detect spans Yosry Ahmed
  0 siblings, 2 replies; 7+ messages in thread
From: Sergey Senozhatsky @ 2026-01-07  5:21 UTC (permalink / raw)
  To: Andrew Morton, Yosry Ahmed, Nhat Pham
  Cc: Minchan Kim, Johannes Weiner, Brian Geffon, linux-kernel,
	linux-mm, Sergey Senozhatsky

Using class->size to detect spanning objects is not entirely correct,
because some size classes can hold a range of object sizes of up to
class->size bytes in length, due to size-classes merge.  Such classes
use padding for cases when actually written objects are smaller than
class->size.  zs_obj_read_begin() can incorrectly hit the slow path
and perform memcpy of such objects, basically copying padding bytes.
Instead of class->size zs_obj_read_begin() should use the actual
compressed object length (both zram and zswap know it) so that it can
correctly handle situations when a written object is small enough to
fit into the first physical page.

Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
---

v1->v2:
- use mem_len for second memcpy size calculation (Yosry)
- simplified read_begin/end logic (Yosry)

 drivers/block/zram/zram_drv.c | 14 ++++++++------
 include/linux/zsmalloc.h      |  4 ++--
 mm/zsmalloc.c                 | 16 +++++++++++-----
 mm/zswap.c                    |  5 +++--
 4 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index a6587bed6a03..76a54eabe889 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -2065,11 +2065,11 @@ static int read_incompressible_page(struct zram *zram, struct page *page,
 	void *src, *dst;
 
 	handle = get_slot_handle(zram, index);
-	src = zs_obj_read_begin(zram->mem_pool, handle, NULL);
+	src = zs_obj_read_begin(zram->mem_pool, handle, PAGE_SIZE, NULL);
 	dst = kmap_local_page(page);
 	copy_page(dst, src);
 	kunmap_local(dst);
-	zs_obj_read_end(zram->mem_pool, handle, src);
+	zs_obj_read_end(zram->mem_pool, handle, PAGE_SIZE, src);
 
 	return 0;
 }
@@ -2087,11 +2087,12 @@ static int read_compressed_page(struct zram *zram, struct page *page, u32 index)
 	prio = get_slot_comp_priority(zram, index);
 
 	zstrm = zcomp_stream_get(zram->comps[prio]);
-	src = zs_obj_read_begin(zram->mem_pool, handle, zstrm->local_copy);
+	src = zs_obj_read_begin(zram->mem_pool, handle, size,
+				zstrm->local_copy);
 	dst = kmap_local_page(page);
 	ret = zcomp_decompress(zram->comps[prio], zstrm, src, size, dst);
 	kunmap_local(dst);
-	zs_obj_read_end(zram->mem_pool, handle, src);
+	zs_obj_read_end(zram->mem_pool, handle, size, src);
 	zcomp_stream_put(zstrm);
 
 	return ret;
@@ -2114,9 +2115,10 @@ static int read_from_zspool_raw(struct zram *zram, struct page *page, u32 index)
 	 * takes place here, as we read raw compressed data.
 	 */
 	zstrm = zcomp_stream_get(zram->comps[ZRAM_PRIMARY_COMP]);
-	src = zs_obj_read_begin(zram->mem_pool, handle, zstrm->local_copy);
+	src = zs_obj_read_begin(zram->mem_pool, handle, size,
+				zstrm->local_copy);
 	memcpy_to_page(page, 0, src, size);
-	zs_obj_read_end(zram->mem_pool, handle, src);
+	zs_obj_read_end(zram->mem_pool, handle, size, src);
 	zcomp_stream_put(zstrm);
 
 	return 0;
diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
index f3ccff2d966c..5565c3171007 100644
--- a/include/linux/zsmalloc.h
+++ b/include/linux/zsmalloc.h
@@ -40,9 +40,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_begin(struct zs_pool *pool, unsigned long handle,
-			void *local_copy);
+			size_t mem_len, void *local_copy);
 void zs_obj_read_end(struct zs_pool *pool, unsigned long handle,
-		     void *handle_mem);
+		     size_t mem_len, void *handle_mem);
 void zs_obj_write(struct zs_pool *pool, unsigned long handle,
 		  void *handle_mem, size_t mem_len);
 
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 84da164dcbc5..119c196a287a 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1065,7 +1065,7 @@ unsigned long zs_get_total_pages(struct zs_pool *pool)
 EXPORT_SYMBOL_GPL(zs_get_total_pages);
 
 void *zs_obj_read_begin(struct zs_pool *pool, unsigned long handle,
-			void *local_copy)
+			size_t mem_len, void *local_copy)
 {
 	struct zspage *zspage;
 	struct zpdesc *zpdesc;
@@ -1087,7 +1087,10 @@ void *zs_obj_read_begin(struct zs_pool *pool, unsigned long handle,
 	class = zspage_class(pool, zspage);
 	off = offset_in_page(class->size * obj_idx);
 
-	if (off + class->size <= PAGE_SIZE) {
+	if (!ZsHugePage(zspage))
+		mem_len += ZS_HANDLE_SIZE;
+
+	if (off + mem_len <= PAGE_SIZE) {
 		/* this object is contained entirely within a page */
 		addr = kmap_local_zpdesc(zpdesc);
 		addr += off;
@@ -1096,7 +1099,7 @@ void *zs_obj_read_begin(struct zs_pool *pool, unsigned long handle,
 
 		/* this object spans two pages */
 		sizes[0] = PAGE_SIZE - off;
-		sizes[1] = class->size - sizes[0];
+		sizes[1] = mem_len - sizes[0];
 		addr = local_copy;
 
 		memcpy_from_page(addr, zpdesc_page(zpdesc),
@@ -1115,7 +1118,7 @@ void *zs_obj_read_begin(struct zs_pool *pool, unsigned long handle,
 EXPORT_SYMBOL_GPL(zs_obj_read_begin);
 
 void zs_obj_read_end(struct zs_pool *pool, unsigned long handle,
-		     void *handle_mem)
+		     size_t mem_len, void *handle_mem)
 {
 	struct zspage *zspage;
 	struct zpdesc *zpdesc;
@@ -1129,7 +1132,10 @@ void zs_obj_read_end(struct zs_pool *pool, unsigned long handle,
 	class = zspage_class(pool, zspage);
 	off = offset_in_page(class->size * obj_idx);
 
-	if (off + class->size <= PAGE_SIZE) {
+	if (!ZsHugePage(zspage))
+		mem_len += ZS_HANDLE_SIZE;
+
+	if (off + mem_len <= PAGE_SIZE) {
 		if (!ZsHugePage(zspage))
 			off += ZS_HANDLE_SIZE;
 		handle_mem -= off;
diff --git a/mm/zswap.c b/mm/zswap.c
index de8858ff1521..a3811b05ab57 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -937,7 +937,8 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
 	u8 *src, *obj;
 
 	acomp_ctx = acomp_ctx_get_cpu_lock(pool);
-	obj = zs_obj_read_begin(pool->zs_pool, entry->handle, acomp_ctx->buffer);
+	obj = zs_obj_read_begin(pool->zs_pool, entry->handle, entry->length,
+				acomp_ctx->buffer);
 
 	/* zswap entries of length PAGE_SIZE are not compressed. */
 	if (entry->length == PAGE_SIZE) {
@@ -966,7 +967,7 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
 	dlen = acomp_ctx->req->dlen;
 
 read_done:
-	zs_obj_read_end(pool->zs_pool, entry->handle, obj);
+	zs_obj_read_end(pool->zs_pool, entry->handle, entry->length, obj);
 	acomp_ctx_put_unlock(acomp_ctx);
 
 	if (!decomp_ret && dlen == PAGE_SIZE)
-- 
2.52.0.351.gbe84eed79e-goog



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCHv2 2/2] zsmalloc: simplify read begin/end logic
  2026-01-07  5:21 [PATCHv2 1/2] zsmalloc: use actual object size to detect spans Sergey Senozhatsky
@ 2026-01-07  5:21 ` Sergey Senozhatsky
  2026-01-07 18:17   ` Andrew Morton
  2026-01-07 19:03   ` Yosry Ahmed
  2026-01-07 18:57 ` [PATCHv2 1/2] zsmalloc: use actual object size to detect spans Yosry Ahmed
  1 sibling, 2 replies; 7+ messages in thread
From: Sergey Senozhatsky @ 2026-01-07  5:21 UTC (permalink / raw)
  To: Andrew Morton, Yosry Ahmed, Nhat Pham
  Cc: Minchan Kim, Johannes Weiner, Brian Geffon, linux-kernel, linux-mm

From: Yosry Ahmed <yosry.ahmed@linux.dev>

When we switched from using class->size (for spans detection)
to actual compressed object size, we had to compensate for
the fact that class->size implicitly took inlined handle
into consideration.  In fact, instead of adjusting the size
of compressed object (adding handle offset for non-huge size
classes), we can move some lines around and simplify the
code: there are already paths in read_begin/end that compensate
for inlined object handle offset.

Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
---
 mm/zsmalloc.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 119c196a287a..cc3d9501ae21 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1088,7 +1088,7 @@ void *zs_obj_read_begin(struct zs_pool *pool, unsigned long handle,
 	off = offset_in_page(class->size * obj_idx);
 
 	if (!ZsHugePage(zspage))
-		mem_len += ZS_HANDLE_SIZE;
+		off += ZS_HANDLE_SIZE;
 
 	if (off + mem_len <= PAGE_SIZE) {
 		/* this object is contained entirely within a page */
@@ -1110,9 +1110,6 @@ void *zs_obj_read_begin(struct zs_pool *pool, unsigned long handle,
 				 0, sizes[1]);
 	}
 
-	if (!ZsHugePage(zspage))
-		addr += ZS_HANDLE_SIZE;
-
 	return addr;
 }
 EXPORT_SYMBOL_GPL(zs_obj_read_begin);
@@ -1133,11 +1130,9 @@ void zs_obj_read_end(struct zs_pool *pool, unsigned long handle,
 	off = offset_in_page(class->size * obj_idx);
 
 	if (!ZsHugePage(zspage))
-		mem_len += ZS_HANDLE_SIZE;
+		off += ZS_HANDLE_SIZE;
 
 	if (off + mem_len <= PAGE_SIZE) {
-		if (!ZsHugePage(zspage))
-			off += ZS_HANDLE_SIZE;
 		handle_mem -= off;
 		kunmap_local(handle_mem);
 	}
-- 
2.52.0.351.gbe84eed79e-goog



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2 2/2] zsmalloc: simplify read begin/end logic
  2026-01-07  5:21 ` [PATCHv2 2/2] zsmalloc: simplify read begin/end logic Sergey Senozhatsky
@ 2026-01-07 18:17   ` Andrew Morton
  2026-01-08  1:14     ` Sergey Senozhatsky
  2026-01-07 19:03   ` Yosry Ahmed
  1 sibling, 1 reply; 7+ messages in thread
From: Andrew Morton @ 2026-01-07 18:17 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Yosry Ahmed, Nhat Pham, Minchan Kim, Johannes Weiner,
	Brian Geffon, linux-kernel, linux-mm

On Wed,  7 Jan 2026 14:21:45 +0900 Sergey Senozhatsky <senozhatsky@chromium.org> wrote:

> From: Yosry Ahmed <yosry.ahmed@linux.dev>
> 
> When we switched from using class->size (for spans detection)
> to actual compressed object size, we had to compensate for
> the fact that class->size implicitly took inlined handle
> into consideration.  In fact, instead of adjusting the size
> of compressed object (adding handle offset for non-huge size
> classes), we can move some lines around and simplify the
> code: there are already paths in read_begin/end that compensate
> for inlined object handle offset.

Updated, thanks.

> Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> ---

This patch should have had your Signed-off-by: also, as you were on the
delivery path.  I have made that change to the mm.gt copy.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2 1/2] zsmalloc: use actual object size to detect spans
  2026-01-07  5:21 [PATCHv2 1/2] zsmalloc: use actual object size to detect spans Sergey Senozhatsky
  2026-01-07  5:21 ` [PATCHv2 2/2] zsmalloc: simplify read begin/end logic Sergey Senozhatsky
@ 2026-01-07 18:57 ` Yosry Ahmed
  1 sibling, 0 replies; 7+ messages in thread
From: Yosry Ahmed @ 2026-01-07 18:57 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Andrew Morton, Nhat Pham, Minchan Kim, Johannes Weiner,
	Brian Geffon, linux-kernel, linux-mm

On Wed, Jan 07, 2026 at 02:21:44PM +0900, Sergey Senozhatsky wrote:
> Using class->size to detect spanning objects is not entirely correct,
> because some size classes can hold a range of object sizes of up to
> class->size bytes in length, due to size-classes merge.  Such classes
> use padding for cases when actually written objects are smaller than
> class->size.  zs_obj_read_begin() can incorrectly hit the slow path
> and perform memcpy of such objects, basically copying padding bytes.
> Instead of class->size zs_obj_read_begin() should use the actual
> compressed object length (both zram and zswap know it) so that it can
> correctly handle situations when a written object is small enough to
> fit into the first physical page.
> 
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>

For zsmalloc and zswap bits:
Reviewed-by: Yosry Ahmed <yosry.ahmed@linux.dev>

> ---
> 
> v1->v2:
> - use mem_len for second memcpy size calculation (Yosry)
> - simplified read_begin/end logic (Yosry)
> 
>  drivers/block/zram/zram_drv.c | 14 ++++++++------
>  include/linux/zsmalloc.h      |  4 ++--
>  mm/zsmalloc.c                 | 16 +++++++++++-----
>  mm/zswap.c                    |  5 +++--
>  4 files changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index a6587bed6a03..76a54eabe889 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -2065,11 +2065,11 @@ static int read_incompressible_page(struct zram *zram, struct page *page,
>  	void *src, *dst;
>  
>  	handle = get_slot_handle(zram, index);
> -	src = zs_obj_read_begin(zram->mem_pool, handle, NULL);
> +	src = zs_obj_read_begin(zram->mem_pool, handle, PAGE_SIZE, NULL);
>  	dst = kmap_local_page(page);
>  	copy_page(dst, src);
>  	kunmap_local(dst);
> -	zs_obj_read_end(zram->mem_pool, handle, src);
> +	zs_obj_read_end(zram->mem_pool, handle, PAGE_SIZE, src);
>  
>  	return 0;
>  }
> @@ -2087,11 +2087,12 @@ static int read_compressed_page(struct zram *zram, struct page *page, u32 index)
>  	prio = get_slot_comp_priority(zram, index);
>  
>  	zstrm = zcomp_stream_get(zram->comps[prio]);
> -	src = zs_obj_read_begin(zram->mem_pool, handle, zstrm->local_copy);
> +	src = zs_obj_read_begin(zram->mem_pool, handle, size,
> +				zstrm->local_copy);
>  	dst = kmap_local_page(page);
>  	ret = zcomp_decompress(zram->comps[prio], zstrm, src, size, dst);
>  	kunmap_local(dst);
> -	zs_obj_read_end(zram->mem_pool, handle, src);
> +	zs_obj_read_end(zram->mem_pool, handle, size, src);
>  	zcomp_stream_put(zstrm);
>  
>  	return ret;
> @@ -2114,9 +2115,10 @@ static int read_from_zspool_raw(struct zram *zram, struct page *page, u32 index)
>  	 * takes place here, as we read raw compressed data.
>  	 */
>  	zstrm = zcomp_stream_get(zram->comps[ZRAM_PRIMARY_COMP]);
> -	src = zs_obj_read_begin(zram->mem_pool, handle, zstrm->local_copy);
> +	src = zs_obj_read_begin(zram->mem_pool, handle, size,
> +				zstrm->local_copy);
>  	memcpy_to_page(page, 0, src, size);
> -	zs_obj_read_end(zram->mem_pool, handle, src);
> +	zs_obj_read_end(zram->mem_pool, handle, size, src);
>  	zcomp_stream_put(zstrm);
>  
>  	return 0;
> diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
> index f3ccff2d966c..5565c3171007 100644
> --- a/include/linux/zsmalloc.h
> +++ b/include/linux/zsmalloc.h
> @@ -40,9 +40,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_begin(struct zs_pool *pool, unsigned long handle,
> -			void *local_copy);
> +			size_t mem_len, void *local_copy);
>  void zs_obj_read_end(struct zs_pool *pool, unsigned long handle,
> -		     void *handle_mem);
> +		     size_t mem_len, void *handle_mem);
>  void zs_obj_write(struct zs_pool *pool, unsigned long handle,
>  		  void *handle_mem, size_t mem_len);
>  
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 84da164dcbc5..119c196a287a 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -1065,7 +1065,7 @@ unsigned long zs_get_total_pages(struct zs_pool *pool)
>  EXPORT_SYMBOL_GPL(zs_get_total_pages);
>  
>  void *zs_obj_read_begin(struct zs_pool *pool, unsigned long handle,
> -			void *local_copy)
> +			size_t mem_len, void *local_copy)
>  {
>  	struct zspage *zspage;
>  	struct zpdesc *zpdesc;
> @@ -1087,7 +1087,10 @@ void *zs_obj_read_begin(struct zs_pool *pool, unsigned long handle,
>  	class = zspage_class(pool, zspage);
>  	off = offset_in_page(class->size * obj_idx);
>  
> -	if (off + class->size <= PAGE_SIZE) {
> +	if (!ZsHugePage(zspage))
> +		mem_len += ZS_HANDLE_SIZE;
> +
> +	if (off + mem_len <= PAGE_SIZE) {
>  		/* this object is contained entirely within a page */
>  		addr = kmap_local_zpdesc(zpdesc);
>  		addr += off;
> @@ -1096,7 +1099,7 @@ void *zs_obj_read_begin(struct zs_pool *pool, unsigned long handle,
>  
>  		/* this object spans two pages */
>  		sizes[0] = PAGE_SIZE - off;
> -		sizes[1] = class->size - sizes[0];
> +		sizes[1] = mem_len - sizes[0];
>  		addr = local_copy;
>  
>  		memcpy_from_page(addr, zpdesc_page(zpdesc),
> @@ -1115,7 +1118,7 @@ void *zs_obj_read_begin(struct zs_pool *pool, unsigned long handle,
>  EXPORT_SYMBOL_GPL(zs_obj_read_begin);
>  
>  void zs_obj_read_end(struct zs_pool *pool, unsigned long handle,
> -		     void *handle_mem)
> +		     size_t mem_len, void *handle_mem)
>  {
>  	struct zspage *zspage;
>  	struct zpdesc *zpdesc;
> @@ -1129,7 +1132,10 @@ void zs_obj_read_end(struct zs_pool *pool, unsigned long handle,
>  	class = zspage_class(pool, zspage);
>  	off = offset_in_page(class->size * obj_idx);
>  
> -	if (off + class->size <= PAGE_SIZE) {
> +	if (!ZsHugePage(zspage))
> +		mem_len += ZS_HANDLE_SIZE;
> +
> +	if (off + mem_len <= PAGE_SIZE) {
>  		if (!ZsHugePage(zspage))
>  			off += ZS_HANDLE_SIZE;
>  		handle_mem -= off;
> diff --git a/mm/zswap.c b/mm/zswap.c
> index de8858ff1521..a3811b05ab57 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -937,7 +937,8 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>  	u8 *src, *obj;
>  
>  	acomp_ctx = acomp_ctx_get_cpu_lock(pool);
> -	obj = zs_obj_read_begin(pool->zs_pool, entry->handle, acomp_ctx->buffer);
> +	obj = zs_obj_read_begin(pool->zs_pool, entry->handle, entry->length,
> +				acomp_ctx->buffer);
>  
>  	/* zswap entries of length PAGE_SIZE are not compressed. */
>  	if (entry->length == PAGE_SIZE) {
> @@ -966,7 +967,7 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>  	dlen = acomp_ctx->req->dlen;
>  
>  read_done:
> -	zs_obj_read_end(pool->zs_pool, entry->handle, obj);
> +	zs_obj_read_end(pool->zs_pool, entry->handle, entry->length, obj);
>  	acomp_ctx_put_unlock(acomp_ctx);
>  
>  	if (!decomp_ret && dlen == PAGE_SIZE)
> -- 
> 2.52.0.351.gbe84eed79e-goog
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2 2/2] zsmalloc: simplify read begin/end logic
  2026-01-07  5:21 ` [PATCHv2 2/2] zsmalloc: simplify read begin/end logic Sergey Senozhatsky
  2026-01-07 18:17   ` Andrew Morton
@ 2026-01-07 19:03   ` Yosry Ahmed
  2026-01-08  1:17     ` Sergey Senozhatsky
  1 sibling, 1 reply; 7+ messages in thread
From: Yosry Ahmed @ 2026-01-07 19:03 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Andrew Morton, Nhat Pham, Minchan Kim, Johannes Weiner,
	Brian Geffon, linux-kernel, linux-mm

On Wed, Jan 07, 2026 at 02:21:45PM +0900, Sergey Senozhatsky wrote:
> From: Yosry Ahmed <yosry.ahmed@linux.dev>

While I appreciate this, I think for all intents and purposes this patch
should be credited to you, it's different from the diff I said as it
applies on top of your change.

If you're feeling really generous, I think Suggested-by or
Co-developed-by + Signed-off-by is sufficient :)

> 
> When we switched from using class->size (for spans detection)
> to actual compressed object size, we had to compensate for
> the fact that class->size implicitly took inlined handle
> into consideration.  In fact, instead of adjusting the size
> of compressed object (adding handle offset for non-huge size
> classes), we can move some lines around and simplify the
> code: there are already paths in read_begin/end that compensate
> for inlined object handle offset.

I think the commit log is not clear in isolation.

How about something like this:

zs_obj_read_begin() currently maps or copies the the compressed object
with the prefix handle for !ZsHugePage case.  Make the logic clearer and
more officient by moving the offset of the object in the page after the
prefix handle instead, only copying the actual object and avoiding the
need to adjust the returned address to account for the prefix.

Adjust the logic to detect spanning objects in zs_obj_read_end()
accordingly, slightly simplifying it by avoiding the need to account for
the handle in both the offset and the object size.

> 
> Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> ---
>  mm/zsmalloc.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 119c196a287a..cc3d9501ae21 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -1088,7 +1088,7 @@ void *zs_obj_read_begin(struct zs_pool *pool, unsigned long handle,
>  	off = offset_in_page(class->size * obj_idx);
>  
>  	if (!ZsHugePage(zspage))
> -		mem_len += ZS_HANDLE_SIZE;
> +		off += ZS_HANDLE_SIZE;
>  
>  	if (off + mem_len <= PAGE_SIZE) {
>  		/* this object is contained entirely within a page */
> @@ -1110,9 +1110,6 @@ void *zs_obj_read_begin(struct zs_pool *pool, unsigned long handle,
>  				 0, sizes[1]);
>  	}
>  
> -	if (!ZsHugePage(zspage))
> -		addr += ZS_HANDLE_SIZE;
> -
>  	return addr;
>  }
>  EXPORT_SYMBOL_GPL(zs_obj_read_begin);
> @@ -1133,11 +1130,9 @@ void zs_obj_read_end(struct zs_pool *pool, unsigned long handle,
>  	off = offset_in_page(class->size * obj_idx);
>  
>  	if (!ZsHugePage(zspage))
> -		mem_len += ZS_HANDLE_SIZE;
> +		off += ZS_HANDLE_SIZE;
>  
>  	if (off + mem_len <= PAGE_SIZE) {
> -		if (!ZsHugePage(zspage))
> -			off += ZS_HANDLE_SIZE;
>  		handle_mem -= off;
>  		kunmap_local(handle_mem);
>  	}
> -- 
> 2.52.0.351.gbe84eed79e-goog
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2 2/2] zsmalloc: simplify read begin/end logic
  2026-01-07 18:17   ` Andrew Morton
@ 2026-01-08  1:14     ` Sergey Senozhatsky
  0 siblings, 0 replies; 7+ messages in thread
From: Sergey Senozhatsky @ 2026-01-08  1:14 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Sergey Senozhatsky, Yosry Ahmed, Nhat Pham, Minchan Kim,
	Johannes Weiner, Brian Geffon, linux-kernel, linux-mm

On (26/01/07 10:17), Andrew Morton wrote:
> > From: Yosry Ahmed <yosry.ahmed@linux.dev>
> > 
> > When we switched from using class->size (for spans detection)
> > to actual compressed object size, we had to compensate for
> > the fact that class->size implicitly took inlined handle
> > into consideration.  In fact, instead of adjusting the size
> > of compressed object (adding handle offset for non-huge size
> > classes), we can move some lines around and simplify the
> > code: there are already paths in read_begin/end that compensate
> > for inlined object handle offset.
> 
> Updated, thanks.
> 
> > Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> > ---
> 
> This patch should have had your Signed-off-by: also, as you were on the
> delivery path.  I have made that change to the mm.gt copy.

Thanks!


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2 2/2] zsmalloc: simplify read begin/end logic
  2026-01-07 19:03   ` Yosry Ahmed
@ 2026-01-08  1:17     ` Sergey Senozhatsky
  0 siblings, 0 replies; 7+ messages in thread
From: Sergey Senozhatsky @ 2026-01-08  1:17 UTC (permalink / raw)
  To: Yosry Ahmed
  Cc: Sergey Senozhatsky, Andrew Morton, Nhat Pham, Minchan Kim,
	Johannes Weiner, Brian Geffon, linux-kernel, linux-mm

On (26/01/07 19:03), Yosry Ahmed wrote:
> On Wed, Jan 07, 2026 at 02:21:45PM +0900, Sergey Senozhatsky wrote:
> > From: Yosry Ahmed <yosry.ahmed@linux.dev>
> 
> While I appreciate this, I think for all intents and purposes this patch
> should be credited to you, it's different from the diff I said as it
> applies on top of your change.
> 
> If you're feeling really generous, I think Suggested-by or
> Co-developed-by + Signed-off-by is sufficient :)

Okey-dokey
Co-developed-by: Yosry Ahmed <yosry.ahmed@linux.dev>

> > When we switched from using class->size (for spans detection)
> > to actual compressed object size, we had to compensate for
> > the fact that class->size implicitly took inlined handle
> > into consideration.  In fact, instead of adjusting the size
> > of compressed object (adding handle offset for non-huge size
> > classes), we can move some lines around and simplify the
> > code: there are already paths in read_begin/end that compensate
> > for inlined object handle offset.
> 
> I think the commit log is not clear in isolation.
> 
> How about something like this:
> 
> zs_obj_read_begin() currently maps or copies the the compressed object
> with the prefix handle for !ZsHugePage case.  Make the logic clearer and
> more officient by moving the offset of the object in the page after the
> prefix handle instead, only copying the actual object and avoiding the
> need to adjust the returned address to account for the prefix.
> 
> Adjust the logic to detect spanning objects in zs_obj_read_end()
> accordingly, slightly simplifying it by avoiding the need to account for
> the handle in both the offset and the object size.

Looks good to me.

Andrew, can we trouble you with commit message update or should I resend?


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-01-08  1:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-01-07  5:21 [PATCHv2 1/2] zsmalloc: use actual object size to detect spans Sergey Senozhatsky
2026-01-07  5:21 ` [PATCHv2 2/2] zsmalloc: simplify read begin/end logic Sergey Senozhatsky
2026-01-07 18:17   ` Andrew Morton
2026-01-08  1:14     ` Sergey Senozhatsky
2026-01-07 19:03   ` Yosry Ahmed
2026-01-08  1:17     ` Sergey Senozhatsky
2026-01-07 18:57 ` [PATCHv2 1/2] zsmalloc: use actual object size to detect spans Yosry Ahmed

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox