Re: [PATCH RFC 01/14] mm/zsmalloc: replace PG_private with pointer comparison

[Usama Arif <usama.arif@linux.dev>]

On Fri, 31 Jul 2026 22:13:24 -0400 Zi Yan <ziy@nvidia.com> wrote:

> zsmalloc uses PG_private to indicate first zpdesc in the zspage chain.
> Replace it with zpdesc->zspage->first_zpdesc == zpdesc. The check,
> is_first_zpdesc(), is only used in VM_BUG_ON(), so performance impact
> should be negligible.
> 
> It prepares for a future commit that remove PG_private.
> 
> No functional change intended.
> 
> Assisted-by: Claude:claude-opus-4-8
> Assisted-by: Codex:gpt-5
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> To: Minchan Kim <minchan@kernel.org>
> To: Sergey Senozhatsky <senozhatsky@chromium.org>
> To: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> ---
>  mm/zpdesc.h   |  2 +-
>  mm/zsmalloc.c | 15 +++------------
>  2 files changed, 4 insertions(+), 13 deletions(-)
> 

create_page_chain() sets zpdesc->zspage = zspage before assigning zspage->first_zpdesc,
so LGTM.

I think the assertion in get_first_zpdesc()
should be changed to

VM_BUG_ON_PAGE(first_zpdesc->zspage != zspage, zpdesc_page(first_zpdesc));

in the current check, we are checking

zspage->first_zpdesc->zspage->first_zpdesc == zspage->first_zpdesc

which is just testing the backpointer.

With the above VM_BUG_ON check change, please feel free to add:

Acked-by: Usama Arif <usama.arif@linux.dev>


> diff --git a/mm/zpdesc.h b/mm/zpdesc.h
> index b8258dc78548d..4fd81c2e80769 100644
> --- a/mm/zpdesc.h
> +++ b/mm/zpdesc.h
> @@ -26,8 +26,8 @@
>   * with memcg_data.
>   *
>   * Page flags used:
> - * * PG_private identifies the first component page.
>   * * PG_locked is used by page migration code.
> + * The first component page has zpdesc->zspage->first_zpdesc == zpdesc
>   */
>  struct zpdesc {
>  	unsigned long flags;
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 8204b76f78308..e8ef227624efa 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -290,11 +290,6 @@ struct zs_pool {
>  	atomic_t compaction_in_progress;
>  };
>  
> -static inline void zpdesc_set_first(struct zpdesc *zpdesc)
> -{
> -	SetPagePrivate(zpdesc_page(zpdesc));
> -}
> -
>  static inline void zpdesc_inc_zone_page_state(struct zpdesc *zpdesc)
>  {
>  	inc_zone_page_state(zpdesc_page(zpdesc), NR_ZSPAGES);
> @@ -478,7 +473,7 @@ static void record_obj(unsigned long handle, unsigned long obj)
>  
>  static inline bool __maybe_unused is_first_zpdesc(struct zpdesc *zpdesc)
>  {
> -	return PagePrivate(zpdesc_page(zpdesc));
> +	return zpdesc->zspage->first_zpdesc == zpdesc;
>  }
>  
>  /* Protected by class->lock */
> @@ -848,9 +843,6 @@ static inline bool obj_allocated(struct zpdesc *zpdesc, void *obj,
>  
>  static void reset_zpdesc(struct zpdesc *zpdesc)
>  {
> -	struct page *page = zpdesc_page(zpdesc);
> -
> -	ClearPagePrivate(page);
>  	zpdesc->zspage = NULL;
>  	zpdesc->next = NULL;
>  	/* PageZsmalloc is sticky until the page is freed to the buddy. */
> @@ -1001,8 +993,8 @@ static void create_page_chain(struct size_class *class, struct zspage *zspage,
>  	 * 1. all pages are linked together using zpdesc->next
>  	 * 2. each sub-page point to zspage using zpdesc->zspage
>  	 *
> -	 * we set PG_private to identify the first zpdesc (i.e. no other zpdesc
> -	 * has this flag set).
> +	 * The first zpdesc has its zspage->first_zpdesc set to itself, no
> +	 * other zpdesc has this set.
>  	 */
>  	for (i = 0; i < nr_zpdescs; i++) {
>  		zpdesc = zpdescs[i];
> @@ -1010,7 +1002,6 @@ static void create_page_chain(struct size_class *class, struct zspage *zspage,
>  		zpdesc->next = NULL;
>  		if (i == 0) {
>  			zspage->first_zpdesc = zpdesc;
> -			zpdesc_set_first(zpdesc);
>  			if (unlikely(class->objs_per_zspage == 1 &&
>  					class->pages_per_zspage == 1))
>  				SetZsHugePage(zspage);
> 
> -- 
> 2.53.0
> 
>