[RFC PATCH v2 0/4] make vmalloc gfp flags usage more apparent

[RFC PATCH v2 0/4] make vmalloc gfp flags usage more apparent

[Vishal Moola (Oracle)]

We should do a better job at enforcing gfp flags for vmalloc. Right now, we
have a kernel-doc for __vmalloc_node_range(), and hope callers pass in
supported flags. If a caller were to pass in an unsupported flag, we may
BUG, silently clear it, or completely ignore it.

If we are more proactive about enforcing gfp flags, we can making sure
callers know when they may be asking for unsupported behavior.

This patchset lets vmalloc control the incoming gfp flags, and cleans up
some hard to read gfp code.

---
Based on mm-new.

v2:
  - Whitelist supported gfp flags instead of blacklisting the unsupported
  - Move the flags check up to the only exported functions that accept
    flags:
	__vmalloc_noprof() and vmalloc_huge_node_prof

Vishal Moola (Oracle) (4):
  mm/vmalloc: warn on invalid vmalloc gfp flags
  mm/vmalloc: Add a helper to optimize vmalloc allocation gfps
  mm/vmalloc: cleanup large_gfp in vm_area_alloc_pages()
  mm/vmalloc: cleanup gfp flag use in new_vmap_block()

 mm/vmalloc.c | 48 ++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 40 insertions(+), 8 deletions(-)

-- 
2.51.1

[RFC PATCH v2 1/4] mm/vmalloc: warn on invalid vmalloc gfp flags

[Vishal Moola (Oracle)]

Vmalloc explicitly supports a list of flags, but we never enforce them.
vmalloc has been trying to handle unsupported flags by clearing and
setting flags wherever necessary. This is messy and makes the code
harder to understand, when we could simply check for a supported input
immediately instead.

Define a helper mask and function telling callers they have passed in
invalid flags, and clear those unsupported vmalloc flags.

Suggested-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 mm/vmalloc.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 0832f944544c..290016c7fb58 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -3911,6 +3911,26 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
 	return NULL;
 }
 
