Messages in this thread | | | From | "Zi Yan" <> | Subject | Re: [RFC PATCH 3/6] mm: page_owner: add support for splitting to any order in split page_owner. | Date | Fri, 13 Nov 2020 20:08:58 -0500 |
| |
On 13 Nov 2020, at 19:15, Roman Gushchin wrote:
> On Wed, Nov 11, 2020 at 03:40:05PM -0500, 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> >> --- >> 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) > > With the addition of the new argument it's a bit hard to understand > what the function is supposed to do. It seems like nr == > page_order(page), > is it right? Maybe we can pass old_order and new_order? Or just the > page > and the new order?
Yeah, it is a bit confusing. Please see more below.
> >> { >> } >> 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); >> >> /* 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); >> } >> EXPORT_SYMBOL_GPL(split_page); >> >> diff --git a/mm/page_owner.c b/mm/page_owner.c >> index b735a8eafcdb..2b7f7e9056dc 100644 >> --- a/mm/page_owner.c >> +++ b/mm/page_owner.c >> @@ -204,7 +204,7 @@ void __set_page_owner_migrate_reason(struct page >> *page, int reason) >> page_owner->last_migrate_reason = reason; >> } >> >> -void __split_page_owner(struct page *page, unsigned int nr) >> +void __split_page_owner(struct page *page, unsigned int nr, unsigned >> int new_order) >> { >> int i; >> struct page_ext *page_ext = lookup_page_ext(page); >> @@ -213,9 +213,9 @@ void __split_page_owner(struct page *page, >> unsigned int nr) >> if (unlikely(!page_ext)) >> return; >> >> - for (i = 0; i < nr; i++) { >> + for (i = 0; i < nr; i += (1 << new_order)) { >> page_owner = get_page_owner(page_ext); >> - page_owner->order = 0; >> + page_owner->order = new_order; >> page_ext = page_ext_next(page_ext); > > I believe there cannot be any leftovers because nr is always a power > of 2. > Is it true? Converting nr argument to order (if it's possible) will > make it obvious.
Right. nr = thp_nr_pages(head), which is a power of 2. There would not be any leftover.
Matthew recently converted split_page_owner to take nr instead of order.[1] But I am not sure why, since it seems to me that two call sites (__split_huge_page in mm/huge_memory.c and split_page in mm/page_alloc.c) can pass the order information.
[1]https://lore.kernel.org/linux-mm/20200908195539.25896-4-willy@infradead.org/
— Best Regards, Yan Zi
| |