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

[Michal Hocko <mhocko@suse.com>]

On Tue 23-09-25 00:19:43, 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.
> 
> Use roundup() to calculate the correct alignment.

What is the problem you are trying to fix here? The ALIGN is certainly
not well defined for nr_pages that is not power of two but does any
caller of alloc_contig_pages assumes any specific alignment, especially
when nr_pages is not power of two?

> 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

-- 
Michal Hocko
SUSE Labs