+/*
+ * See __vmalloc_node_range() for a clear list of supported vmalloc flags.
+ * This gfp lists all flags currently passed through vmalloc. Currently,
+ * __GFP_ZERO is used by BFP and __GFP_NORETRY is used by percpu.
+ */
+#define GFP_VMALLOC_SUPPORTED (GFP_KERNEL | GFP_ATOMIC | GFP_NOWAIT |\
+				__GFP_NOFAIL |  __GFP_ZERO | __GFP_NORETRY)
+
+static gfp_t vmalloc_fix_flags(gfp_t flags)
+{
+	gfp_t invalid_mask = flags & ~GFP_VMALLOC_SUPPORTED;
+
+	flags &= GFP_VMALLOC_SUPPORTED;
+	pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
+			invalid_mask, &invalid_mask, flags, &flags);
+	dump_stack();
+
+	return flags;
+}
+
 /**
  * __vmalloc_node_range - allocate virtually contiguous memory
  * @size:		  allocation size
@@ -4092,6 +4112,8 @@ EXPORT_SYMBOL_GPL(__vmalloc_node_noprof);
 
 void *__vmalloc_noprof(unsigned long size, gfp_t gfp_mask)
 {
+	if (gfp_mask & ~GFP_VMALLOC_SUPPORTED)
+		gfp_mask = vmalloc_fix_flags(gfp_mask);
 	return __vmalloc_node_noprof(size, 1, gfp_mask, NUMA_NO_NODE,
 				__builtin_return_address(0));
 }
@@ -4131,6 +4153,8 @@ EXPORT_SYMBOL(vmalloc_noprof);
  */
 void *vmalloc_huge_node_noprof(unsigned long size, gfp_t gfp_mask, int node)
 {
+	if (gfp_mask & ~GFP_VMALLOC_SUPPORTED)
+		gfp_mask = vmalloc_fix_flags(gfp_mask);
 	return __vmalloc_node_range_noprof(size, 1, VMALLOC_START, VMALLOC_END,
 					   gfp_mask, PAGE_KERNEL, VM_ALLOW_HUGE_VMAP,
 					   node, __builtin_return_address(0));
-- 
2.51.1

[RFC PATCH v2 2/4] mm/vmalloc: Add a helper to optimize vmalloc allocation gfps

[Vishal Moola (Oracle)]

vm_area_alloc_pages() attempts to use different gfp flags as a way
to optimize allocations. This has been done inline which makes things
harder to read.

Add a helper function to make the code more readable.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 mm/vmalloc.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 290016c7fb58..38d94c0c8153 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -3614,6 +3614,17 @@ void *vmap_pfn(unsigned long *pfns, unsigned int count, pgprot_t prot)
 EXPORT_SYMBOL_GPL(vmap_pfn);
 #endif /* CONFIG_VMAP_PFN */
 
+/*
+ * Helper for vmalloc to adjust the gfp flags for certain allocations.
+ */
+static inline gfp_t vmalloc_gfp_adjust(gfp_t flags, const bool large)
+{
+	flags |= __GFP_NOWARN;
+	if (large)
+		flags &= ~__GFP_NOFAIL;
+	return flags;
+}
+
 static inline unsigned int
 vm_area_alloc_pages(gfp_t gfp, int nid,
 		unsigned int order, unsigned int nr_pages, struct page **pages)
@@ -3852,9 +3863,9 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
 	 * Please note, the __vmalloc_node_range_noprof() falls-back
 	 * to order-0 pages if high-order attempt is unsuccessful.
 	 */
-	area->nr_pages = vm_area_alloc_pages((page_order ?
-		gfp_mask & ~__GFP_NOFAIL : gfp_mask) | __GFP_NOWARN,
-		node, page_order, nr_small_pages, area->pages);
+	area->nr_pages = vm_area_alloc_pages(
+			vmalloc_gfp_adjust(gfp_mask, page_order), node,
+			page_order, nr_small_pages, area->pages);
 
 	atomic_long_add(area->nr_pages, &nr_vmalloc_pages);
 	/* All pages of vm should be charged to same memcg, so use first one. */
-- 
2.51.1

[RFC PATCH v2 3/4] mm/vmalloc: cleanup large_gfp in vm_area_alloc_pages()

[Vishal Moola (Oracle)]

Now that we have already checked for unsupported flags, we can use the
helper function to set the necessary gfp flags for the large order
allocation optimization.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 mm/vmalloc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 38d94c0c8153..ced77fc65ce3 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -3634,10 +3634,8 @@ vm_area_alloc_pages(gfp_t gfp, int nid,
 	unsigned int max_attempt_order = MAX_PAGE_ORDER;
 	struct page *page;
 	int i;
-	gfp_t large_gfp = (gfp &
-		~(__GFP_DIRECT_RECLAIM | __GFP_NOFAIL | __GFP_COMP))
-		| __GFP_NOWARN;
 	unsigned int large_order = ilog2(nr_remaining);
+	gfp_t large_gfp = vmalloc_gfp_adjust(gfp, large_order) & ~__GFP_DIRECT_RECLAIM;
 
 	large_order = min(max_attempt_order, large_order);
 
-- 
2.51.1

[RFC PATCH v2 4/4] mm/vmalloc: cleanup gfp flag use in new_vmap_block()

[Vishal Moola (Oracle)]

The only caller, vb_alloc(), passes GFP_KERNEL into new_vmap_block()
which is a subset of GFP_RECLAIM_MASK. Since there's no reason to use
this mask here, remove it.

Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
---
 mm/vmalloc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index ced77fc65ce3..e6cca6219e48 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2699,8 +2699,7 @@ static void *new_vmap_block(unsigned int order, gfp_t gfp_mask)
 
 	node = numa_node_id();
 
-	vb = kmalloc_node(sizeof(struct vmap_block),
-			gfp_mask & GFP_RECLAIM_MASK, node);
+	vb = kmalloc_node(sizeof(struct vmap_block), gfp_mask, node);
 	if (unlikely(!vb))
 		return ERR_PTR(-ENOMEM);
 
-- 
2.51.1

Re: [RFC PATCH v2 1/4] mm/vmalloc: warn on invalid vmalloc gfp flags

[Christoph Hellwig]

On Mon, Nov 03, 2025 at 11:04:26AM -0800, Vishal Moola (Oracle) wrote:
> Vmalloc explicitly supports a list of flags, but we never enforce them.
> vmalloc has been trying to handle unsupported flags by clearing and
> setting flags wherever necessary. This is messy and makes the code
> harder to understand, when we could simply check for a supported input
> immediately instead.
> 
> Define a helper mask and function telling callers they have passed in
> invalid flags, and clear those unsupported vmalloc flags.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

And just for the record:  I very much suspect we'll find someone
passing unsuported flags soon with this hidden somewhere that will
need fixing or at least temporarily extending the mask.

Re: [RFC PATCH v2 1/4] mm/vmalloc: warn on invalid vmalloc gfp flags

[Uladzislau Rezki]

On Mon, Nov 03, 2025 at 11:04:26AM -0800, Vishal Moola (Oracle) wrote:
> Vmalloc explicitly supports a list of flags, but we never enforce them.
> vmalloc has been trying to handle unsupported flags by clearing and
> setting flags wherever necessary. This is messy and makes the code
> harder to understand, when we could simply check for a supported input
> immediately instead.
> 
> Define a helper mask and function telling callers they have passed in
> invalid flags, and clear those unsupported vmalloc flags.
> 
> Suggested-by: Christoph Hellwig <hch@infradead.org>
> Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> ---
>  mm/vmalloc.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 0832f944544c..290016c7fb58 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3911,6 +3911,26 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
>  	return NULL;
>  }
>  
> +/*
> + * See __vmalloc_node_range() for a clear list of supported vmalloc flags.
> + * This gfp lists all flags currently passed through vmalloc. Currently,
> + * __GFP_ZERO is used by BFP and __GFP_NORETRY is used by percpu.
> + */
> +#define GFP_VMALLOC_SUPPORTED (GFP_KERNEL | GFP_ATOMIC | GFP_NOWAIT |\
> +				__GFP_NOFAIL |  __GFP_ZERO | __GFP_NORETRY)
> +
> +static gfp_t vmalloc_fix_flags(gfp_t flags)
> +{
> +	gfp_t invalid_mask = flags & ~GFP_VMALLOC_SUPPORTED;
> +
> +	flags &= GFP_VMALLOC_SUPPORTED;
> +	pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
> +			invalid_mask, &invalid_mask, flags, &flags);
> +	dump_stack();
>
WARN_ON() or friends? It prints the stack.

