On Mon, Apr 20, 2026 at 11:30:23AM +0200, Marco Elver wrote:
> On Mon, 20 Apr 2026 at 09:25, Harry Yoo (Oracle) <[email protected]> wrote:
> > [CC'ing RUST ALLOC folks for rust bindings]
> > On Wed, Apr 15, 2026 at 04:37:05PM +0200, Marco Elver wrote:
> > A few comments on V2:
> >
> > # comment 1
> >
> > I'm not a big fan of how k[v]realloc_node_align()
> > and kmalloc_nolock() define and pass the token parameter.
> >
> > IMHO it'll be fine to use {DECL,PASS}_KMALLOC_PARAMS() in those
> > functions, since SLAB_BUCKETS users already passes NULL bucket
> > to most of __kmalloc*() calls anyway.
>
> I'm not sure I agree. 2 reasons:
>
> 1. Even though it's "just" k[v]realloc_node_align() and
> kmalloc_nolock() - despite their relatively less frequent use - just
> put one of them in a hot path and you're sacrificing performance even
> further. There are only so many arguments that can be passed in
> registers (depending on arch), and may cause more stack spilling.
>
> 2. We'd misleadingly declare that these functions do something with
> the bucket arg. This is wrong.
Both are valid points. But it still feels wrong to have:
void *krealloc_node_align_noprof(const void *objp, size_t new_size,
unsigned long align,
gfp_t flags, int nid DECL_TOKEN_PARAM(token));
n = krealloc_node_align_noprof(p, size, align, kmalloc_gfp_adjust(flags, size),
nid _PASS_TOKEN_PARAM(token));
Actually the problem here is that some of parameters in
DECL_KMALLOC_PARAMS() are not necessary in some functions.
Perhaps we could have
DECL_KMALLOC_PARAMS(size, b, token) # declare size, bucket, token
DECL_BUCKET_PARAMS(size, token) # declare size, bucket;
# but, actually, we don't need this!
DECL_TOKEN_PARAMS(size, b) # declare size, token only;
# for kmalloc_nolock and k[v]realloc_node_align()
and use DECL_TOKEN_PARAMS(), PASS_TOKEN_PARAMS() for those functions?
(just like how DECL_BUCKET_PARAMS() worked before)
What do you think?
> Both feels wrong, and would only make this change if you confirm both
> are trade-offs that you strongly prefer.
>
> > # comment 2
> >
> > This breaks Documentation/.
> >
> > Problems:
> >
> > - The document generator doesn't handle DECL_KMALLOC_PARAMS() well.
> >
> > - The signature of the function that users call (krealloc_node_align())
> > and the function that has kerneldoc (krealloc_node_align_noprof())
> > don't match.
> >
> > - Even worse, moving kerneldoc to the macro doesn't work because
> > it uses variable arguments (...)
>
> Well, some were broken before, now it's just broken more. :-)
Ouch... ;-)
> We could move the documentation to macros and switch to explicit args
> instead of (...).
That works for me!
> Otherwise, I don't see any way to fix this. Preferences?
>
> > # comment 3
> >
> > Looking at how rust generates helper functions,
> > in rust/helpers/slab.c:
> > | // SPDX-License-Identifier: GPL-2.0
> > |
> > | #include <linux/slab.h>
> > |
> > | __rust_helper void *__must_check __realloc_size(2)
> > | rust_helper_krealloc_node_align(const void *objp, size_t new_size,
> > unsigned long align,
> > | gfp_t flags, int node)
> > | {
> > | return krealloc_node_align(objp, new_size, align, flags, node);
> > | }
> > |
> > | __rust_helper void *__must_check __realloc_size(2)
> > | rust_helper_kvrealloc_node_align(const void *p, size_t size, unsigned
> > long align,
> > | gfp_t flags, int node)
> > | {
> > | return kvrealloc_node_align(p, size, align, flags, node);
> > | }
> >
> > Rust code probably won't pass any meaningful token?
> > (something you may want to address in the future)
>
> Yes, it'll just pass '0' by default. We could force Rust's allocation
> to be in the pointer-containing range - if we assume Rust code is less
> prone to contain bugs, but the assumption is that such allocations
> both originate and are confined to the Rust side. One easy way to do
> this is to write:
>
> return kvrealloc_node_align(p, size + 0 * sizeof(long*), align,
> flags, node);
>
> But I'd defer that for now, until we're sure the above assumption
> holds (Rust originated + confined).
Ack.
By the way, since Allocator trait uses realloc() to allocate new memory,
IIUC all k[v]malloc, k[v]realloc usage from Rust will be affected.
--
Cheers,
Harry / Hyeonggon
