Re: [PATCH v3 2/2] rust: support align and NUMA id in allocations

[Danilo Krummrich]

On Wed, Jun 25, 2025 at 08:56:05PM +0200, Danilo Krummrich wrote:
> On Wed, Jun 25, 2025 at 08:30:26AM +0200, Vitaly Wool wrote:
> > Add support for large (> PAGE_SIZE) alignments in Rust allocators
> > (Kmalloc support for large alignments is limited to the requested
> > size, which is a reasonable limitation anyway).
> 
> Please split this..
> 
> > Besides, add support for NUMA id to Vmalloc.
> 
> and this into separate patches.
> 
> Please also add some information to the commit message what you need node
> support for. Do you also have patches to add node support to Box and Vec?
> 
> > 
> > Signed-off-by: Vitaly Wool <vitaly.wool@xxxxxxxxxxx>
> > ---
> >  rust/helpers/slab.c            |  8 +++++--
> >  rust/helpers/vmalloc.c         |  4 ++--
> >  rust/kernel/alloc.rs           | 28 ++++++++++++++++++++++--
> >  rust/kernel/alloc/allocator.rs | 40 +++++++++++++++++++---------------
> >  rust/kernel/alloc/kvec.rs      |  3 ++-
> >  5 files changed, 59 insertions(+), 24 deletions(-)
> > 
> > diff --git a/rust/helpers/slab.c b/rust/helpers/slab.c
> > index a842bfbddcba..221c517f57a1 100644
> > --- a/rust/helpers/slab.c
> > +++ b/rust/helpers/slab.c
> > @@ -3,13 +3,17 @@
> >  #include <linux/slab.h>
> >  
> >  void * __must_check __realloc_size(2)
> > -rust_helper_krealloc(const void *objp, size_t new_size, gfp_t flags)
> > +rust_helper_krealloc(const void *objp, size_t new_size, unsigned long align, gfp_t flags, int nid)
> 
> This should have a comment making it obvious why the function has two arguments
> that are discarded. I think we should even separate it with an additional inline
> function.
> 
> I do agree with discarding the align argument, given that it's not exposed to
> users though the Allocator API.

What I meant is that proper alignment is implied when krealloc() succeeds.

> I do disagree with discarding the nid argument though, since you change the
> generic Allocator::realloc() API to take a node argument, which for KREALLOC and
> KVREALLOC is silently discarded. If we introduce it, we should do so for all
> three allocators.
> 
> >  {
> > +	if (WARN_ON(new_size & (align - 1)))
> > +		return NULL;
> 
> I don't think we should have this WARN_ON(). If we want to warn about this, we
> should already do so on the Rust side. The helper functions in this file should
> not contain any logic.
> 
> >  	return krealloc(objp, new_size, flags);
> >  }
> >  
> >  void * __must_check __realloc_size(2)
> > -rust_helper_kvrealloc(const void *p, size_t size, gfp_t flags)
> > +rust_helper_kvrealloc(const void *p, size_t size, unsigned long align, gfp_t flags, int nid)
> >  {
> > +	if (WARN_ON(size & (align - 1)))
> > +		return NULL;
> >  	return kvrealloc(p, size, flags);
> >  }
> 
> Same as above.

This is actually different though, here kvrealloc() may succeed even if the
requested alignment is not fulfilled, so this is incorrect.