[PATCH v12 1/4] mm/vmalloc: allow to set node and align in vrealloc

Vitaly Wool posted 4 patches 2 months, 4 weeks ago
There is a newer version of this series
[PATCH v12 1/4] mm/vmalloc: allow to set node and align in vrealloc
Posted by Vitaly Wool 2 months, 4 weeks ago
Reimplement vrealloc() to be able to set node and alignment should
a user need to do so. Rename the function to vrealloc_node_align()
to better match what it actually does now and introduce macros for
vrealloc() and friends for backward compatibility.

With that change we also provide the ability for the Rust part of
the kernel to set node and alignment in its allocations.

Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
---
 include/linux/vmalloc.h | 12 +++++++++---
 mm/nommu.c              |  3 ++-
 mm/vmalloc.c            | 31 ++++++++++++++++++++++++++-----
 3 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index fdc9aeb74a44..68791f7cb3ba 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -197,9 +197,15 @@ extern void *__vcalloc_noprof(size_t n, size_t size, gfp_t flags) __alloc_size(1
 extern void *vcalloc_noprof(size_t n, size_t size) __alloc_size(1, 2);
 #define vcalloc(...)		alloc_hooks(vcalloc_noprof(__VA_ARGS__))
 
-void * __must_check vrealloc_noprof(const void *p, size_t size, gfp_t flags)
-		__realloc_size(2);
-#define vrealloc(...)		alloc_hooks(vrealloc_noprof(__VA_ARGS__))
+void *__must_check vrealloc_node_align_noprof(const void *p, size_t size,
+		unsigned long align, gfp_t flags, int nid) __realloc_size(2);
+#define vrealloc_node_noprof(_p, _s, _f, _nid)	\
+	vrealloc_node_align_noprof(_p, _s, 1, _f, _nid)
+#define vrealloc_noprof(_p, _s, _f)		\
+	vrealloc_node_align_noprof(_p, _s, 1, _f, NUMA_NO_NODE)
+#define vrealloc_node_align(...)		alloc_hooks(vrealloc_node_align_noprof(__VA_ARGS__))
+#define vrealloc_node(...)			alloc_hooks(vrealloc_node_noprof(__VA_ARGS__))
+#define vrealloc(...)				alloc_hooks(vrealloc_noprof(__VA_ARGS__))
 
 extern void vfree(const void *addr);
 extern void vfree_atomic(const void *addr);
diff --git a/mm/nommu.c b/mm/nommu.c
index 87e1acab0d64..8359b2025b9f 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -119,7 +119,8 @@ void *__vmalloc_noprof(unsigned long size, gfp_t gfp_mask)
 }
 EXPORT_SYMBOL(__vmalloc_noprof);
 
-void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)
+void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align,
+				 gfp_t flags, int node)
 {
 	return krealloc_noprof(p, size, (flags | __GFP_COMP) & ~__GFP_HIGHMEM);
 }
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 6dbcdceecae1..03dd06097b25 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -4089,19 +4089,31 @@ void *vzalloc_node_noprof(unsigned long size, int node)
 EXPORT_SYMBOL(vzalloc_node_noprof);
 
 /**
- * vrealloc - reallocate virtually contiguous memory; contents remain unchanged
+ * vrealloc_node_align_noprof - reallocate virtually contiguous memory; contents
+ * remain unchanged
  * @p: object to reallocate memory for
  * @size: the size to reallocate
+ * @align: requested alignment
  * @flags: the flags for the page level allocator
+ * @nid: node number of the target node
+ *
+ * If @p is %NULL, vrealloc_XXX() behaves exactly like vmalloc(). If @size is
+ * 0 and @p is not a %NULL pointer, the object pointed to is freed.
  *
- * If @p is %NULL, vrealloc() behaves exactly like vmalloc(). If @size is 0 and
- * @p is not a %NULL pointer, the object pointed to is freed.
+ * if @nid is not NUMA_NO_NODE, this function will try to allocate memory on
+ * the given node. If reallocation is not necessary (e. g. the new size is less
+ * than the current allocated size), the current allocation will be preserved
+ * unless __GFP_THISNODE is set. In the latter case a new allocation on the
+ * requested node will be attempted.
  *
  * If __GFP_ZERO logic is requested, callers must ensure that, starting with the
  * initial memory allocation, every subsequent call to this API for the same
  * memory allocation is flagged with __GFP_ZERO. Otherwise, it is possible that
  * __GFP_ZERO is not fully honored by this API.
  *
+ * If the requested alignment is bigger than the one the *existing* allocation
+ * has, this function will fail.
+ *
  * In any case, the contents of the object pointed to are preserved up to the
  * lesser of the new and old sizes.
  *
@@ -4111,7 +4123,8 @@ EXPORT_SYMBOL(vzalloc_node_noprof);
  * Return: pointer to the allocated memory; %NULL if @size is zero or in case of
  *         failure
  */
