Re: [PATCH v9 mm-unstable 11/18] mm/zsmalloc: convert reset_page to reset_zpdesc

[Hyeonggon Yoo <hyeonggon.yoo@sk.com>]

On 1/10/2025 1:43 PM, Matthew Wilcox wrote:
> On Tue, Dec 17, 2024 at 12:04:42AM +0900, Hyeonggon Yoo wrote:
>> From: Alex Shi <alexs@kernel.org>
>>
>> zpdesc.zspage matches with page.private, zpdesc.next matches with
>> page.index. They will be reset in reset_page() which is called prior to
>> free base pages of a zspage.
>>
>> Since the fields that need to be initialized are independent of the
>> order in struct zpdesc, Keep it to use struct page to ensure robustness
>> against potential rearrangements of struct zpdesc fields in the future.
>>
>> [42.hyeyoo: keep reset_zpdesc() to use struct page fields]
> 
> Ummm ... why did you make this change?  Now page->index still has a user
> in zsmalloc ;-(

Ohhh.., my bad :/

I was thinking of a situation when someone re-arranges zpdesc fields
and forgets to reset the field that overlaps page->index now.
And when writing it did not consider that we're going to remove usages
of page->index anyway...

Let me fix this - will send an updated version of this patch or series
(whatever way that is convenient for You and Andrew)

---
Hyeonggon

>> Signed-off-by: Alex Shi <alexs@kernel.org>
>> Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
>> ---
>>   mm/zsmalloc.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
>> index 432e78e61d2e..dded6d1f3b7a 100644
>> --- a/mm/zsmalloc.c
>> +++ b/mm/zsmalloc.c
>> @@ -843,8 +843,10 @@ static inline bool obj_allocated(struct zpdesc *zpdesc, void *obj,
>>   	return true;
>>   }
>>   
>> -static void reset_page(struct page *page)
>> +static void reset_zpdesc(struct zpdesc *zpdesc)
>>   {
>> +	struct page *page = zpdesc_page(zpdesc);
>> +
>>   	__ClearPageMovable(page);
>>   	ClearPagePrivate(page);
>>   	set_page_private(page, 0);
>> @@ -887,7 +889,7 @@ static void __free_zspage(struct zs_pool *pool, struct size_class *class,
>>   	do {
>>   		VM_BUG_ON_PAGE(!PageLocked(page), page);
>>   		next = get_next_page(page);
>> -		reset_page(page);
>> +		reset_zpdesc(page_zpdesc(page));
>>   		unlock_page(page);
>>   		dec_zone_page_state(page, NR_ZSPAGES);
>>   		put_page(page);
>> @@ -1865,7 +1867,7 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
>>   		zpdesc_inc_zone_page_state(newzpdesc);
>>   	}
>>   
>> -	reset_page(page);
>> +	reset_zpdesc(zpdesc);
>>   	zpdesc_put(zpdesc);
>>   
>>   	return MIGRATEPAGE_SUCCESS;
>> -- 
>> 2.43.5
>>
>