Message-ID: <Z6t51xodSV21ER4M@thinkpad>
Date: Tue, 11 Feb 2025 11:24:55 -0500
From: Yury Norov <yury.norov@...il.com>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
	Miguel Ojeda <miguel.ojeda.sandonis@...il.com>,
	Danilo Krummrich <dakr@...hat.com>, Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	linux-pm@...r.kernel.org,
	Vincent Guittot <vincent.guittot@...aro.org>,
	Stephen Boyd <sboyd@...nel.org>, Nishanth Menon <nm@...com>,
	rust-for-linux@...r.kernel.org,
	Manos Pitsidianakis <manos.pitsidianakis@...aro.org>,
	Erik Schilling <erik.schilling@...aro.org>,
	Alex Bennée <alex.bennee@...aro.org>,
	Joakim Bech <joakim.bech@...aro.org>, Rob Herring <robh@...nel.org>,
	Christoph Hellwig <hch@....de>, Jason Gunthorpe <jgg@...dia.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH V8 04/14] rust: Add cpumask helpers

+ Jason, Christoph

On Tue, Feb 11, 2025 at 09:59:08AM +0530, Viresh Kumar wrote:
> On 10-02-25, 19:02, Yury Norov wrote:
> > On Thu, Feb 06, 2025 at 02:58:25PM +0530, Viresh Kumar wrote:
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index ee6709599df5..bfc1bf2ebd77 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -4021,6 +4021,7 @@ F:	lib/cpumask_kunit.c
> > >  F:	lib/find_bit.c
> > >  F:	lib/find_bit_benchmark.c
> > >  F:	lib/test_bitmap.c
> > > +F:	rust/helpers/cpumask.c
> > 
> > Sorry what? I never committed to maintain this thing, and will
> > definitely not do it for free.
> > 
> > NAK.
> 
> Okay.

No, not Okay.

To begin with, this is the 8th version of the same patch, but you
only now bothered to CC someone who is listed in MAINTAINERS. This is
not how the community works.

You also made it a patch bomb that touches multiple critical and very
sensitive subsystems. You link them to an experimental and unstable
project, and do it in a way that makes it really easy to slip through
maintainers' attention.

Not speaking for others, but please for cpumasks create a separate
series and start thorough discussion.

> I will add a separate entry for Rust cpumask stuff.

That would make things even worse. Before you wanted me to maintain
rust linkage. Now you want me to get approval from someone else who
maintains rust linkage. In case I need to change something, I want to
be able to just change.

I went deeper into the subject, and found that rust team has similar
problems with other maintainers:

https://lore.kernel.org/lkml/20250108122825.136021-3-abdiel.janulgue@gmail.com/

I'm particularly concerned with the following comment from Jason
Gunthorpe:

  All PRs to Linus must not break the rust build and the responsibilty
  for that falls to all the maintainers. If the Rust team is not quick
  enough to resolve any issues during the development window then
  patches must be dropped before sending PRs, or Linus will refuse the
  PR.

https://lore.kernel.org/lkml/20250130154646.GA2298732@nvidia.com/

And that happened at least once, right?

I don't expect that the functions you export now will get changed
anytime soon but it happens from time to time. cpumask_next_wrap()
is one recent example.

https://lore.kernel.org/netdev/20250128164646.4009-7-yury.norov@gmail.com/T/

The more drivers you write, the more functionality you will inevitably
pull. And the risk that something will get broken one day will grow
exponentially. So before we move forward, please explain in very details
how would you act in a scenario described by Jason?

Do you proactively run builds against development branches? If so,
please add my bitmap-for-next.

https://github.com/norov/linux/tree/bitmap-for-next

Have you considered creating a conftest, so that if rust fails to build
against the current kernel, it will get disabled until you fix the
issue?

Maybe you will just teach your language to understand inlines, and
that's it? Does it understand macros?

Thanks,
Yury

> > > +#ifndef CONFIG_CPUMASK_OFFSTACK
> > > +void rust_helper_free_cpumask_var(cpumask_var_t mask)
> > > +{
> > > +	free_cpumask_var(mask);
> > > +}
> > > +#endif
> > 
> > This is most likely wrong because free_cpumask_var() is declared
> > unconditionally, and I suspect the rust helper should be as well.
> 
> Non-trivial C macros and inlined C functions cannot be used directly
> in the Rust code and are used via functions ("helpers") that wrap
> those so that they can be called from Rust.
> 
> The free_cpumask_var() function is defined as inline only for the
> CONFIG_CPUMASK_OFFSTACK=n case and that's where we need this wrapper.
> For the other case (CONFIG_CPUMASK_OFFSTACK=y), Rust code can directly
> call free_cpumask_var() from the C code and we don't need this helper.
> 
> > Please for the next iteration keep me CCed for the whole series. I
> > want to make sure you'll not make me a rust maintainer accidentally.
> 
> Sure.
> 
> -- 
> viresh

Powered by blists - more mailing lists