On 28.08.25 16:37, Lorenzo Stoakes wrote:
On Thu, Aug 28, 2025 at 12:01:10AM +0200, David Hildenbrand wrote:
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 <z...@nvidia.com>
Acked-by: SeongJae Park <s...@kernel.org>
Signed-off-by: David Hildenbrand <da...@redhat.com>

Some nits, but overall LGTM so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoa...@oracle.com>

---
  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.

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

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 ... :)


+
  /*
   * 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)
  {
+       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.

So I'll leave it as is I think.

Thanks for all the review!

--
Cheers

David / dhildenb

Reply via email to