> +
> +	return flags;
> +}
> +
>  /**
>   * __vmalloc_node_range - allocate virtually contiguous memory
>   * @size:		  allocation size
> @@ -4092,6 +4112,8 @@ EXPORT_SYMBOL_GPL(__vmalloc_node_noprof);
>  
>  void *__vmalloc_noprof(unsigned long size, gfp_t gfp_mask)
>  {
> +	if (gfp_mask & ~GFP_VMALLOC_SUPPORTED)
> +		gfp_mask = vmalloc_fix_flags(gfp_mask);
>  	return __vmalloc_node_noprof(size, 1, gfp_mask, NUMA_NO_NODE,
>  				__builtin_return_address(0));
>  }
> @@ -4131,6 +4153,8 @@ EXPORT_SYMBOL(vmalloc_noprof);
>   */
>  void *vmalloc_huge_node_noprof(unsigned long size, gfp_t gfp_mask, int node)
>  {
> +	if (gfp_mask & ~GFP_VMALLOC_SUPPORTED)
> +		gfp_mask = vmalloc_fix_flags(gfp_mask);
>
gfp_mask = check_and_fix_flags()? 

--
Uladzislau Rezki

Re: [RFC PATCH v2 4/4] mm/vmalloc: cleanup gfp flag use in new_vmap_block()

[Uladzislau Rezki]

On Mon, Nov 03, 2025 at 11:04:29AM -0800, Vishal Moola (Oracle) wrote:
> The only caller, vb_alloc(), passes GFP_KERNEL into new_vmap_block()
> which is a subset of GFP_RECLAIM_MASK. Since there's no reason to use
> this mask here, remove it.
> 
> Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> ---
>  mm/vmalloc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index ced77fc65ce3..e6cca6219e48 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2699,8 +2699,7 @@ static void *new_vmap_block(unsigned int order, gfp_t gfp_mask)
>  
>  	node = numa_node_id();
>  
> -	vb = kmalloc_node(sizeof(struct vmap_block),
> -			gfp_mask & GFP_RECLAIM_MASK, node);
> +	vb = kmalloc_node(sizeof(struct vmap_block), gfp_mask, node);
>  	if (unlikely(!vb))
>  		return ERR_PTR(-ENOMEM);
>  
> -- 
> 2.51.1
> 
LGTM:

Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com>

--
Uladzislau Rezki

Re: [RFC PATCH v2 1/4] mm/vmalloc: warn on invalid vmalloc gfp flags

[Uladzislau Rezki]

> On Mon, Nov 03, 2025 at 11:04:26AM -0800, Vishal Moola (Oracle) wrote:
> > Vmalloc explicitly supports a list of flags, but we never enforce them.
> > vmalloc has been trying to handle unsupported flags by clearing and
> > setting flags wherever necessary. This is messy and makes the code
> > harder to understand, when we could simply check for a supported input
> > immediately instead.
> >
> > Define a helper mask and function telling callers they have passed in
> > invalid flags, and clear those unsupported vmalloc flags.
> >
> > Suggested-by: Christoph Hellwig <hch@infradead.org>
> > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> > ---
> >  mm/vmalloc.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > index 0832f944544c..290016c7fb58 100644
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -3911,6 +3911,26 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
> >       return NULL;
> >  }
> >
> > +/*
> > + * See __vmalloc_node_range() for a clear list of supported vmalloc flags.
> > + * This gfp lists all flags currently passed through vmalloc. Currently,
> > + * __GFP_ZERO is used by BFP and __GFP_NORETRY is used by percpu.
> > + */
> > +#define GFP_VMALLOC_SUPPORTED (GFP_KERNEL | GFP_ATOMIC | GFP_NOWAIT |\
> > +                             __GFP_NOFAIL |  __GFP_ZERO | __GFP_NORETRY)
> > +
Also we do support %GFP_NOFS and %GFP_NOIO flags.

