Re: [PATCH 1/5] mm/compaction: allow blockpfn outside of pageblock for high order buddy page

[Kemeng Shi]

on 8/2/2023 9:57 AM, Baolin Wang wrote:
> 
> 
> On 7/30/2023 1:43 AM, Kemeng Shi wrote:
>> Commit 9fcd6d2e052ee ("mm, compaction: skip compound pages by order in
>> free scanner") skiped compound pages to save iterations and limit blockpfn
>> to reach outside of page block in case of bogus compound_order. While this
>> also limit pfnblock outside page block in case a buddy page with order
>> higher than page block is found. After this, isolate_freepages_range will
>> fail unexpectedly as it will fail to isolate the page block which was
>> isolated successfully by high order buddy page in previous page block
>> and abort the free page isolation.
> 
> Not sure I uderstand the problem, why the isolate_freepages_range() will fail? In isolate_freepages_range(), it did not use the 'blockpfn' cursor to try next candidate pfn, instead using the 'isolated' to calculate next cursor. Or I missed something else?
> 
Ah, my bad. I will drop this in next version.

>> Fix this by allow blockpfn outside of pageblock in case of high order
>> buddy page.
>>
>> Fixes: 9fcd6d2e052ee ("mm, compaction: skip compound pages by order in free scanner")
>> Signed-off-by: Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx>
>> ---
>>   mm/compaction.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/compaction.c b/mm/compaction.c
>> index 6841c0496223..d1d28d57e5bd 100644
>> --- a/mm/compaction.c
>> +++ b/mm/compaction.c
>> @@ -681,8 +681,10 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
>>       /*
>>        * There is a tiny chance that we have read bogus compound_order(),
>>        * so be careful to not go outside of the pageblock.
>> +     * Allow blockpfn outside pageblock in normal case that we isolate
>> +     * buddy page with order more than pageblock order.
>>        */
>> -    if (unlikely(blockpfn > end_pfn))
>> +    if (unlikely(blockpfn > end_pfn) && total_isolated <= pageblock_nr_pages)
>>           blockpfn = end_pfn;
>>         trace_mm_compaction_isolate_freepages(*start_pfn, blockpfn,
>> @@ -1418,7 +1420,7 @@ fast_isolate_around(struct compact_control *cc, unsigned long pfn)
>>       isolate_freepages_block(cc, &start_pfn, end_pfn, &cc->freepages, 1, false);
>>         /* Skip this pageblock in the future as it's full or nearly full */
>> -    if (start_pfn == end_pfn && !cc->no_set_skip_hint)
>> +    if (start_pfn >= end_pfn && !cc->no_set_skip_hint)
>>           set_pageblock_skip(page);
>>   }
>>   @@ -1687,7 +1689,7 @@ static void isolate_freepages(struct compact_control *cc)
>>                       block_end_pfn, freelist, stride, false);
>>             /* Update the skip hint if the full pageblock was scanned */
>> -        if (isolate_start_pfn == block_end_pfn)
>> +        if (isolate_start_pfn >= block_end_pfn)
>>               update_pageblock_skip(cc, page, block_start_pfn);
>>             /* Are enough freepages isolated? */
>