-void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)
+void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align,
+				 gfp_t flags, int nid)
 {
 	struct vm_struct *vm = NULL;
 	size_t alloced_size = 0;
@@ -4135,6 +4148,12 @@ void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)
 		if (WARN(alloced_size < old_size,
 			 "vrealloc() has mismatched area vs requested sizes (%p)\n", p))
 			return NULL;
+		if (WARN(!IS_ALIGNED((unsigned long)p, align),
+			 "will not reallocate with a bigger alignment (0x%lx)\n", align))
+			return NULL;
+		if (unlikely(flags & __GFP_THISNODE) && nid != NUMA_NO_NODE &&
+			     nid != page_to_nid(vmalloc_to_page(p)))
+			goto need_realloc;
 	}
 
 	/*
@@ -4165,8 +4184,10 @@ void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)
 		return (void *)p;
 	}
 
+need_realloc:
 	/* TODO: Grow the vm_area, i.e. allocate and map additional pages. */
-	n = __vmalloc_noprof(size, flags);
+	n = __vmalloc_node_noprof(size, align, flags, nid, __builtin_return_address(0));
+
 	if (!n)
 		return NULL;
 
-- 
2.39.2
Re: [PATCH v12 1/4] mm/vmalloc: allow to set node and align in vrealloc
Posted by Alexei Starovoitov 2 months, 4 weeks ago
On Wed, Jul 9, 2025 at 10:25 AM Vitaly Wool <vitaly.wool@konsulko.se> wrote:
>
>
> -void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)
> +void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align,
> +                                gfp_t flags, int node)
>  {

imo this is a silly pattern to rename functions because they
got new arguments.
The names of the args are clear enough "align" and "node".
I see no point in adding the same suffixes to a function name.
In the future this function will receive another argument and
the function would be renamed again?!
"_noprof" suffix makes sense, since it's there for alloc_hooks,
but "_node_align_" is unnecessary.
Re: [PATCH v12 1/4] mm/vmalloc: allow to set node and align in vrealloc
Posted by Danilo Krummrich 2 months, 4 weeks ago
On 7/10/25 12:53 AM, Alexei Starovoitov wrote:
> On Wed, Jul 9, 2025 at 10:25 AM Vitaly Wool <vitaly.wool@konsulko.se> wrote:
>>
>>
>> -void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)
>> +void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align,
>> +                                gfp_t flags, int node)
>>   {
> 
> imo this is a silly pattern to rename functions because they
> got new arguments.
> The names of the args are clear enough "align" and "node".
> I see no point in adding the same suffixes to a function name.
> In the future this function will receive another argument and
> the function would be renamed again?!
> "_noprof" suffix makes sense, since it's there for alloc_hooks,
> but "_node_align_" is unnecessary.

Do you have an alternative proposal given that we also have vrealloc() and
vrealloc_node()?
Re: [PATCH v12 1/4] mm/vmalloc: allow to set node and align in vrealloc
Posted by Alexei Starovoitov 2 months, 4 weeks ago
On Wed, Jul 9, 2025 at 3:57 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On 7/10/25 12:53 AM, Alexei Starovoitov wrote:
> > On Wed, Jul 9, 2025 at 10:25 AM Vitaly Wool <vitaly.wool@konsulko.se> wrote:
> >>
> >>
> >> -void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)
> >> +void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align,
> >> +                                gfp_t flags, int node)
> >>   {
> >
> > imo this is a silly pattern to rename functions because they
> > got new arguments.
> > The names of the args are clear enough "align" and "node".
> > I see no point in adding the same suffixes to a function name.
> > In the future this function will receive another argument and
> > the function would be renamed again?!
> > "_noprof" suffix makes sense, since it's there for alloc_hooks,
> > but "_node_align_" is unnecessary.
>
> Do you have an alternative proposal given that we also have vrealloc() and
> vrealloc_node()?

vrealloc_node()?! There is no such thing in the tree.
There are various k[zm]alloc_node() which are artifacts of the past
when NUMA just appeared and people cared about CONFIG_NUMA vs not.
Nowadays NUMA is everywhere and any new code must support NUMA
from the start. Hence no point in carrying old baggage and obsolete names.
Re: [PATCH v12 1/4] mm/vmalloc: allow to set node and align in vrealloc
Posted by Danilo Krummrich 2 months, 4 weeks ago
On Thu Jul 10, 2025 at 1:14 AM CEST, Alexei Starovoitov wrote:
> On Wed, Jul 9, 2025 at 3:57 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On 7/10/25 12:53 AM, Alexei Starovoitov wrote:
>> > On Wed, Jul 9, 2025 at 10:25 AM Vitaly Wool <vitaly.wool@konsulko.se> wrote:
>> >>
>> >>
>> >> -void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)
>> >> +void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align,
>> >> +                                gfp_t flags, int node)
>> >>   {
>> >
>> > imo this is a silly pattern to rename functions because they
>> > got new arguments.
>> > The names of the args are clear enough "align" and "node".
>> > I see no point in adding the same suffixes to a function name.
>> > In the future this function will receive another argument and
>> > the function would be renamed again?!
>> > "_noprof" suffix makes sense, since it's there for alloc_hooks,
>> > but "_node_align_" is unnecessary.
>>
>> Do you have an alternative proposal given that we also have vrealloc() and
>> vrealloc_node()?
>
> vrealloc_node()?! There is no such thing in the tree.
> There are various k[zm]alloc_node() which are artifacts of the past
> when NUMA just appeared and people cared about CONFIG_NUMA vs not.
> Nowadays NUMA is everywhere and any new code must support NUMA
> from the start. Hence no point in carrying old baggage and obsolete names.

This patch adds it; do you suggest to redefine vrealloc_noprof() to take align
and nid? If we don't mind being inconsistent with krealloc_noprof() and
kvrealloc_noprof() that's fine I guess.

FWIW, I prefer consistency.
Re: [PATCH v12 1/4] mm/vmalloc: allow to set node and align in vrealloc
Posted by Alexei Starovoitov 2 months, 4 weeks ago
On Wed, Jul 9, 2025 at 4:26 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Thu Jul 10, 2025 at 1:14 AM CEST, Alexei Starovoitov wrote:
> > On Wed, Jul 9, 2025 at 3:57 PM Danilo Krummrich <dakr@kernel.org> wrote:
> >>
> >> On 7/10/25 12:53 AM, Alexei Starovoitov wrote:
> >> > On Wed, Jul 9, 2025 at 10:25 AM Vitaly Wool <vitaly.wool@konsulko.se> wrote:
> >> >>
> >> >>
> >> >> -void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)
> >> >> +void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align,
> >> >> +                                gfp_t flags, int node)
> >> >>   {
> >> >
> >> > imo this is a silly pattern to rename functions because they
> >> > got new arguments.
> >> > The names of the args are clear enough "align" and "node".
> >> > I see no point in adding the same suffixes to a function name.
> >> > In the future this function will receive another argument and
> >> > the function would be renamed again?!
> >> > "_noprof" suffix makes sense, since it's there for alloc_hooks,
> >> > but "_node_align_" is unnecessary.
> >>
> >> Do you have an alternative proposal given that we also have vrealloc() and
> >> vrealloc_node()?
> >
> > vrealloc_node()?! There is no such thing in the tree.
> > There are various k[zm]alloc_node() which are artifacts of the past
> > when NUMA just appeared and people cared about CONFIG_NUMA vs not.
> > Nowadays NUMA is everywhere and any new code must support NUMA
> > from the start. Hence no point in carrying old baggage and obsolete names.
>
> This patch adds it; do you suggest to redefine vrealloc_noprof() to take align
> and nid? If we don't mind being inconsistent with krealloc_noprof() and
> kvrealloc_noprof() that's fine I guess.
>
> FWIW, I prefer consistency.

What inconsistency are you talking about? That
krealloc_noprof(const void *p, size_t new_size, gfp_t flags)
and
vrealloc_noprof(const void *p, size_t size, unsigned long align,
                gfp_t flags, int node)
have different number of arguments?!

See:
alloc_pages_noprof(gfp_t gfp, unsigned int order);
__alloc_pages_noprof(gfp_t gfp, unsigned int order, int preferred_nid,
                nodemask_t *nodemask);

Adding double underscore to keep all existing callers of
vrealloc_noprof() without changes and do:

vrealloc_noprof(const void *p, size_t size, gfp_t flags);
__vrealloc_noprof(const void *p, size_t size, unsigned long align,
gfp_t flags, int node);

is fine and consistent with how things were done in the past,
but adding "_node_align_" to the function name and code churn to all
callsites is a cargo cult.
Re: [PATCH v12 1/4] mm/vmalloc: allow to set node and align in vrealloc
Posted by Danilo Krummrich 2 months, 4 weeks ago
On Thu Jul 10, 2025 at 2:39 AM CEST, Alexei Starovoitov wrote:
> On Wed, Jul 9, 2025 at 4:26 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Thu Jul 10, 2025 at 1:14 AM CEST, Alexei Starovoitov wrote:
>> > On Wed, Jul 9, 2025 at 3:57 PM Danilo Krummrich <dakr@kernel.org> wrote:
>> >>
>> >> On 7/10/25 12:53 AM, Alexei Starovoitov wrote:
>> >> > On Wed, Jul 9, 2025 at 10:25 AM Vitaly Wool <vitaly.wool@konsulko.se> wrote:
>> >> >>
>> >> >>
>> >> >> -void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)
>> >> >> +void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align,
>> >> >> +                                gfp_t flags, int node)
>> >> >>   {
>> >> >
>> >> > imo this is a silly pattern to rename functions because they
>> >> > got new arguments.
>> >> > The names of the args are clear enough "align" and "node".
>> >> > I see no point in adding the same suffixes to a function name.
>> >> > In the future this function will receive another argument and
>> >> > the function would be renamed again?!
>> >> > "_noprof" suffix makes sense, since it's there for alloc_hooks,
>> >> > but "_node_align_" is unnecessary.
>> >>
>> >> Do you have an alternative proposal given that we also have vrealloc() and
>> >> vrealloc_node()?
>> >
>> > vrealloc_node()?! There is no such thing in the tree.
>> > There are various k[zm]alloc_node() which are artifacts of the past
>> > when NUMA just appeared and people cared about CONFIG_NUMA vs not.
>> > Nowadays NUMA is everywhere and any new code must support NUMA
>> > from the start. Hence no point in carrying old baggage and obsolete names.
>>
>> This patch adds it; do you suggest to redefine vrealloc_noprof() to take align
>> and nid? If we don't mind being inconsistent with krealloc_noprof() and
>> kvrealloc_noprof() that's fine I guess.
>>
>> FWIW, I prefer consistency.
>
> What inconsistency are you talking about? That
> krealloc_noprof(const void *p, size_t new_size, gfp_t flags)
> and
> vrealloc_noprof(const void *p, size_t size, unsigned long align,
>                 gfp_t flags, int node)
> have different number of arguments?!
>
> See:
> alloc_pages_noprof(gfp_t gfp, unsigned int order);
> __alloc_pages_noprof(gfp_t gfp, unsigned int order, int preferred_nid,
>                 nodemask_t *nodemask);
>
> Adding double underscore to keep all existing callers of
> vrealloc_noprof() without changes and do:
>
> vrealloc_noprof(const void *p, size_t size, gfp_t flags);
> __vrealloc_noprof(const void *p, size_t size, unsigned long align,
> gfp_t flags, int node);
>
> is fine and consistent with how things were done in the past,
> but adding "_node_align_" to the function name and code churn to all
> callsites is a cargo cult.

As Vitaly mentioned in a different reply, this would be inconsistent with the
'k' and 'kv' variants, which have the suffix '_node'.

Anyways, in general I don't think that adding underscores for functions that
basically do the same thing but are getting more specialized is a great pattern
for things that are not strictly limited to a narrow context.

Please note, I'm not saying we should encode additional arguments in the name
either. I think it really depends on the actual case.

In this case, it seems to make sense to me that there is e.g. kmalloc() and
kmalloc_node().

For a caller that's much more useful, i.e. I want classic kmalloc(), but want to
set the node, hence kmalloc_node(). Calling it __kmalloc() instead seems a bit
random.

Or do you only refer to the *_noprof() variants, which are not exported to
users? But even then, underscores still don't seem very expressive.

I'm not maintaining this code though, so just take it FWIW. :)
Re: [PATCH v12 1/4] mm/vmalloc: allow to set node and align in vrealloc
Posted by Vitaly Wool 2 months, 4 weeks ago

> On Jul 10, 2025, at 2:39 AM, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> 
> On Wed, Jul 9, 2025 at 4:26 PM Danilo Krummrich <dakr@kernel.org> wrote:
>> 
>> On Thu Jul 10, 2025 at 1:14 AM CEST, Alexei Starovoitov wrote:
>>> On Wed, Jul 9, 2025 at 3:57 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>>> 
>>>> On 7/10/25 12:53 AM, Alexei Starovoitov wrote:
>>>>> On Wed, Jul 9, 2025 at 10:25 AM Vitaly Wool <vitaly.wool@konsulko.se> wrote:
>>>>>> 
>>>>>> 
>>>>>> -void *vrealloc_noprof(const void *p, size_t size, gfp_t flags)
>>>>>> +void *vrealloc_node_align_noprof(const void *p, size_t size, unsigned long align,
>>>>>> +                                gfp_t flags, int node)
>>>>>>  {
>>>>> 
>>>>> imo this is a silly pattern to rename functions because they
>>>>> got new arguments.
>>>>> The names of the args are clear enough "align" and "node".
>>>>> I see no point in adding the same suffixes to a function name.
>>>>> In the future this function will receive another argument and
>>>>> the function would be renamed again?!
>>>>> "_noprof" suffix makes sense, since it's there for alloc_hooks,
>>>>> but "_node_align_" is unnecessary.
>>>> 
>>>> Do you have an alternative proposal given that we also have vrealloc() and
>>>> vrealloc_node()?
>>> 
>>> vrealloc_node()?! There is no such thing in the tree.
>>> There are various k[zm]alloc_node() which are artifacts of the past
>>> when NUMA just appeared and people cared about CONFIG_NUMA vs not.
>>> Nowadays NUMA is everywhere and any new code must support NUMA
>>> from the start. Hence no point in carrying old baggage and obsolete names.
>> 
>> This patch adds it; do you suggest to redefine vrealloc_noprof() to take align
>> and nid? If we don't mind being inconsistent with krealloc_noprof() and
>> kvrealloc_noprof() that's fine I guess.
>> 
>> FWIW, I prefer consistency.
> 
> What inconsistency are you talking about? That
> krealloc_noprof(const void *p, size_t new_size, gfp_t flags)
> and
> vrealloc_noprof(const void *p, size_t size, unsigned long align,
>                gfp_t flags, int node)
> have different number of arguments?!
> 
> See:
> alloc_pages_noprof(gfp_t gfp, unsigned int order);
> __alloc_pages_noprof(gfp_t gfp, unsigned int order, int preferred_nid,
>                nodemask_t *nodemask);
> 
> Adding double underscore to keep all existing callers of
> vrealloc_noprof() without changes and do:
> 
> vrealloc_noprof(const void *p, size_t size, gfp_t flags);
> __vrealloc_noprof(const void *p, size_t size, unsigned long align,
> gfp_t flags, int node);
> 
> is fine and consistent with how things were done in the past,
> but adding "_node_align_" to the function name and code churn to all
> callsites is a cargo cult.
> 

I see your point but your approach is currently only applicable to vmalloc and it will not work for slub because the latter already has __kmalloc_node, __kvmalloc_node etc. and we want to keep at least some naming consistency between k[v]* and v* functions.

This whole patchset is only intended to add the capability to set node and properly handle alignment requests in Rust allocators, and is thus well aligned with your idea that the new code must support NUMA (which I do share). I would suggest that we get this in as it is, and then I can take the burden of straightening out the naming which will inevitably lead to many modifications in other parts of the kernel. The latter I am fine with, too, but not in this series.

~Vitaly
Re: [PATCH v12 1/4] mm/vmalloc: allow to set node and align in vrealloc
Posted by Liam R. Howlett 2 months, 4 weeks ago
* Vitaly Wool <vitaly.wool@konsulko.se> [250709 13:24]:
> Reimplement vrealloc() to be able to set node and alignment should
> a user need to do so. Rename the function to vrealloc_node_align()
> to better match what it actually does now and introduce macros for
> vrealloc() and friends for backward compatibility.
> 
> With that change we also provide the ability for the Rust part of
> the kernel to set node and alignment in its allocations.
> 
> Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
> Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
> ---
>  include/linux/vmalloc.h | 12 +++++++++---
>  mm/nommu.c              |  3 ++-
>  mm/vmalloc.c            | 31 ++++++++++++++++++++++++++-----
>  3 files changed, 37 insertions(+), 9 deletions(-)
> 
...

> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 6dbcdceecae1..03dd06097b25 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -4089,19 +4089,31 @@ void *vzalloc_node_noprof(unsigned long size, int node)
>  EXPORT_SYMBOL(vzalloc_node_noprof);
>  
>  /**
> - * vrealloc - reallocate virtually contiguous memory; contents remain unchanged
> + * vrealloc_node_align_noprof - reallocate virtually contiguous memory; contents
> + * remain unchanged
>   * @p: object to reallocate memory for
>   * @size: the size to reallocate
> + * @align: requested alignment
>   * @flags: the flags for the page level allocator
> + * @nid: node number of the target node
> + *
> + * If @p is %NULL, vrealloc_XXX() behaves exactly like vmalloc(). If @size is
> + * 0 and @p is not a %NULL pointer, the object pointed to is freed.
>   *
> - * If @p is %NULL, vrealloc() behaves exactly like vmalloc(). If @size is 0 and
> - * @p is not a %NULL pointer, the object pointed to is freed.
> + * if @nid is not NUMA_NO_NODE, this function will try to allocate memory on
> + * the given node. If reallocation is not necessary (e. g. the new size is less
> + * than the current allocated size), the current allocation will be preserved
> + * unless __GFP_THISNODE is set. In the latter case a new allocation on the
> + * requested node will be attempted.

I am having a very hard time understanding what you mean here.  What is
the latter case?

If @nis is !NUMA_NO_NODE, the allocation will be attempted on the given
node.  Then things sort of get confusing.  What is the latter case?

>   *
>   * If __GFP_ZERO logic is requested, callers must ensure that, starting with the
>   * initial memory allocation, every subsequent call to this API for the same
>   * memory allocation is flagged with __GFP_ZERO. Otherwise, it is possible that
>   * __GFP_ZERO is not fully honored by this API.
>   *
> + * If the requested alignment is bigger than the one the *existing* allocation
> + * has, this function will fail.
> + *

It might be better to say something like:
Requesting an alignment that is bigger than the alignment of the
*existing* allocation will fail.

...

Thanks,
Liam
Re: [PATCH v12 1/4] mm/vmalloc: allow to set node and align in vrealloc
Posted by Vitaly Wool 2 months, 4 weeks ago

> On Jul 9, 2025, at 9:01 PM, Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> 
> * Vitaly Wool <vitaly.wool@konsulko.se> [250709 13:24]:
>> Reimplement vrealloc() to be able to set node and alignment should
>> a user need to do so. Rename the function to vrealloc_node_align()
>> to better match what it actually does now and introduce macros for
>> vrealloc() and friends for backward compatibility.
>> 
>> With that change we also provide the ability for the Rust part of
>> the kernel to set node and alignment in its allocations.
>> 
>> Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
>> Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
>> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
>> ---
>> include/linux/vmalloc.h | 12 +++++++++---
>> mm/nommu.c              |  3 ++-
>> mm/vmalloc.c            | 31 ++++++++++++++++++++++++++-----
>> 3 files changed, 37 insertions(+), 9 deletions(-)
>> 
> ...
> 
>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>> index 6dbcdceecae1..03dd06097b25 100644
>> --- a/mm/vmalloc.c
>> +++ b/mm/vmalloc.c
>> @@ -4089,19 +4089,31 @@ void *vzalloc_node_noprof(unsigned long size, int node)
>> EXPORT_SYMBOL(vzalloc_node_noprof);
>> 
>> /**
>> - * vrealloc - reallocate virtually contiguous memory; contents remain unchanged
>> + * vrealloc_node_align_noprof - reallocate virtually contiguous memory; contents
>> + * remain unchanged
>>  * @p: object to reallocate memory for
>>  * @size: the size to reallocate
>> + * @align: requested alignment
>>  * @flags: the flags for the page level allocator
>> + * @nid: node number of the target node
>> + *
>> + * If @p is %NULL, vrealloc_XXX() behaves exactly like vmalloc(). If @size is
>> + * 0 and @p is not a %NULL pointer, the object pointed to is freed.
>>  *
>> - * If @p is %NULL, vrealloc() behaves exactly like vmalloc(). If @size is 0 and
>> - * @p is not a %NULL pointer, the object pointed to is freed.
>> + * if @nid is not NUMA_NO_NODE, this function will try to allocate memory on
>> + * the given node. If reallocation is not necessary (e. g. the new size is less
>> + * than the current allocated size), the current allocation will be preserved
>> + * unless __GFP_THISNODE is set. In the latter case a new allocation on the
>> + * requested node will be attempted.
> 
> I am having a very hard time understanding what you mean here.  What is
> the latter case?
> 
> If @nis is !NUMA_NO_NODE, the allocation will be attempted on the given
> node.  Then things sort of get confusing.  What is the latter case?

The latter case is __GFP_THISNODE present in flags. That’s the latest if-clause in this paragraph.

> 
>>  *
>>  * If __GFP_ZERO logic is requested, callers must ensure that, starting with the
>>  * initial memory allocation, every subsequent call to this API for the same
>>  * memory allocation is flagged with __GFP_ZERO. Otherwise, it is possible that
>>  * __GFP_ZERO is not fully honored by this API.
>>  *
>> + * If the requested alignment is bigger than the one the *existing* allocation
>> + * has, this function will fail.
>> + *
> 
> It might be better to say something like:
> Requesting an alignment that is bigger than the alignment of the
> *existing* allocation will fail.
> 
The whole function description in fact consists of several if-clauses (some of which are nested) so I am just following the pattern here.

~Vitaly