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? */
>