-- 
Uladzislau Rezki

Re: [RFC PATCH v2 1/4] mm/vmalloc: warn on invalid vmalloc gfp flags

[Vishal Moola (Oracle)]

On Tue, Nov 04, 2025 at 05:28:16PM +0100, Uladzislau Rezki wrote:
> On Mon, Nov 03, 2025 at 11:04:26AM -0800, Vishal Moola (Oracle) wrote:
> > Vmalloc explicitly supports a list of flags, but we never enforce them.
> > vmalloc has been trying to handle unsupported flags by clearing and
> > setting flags wherever necessary. This is messy and makes the code
> > harder to understand, when we could simply check for a supported input
> > immediately instead.
> > 
> > Define a helper mask and function telling callers they have passed in
> > invalid flags, and clear those unsupported vmalloc flags.
> > 
> > Suggested-by: Christoph Hellwig <hch@infradead.org>
> > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> > ---
> >  mm/vmalloc.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> > 
> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > index 0832f944544c..290016c7fb58 100644
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -3911,6 +3911,26 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
> >  	return NULL;
> >  }
> >  
> > +/*
> > + * See __vmalloc_node_range() for a clear list of supported vmalloc flags.
> > + * This gfp lists all flags currently passed through vmalloc. Currently,
> > + * __GFP_ZERO is used by BFP and __GFP_NORETRY is used by percpu.
> > + */
> > +#define GFP_VMALLOC_SUPPORTED (GFP_KERNEL | GFP_ATOMIC | GFP_NOWAIT |\
> > +				__GFP_NOFAIL |  __GFP_ZERO | __GFP_NORETRY)
> > +
> > +static gfp_t vmalloc_fix_flags(gfp_t flags)
> > +{
> > +	gfp_t invalid_mask = flags & ~GFP_VMALLOC_SUPPORTED;
> > +
> > +	flags &= GFP_VMALLOC_SUPPORTED;
> > +	pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
> > +			invalid_mask, &invalid_mask, flags, &flags);
> > +	dump_stack();
> >
> WARN_ON() or friends? It prints the stack.

