Re: [RFC PATCH 3/6] mm: page_owner: add support for splitting to any order in split page_owner.

[Zi Yan <ziy@nvidia.com>]

[-- Attachment #1: Type: text/plain, Size: 2844 bytes --]

On 12 Nov 2020, at 12:57, Ralph Campbell wrote:

> On 11/11/20 12:40 PM, Zi Yan wrote:
>> From: Zi Yan <ziy@nvidia.com>
>>
>> It adds a new_order parameter to set new page order in page owner.
>> It prepares for upcoming changes to support split huge page to any lower
>> order.
>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>
> Except for a minor fix below, you can add:
> Reviewed-by: Ralph Campbell <rcampbell@nvidia.com>

Thanks.

>
>> ---
>>   include/linux/page_owner.h | 7 ++++---
>>   mm/huge_memory.c           | 2 +-
>>   mm/page_alloc.c            | 2 +-
>>   mm/page_owner.c            | 6 +++---
>>   4 files changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/linux/page_owner.h b/include/linux/page_owner.h
>> index 3468794f83d2..215cbb159568 100644
>> --- a/include/linux/page_owner.h
>> +++ b/include/linux/page_owner.h
>> @@ -31,10 +31,11 @@ static inline void set_page_owner(struct page *page,
>>   		__set_page_owner(page, order, gfp_mask);
>>   }
>>  -static inline void split_page_owner(struct page *page, unsigned int nr)
>> +static inline void split_page_owner(struct page *page, unsigned int nr,
>> +			unsigned int new_order)
>>   {
>>   	if (static_branch_unlikely(&page_owner_inited))
>> -		__split_page_owner(page, nr);
>> +		__split_page_owner(page, nr, new_order);
>>   }
>>   static inline void copy_page_owner(struct page *oldpage, struct page *newpage)
>>   {
>> @@ -60,7 +61,7 @@ static inline void set_page_owner(struct page *page,
>>   {
>>   }
>>   static inline void split_page_owner(struct page *page,
>> -			unsigned int order)
>> +			unsigned int nr, unsigned int new_order)
>>   {
>>   }
>>   static inline void copy_page_owner(struct page *oldpage, struct page *newpage)
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index f599f5b9bf7f..8b7d771ee962 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -2459,7 +2459,7 @@ static void __split_huge_page(struct page *page, struct list_head *list,
>>    	ClearPageCompound(head);
>>  -	split_page_owner(head, nr);
>> +	split_page_owner(head, nr, 1);
>
> Shouldn't this be 0, not 1?
> (new_order not new_nr).
>

Yes, I forgot to fix the call site after I change the function signature. Thanks.

>>   	/* See comment in __split_huge_page_tail() */
>>   	if (PageAnon(head)) {
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index d77220615fd5..a9eead0e091a 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -3284,7 +3284,7 @@ void split_page(struct page *page, unsigned int order)
>>    	for (i = 1; i < (1 << order); i++)
>>   		set_page_refcounted(page + i);
>> -	split_page_owner(page, 1 << order);
>> +	split_page_owner(page, 1 << order, 1);
>
> Ditto, 0.
>

Sure, will fix this too.


—
Best Regards,
Yan Zi

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]