Message-ID: <8a08931c-ce54-4e96-9e99-e7c696389dc3@lucifer.local>
Date: Tue, 8 Jul 2025 13:36:05 +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

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@...il.com> (maintainer:RUST)
Boqun Feng <boqun.feng@...il.com> (reviewer:RUST)
Gary Guo <gary@...yguo.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@...tonmail.com> (reviewer:RUST)
Benno Lossin <lossin@...nel.org> (reviewer:RUST,commit_signer:2/5=40%)
Andreas Hindborg <a.hindborg@...nel.org> (reviewer:RUST,authored:1/5=20%,added_lines:10/26=38%)
Alice Ryhl <aliceryhl@...gle.com> (reviewer:RUST,commit_signer:2/5=40%,commit_signer:1/3=33%)
Trevor Gross <tmgross@...ch.edu> (reviewer:RUST)
Danilo Krummrich <dakr@...nel.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.

Powered by blists - more mailing lists