Re: [PATCH] mm/page_alloc: fix alignment for alloc_contig_pages_noprof()

[Anshuman Khandual <anshuman.khandual@arm.com>]

On 23/09/25 7:16 AM, Zi Yan wrote:
> On 22 Sep 2025, at 20:19, Wei Yang wrote:
> 
>> Commit 5e27a2df03b8 ("mm/page_alloc: add alloc_contig_pages()")
>> introduced generic method for alloc_contig_pages(). But the alignment
>> calculation seems wrong.
>>
>> Since ALIGN() only accept power of two value, while nr_pages could be
>> any positive one, the result is not defined.
> 
> The result would not be any value lower than zone->zone_start_pfn,
> so the worst case is getting an unaligned PFN range. I guess
> most of the time nr_pages would be power of 2.

Agreed.

Also as Dev had pointed out earlier, this function gives no
guarantee on alignment of non-power-of-2 requests. Hence I
don't have a strong opinion either way, but does it really
qualify for a "Fixes:" tag ?
> 
>>
>> Use roundup() to calculate the correct alignment.
>>
>> Fixes: 5e27a2df03b8 ("mm/page_alloc: add alloc_contig_pages()")
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
>> Cc: Oscar Salvador <osalvador@suse.de>
>> Cc: Brendan Jackman <jackmanb@google.com>
>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: Suren Baghdasaryan <surenb@google.com>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> Cc: Wei Yang <richard.weiyang@gmail.com>
>> Cc: Zi Yan <ziy@nvidia.com>
>> ---
>>  mm/page_alloc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index a1bcc1e003c7..a17a6014e3db 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -7095,7 +7095,7 @@ struct page *alloc_contig_pages_noprof(unsigned long nr_pages, gfp_t gfp_mask,
>>  					gfp_zone(gfp_mask), nodemask) {
>>  		spin_lock_irqsave(&zone->lock, flags);
>>
>> -		pfn = ALIGN(zone->zone_start_pfn, nr_pages);
>> +		pfn = roundup(zone->zone_start_pfn, nr_pages);
>>  		while (zone_spans_last_pfn(zone, pfn, nr_pages)) {
>>  			if (pfn_range_valid_contig(pfn, nr_pages)) {
>>  				/*
>> -- 
>> 2.34.1
> 
> LGTM. Reviewed-by: Zi Yan <ziy@nvidia.com>
> 
> Best Regards,
> Yan, Zi