My understanding of WARN_ON() variants is they're used for internal
kernel concerns, while the pr_warn() variants are for situations like
this where proprietary drivers can cause unexpected behavior.

> > +
> > +	return flags;
> > +}
> > +
> >  /**
> >   * __vmalloc_node_range - allocate virtually contiguous memory
> >   * @size:		  allocation size
> > @@ -4092,6 +4112,8 @@ EXPORT_SYMBOL_GPL(__vmalloc_node_noprof);
> >  
> >  void *__vmalloc_noprof(unsigned long size, gfp_t gfp_mask)
> >  {
> > +	if (gfp_mask & ~GFP_VMALLOC_SUPPORTED)
> > +		gfp_mask = vmalloc_fix_flags(gfp_mask);
> >  	return __vmalloc_node_noprof(size, 1, gfp_mask, NUMA_NO_NODE,
> >  				__builtin_return_address(0));
> >  }
> > @@ -4131,6 +4153,8 @@ EXPORT_SYMBOL(vmalloc_noprof);
> >   */
> >  void *vmalloc_huge_node_noprof(unsigned long size, gfp_t gfp_mask, int node)
> >  {
> > +	if (gfp_mask & ~GFP_VMALLOC_SUPPORTED)
> > +		gfp_mask = vmalloc_fix_flags(gfp_mask);
> >
> gfp_mask = check_and_fix_flags()? 

I just mirrored how its done in mm/slab.h. IMO its cleaner to keep
the check out here and have vmalloc_fix_flags() stick to one thing.

If you really want it as check_and_fix_flags(), let me know and I'm open
to changing it in the next version.

On another note, I'm now realizing I forgot to mark the check as
unlikey(). I'll include that in a final version once the other 2
patches have been looked at.

Re: [RFC PATCH v2 1/4] mm/vmalloc: warn on invalid vmalloc gfp flags

[Vishal Moola (Oracle)]

On Wed, Nov 05, 2025 at 02:16:24PM +0100, Uladzislau Rezki wrote:
> > On Mon, Nov 03, 2025 at 11:04:26AM -0800, Vishal Moola (Oracle) wrote:
> > > Vmalloc explicitly supports a list of flags, but we never enforce them.
> > > vmalloc has been trying to handle unsupported flags by clearing and
> > > setting flags wherever necessary. This is messy and makes the code
> > > harder to understand, when we could simply check for a supported input
> > > immediately instead.
> > >
> > > Define a helper mask and function telling callers they have passed in
> > > invalid flags, and clear those unsupported vmalloc flags.
> > >
> > > Suggested-by: Christoph Hellwig <hch@infradead.org>
> > > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> > > ---
> > >  mm/vmalloc.c | 24 ++++++++++++++++++++++++
> > >  1 file changed, 24 insertions(+)
> > >
> > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > > index 0832f944544c..290016c7fb58 100644
> > > --- a/mm/vmalloc.c
> > > +++ b/mm/vmalloc.c
> > > @@ -3911,6 +3911,26 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
> > >       return NULL;
> > >  }
> > >
> > > +/*
> > > + * See __vmalloc_node_range() for a clear list of supported vmalloc flags.
> > > + * This gfp lists all flags currently passed through vmalloc. Currently,
> > > + * __GFP_ZERO is used by BFP and __GFP_NORETRY is used by percpu.
> > > + */
> > > +#define GFP_VMALLOC_SUPPORTED (GFP_KERNEL | GFP_ATOMIC | GFP_NOWAIT |\
> > > +                             __GFP_NOFAIL |  __GFP_ZERO | __GFP_NORETRY)
> > > +
> Also we do support %GFP_NOFS and %GFP_NOIO flags.

