On Tue, Nov 17, 2020 at 01:35:37PM -0800, Roman Gushchin wrote:
On Tue, Nov 17, 2020 at 09:22:55PM +0000, Matthew Wilcox wrote:
On Tue, Nov 17, 2020 at 04:12:03PM -0500, Zi Yan wrote:
On 17 Nov 2020, at 16:05, Matthew Wilcox wrote:
On Fri, Nov 13, 2020 at 05:38:01PM -0800, Roman Gushchin wrote:
On Fri, Nov 13, 2020 at 08:08:58PM -0500, Zi Yan wrote:
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.
Yeah, I'm not sure why too. Maybe Matthew has some input here? You can also pass new_nr, but IMO orders look so much better here.
If only I'd written that information in the changelog ... oh wait, I did!
mm/page_owner: change split_page_owner to take a count The implementation of split_page_owner() prefers a count rather than the old order of the page. When we support a variable size THP, we won't have the order at this point, but we will have the number of pages. So change the interface to what the caller and callee would prefer.There are two callers, split_page in mm/page_alloc.c and __split_huge_page in mm/huge_memory.c. The former has the page order. The latter has the page order information before __split_huge_page_tail is called, so we can do old_order = thp_order(head) instead of nr = thp_nr_page(head) and use old_order. What am I missing there?
Sure, we could also do that. But what I wrote was true at the time I wrote it.
Sure, I was asking about if you're ok with going back to orders or there are better ideas. I'm sorry if it wasn't clear and sounded differently.
It just seems to me than a function is taking nr and order (as in Zi's last version), I'd expect that it's a number of pages of given order, or something like this. So I'd avoid mixing them. Orders are slightly better if nr is always a power of two, it's just more obvious from looking at the code.
I think it's awkward no matter which way round we do it.
If we pass old_order, new_order then we create extra work for both caller and callee.
If we pass old_nr, new_order, it looks weird for humans.
At the end of the day, I'm not that invested in which we do.