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

[Vitaly Wool <vitaly.wool@konsulko.se>]

[-- Attachment #1: Type: text/plain, Size: 4635 bytes --]



> On Jun 30, 2025, at 12:30 PM, Uladzislau Rezki <urezki@gmail.com> wrote:
> 
> On Sat, Jun 28, 2025 at 12:25:37PM +0200, Vitaly Wool wrote:
>> 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 aligmnent in its allocations.
>> 
>> Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
>> ---
>> include/linux/vmalloc.h | 12 +++++++++---
>> mm/vmalloc.c            | 20 ++++++++++++++++----
>> 2 files changed, 25 insertions(+), 7 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/vmalloc.c b/mm/vmalloc.c
>> index 6dbcdceecae1..d633ac0ff977 100644
>> --- a/mm/vmalloc.c
>> +++ b/mm/vmalloc.c
>> @@ -4089,12 +4089,15 @@ 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 id
>>  *
>> - * If @p is %NULL, vrealloc() behaves exactly like vmalloc(). If @size is 0 and
>> + * 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 __GFP_ZERO logic is requested, callers must ensure that, starting with the
>> @@ -4111,7 +4114,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 +4139,13 @@ 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(nid != NUMA_NO_NODE && nid != page_to_nid(vmalloc_to_page(p)),
>> +			 "vrealloc() has mismatched nids\n"))
>> +			return NULL;
>> +		if (WARN((uintptr_t)p & (align - 1),
>> +			 "will not reallocate with a bigger alignment (0x%lx)\n",
>> +			 align))
>> +			return NULL;
>> 
> IMO, IS_ALIGNED() should be used instead. We have already a macro for this
> purpose, i.e. the idea is just to check that "p" is aligned with "align"
> request.
> 
> Can you replace the (uintptr_t) casting to (ulong) or (unsigned long)
> this is how we mostly cast in vmalloc code?

Thanks, noted.
> 
> WARN() probably is worth to replace. Use WARN_ON_ONCE() to prevent
> flooding.

I am not sure i totally agree, because:
a) there’s already one WARN() in that block and I’m just following the pattern
b) I don’t think this will be a frequent error.

~Vitaly


[-- Attachment #2: Type: text/html, Size: 15060 bytes --]