Both of those are subsets of GFP_KERNEL, so I felt it was redundant to
add.

Re: [RFC PATCH v2 1/4] mm/vmalloc: warn on invalid vmalloc gfp flags

[Uladzislau Rezki]

On Wed, Nov 05, 2025 at 04:00:19PM -0800, Vishal Moola (Oracle) wrote:
> On Wed, Nov 05, 2025 at 02:16:24PM +0100, Uladzislau Rezki wrote:
> > > On Mon, Nov 03, 2025 at 11:04:26AM -0800, Vishal Moola (Oracle) wrote:
> > > > Vmalloc explicitly supports a list of flags, but we never enforce them.
> > > > vmalloc has been trying to handle unsupported flags by clearing and
> > > > setting flags wherever necessary. This is messy and makes the code
> > > > harder to understand, when we could simply check for a supported input
> > > > immediately instead.
> > > >
> > > > Define a helper mask and function telling callers they have passed in
> > > > invalid flags, and clear those unsupported vmalloc flags.
> > > >
> > > > Suggested-by: Christoph Hellwig <hch@infradead.org>
> > > > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> > > > ---
> > > >  mm/vmalloc.c | 24 ++++++++++++++++++++++++
> > > >  1 file changed, 24 insertions(+)
> > > >
> > > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > > > index 0832f944544c..290016c7fb58 100644
> > > > --- a/mm/vmalloc.c
> > > > +++ b/mm/vmalloc.c
> > > > @@ -3911,6 +3911,26 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
> > > >       return NULL;
> > > >  }
> > > >
> > > > +/*
> > > > + * See __vmalloc_node_range() for a clear list of supported vmalloc flags.
> > > > + * This gfp lists all flags currently passed through vmalloc. Currently,
> > > > + * __GFP_ZERO is used by BFP and __GFP_NORETRY is used by percpu.
> > > > + */
> > > > +#define GFP_VMALLOC_SUPPORTED (GFP_KERNEL | GFP_ATOMIC | GFP_NOWAIT |\
> > > > +                             __GFP_NOFAIL |  __GFP_ZERO | __GFP_NORETRY)
> > > > +
> > Also we do support %GFP_NOFS and %GFP_NOIO flags.
> 
> Both of those are subsets of GFP_KERNEL, so I felt it was redundant to
> add.
>
Yep, that is true. But then you need to explicitly check which bits
GFP_KERNEL includes. I mean the white-list mask becomes less informative
for people who check or review the vmalloc code.

But you decide, i do not have a strong opinion.

--
Uladzislau Rezki

Re: [RFC PATCH v2 1/4] mm/vmalloc: warn on invalid vmalloc gfp flags

[Uladzislau Rezki]

