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

[David Hildenbrand (Arm)]

On 9/8/26 04:56, Zi Yan wrote:
> zsmalloc uses PG_private to indicate first zpdesc in a zspage chain. It is
> equivalent to check zpdesc == zspage->first_zpdesc. Replace
> is_first_zpdesc() with zpdesc == zspage->first_zpdesc in obj_allocated().
> 
> For get_first_zpdesc(), first_zpdesc is from zspage->first_zpdesc, so
> replace is_first_zpdesc() with first_zpdesc->zspage == zspage, the second
> requirement of a zspage chain, where all zpdescs point to the same zspage.
> 
> is_first_zpdesc(), is only used in VM_BUG_ON_PAGE(), so performance impact
> should be negligible. While at it, change VM_BUG_ON() to
> VM_WARN_ON_ONCE_PAGE().
> 
> 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

Assied-by: LLM :)

> To: Minchan Kim <minchan@xxxxxxxxxx>
> To: Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx>
> To: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: linux-mm@xxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Acked-by: Johannes Weiner <hannes@xxxxxxxxxxx>
> Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
> ---

Nice cleanup, one comment below.

Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

>  mm/zpdesc.h   |  2 +-
>  mm/zsmalloc.c | 24 ++++++------------------
>  2 files changed, 7 insertions(+), 19 deletions(-)
> 
> 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 825022a7a328f..24526185fd31f 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);
> @@ -476,11 +471,6 @@ static void record_obj(unsigned long handle, unsigned long obj)
>  	WRITE_ONCE(*(unsigned long *)handle, obj);
>  }
>  
> -static inline bool __maybe_unused is_first_zpdesc(struct zpdesc *zpdesc)
> -{
> -	return PagePrivate(zpdesc_page(zpdesc));
> -}

I would keep this helper, reworking it and moving all comments regarding *how*
first detection works in there

-static inline bool __maybe_unused is_first_zpdesc(struct zpdesc *zpdesc)
+static inline bool __maybe_unused is_first_zpdesc(const struct zpdesc *zpdesc)
 {
-       return PagePrivate(zpdesc_page(zpdesc));
+       /* Only the first zpdesc has its zspage->first_zpdesc set to itself. */
+       return zpdesc == zpdesc->zspage->first_zpdesc;
 }

-- 
Cheers,

David