On 11 Oct 2023, at 11:45, Johannes Weiner wrote:
> On Wed, Oct 11, 2023 at 11:25:27AM -0400, Johannes Weiner wrote:
>> On Tue, Oct 10, 2023 at 05:12:01PM -0400, Johannes Weiner wrote:
>>> On Mon, Oct 02, 2023 at 10:26:44PM -0400, Zi Yan wrote:
>>>> @@ -1614,10 +1652,43 @@ static int move_freepages(struct zone *zone, unsigned long start_pfn,
>>>>
>>>> order = buddy_order(page);
>>>> move_to_free_list(page, zone, order, old_mt, new_mt);
>>>> + /*
>>>> + * set page migratetype 1) only after we move all free pages in
>>>> + * one pageblock and 2) for all pageblocks within the page.
>>>> + *
>>>> + * for 1), since move_to_free_list() checks page migratetype with
>>>> + * old_mt and changing one page migratetype affects all pages
>>>> + * within the same pageblock, if we are moving more than
>>>> + * one free pages in the same pageblock, setting migratetype
>>>> + * right after first move_to_free_list() triggers the warning
>>>> + * in the following move_to_free_list().
>>>> + *
>>>> + * for 2), when a free page order is greater than pageblock_order,
>>>> + * all pageblocks within the free page need to be changed after
>>>> + * move_to_free_list().
>>>
>>> I think this can be somewhat simplified.
>>>
>>> There are two assumptions we can make. Buddies always consist of 2^n
>>> pages. And buddies and pageblocks are naturally aligned. This means
>>> that if this pageblock has the start of a buddy that straddles into
>>> the next pageblock(s), it must be the first page in the block. That in
>>> turn means we can move the handling before the loop.
>>
>> Eh, scratch that. Obviously, a sub-block buddy can straddle blocks :(
>
> I apologize for the back and forth, but I think I had it right the
> first time. Say we have order-0 frees at pfn 511 and 512. Those can't
> merge because their order-0 buddies are 510 and 513 respectively. The
> same keeps higher-order merges below block size within the pageblock.
> So again, due to the pow2 alignment, the only way for a buddy to
> straddle a pageblock boundary is if it's >pageblock_order.
>
> Please double check me on this, because I've stared at your patches
> and the allocator code long enough now to thoroughly confuse myself.
>
> My proposal for the follow-up changes still stands for now.
Sure. I admit that current alloc_contig_range() code is too complicated
and I am going to refactor it.
find_straddling_buddy() looks good to me. You will this change in
alloc_contig_range() to replace get_freepage_start_pfn():
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a86025f5e80a..e8ed25c94863 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6209,7 +6209,6 @@ int alloc_contig_range(unsigned long start, unsigned long end,
unsigned migratetype, gfp_t gfp_mask)
{
unsigned long outer_start, outer_end;
- int order;
int ret = 0;
struct compact_control cc = {
@@ -6283,21 +6282,13 @@ int alloc_contig_range(unsigned long start, unsigned long end,
* isolated thus they won't get removed from buddy.
*/
- order = 0;
- outer_start = get_freepage_start_pfn(start);
-
- if (outer_start != start) {
- order = buddy_order(pfn_to_page(outer_start));
-
- /*
- * outer_start page could be small order buddy page and
- * it doesn't include start page. Adjust outer_start
- * in this case to report failed page properly
- * on tracepoint in test_pages_isolated()
- */
- if (outer_start + (1UL << order) <= start)
- outer_start = start;
- }
+ /*
+ * outer_start page could be small order buddy page and it doesn't
+ * include start page. outer_start is set to start in
+ * find_straddling_buddy() to report failed page properly on tracepoint
+ * in test_pages_isolated()
+ */
+ outer_start = find_straddling_buddy(start);
/* Make sure the range is really isolated. */
if (test_pages_isolated(outer_start, end, 0)) {
Let me go through your move_freepages() in details and get back to you.
Thank you for the feedback!
--
Best Regards,
Yan, Zi
Attachment:
signature.asc
Description: OpenPGP digital signature
