Message-ID: <4c122436-db58-4ca5-bc64-5ca596f03952@lucifer.local>
Date: Tue, 8 Jul 2025 15:39:44 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Vitaly Wool <vitaly.wool@...sulko.se>, linux-mm@...ck.org,
        akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
        Uladzislau Rezki <urezki@...il.com>, Alice Ryhl <aliceryhl@...gle.com>,
        Vlastimil Babka <vbabka@...e.cz>, rust-for-linux@...r.kernel.org,
        Liam Howlett <liam.howlett@...cle.com>
Subject: Re: [PATCH v11 0/4] support large align and nid in Rust allocators

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

Powered by blists - more mailing lists