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

On Sun Aug 2, 2026 at 8:05 AM EDT, Usama Arif wrote:
>
>
> On 02/08/2026 00:49, Zi Yan wrote:
>> On Sat Aug 1, 2026 at 10:12 AM EDT, Usama Arif wrote:
>>> On Fri, 31 Jul 2026 22:13:24 -0400 Zi Yan <[email protected]> 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 <[email protected]>
>>>> To: Minchan Kim <[email protected]>
>>>> To: Sergey Senozhatsky <[email protected]>
>>>> To: Andrew Morton <[email protected]>
>>>> Cc: [email protected]
>>>> Cc: [email protected]
>>>> ---
>>>>  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.
>> 
>> Got it. Thank you for pointing this out.
>> 
>> is_first_zpdesc() is also used in obj_allocated() for the same check and
>> getting rid of it in get_first_zpdesc() causes inconsistency.
>> 
>> How about the changes below on top of this patch? is_first_zpdesc() can
>> be used for both sites and looks cleaner. And VM_BUG_ON_PAGE is replaced
>> with VM_WARN_ON_ONCE_PAGE.
>> 
>> 
>> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
>> index e8ef227624efa..f021d2df99404 100644
>> --- a/mm/zsmalloc.c
>> +++ b/mm/zsmalloc.c
>> @@ -471,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 zpdesc->zspage->first_zpdesc == zpdesc;
>> +	return zpdesc->zspage == zspage && zspage->first_zpdesc == zpdesc;
>>  }
>>  
>>  /* Protected by class->lock */
>> @@ -491,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));
>
> mhmm do you mean 
>
> VM_WARN_ON_ONCE_PAGE(!is_first_zpdesc(first_zpdesc, zspage), zpdesc_page(first_zpdesc)); 
>
> here? ! is missing I think?
>
>>  	return first_zpdesc;
>>  }
>>  
>> @@ -828,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));
>
> same here? ! is missing?
>
>
Oops, sorry, will fix both. Thanks.



-- 
Best Regards,
Yan, Zi