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>
+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?
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.
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?
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
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
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/
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
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. :)
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
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.
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.
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.
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.
© 2016 - 2025 Red Hat, Inc.