Re: [PATCH RFC 01/14] mm/zsmalloc: replace PG_private with pointer comparison
From: Zi Yan
Date: Mon Aug 03 2026 - 13:03:54 EST
On Mon Aug 3, 2026 at 11:04 AM EDT, Johannes Weiner wrote:
> 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@xxxxxxxxxx>
>> To: Minchan Kim <minchan@xxxxxxxxxx>
>> To: Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx>
>> To: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>> Cc: linux-mm@xxxxxxxxx
>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>> ---
>> 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
Usama has the same comment about this.
> 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.
How about the version below? Basically, I made is_first_zpdesc() more
straightforward for backpointer checking and first_zpdesc checking.
1. get_first_zpdesc() needs the backpointer check; the first_zpdesc check is
meaningless, since the assignment is done above.
2. obj_allocated() needs the first_zpdesc check; the backpointer check
is meaningless, since the zspage is from get_zspage().
Both conditions are needed for is_first_zpdesc().
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..d89253dc6dd40 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,9 +471,10 @@ 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)
+static inline bool __maybe_unused is_first_zpdesc(struct zpdesc *zpdesc,
+ struct zspage *zspage)
{
- return PagePrivate(zpdesc_page(zpdesc));
+ return zpdesc->zspage == zspage && zspage->first_zpdesc == zpdesc;
}
/* Protected by class->lock */
@@ -496,7 +492,7 @@ 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));
+ VM_WARN_ON_ONCE_PAGE(!is_first_zpdesc(first_zpdesc, zspage), zpdesc_page(first_zpdesc));
return first_zpdesc;
}
@@ -833,7 +829,7 @@ static inline bool obj_allocated(struct zpdesc *zpdesc, void *obj,
struct zspage *zspage = get_zspage(zpdesc);
if (unlikely(ZsHugePage(zspage))) {
- VM_BUG_ON_PAGE(!is_first_zpdesc(zpdesc), zpdesc_page(zpdesc));
+ VM_WARN_ON_ONCE_PAGE(!is_first_zpdesc(zpdesc, zspage), zpdesc_page(zpdesc));
handle = zpdesc->handle;
} else
handle = *(unsigned long *)obj;
@@ -848,9 +844,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 +994,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 +1003,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);
--
Best Regards,
Yan, Zi