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

[Johannes Weiner <hannes@cmpxchg.org>]

On Fri, Jul 31, 2026 at 10:13:24PM -0400, Zi Yan 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(-)
> 
> 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;
>  }

There are two checks: get_first_zpdesc() and obj_allocated().

static struct zpdesc *get_first_zpdesc(struct zspage *zspage)
{
	struct zpdesc *first_zpdesc = zspage->first_zpdesc;

	VM_BUG_ON_PAGE(is_first_zpdesc(first_zpdesc), zpdesc_page(first_zpdesc));
	return first_zpdesc;
}

If you expand the helper, this seems kind of pointless now:

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

Mayyybe it could make sense to assert first_zpdesc->zspage !=
zspage. But that's a separate issue that the previous check didn't
necessarily catch. And might not be worth checking, considering how
trivial create_page_chain() is.

In any case, it doesn't seem worth keeping the check as-is.

And with one caller remaining, you could delete the helper and inline
that expression into the check in obj_allocated(). What it does now is
self-explanatory; it doesn't need another name like that PagePrivate()
check before did.