Re: [PATCH v2] mm/contig_alloc: fix alloc_contig_range when __GFP_COMP and order < MAX_ORDER

[Jinjiang Tu <tujinjiang@huawei.com>]

在 2025/5/11 16:04, David Hildenbrand 写道:
> On 25.04.25 13:04, Zi Yan wrote:
>> On 25 Apr 2025, at 6:33, David Hildenbrand wrote:
>>
>>> On 21.04.25 03:36, Jinjiang Tu wrote:
>>>> When calling alloc_contig_range() with __GFP_COMP and the order of
>>>> requested pfn range is pageblock_order, less than MAX_ORDER, I 
>>>> triggered
>>>> WARNING as follows:
>>>>
>>>>    PFN range: requested [2150105088, 2150105600), allocated 
>>>> [2150105088, 2150106112)
>>>>    WARNING: CPU: 3 PID: 580 at mm/page_alloc.c:6877 
>>>> alloc_contig_range+0x280/0x340
>>>>
>>>
>>> Just to verify: there is no such in-tree user, right?
>>>
>>>> alloc_contig_range() marks pageblocks of the requested pfn range to be
>>>> isolated, migrate these pages if they are in use and will be freed to
>>>> MIGRATE_ISOLATED freelist.
>>>>
>>>> Suppose two alloc_contig_range() calls at the same time and the 
>>>> requested
>>>> pfn range are [0x80280000, 0x80280200) and [0x80280200, 0x80280400)
>>>> respectively. Suppose the two memory range are in use, then
>>>> alloc_contig_range() will migrate and free these pages to 
>>>> MIGRATE_ISOLATED
>>>> freelist. __free_one_page() will merge MIGRATE_ISOLATE buddy to larger
>>>> buddy, resulting in a MAX_ORDER buddy. Finally, find_large_buddy() in
>>>> alloc_contig_range() returns a MAX_ORDER buddy and results in WARNING.
>>>>
>>>> To fix it, call free_contig_range() to free the excess pfn range.
>>>>
>>>> Fixes: e98337d11bbd ("mm/contig_alloc: support __GFP_COMP")
>>>> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
>>>> ---
>>>> Changelog since v1:
>>>>    * Add comment and remove redundant code, suggested by Zi Yan
>>>>
>>>>    mm/page_alloc.c | 20 ++++++++++++++++++--
>>>>    1 file changed, 18 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>>> index 579789600a3c..f0162ab991ad 100644
>>>> --- a/mm/page_alloc.c
>>>> +++ b/mm/page_alloc.c
>>>> @@ -6440,6 +6440,7 @@ int alloc_contig_range_noprof(unsigned long 
>>>> start, unsigned long end,
>>>>            .alloc_contig = true,
>>>>        };
>>>>        INIT_LIST_HEAD(&cc.migratepages);
>>>> +    bool is_range_aligned;
>>>
>>> is "aligned" the right word? Aligned to what?
>>>
>>> I do wonder if we could do the following on top, checking that the 
>>> range is suitable for __GFP_COMP earlier.
>>>
>>
>> The change below makes the code cleaner. Acked-by: Zi Yan 
>> <ziy@nvidia.com>
>
> Jinjiang, can you integrate that into your patch and resend?

Sorry for late reply. I will do it today.

Thanks for review.

>
> Thanks!
>