Re: [PATCH] mm/compaction: fix low_pfn advance on isolating hugetlb

[Wei Yang <richard.weiyang@gmail.com>]

On Wed, Sep 10, 2025 at 09:35:53PM -0400, Zi Yan wrote:
>On 10 Sep 2025, at 21:25, Wei Yang wrote:
>
>> On Wed, Sep 10, 2025 at 09:22:40AM +0000, Wei Yang wrote:
>>> Commit 56ae0bb349b4 ("mm: compaction: convert to use a folio in
>>> isolate_migratepages_block()") converts api from page to folio. But the
>>> low_pfn advance for hugetlb page seems wrong when low_pfn doesn't point
>>> to head page.
>>>
>>> Originally, if page is a hugetlb tail page, compound_nr() return 1,
>>> which means low_pfn only advance one in next iteration. After the
>>> change, low_pfn would advance more than the hugetlb range, since
>>> folio_nr_pages() always return total number of the large page. This
>>> results in skipping some range to isolate and then to migrate.
>>>
>>> The worst case for alloc_contig is it does all the isolation and
>>> migration, but finally find some range is still not isolated. And then
>>> undo all the work and try a new range.
>>>
>>> Advance low_pfn to the end of hugetlb.
>>>
>>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>>> Fixes: 56ae0bb349b4 ("mm: compaction: convert to use a folio in isolate_migratepages_block()")
>>> Cc: Kefeng Wang <wangkefeng.wang@huawei.com>
>>> Cc: Oscar Salvador <osalvador@suse.de>
>>
>> Forgot to cc stable.
>>
>> Cc: <stable@vger.kernel.org>
>
>Is there any bug report to justify the backport? Since it is more likely
>to be a performance issue instead of a correctness issue.
>

OK, I thought cc-stable is paired with fixes tag.

If not, please drop it.

>>
>>> ---
>>> mm/compaction.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/mm/compaction.c b/mm/compaction.c
>>> index bf021b31c7ec..1e8f8eca318c 100644
>>> --- a/mm/compaction.c
>>> +++ b/mm/compaction.c
>>> @@ -989,7 +989,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
>>> 				 * Hugepage was successfully isolated and placed
>>> 				 * on the cc->migratepages list.
>>> 				 */
>>> -				low_pfn += folio_nr_pages(folio) - 1;
>>> +				low_pfn += folio_nr_pages(folio) - folio_page_idx(folio, page) - 1;
>>
>> One question is why we advance compound_nr() in original version.
>>
>> Yes, there are several places advancing compound_nr(), but it seems to iterate
>> on the same large page and do the same thing and advance 1 again.
>>
>> Not sure which part story I missed.
>
>isolate_migratepages_block() starts from the beginning of a pageblock.
>How likely the code hit in the middle of a hugetlb?
>

OK, this is a kind of optimization based on the knowledge it is not likely to
be a tail page?

-- 
Wei Yang
Help you, Help me