On 12 Nov 2020, at 12:57, Ralph Campbell wrote:
> On 11/11/20 12:40 PM, Zi Yan wrote:
>> From: Zi Yan <ziy@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>
> Except for a minor fix below, you can add:
> Reviewed-by: Ralph Campbell <rcampbell@xxxxxxxxxx>
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:
signature.asc
Description: OpenPGP digital signature
