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

[Baolin Wang]

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?

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