On Wed, Nov 05, 2025 at 03:58:22PM -0800, Vishal Moola (Oracle) wrote:
> On Tue, Nov 04, 2025 at 05:28:16PM +0100, Uladzislau Rezki wrote:
> > On Mon, Nov 03, 2025 at 11:04:26AM -0800, Vishal Moola (Oracle) wrote:
> > > Vmalloc explicitly supports a list of flags, but we never enforce them.
> > > vmalloc has been trying to handle unsupported flags by clearing and
> > > setting flags wherever necessary. This is messy and makes the code
> > > harder to understand, when we could simply check for a supported input
> > > immediately instead.
> > > 
> > > Define a helper mask and function telling callers they have passed in
> > > invalid flags, and clear those unsupported vmalloc flags.
> > > 
> > > Suggested-by: Christoph Hellwig <hch@infradead.org>
> > > Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> > > ---
> > >  mm/vmalloc.c | 24 ++++++++++++++++++++++++
> > >  1 file changed, 24 insertions(+)
> > > 
> > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > > index 0832f944544c..290016c7fb58 100644
> > > --- a/mm/vmalloc.c
> > > +++ b/mm/vmalloc.c
> > > @@ -3911,6 +3911,26 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,
> > >  	return NULL;
> > >  }
> > >  
> > > +/*
> > > + * See __vmalloc_node_range() for a clear list of supported vmalloc flags.
> > > + * This gfp lists all flags currently passed through vmalloc. Currently,
> > > + * __GFP_ZERO is used by BFP and __GFP_NORETRY is used by percpu.
> > > + */
> > > +#define GFP_VMALLOC_SUPPORTED (GFP_KERNEL | GFP_ATOMIC | GFP_NOWAIT |\
> > > +				__GFP_NOFAIL |  __GFP_ZERO | __GFP_NORETRY)
> > > +
> > > +static gfp_t vmalloc_fix_flags(gfp_t flags)
> > > +{
> > > +	gfp_t invalid_mask = flags & ~GFP_VMALLOC_SUPPORTED;
> > > +
> > > +	flags &= GFP_VMALLOC_SUPPORTED;
> > > +	pr_warn("Unexpected gfp: %#x (%pGg). Fixing up to gfp: %#x (%pGg). Fix your code!\n",
> > > +			invalid_mask, &invalid_mask, flags, &flags);
> > > +	dump_stack();
> > >
> > WARN_ON() or friends? It prints the stack.
> 
> My understanding of WARN_ON() variants is they're used for internal
> kernel concerns, while the pr_warn() variants are for situations like
> this where proprietary drivers can cause unexpected behavior.
> 
As far as i got, we fix the mask if it contains buggy flags. In that
sense it is absolutely OK to warn and print the stack of caller that
violates the rules.

> > > +
> > > +	return flags;
> > > +}
> > > +
> > >  /**
> > >   * __vmalloc_node_range - allocate virtually contiguous memory
> > >   * @size:		  allocation size
> > > @@ -4092,6 +4112,8 @@ EXPORT_SYMBOL_GPL(__vmalloc_node_noprof);
> > >  
> > >  void *__vmalloc_noprof(unsigned long size, gfp_t gfp_mask)
> > >  {
> > > +	if (gfp_mask & ~GFP_VMALLOC_SUPPORTED)
> > > +		gfp_mask = vmalloc_fix_flags(gfp_mask);
> > >  	return __vmalloc_node_noprof(size, 1, gfp_mask, NUMA_NO_NODE,
> > >  				__builtin_return_address(0));
> > >  }
> > > @@ -4131,6 +4153,8 @@ EXPORT_SYMBOL(vmalloc_noprof);
> > >   */
> > >  void *vmalloc_huge_node_noprof(unsigned long size, gfp_t gfp_mask, int node)
> > >  {
> > > +	if (gfp_mask & ~GFP_VMALLOC_SUPPORTED)
> > > +		gfp_mask = vmalloc_fix_flags(gfp_mask);
> > >
> > gfp_mask = check_and_fix_flags()? 
> 
> I just mirrored how its done in mm/slab.h. IMO its cleaner to keep
> the check out here and have vmalloc_fix_flags() stick to one thing.
> 
> If you really want it as check_and_fix_flags(), let me know and I'm open
> to changing it in the next version.
> 
Well, i will not insist on. You decide :)

>
> On another note, I'm now realizing I forgot to mark the check as
> unlikey(). I'll include that in a final version once the other 2
> patches have been looked at.
>
Sounds good. One thing i have noticed, it is below peace of code:


/* __GFP_NOFAIL and "noblock" flags are mutually exclusive. */
if (!gfpflags_allow_blocking(gfp_mask))
	nofail = false;

i forgot to drop the __GFP_NOFAIL for non-blocking flags. But this
is not a problem of this series. I will fix it by sending the patch.

--
Uladzislau Rezki