[PATCH v11 0/4] support large align and nid in Rust allocators

Vitaly Wool posted 4 patches 3 months ago
There is a newer version of this series
[PATCH v11 0/4] support large align and nid in Rust allocators
Posted by Vitaly Wool 3 months ago
The coming patches provide the ability for Rust allocators to set
NUMA node and large alignment.

Changelog:
v2 -> v3:
* fixed the build breakage for non-MMU configs
v3 -> v4:
* added NUMA node support for k[v]realloc (patch #2)
* removed extra logic in Rust helpers
* patch for Rust allocators split into 2 (align: patch #3 and
  NUMA ids: patch #4)
v4 -> v5:
* reworked NUMA node support for k[v]realloc for all 3 <alloc>_node
  functions to have the same signature
* all 3 <alloc>_node slab/vmalloc functions now support alignment
  specification
* Rust helpers are extended with new functions, the old ones are left
  intact
* Rust support for NUMA nodes comes first now (as patch #3)
v5 -> v6:
* added <alloc>_node_align functions to keep the existing interfaces
  intact
* clearer separation for Rust support of MUNA ids and large alignments
v6 -> v7:
* NUMA identifier as a new Rust type (NumaNode)
* better documentation for changed and new functions and constants
v7 -> v8:
* removed NumaError
* small cleanups per reviewers' comments
v8 -> v9:
* realloc functions can now reallocate memory for a different NUMA
  node
* better comments/explanations in the Rust part
v9 -> v10:
* refined behavior when memory is being reallocated for a different
  NUMA node, comments added
* cleanups in the Rust part, rustfmt ran
* typos corrected
v10 -> v11:
* added documentation for the NO_NODE constant
* added node parameter to Allocator's alloc/realloc instead of adding
  separate alloc_node resp. realloc_node functions, modified users of
  alloc/realloc in accordance with that

Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
Re: [PATCH v11 0/4] support large align and nid in Rust allocators
Posted by Lorenzo Stoakes 3 months ago
+cc Liam

Hi guys,

We have a section in MAINTAINERS for mm rust (MEMORY MANAGEMENT - RUST), so
it's slightly concerning to find a series (at v11!) like this that changes
mm-related stuff and it involves files not listed there and nobody bothered
to cc- the people listed there.

I can fully understand there being some process fail here meaning you
missed it - fine if so - but let's fix it please moving forwards.

It's really important to me that the rust efforts in mm are collaborative -
I really believe in your mission (well - for me it's about the compiler
_helping_ me not shooting me in the foot :) - and have put substantial
effort in assisting initial work there. So let's make sure we're
collaborative in both directions please.

We have rust/kernel/mm/ under MEMORY MANAGEMENT - RUST too, I'm not au fait
with your approach to structuring in these folders but seems to me these
helpers should be there? I may be unaware of some rust aspect of this
however.

Can we please add these files to this section and in future cc people
listed there? We're here to help!

A side-note I wonder if we also need to put specific files also in relevant
mm sections? E.g. the slab helper should also be put under the slab section
perhaps?

You can have files in multiple entries in MAINTAINERS so it's flexible
enough to allow it to be both in the mm rust section and also the slab
section for instance.

Thanks, Lorenzo

On Mon, Jul 07, 2025 at 06:47:55PM +0200, Vitaly Wool wrote:
> The coming patches provide the ability for Rust allocators to set
> NUMA node and large alignment.
>
> Changelog:
> v2 -> v3:
> * fixed the build breakage for non-MMU configs
> v3 -> v4:
> * added NUMA node support for k[v]realloc (patch #2)
> * removed extra logic in Rust helpers
> * patch for Rust allocators split into 2 (align: patch #3 and
>   NUMA ids: patch #4)
> v4 -> v5:
> * reworked NUMA node support for k[v]realloc for all 3 <alloc>_node
>   functions to have the same signature
> * all 3 <alloc>_node slab/vmalloc functions now support alignment
>   specification
> * Rust helpers are extended with new functions, the old ones are left
>   intact
> * Rust support for NUMA nodes comes first now (as patch #3)
> v5 -> v6:
> * added <alloc>_node_align functions to keep the existing interfaces
>   intact
> * clearer separation for Rust support of MUNA ids and large alignments
> v6 -> v7:
> * NUMA identifier as a new Rust type (NumaNode)
> * better documentation for changed and new functions and constants
> v7 -> v8:
> * removed NumaError
> * small cleanups per reviewers' comments
> v8 -> v9:
> * realloc functions can now reallocate memory for a different NUMA
>   node
> * better comments/explanations in the Rust part
> v9 -> v10:
> * refined behavior when memory is being reallocated for a different
>   NUMA node, comments added
> * cleanups in the Rust part, rustfmt ran
> * typos corrected
> v10 -> v11:
> * added documentation for the NO_NODE constant
> * added node parameter to Allocator's alloc/realloc instead of adding
>   separate alloc_node resp. realloc_node functions, modified users of
>   alloc/realloc in accordance with that
>
> Signed-off-by: Vitaly Wool <vitaly.wool@konsulko.se>
>
>

Odd you don't have a diffstat here?
Re: [PATCH v11 0/4] support large align and nid in Rust allocators
Posted by Danilo Krummrich 3 months ago
On Tue, Jul 08, 2025 at 11:58:06AM +0100, Lorenzo Stoakes wrote:
> +cc Liam
> 
> Hi guys,
> 
> We have a section in MAINTAINERS for mm rust (MEMORY MANAGEMENT - RUST), so
> it's slightly concerning to find a series (at v11!) like this that changes
> mm-related stuff and it involves files not listed there and nobody bothered
> to cc- the people listed there.

What files are you referring to? Are you referring to:

	rust/kernel/alloc.rs
	rust/kernel/alloc/*

If so, they're indeed not under the "MEMORY MANAGEMENT - RUST" entry, which
so far seems correct.

Please also note that we had "RUST [ALLOC]" before "MEMORY MANAGEMENT - RUST"
did exist.

> I can fully understand there being some process fail here meaning you
> missed it - fine if so - but let's fix it please moving forwards.

I agree that this series should have a couple more people in Cc.

Given the existing entries in the MAINTAINERS file the Rust parts seems to be
correct though.

> It's really important to me that the rust efforts in mm are collaborative -
> I really believe in your mission (well - for me it's about the compiler
> _helping_ me not shooting me in the foot :) - and have put substantial
> effort in assisting initial work there. So let's make sure we're
> collaborative in both directions please.

AFAICT, those efforts are collaborative.

Back then I sent patches to introduce vrealloc() and improve and align
kvrealloc() and krealloc() [1]; it was also mentioned that this was, besides the
other advantages, prerequisite work for the Rust allocator patch series [2].

The subsequent Rust allocator patch series [2] was also sent to Andrew and the
-mm mailing list; the previous code replaced by this series was maintained under
the "RUST" entry in the maintainers file.

With the introduction of the new Rust allocator code I took over maintainership.

So, Andrew is aware of the Rust allocator tree, please see also [3].

[1] https://lore.kernel.org/all/20240722163111.4766-1-dakr@kernel.org/
[2] https://lore.kernel.org/all/20241004154149.93856-1-dakr@kernel.org/
[3] https://lore.kernel.org/all/20250625143450.2afc473fc0e7124a5108c187@linux-foundation.org/

> We have rust/kernel/mm/ under MEMORY MANAGEMENT - RUST too, I'm not au fait
> with your approach to structuring in these folders but seems to me these
> helpers should be there? I may be unaware of some rust aspect of this
> however.

The Rust allocator module is a user of exactly three functions of mm, i.e.
krealloc(), vrealloc(), kvrealloc(), with a thin abstraction layer for those
three allocator backends. Everything else is rather Rust core infrastructure
than mm infrastructure.

> Can we please add these files to this section and in future cc people
> listed there? We're here to help!

What's your proposal regarding maintainership? Are you asking me to drop it to
"MEMORY MANAGEMENT - RUST"?

> A side-note I wonder if we also need to put specific files also in relevant
> mm sections? E.g. the slab helper should also be put under the slab section
> perhaps?

Yes, we could. But in the end all Rust helper functions are transparent
wrappers, simply forwarding a function call *without* any additional logic.
They don't really require maintainence effort, and, in the end, are just
trivial boilerplate.
Re: [PATCH v11 0/4] support large align and nid in Rust allocators
Posted by Lorenzo Stoakes 3 months ago
On Tue, Jul 08, 2025 at 01:55:18PM +0200, Danilo Krummrich wrote:
> On Tue, Jul 08, 2025 at 11:58:06AM +0100, Lorenzo Stoakes wrote:
> > +cc Liam
> >
> > Hi guys,
> >
> > We have a section in MAINTAINERS for mm rust (MEMORY MANAGEMENT - RUST), so
> > it's slightly concerning to find a series (at v11!) like this that changes
> > mm-related stuff and it involves files not listed there and nobody bothered
> > to cc- the people listed there.
>
> What files are you referring to? Are you referring to:
>
> 	rust/kernel/alloc.rs
> 	rust/kernel/alloc/*
>
> If so, they're indeed not under the "MEMORY MANAGEMENT - RUST" entry, which
> so far seems correct.

Looking at these, they seem to be intended to be the primary means by which
slab/vmalloc allocations will be managed in rust kernel code correct?

There's also stuff relating to NUMA etc.

I really do wonder where the line between this and the mm stuff is. Because
if the idea is 'well this is just a wrapper around slab/vmalloc' surely the
same can be said of what's in rust/kernel/mm.rs re: VMAs?

So if this is the rust equivalent of include/linux/slab.h and mm/slub.c
then that does seem to me to suggest this should be considered an mm/rust
thing right?

It'd be good to know exactly what is considered mm rust and should go
through the mm tree and what isn't.

Maybe Alice has some insights on this?
Re: [PATCH v11 0/4] support large align and nid in Rust allocators
Posted by Alice Ryhl 3 months ago
On Tue, Jul 08, 2025 at 02:19:38PM +0100, Lorenzo Stoakes wrote:
> On Tue, Jul 08, 2025 at 01:55:18PM +0200, Danilo Krummrich wrote:
> > On Tue, Jul 08, 2025 at 11:58:06AM +0100, Lorenzo Stoakes wrote:
> > > +cc Liam
> > >
> > > Hi guys,
> > >
> > > We have a section in MAINTAINERS for mm rust (MEMORY MANAGEMENT - RUST), so
> > > it's slightly concerning to find a series (at v11!) like this that changes
> > > mm-related stuff and it involves files not listed there and nobody bothered
> > > to cc- the people listed there.
> >
> > What files are you referring to? Are you referring to:
> >
> > 	rust/kernel/alloc.rs
> > 	rust/kernel/alloc/*
> >
> > If so, they're indeed not under the "MEMORY MANAGEMENT - RUST" entry, which
> > so far seems correct.
> 
> Looking at these, they seem to be intended to be the primary means by which
> slab/vmalloc allocations will be managed in rust kernel code correct?
> 
> There's also stuff relating to NUMA etc.
> 
> I really do wonder where the line between this and the mm stuff is. Because
> if the idea is 'well this is just a wrapper around slab/vmalloc' surely the
> same can be said of what's in rust/kernel/mm.rs re: VMAs?
> 
> So if this is the rust equivalent of include/linux/slab.h and mm/slub.c
> then that does seem to me to suggest this should be considered an mm/rust
> thing right?
> 
> It'd be good to know exactly what is considered mm rust and should go
> through the mm tree and what isn't.
> 
> Maybe Alice has some insights on this?

The Rust standard library has three pieces:

- core. Defines standard types that can work anywhere. (such as ints)
- alloc. Defines standard types that require an allocator. (such as vectors)
- std. Defines standard types that require an OS. (such as File or TcpStream)

In the kernel we used to use both core and alloc, but we switched away
from alloc because it doesn't support GFP flags well. The 'RUST [ALLOC]'
subsystem originates from that transition from the Rust stdlib alloc to
our own implementation. It contains essentially three pieces:

- Two data structures Vec and Box.
  - The Box data structure is the simplest possible user of allocation:
    A Box<T> stores a single instance of the struct T in its own
    allocation.
  - The Vec data structure stores a resizable array and maintains a
    pointer, length, capacity triplet. There is a bunch of logic to
    manipulate these to correctly keep track of which parts of the
    vector are in use.
- The Allocator trait.
  - This trait defines what functions an allocator must provide.
  - The data structures Box or Vec require you to specify an allocator,
    and internally it calls into the allocator to manage the backing
    memory for its data.
- Three concrete implementations of the Allocator trait.
  - These are kmalloc, vmalloc, and kvmalloc respectively.

In my eyes, the further down this list you get, the more likely it is
that the patch needs to go through the MM tree.

Alice
Re: [PATCH v11 0/4] support large align and nid in Rust allocators
Posted by Lorenzo Stoakes 3 months ago
On Wed, Jul 09, 2025 at 11:31:31AM +0000, Alice Ryhl wrote:
> On Tue, Jul 08, 2025 at 02:19:38PM +0100, Lorenzo Stoakes wrote:
> > On Tue, Jul 08, 2025 at 01:55:18PM +0200, Danilo Krummrich wrote:
> > > On Tue, Jul 08, 2025 at 11:58:06AM +0100, Lorenzo Stoakes wrote:
> > > > +cc Liam
> > > >
> > > > Hi guys,
> > > >
> > > > We have a section in MAINTAINERS for mm rust (MEMORY MANAGEMENT - RUST), so
> > > > it's slightly concerning to find a series (at v11!) like this that changes
> > > > mm-related stuff and it involves files not listed there and nobody bothered
> > > > to cc- the people listed there.
> > >
> > > What files are you referring to? Are you referring to:
> > >
> > > 	rust/kernel/alloc.rs
> > > 	rust/kernel/alloc/*
> > >
> > > If so, they're indeed not under the "MEMORY MANAGEMENT - RUST" entry, which
> > > so far seems correct.
> >
> > Looking at these, they seem to be intended to be the primary means by which
> > slab/vmalloc allocations will be managed in rust kernel code correct?
> >
> > There's also stuff relating to NUMA etc.
> >
> > I really do wonder where the line between this and the mm stuff is. Because
> > if the idea is 'well this is just a wrapper around slab/vmalloc' surely the
> > same can be said of what's in rust/kernel/mm.rs re: VMAs?
> >
> > So if this is the rust equivalent of include/linux/slab.h and mm/slub.c
> > then that does seem to me to suggest this should be considered an mm/rust
> > thing right?
> >
> > It'd be good to know exactly what is considered mm rust and should go
> > through the mm tree and what isn't.
> >
> > Maybe Alice has some insights on this?
>
> The Rust standard library has three pieces:
>
> - core. Defines standard types that can work anywhere. (such as ints)
> - alloc. Defines standard types that require an allocator. (such as vectors)
> - std. Defines standard types that require an OS. (such as File or TcpStream)

Ahh this makes a lot of sense.

>
> In the kernel we used to use both core and alloc, but we switched away
> from alloc because it doesn't support GFP flags well. The 'RUST [ALLOC]'
> subsystem originates from that transition from the Rust stdlib alloc to
> our own implementation. It contains essentially three pieces:
>
> - Two data structures Vec and Box.
>   - The Box data structure is the simplest possible user of allocation:
>     A Box<T> stores a single instance of the struct T in its own
>     allocation.
>   - The Vec data structure stores a resizable array and maintains a
>     pointer, length, capacity triplet. There is a bunch of logic to
>     manipulate these to correctly keep track of which parts of the
>     vector are in use.
> - The Allocator trait.
>   - This trait defines what functions an allocator must provide.
>   - The data structures Box or Vec require you to specify an allocator,
>     and internally it calls into the allocator to manage the backing
>     memory for its data.
> - Three concrete implementations of the Allocator trait.
>   - These are kmalloc, vmalloc, and kvmalloc respectively.
>
> In my eyes, the further down this list you get, the more likely it is
> that the patch needs to go through the MM tree.

Thanks that's a really useful explanation.

And makes sense that the closer we get to the underlying bits that provide
the actual memory used by all of the above naturally we get closer to core
mm stuff.

I think the implementation details of all of this, looking far into the
future with rust doing a lot more, do end up constituting essentially mm
semantics, as if a decision was made let's say (for the sake of argument)
to - rather than use kvalloc to back an allocation, introduce a new
heuristic that determined whether to use vmalloc or kmalloc - then of
course this would have a very big impact on how slab/vmalloc allocations
were done in the kernel.

Overall I think the approach we're taking - simply adding some mm folks as
reviewers to 'RUST [ALLOC]' - solves the issue wrt being aware of changes
in this area without too much fuss.

>
> Alice

Thanks, Lorenzo
Re: [PATCH v11 0/4] support large align and nid in Rust allocators
Posted by Danilo Krummrich 3 months ago
On Tue Jul 8, 2025 at 3:19 PM CEST, Lorenzo Stoakes wrote:
> On Tue, Jul 08, 2025 at 01:55:18PM +0200, Danilo Krummrich wrote:
>> On Tue, Jul 08, 2025 at 11:58:06AM +0100, Lorenzo Stoakes wrote:
>> > +cc Liam
>> >
>> > Hi guys,
>> >
>> > We have a section in MAINTAINERS for mm rust (MEMORY MANAGEMENT - RUST), so
>> > it's slightly concerning to find a series (at v11!) like this that changes
>> > mm-related stuff and it involves files not listed there and nobody bothered
>> > to cc- the people listed there.
>>
>> What files are you referring to? Are you referring to:
>>
>> 	rust/kernel/alloc.rs
>> 	rust/kernel/alloc/*
>>
>> If so, they're indeed not under the "MEMORY MANAGEMENT - RUST" entry, which
>> so far seems correct.
>
> Looking at these, they seem to be intended to be the primary means by which
> slab/vmalloc allocations will be managed in rust kernel code correct?
>
> There's also stuff relating to NUMA etc.
>
> I really do wonder where the line between this and the mm stuff is. Because
> if the idea is 'well this is just a wrapper around slab/vmalloc' surely the
> same can be said of what's in rust/kernel/mm.rs re: VMAs?
>
> So if this is the rust equivalent of include/linux/slab.h and mm/slub.c
> then that does seem to me to suggest this should be considered an mm/rust
> thing right?
>
> It'd be good to know exactly what is considered mm rust and should go
> through the mm tree and what isn't.

(Please also see the explanation in [1].)

There's a thin abstraction layer for allocators in Rust, represented by the
kernel's Allocator trait [2] (which has a few differences to the Allocator trait in
upstream Rust, which, for instance, can't deal with GFP flags).

This allocator trait is implemented by three backends, one for each of
krealloc(), vrealloc() and kvrealloc() [3].

Otherwise, the alloc module implements Rust's core allocation primitives Box and
Vec, which each of them have a type alias for allocator backends. For instance,
there is KBox, VBox and KVBox [4].

This was also mentioned in the mm rework in [5] and in the subsequent patch
series reworking the Rust allocator module [6].

Before [6], the Rust allocator module only covered the kmalloc allocator (i.e.
krealloc()) and was maintained under the "RUST" entry.

Since [6], this is maintained under the "RUST [ALLOC]" entry by me.

Given that, there is a clear and documented responsibility, which also Andrew is
aware of.

To me the current setup looks reasonable, but feel free to take a look at the
code and its relationship to mm and Rust core infrastructure and let me know
what you think -- I'm happy to discuss other proposals.

[1] https://lore.kernel.org/all/aG0HJte0Xw55z_y4@pollux/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/alloc.rs#n139
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/alloc/allocator.rs#n130
[4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/alloc/kbox.rs
[5] https://lore.kernel.org/all/20240722163111.4766-1-dakr@kernel.org/
[6] https://lore.kernel.org/all/20241004154149.93856-1-dakr@kernel.org/
Re: [PATCH v11 0/4] support large align and nid in Rust allocators
Posted by Lorenzo Stoakes 3 months ago
On Tue, Jul 08, 2025 at 04:16:48PM +0200, Danilo Krummrich wrote:
> On Tue Jul 8, 2025 at 3:19 PM CEST, Lorenzo Stoakes wrote:
> > On Tue, Jul 08, 2025 at 01:55:18PM +0200, Danilo Krummrich wrote:
> >> On Tue, Jul 08, 2025 at 11:58:06AM +0100, Lorenzo Stoakes wrote:
> >> > +cc Liam
> >> >
> >> > Hi guys,
> >> >
> >> > We have a section in MAINTAINERS for mm rust (MEMORY MANAGEMENT - RUST), so
> >> > it's slightly concerning to find a series (at v11!) like this that changes
> >> > mm-related stuff and it involves files not listed there and nobody bothered
> >> > to cc- the people listed there.
> >>
> >> What files are you referring to? Are you referring to:
> >>
> >> 	rust/kernel/alloc.rs
> >> 	rust/kernel/alloc/*
> >>
> >> If so, they're indeed not under the "MEMORY MANAGEMENT - RUST" entry, which
> >> so far seems correct.
> >
> > Looking at these, they seem to be intended to be the primary means by which
> > slab/vmalloc allocations will be managed in rust kernel code correct?
> >
> > There's also stuff relating to NUMA etc.
> >
> > I really do wonder where the line between this and the mm stuff is. Because
> > if the idea is 'well this is just a wrapper around slab/vmalloc' surely the
> > same can be said of what's in rust/kernel/mm.rs re: VMAs?
> >
> > So if this is the rust equivalent of include/linux/slab.h and mm/slub.c
> > then that does seem to me to suggest this should be considered an mm/rust
> > thing right?
> >
> > It'd be good to know exactly what is considered mm rust and should go
> > through the mm tree and what isn't.
>
> (Please also see the explanation in [1].)
>
> There's a thin abstraction layer for allocators in Rust, represented by the
> kernel's Allocator trait [2] (which has a few differences to the Allocator trait in
> upstream Rust, which, for instance, can't deal with GFP flags).
>
> This allocator trait is implemented by three backends, one for each of
> krealloc(), vrealloc() and kvrealloc() [3].
>
> Otherwise, the alloc module implements Rust's core allocation primitives Box and
> Vec, which each of them have a type alias for allocator backends. For instance,
> there is KBox, VBox and KVBox [4].
>
> This was also mentioned in the mm rework in [5] and in the subsequent patch
> series reworking the Rust allocator module [6].
>
> Before [6], the Rust allocator module only covered the kmalloc allocator (i.e.
> krealloc()) and was maintained under the "RUST" entry.
>
> Since [6], this is maintained under the "RUST [ALLOC]" entry by me.
>
> Given that, there is a clear and documented responsibility, which also Andrew is
> aware of.
>
> To me the current setup looks reasonable, but feel free to take a look at the
> code and its relationship to mm and Rust core infrastructure and let me know
> what you think -- I'm happy to discuss other proposals.

Thanks for the explanation.

To me this is clearly mm rust code. This is an abstraction over mm bits to
provide slab or vmalloc allocations for rust bits.

To be clear - I'm not suggesting anything dramatic here, nor in any way
suggesting you ought not maintain this (apologies if this wasn't clear :)

It's really a couple points:

1. Purely pragmatically - how can we make sure relevant people are pinged?

2. Having clarity on what does/does not constitute 'MEMORY MANAGEMENT - RUST'
   (again, perhaps Alice best placed to give some input here from her point of
   view).

We could solve 1 very simply by just using the fact we can have files in
multiple sections in MAINTAINERS.

Doing a scripts/get_maintainers.pl invocation will very clearly tell you
who's in charge of what so there'd be no lack of clarity on this.

It's a bit messy, obviously. But it'd solve the issue of mm people not
getting notified when things change.

However, at this stage you might want to just limit this to people who have
_opted in_ to look at mm/rust stuff. In which case then it'd make sense to
add only to the "MEMORY MANAGEMENT - RUST" section (but here we need to
address point 2 obviously).

Alternatively we could just add reviewers to the rust alloc bit.

>
> [1] https://lore.kernel.org/all/aG0HJte0Xw55z_y4@pollux/
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/alloc.rs#n139
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/alloc/allocator.rs#n130
> [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/alloc/kbox.rs
> [5] https://lore.kernel.org/all/20240722163111.4766-1-dakr@kernel.org/
> [6] https://lore.kernel.org/all/20241004154149.93856-1-dakr@kernel.org/


I feel it's really important to not separate rust _too much_ from the
subsystems it utilises - if we intend to have rust be used more and more
and integrated further in the kernel (something I'd like to see, more so
when I learn it :P) - the only practical way forward is for the rust stuff
to be considered a first class citizen and managed hand-in-glove with the
not-rust stuff.

Cheers, Lorenzo
Re: [PATCH v11 0/4] support large align and nid in Rust allocators
Posted by Danilo Krummrich 3 months ago
On Tue Jul 8, 2025 at 4:39 PM CEST, Lorenzo Stoakes wrote:
> On Tue, Jul 08, 2025 at 04:16:48PM +0200, Danilo Krummrich wrote:
>> (Please also see the explanation in [1].)
>>
>> There's a thin abstraction layer for allocators in Rust, represented by the
>> kernel's Allocator trait [2] (which has a few differences to the Allocator trait in
>> upstream Rust, which, for instance, can't deal with GFP flags).
>>
>> This allocator trait is implemented by three backends, one for each of
>> krealloc(), vrealloc() and kvrealloc() [3].
>>
>> Otherwise, the alloc module implements Rust's core allocation primitives Box and
>> Vec, which each of them have a type alias for allocator backends. For instance,
>> there is KBox, VBox and KVBox [4].
>>
>> This was also mentioned in the mm rework in [5] and in the subsequent patch
>> series reworking the Rust allocator module [6].
>>
>> Before [6], the Rust allocator module only covered the kmalloc allocator (i.e.
>> krealloc()) and was maintained under the "RUST" entry.
>>
>> Since [6], this is maintained under the "RUST [ALLOC]" entry by me.
>>
>> Given that, there is a clear and documented responsibility, which also Andrew is
>> aware of.
>>
>> To me the current setup looks reasonable, but feel free to take a look at the
>> code and its relationship to mm and Rust core infrastructure and let me know
>> what you think -- I'm happy to discuss other proposals.
>
> Thanks for the explanation.
>
> To me this is clearly mm rust code. This is an abstraction over mm bits to
> provide slab or vmalloc allocations for rust bits.
>
> To be clear - I'm not suggesting anything dramatic here, nor in any way
> suggesting you ought not maintain this (apologies if this wasn't clear :)
>
> It's really a couple points:
>
> 1. Purely pragmatically - how can we make sure relevant people are pinged?

I'm very happy to add more reviewers to the RUST [ALLOC] entry for this purpose.

Can you please send a patch for this?

> 2. Having clarity on what does/does not constitute 'MEMORY MANAGEMENT - RUST'
>    (again, perhaps Alice best placed to give some input here from her point of
>    view).

In the end that's a question of definition.

The reason RUST [ALLOC] is a thing is because it is very closely related to Rust
core infrastructure with only a thin backend using {k,v}realloc().

Compared to other abstractions, the main purpose is not to expose a Rust
interface for an existing kernel specific API, but rather implement a very Rust
specific concept while being a user of an existing kernel C API.

> We could solve 1 very simply by just using the fact we can have files in
> multiple sections in MAINTAINERS.

Please not -- I don't want to have files in multiple entries in MAINTAINERS,
especially when there are different maintainers and different trees. I prefer
clear responsibility.

> Doing a scripts/get_maintainers.pl invocation will very clearly tell you
> who's in charge of what so there'd be no lack of clarity on this.

How's that when a file is in multiple entries?

> It's a bit messy, obviously. But it'd solve the issue of mm people not
> getting notified when things change.
>
> However, at this stage you might want to just limit this to people who have
> _opted in_ to look at mm/rust stuff. In which case then it'd make sense to
> add only to the "MEMORY MANAGEMENT - RUST" section (but here we need to
> address point 2 obviously).
>
> Alternatively we could just add reviewers to the rust alloc bit.

Yeah, let's do that instead please.

>>
>> [1] https://lore.kernel.org/all/aG0HJte0Xw55z_y4@pollux/
>> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/alloc.rs#n139
>> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/alloc/allocator.rs#n130
>> [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/alloc/kbox.rs
>> [5] https://lore.kernel.org/all/20240722163111.4766-1-dakr@kernel.org/
>> [6] https://lore.kernel.org/all/20241004154149.93856-1-dakr@kernel.org/
>
>
> I feel it's really important to not separate rust _too much_ from the
> subsystems it utilises - if we intend to have rust be used more and more
> and integrated further in the kernel (something I'd like to see, more so
> when I learn it :P) - the only practical way forward is for the rust stuff
> to be considered a first class citizen and managed hand-in-glove with the
> not-rust stuff.

You're preaching to the choir with this on my end. I'm recommending subsystems
that receive the first Rust bits to get involved in one or the other way all
the time. And if it's only by reading along. :)
Re: [PATCH v11 0/4] support large align and nid in Rust allocators
Posted by Lorenzo Stoakes 3 months ago
On Tue, Jul 08, 2025 at 05:11:28PM +0200, Danilo Krummrich wrote:
> On Tue Jul 8, 2025 at 4:39 PM CEST, Lorenzo Stoakes wrote:
> > On Tue, Jul 08, 2025 at 04:16:48PM +0200, Danilo Krummrich wrote:
> >> (Please also see the explanation in [1].)
> >>
> >> There's a thin abstraction layer for allocators in Rust, represented by the
> >> kernel's Allocator trait [2] (which has a few differences to the Allocator trait in
> >> upstream Rust, which, for instance, can't deal with GFP flags).
> >>
> >> This allocator trait is implemented by three backends, one for each of
> >> krealloc(), vrealloc() and kvrealloc() [3].
> >>
> >> Otherwise, the alloc module implements Rust's core allocation primitives Box and
> >> Vec, which each of them have a type alias for allocator backends. For instance,
> >> there is KBox, VBox and KVBox [4].
> >>
> >> This was also mentioned in the mm rework in [5] and in the subsequent patch
> >> series reworking the Rust allocator module [6].
> >>
> >> Before [6], the Rust allocator module only covered the kmalloc allocator (i.e.
> >> krealloc()) and was maintained under the "RUST" entry.
> >>
> >> Since [6], this is maintained under the "RUST [ALLOC]" entry by me.
> >>
> >> Given that, there is a clear and documented responsibility, which also Andrew is
> >> aware of.
> >>
> >> To me the current setup looks reasonable, but feel free to take a look at the
> >> code and its relationship to mm and Rust core infrastructure and let me know
> >> what you think -- I'm happy to discuss other proposals.
> >
> > Thanks for the explanation.
> >
> > To me this is clearly mm rust code. This is an abstraction over mm bits to
> > provide slab or vmalloc allocations for rust bits.
> >
> > To be clear - I'm not suggesting anything dramatic here, nor in any way
> > suggesting you ought not maintain this (apologies if this wasn't clear :)
> >
> > It's really a couple points:
> >
> > 1. Purely pragmatically - how can we make sure relevant people are pinged?
>
> I'm very happy to add more reviewers to the RUST [ALLOC] entry for this purpose.

Thanks!

>
> Can you please send a patch for this?

Ack will do.

>
> > 2. Having clarity on what does/does not constitute 'MEMORY MANAGEMENT - RUST'
> >    (again, perhaps Alice best placed to give some input here from her point of
> >    view).
>
> In the end that's a question of definition.
>
> The reason RUST [ALLOC] is a thing is because it is very closely related to Rust
> core infrastructure with only a thin backend using {k,v}realloc().
>
> Compared to other abstractions, the main purpose is not to expose a Rust
> interface for an existing kernel specific API, but rather implement a very Rust
> specific concept while being a user of an existing kernel C API.

Right sure. this stuff gets blurry...

>
> > We could solve 1 very simply by just using the fact we can have files in
> > multiple sections in MAINTAINERS.
>
> Please not -- I don't want to have files in multiple entries in MAINTAINERS,
> especially when there are different maintainers and different trees. I prefer
> clear responsibility.

Ack :) No probs.

>
> > Doing a scripts/get_maintainers.pl invocation will very clearly tell you
> > who's in charge of what so there'd be no lack of clarity on this.
>
> How's that when a file is in multiple entries?

Well you can see which is directly related which not. But I get this is not what
you want! :)

>
> > It's a bit messy, obviously. But it'd solve the issue of mm people not
> > getting notified when things change.
> >
> > However, at this stage you might want to just limit this to people who have
> > _opted in_ to look at mm/rust stuff. In which case then it'd make sense to
> > add only to the "MEMORY MANAGEMENT - RUST" section (but here we need to
> > address point 2 obviously).
> >
> > Alternatively we could just add reviewers to the rust alloc bit.
>
> Yeah, let's do that instead please.

Sure.

>
> >>
> >> [1] https://lore.kernel.org/all/aG0HJte0Xw55z_y4@pollux/
> >> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/alloc.rs#n139
> >> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/alloc/allocator.rs#n130
> >> [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/alloc/kbox.rs
> >> [5] https://lore.kernel.org/all/20240722163111.4766-1-dakr@kernel.org/
> >> [6] https://lore.kernel.org/all/20241004154149.93856-1-dakr@kernel.org/
> >
> >
> > I feel it's really important to not separate rust _too much_ from the
> > subsystems it utilises - if we intend to have rust be used more and more
> > and integrated further in the kernel (something I'd like to see, more so
> > when I learn it :P) - the only practical way forward is for the rust stuff
> > to be considered a first class citizen and managed hand-in-glove with the
> > not-rust stuff.
>
> You're preaching to the choir with this on my end. I'm recommending subsystems
> that receive the first Rust bits to get involved in one or the other way all
> the time. And if it's only by reading along. :)

:)))

Yes well I like to think we in mm are forward thinking about this!

In the end rust is here to stay and it's important to embrace it and find the
best means of collaborating!

Cheers, Lorenzo
Re: [PATCH v11 0/4] support large align and nid in Rust allocators
Posted by Lorenzo Stoakes 3 months ago
TL;DR - the real issue here is not cc'ing the right people (Vlastimil was
not cc'd until v11 for instance). Beyond that there's some process things
to think about re: rust/mm section.

On Tue, Jul 08, 2025 at 01:55:18PM +0200, Danilo Krummrich wrote:
> On Tue, Jul 08, 2025 at 11:58:06AM +0100, Lorenzo Stoakes wrote:
> > +cc Liam
> >
> > Hi guys,
> >
> > We have a section in MAINTAINERS for mm rust (MEMORY MANAGEMENT - RUST), so
> > it's slightly concerning to find a series (at v11!) like this that changes
> > mm-related stuff and it involves files not listed there and nobody bothered
> > to cc- the people listed there.
>
> What files are you referring to? Are you referring to:
>
> 	rust/kernel/alloc.rs
> 	rust/kernel/alloc/*

 include/linux/slab.h           | 40 ++++++++++++++-------
 include/linux/vmalloc.h        | 12 +++++--
 mm/nommu.c                     |  3 +-
 mm/slub.c                      | 64 +++++++++++++++++++++++-----------
 mm/vmalloc.c                   | 28 ++++++++++++---
this ---> rust/helpers/slab.c            | 10 +++---
this ---> rust/helpers/vmalloc.c         |  5 +--
 rust/kernel/alloc.rs           | 52 ++++++++++++++++++++++++---
 rust/kernel/alloc/allocator.rs | 46 ++++++++++++------------
 rust/kernel/alloc/kbox.rs      |  4 +--
 rust/kernel/alloc/kvec.rs      | 11 ++++--
 11 files changed, 194 insertions(+), 81 deletions(-)

These are clearly specifically related to mm no?

Apologies with comment re rust/kernel/mm/... I was misreading the changes here
(lack of diffstat unhelpful).

>
> If so, they're indeed not under the "MEMORY MANAGEMENT - RUST" entry, which
> so far seems correct.

I think the sticking point here is that these helpers are considered
trivial wrappers around mm bits. More below.

>
> Please also note that we had "RUST [ALLOC]" before "MEMORY MANAGEMENT - RUST"
> did exist.

I'm talking about the mm-specific bits. See above.

>
> > I can fully understand there being some process fail here meaning you
> > missed it - fine if so - but let's fix it please moving forwards.
>
> I agree that this series should have a couple more people in Cc.

There were 17 people missing. So more than a couple.

Until v11 the slab maintainer wasn't even cc'd for changes to slab :)

v10 at https://lore.kernel.org/linux-mm/20250702160758.3609992-1-vitaly.wool@konsulko.se/

This definitely isn't ok.

>
> Given the existing entries in the MAINTAINERS file the Rust parts seems to be
> correct though.

scripts/get_maintainers.pl says:

Alex Gaynor <alex.gaynor@gmail.com> (maintainer:RUST)
Boqun Feng <boqun.feng@gmail.com> (reviewer:RUST)
Gary Guo <gary@garyguo.net> (reviewer:RUST,commit_signer:3/5=60%,authored:1/5=20%,removed_lines:1/9=11%,commit_signer:1/3=33%)
"Björn Roy Baron" <bjorn3_gh@protonmail.com> (reviewer:RUST)
Benno Lossin <lossin@kernel.org> (reviewer:RUST,commit_signer:2/5=40%)
Andreas Hindborg <a.hindborg@kernel.org> (reviewer:RUST,authored:1/5=20%,added_lines:10/26=38%)
Alice Ryhl <aliceryhl@google.com> (reviewer:RUST,commit_signer:2/5=40%,commit_signer:1/3=33%)
Trevor Gross <tmgross@umich.edu> (reviewer:RUST)
Danilo Krummrich <dakr@kernel.org> (reviewer:RUST,authored:1/5=20%,added_lines:6/26=23%,commit_signer:1/3=33%,authored:1/3=33%,added_lines:9/14=64%)

Most of whom aren't cc'd.

This is based on mm-new's MAINTAINERS though so it may not be up-to-date.

>
> > It's really important to me that the rust efforts in mm are collaborative -
> > I really believe in your mission (well - for me it's about the compiler
> > _helping_ me not shooting me in the foot :) - and have put substantial
> > effort in assisting initial work there. So let's make sure we're
> > collaborative in both directions please.
>
> AFAICT, those efforts are collaborative.
>
> Back then I sent patches to introduce vrealloc() and improve and align
> kvrealloc() and krealloc() [1]; it was also mentioned that this was, besides the
> other advantages, prerequisite work for the Rust allocator patch series [2].
>
> The subsequent Rust allocator patch series [2] was also sent to Andrew and the
> -mm mailing list; the previous code replaced by this series was maintained under
> the "RUST" entry in the maintainers file.
>
> With the introduction of the new Rust allocator code I took over maintainership.
>
> So, Andrew is aware of the Rust allocator tree, please see also [3].

I mean there's process issues here too I think. I think ideally best to cc mm
rust people too, sending to linux-mm is usually not enough, since we are all so
busy it's hard to keep up.

I'm making real efforts to improve this by adding explicit MAINTAINERS entries
for things as best I can so everyone's life is easier - and absolutely this is a
bit in flux atm - so forgivable to not be aware/miss entries that were only
added recently.

Anyway, that series appears to me to be more so _internal_.

The important stuff to have mm input on are things that _interface_ with
mm. Even trivial wrappers should be at least tracked so people can at least be
aware of things that might change.

And absolutely I couldn't agree more with this going through the mm tree to be
sync'd up with the mm changes - there was broad agreement on this at LSF/MM.

>
> [1] https://lore.kernel.org/all/20240722163111.4766-1-dakr@kernel.org/
> [2] https://lore.kernel.org/all/20241004154149.93856-1-dakr@kernel.org/
> [3] https://lore.kernel.org/all/20250625143450.2afc473fc0e7124a5108c187@linux-foundation.org/
>
> > We have rust/kernel/mm/ under MEMORY MANAGEMENT - RUST too, I'm not au fait
> > with your approach to structuring in these folders but seems to me these
> > helpers should be there? I may be unaware of some rust aspect of this
> > however.
>
> The Rust allocator module is a user of exactly three functions of mm, i.e.
> krealloc(), vrealloc(), kvrealloc(), with a thin abstraction layer for those
> three allocator backends. Everything else is rather Rust core infrastructure
> than mm infrastructure.

I would argue that making use of mm interfaces would make it important to cc
relevant maintainers.

>
> > Can we please add these files to this section and in future cc people
> > listed there? We're here to help!
>
> What's your proposal regarding maintainership? Are you asking me to drop it to
> "MEMORY MANAGEMENT - RUST"?

I'm not making any suggestions re: maintainership, I'm suggesting mm-related
rust files should belong in the mm rust section and that people who've
volunteered to review mm-related rust code should be cc'd on series relating to
rust + mm.

>
> > A side-note I wonder if we also need to put specific files also in relevant
> > mm sections? E.g. the slab helper should also be put under the slab section
> > perhaps?
>
> Yes, we could. But in the end all Rust helper functions are transparent
> wrappers, simply forwarding a function call *without* any additional logic.
> They don't really require maintainence effort, and, in the end, are just
> trivial boilerplate.

It'd be good to keep track of files like this and to know who to cc when
you change them.
Re: [PATCH v11 0/4] support large align and nid in Rust allocators
Posted by Danilo Krummrich 3 months ago
On Tue Jul 8, 2025 at 2:36 PM CEST, Lorenzo Stoakes wrote:
> TL;DR - the real issue here is not cc'ing the right people (Vlastimil was
> not cc'd until v11 for instance).

Since Andrew was Cc'd and also did reply, but didn't mention anything about
missing receipients on the -mm side of things, I did not see a reason to bring
anything up regarding this from my end.

Thanks for bringing this up.

> On Tue, Jul 08, 2025 at 01:55:18PM +0200, Danilo Krummrich wrote:
>> On Tue, Jul 08, 2025 at 11:58:06AM +0100, Lorenzo Stoakes wrote:
>> > +cc Liam
>> >
>> > Hi guys,
>> >
>> > We have a section in MAINTAINERS for mm rust (MEMORY MANAGEMENT - RUST), so
>> > it's slightly concerning to find a series (at v11!) like this that changes
>> > mm-related stuff and it involves files not listed there and nobody bothered
>> > to cc- the people listed there.
>>
>> What files are you referring to? Are you referring to:
>>
>> 	rust/kernel/alloc.rs
>> 	rust/kernel/alloc/*
>
> this ---> rust/helpers/slab.c            | 10 +++---
> this ---> rust/helpers/vmalloc.c         |  5 +--

So, your concern is about those?

> These are clearly specifically related to mm no?

Yes, and if the maintainers of slab and vmalloc agree we can add them there.

But again, they're just re-exporting inline functions and macros from header
files, which bindgen does not pick up automatically. They do not carry any logic
and purely are a workaround for bindgen.

For instance,

void * __must_check __realloc_size(2)
	rust_helper_vrealloc(const void *p, size_t size, gfp_t flags)
	{
	        return vrealloc(p, size, flags);
	}

works around bindgen not picking up the vrealloc() macro.
Re: [PATCH v11 0/4] support large align and nid in Rust allocators
Posted by Lorenzo Stoakes 3 months ago
On Tue, Jul 08, 2025 at 03:41:31PM +0200, Danilo Krummrich wrote:
> On Tue Jul 8, 2025 at 2:36 PM CEST, Lorenzo Stoakes wrote:
> > TL;DR - the real issue here is not cc'ing the right people (Vlastimil was
> > not cc'd until v11 for instance).
>
> Since Andrew was Cc'd and also did reply, but didn't mention anything about
> missing receipients on the -mm side of things, I did not see a reason to bring
> anything up regarding this from my end.
>
> Thanks for bringing this up.

No problem - and it's understandable that it wouldn't quite be clear that
it's important to cc- as many people, as things have recently changed a lot
in mm re: having a good + specific response from get_maintainers.pl.

At any rate, it's important to always include M's/R's from
get_maintainers.pl as a matter of course when submitting series.

>
> > On Tue, Jul 08, 2025 at 01:55:18PM +0200, Danilo Krummrich wrote:
> >> On Tue, Jul 08, 2025 at 11:58:06AM +0100, Lorenzo Stoakes wrote:
> >> > +cc Liam
> >> >
> >> > Hi guys,
> >> >
> >> > We have a section in MAINTAINERS for mm rust (MEMORY MANAGEMENT - RUST), so
> >> > it's slightly concerning to find a series (at v11!) like this that changes
> >> > mm-related stuff and it involves files not listed there and nobody bothered
> >> > to cc- the people listed there.
> >>
> >> What files are you referring to? Are you referring to:
> >>
> >> 	rust/kernel/alloc.rs
> >> 	rust/kernel/alloc/*
> >
> > this ---> rust/helpers/slab.c            | 10 +++---
> > this ---> rust/helpers/vmalloc.c         |  5 +--
>
> So, your concern is about those?
>
> > These are clearly specifically related to mm no?
>
> Yes, and if the maintainers of slab and vmalloc agree we can add them there.

And the mm/rust section. Because if that's not where we put files that
relate to mm/rust, what is it for?

I think it turns out the larger question here is really about the alloc
stuff. I raised that in a separate thread.

>
> But again, they're just re-exporting inline functions and macros from header
> files, which bindgen does not pick up automatically. They do not carry any logic
> and purely are a workaround for bindgen.
>
> For instance,
>
> void * __must_check __realloc_size(2)
> 	rust_helper_vrealloc(const void *p, size_t size, gfp_t flags)
> 	{
> 	        return vrealloc(p, size, flags);
> 	}
>
> works around bindgen not picking up the vrealloc() macro.

Thanks for the explanation - that's useful.

So the fact these came up by accident more or less I think underlines the
broader need for a conversation about about what does/doesn't constitute
mm/rust.

But perhaps best to keep that to the thread where I raise the alloc stuff.
Re: [PATCH v11 0/4] support large align and nid in Rust allocators
Posted by Lorenzo Stoakes 3 months ago
FYI I ran scripts/get_maintainers.pl on your series and there are 22 people you
should have cc'd and you cc'd 5.

While we'd all love to read all of linux-mm, for the majority of us that isn't
really practical so it's vital you follow correct kernel procedure here.

So I think you need to resend this with correct CC's at this point.

Thanks.