On Fri, Aug 29, 2025 at 12:06:21PM +0200, David Hildenbrand wrote:

On 28.08.25 16:37, Lorenzo Stoakes wrote:

quoted

On Thu, Aug 28, 2025 at 12:01:10AM +0200, David Hildenbrand wrote:

quoted

Let's reject them early, which in turn makes folio_alloc_gigantic() reject
them properly.

To avoid converting from order to nr_pages, let's just add MAX_FOLIO_ORDER
and calculate MAX_FOLIO_NR_PAGES based on that.

Reviewed-by: Zi Yan <ziy@nvidia.com>
Acked-by: SeongJae Park <sj@kernel.org>
Signed-off-by: David Hildenbrand <redacted>

Some nits, but overall LGTM so:

Reviewed-by: Lorenzo Stoakes <redacted>

quoted

---
  include/linux/mm.h | 6 ++++--
  mm/page_alloc.c    | 5 ++++-
  2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 00c8a54127d37..77737cbf2216a 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h

@@ -2055,11 +2055,13 @@ static inline long folio_nr_pages(const struct folio *folio)

  /* Only hugetlbfs can allocate folios larger than MAX_ORDER */
  #ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
-#define MAX_FOLIO_NR_PAGES	(1UL << PUD_ORDER)
+#define MAX_FOLIO_ORDER		PUD_ORDER
  #else
-#define MAX_FOLIO_NR_PAGES	MAX_ORDER_NR_PAGES
+#define MAX_FOLIO_ORDER		MAX_PAGE_ORDER
  #endif

+#define MAX_FOLIO_NR_PAGES	(1UL << MAX_FOLIO_ORDER)

BIT()?

I don't think we want to use BIT whenever we convert from order -> folio --
which is why we also don't do that in other code.

It seems a bit arbitrary, like we open-code this (at risk of making a mistake)
in some places but not others.

BIT() is nice in the context of flags and bitmaps, but not really in the
context of converting orders to pages.

It's nice for setting a specific bit :)

One could argue that maybe one would want a order_to_pages() helper (that
could use BIT() internally), but I am certainly not someone that would
suggest that at this point ...  :)

I mean maybe.

Anyway as I said none of this is massively important, the open-coding here is
correct, just seems silly.

quoted

quoted

+
  /*
   * compound_nr() returns the number of pages in this potentially compound
   * page.  compound_nr() can be called on a tail page, and is defined to

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index baead29b3e67b..426bc404b80cc 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c

@@ -6833,6 +6833,7 @@ static int __alloc_contig_verify_gfp_mask(gfp_t gfp_mask, gfp_t *gfp_cc_mask)
  int alloc_contig_range_noprof(unsigned long start, unsigned long end,
  			      acr_flags_t alloc_flags, gfp_t gfp_mask)

Funny btw th

quoted

quoted

  {
+	const unsigned int order = ilog2(end - start);
  	unsigned long outer_start, outer_end;
  	int ret = 0;

@@ -6850,6 +6851,9 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
  					    PB_ISOLATE_MODE_CMA_ALLOC :
  					    PB_ISOLATE_MODE_OTHER;

+	if (WARN_ON_ONCE((gfp_mask & __GFP_COMP) && order > MAX_FOLIO_ORDER))
+		return -EINVAL;

Possibly not worth it for a one off, but be nice to have this as a helper function, like:

static bool is_valid_order(gfp_t gfp_mask, unsigned int order)
{
	return !(gfp_mask & __GFP_COMP) || order <= MAX_FOLIO_ORDER;
}

Then makes this:

	if (WARN_ON_ONCE(!is_valid_order(gfp_mask, order)))
		return -EINVAL;

Kinda self-documenting!

I don't like it -- especially forwarding __GFP_COMP.

is_valid_folio_order() to wrap the order check? Also not sure.

OK, it's not a big deal.

Can we have a comment explaining this though? As people might be confused
as to why we check this here and not elsewhere.

So I'll leave it as is I think.

Right fine.

Thanks for all the review!

--
Cheers

David / dhildenb