Message-ID: <2024121646-shelve-series-5319@gregkh>
Date: Mon, 16 Dec 2024 13:14:16 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Andreas Hindborg <a.hindborg@...nel.org>
Cc: 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>,
	Alice Ryhl <aliceryhl@...gle.com>,
	Masahiro Yamada <masahiroy@...nel.org>,
	Nathan Chancellor <nathan@...nel.org>,
	Nicolas Schier <nicolas@...sle.eu>,
	Trevor Gross <tmgross@...ch.edu>,
	Adam Bratschi-Kaye <ark.email@...il.com>,
	rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-kbuild@...r.kernel.org, Jens Axboe <axboe@...nel.dk>
Subject: Re: [PATCH v3 0/4] rust: extend `module!` macro with integer
 parameter support

On Mon, Dec 16, 2024 at 10:51:53AM +0100, Andreas Hindborg wrote:
> "Greg KH" <gregkh@...uxfoundation.org> writes:
> 
> > On Fri, Dec 13, 2024 at 04:38:30PM +0100, Andreas Hindborg wrote:
> >> "Greg KH" <gregkh@...uxfoundation.org> writes:
> >> > On Fri, Dec 13, 2024 at 01:24:42PM +0100, Andreas Hindborg wrote:
> >> >> "Greg KH" <gregkh@...uxfoundation.org> writes:
> >> >> > On Fri, Dec 13, 2024 at 12:30:45PM +0100, Andreas Hindborg wrote:
> >> I'm not getting a clear reading on the following, perhaps you can
> >> clarify:
> >>
> >>  - Is the community aligned on dropping module parameters for all new
> >>    drivers?
> >>    - If so, was this decided upon at some point or is this a fluid
> >>      decision that is just manifesting now?
> >
> > It's something that I've been saying in review comments of drivers for
> > many many years now.  Again, it was one of the main reasons we created
> > configfs and sysfs all those decades ago, because module parameters just
> > do not work properly for drivers in almost all cases.
> 
> Thinking a bit about this - are you sure there are no situations where
> module parameters is actually the right choice? I could imagine deeply
> embedded deployments, potentially with all required modules linked in to
> a static kernel image. It is probably desirable to be able to pass
> configuration to the modules via the kernel command line in this
> situation. If not for configuration in the field, then for a development
> situation.

I'm not saying "no situations at all", I'm saying "almost no situations
need a module parameter, and almost NO driver needs one."  That does not
mean "none".

> Surely there are also situations in regular PC setups where it is
> desirable to pass configuration data to a module, so that it is
> available at load time. With configfs, module initialization becomes a
> two stage process of first loading and then configuring. That is not
> always what you would want.

"regular" PC setups do not want module parameters either, they should
all be dynamic busses and everything should "just work".

> Since module parameters actually do show up in sysfs as writable
> entries, when the right flags are passed, they do feel very similar to
> sysfs/configfs for simple use cases. What are the reasons they should
> not be usable for these simple use cases?

Because they are almost never a good idea for a driver because drivers
can control multiple devices and how do you show that in a single value?

For non-drivers, it's a different issue, but that's not what you are
considering here :)

> >>  - Does this ban of module parameters also cover cases where backwards
> >>    compatibility is desirable?
> >
> > No, we don't break existing kernel features, but if you are writing a
> > new driver, don't add them and then there's no compatibility issue.
> >
> > We don't normally allow "rewrites" of drivers, but if we do, yes, you
> > would have to implement the old features if needed.
> >
> > As you just seem to want to write an "example" block driver, no need to
> > add the module option there, just do it right this time in how to
> > properly configure things.
> >
> >>  - Can we merge this so I can move forward at my current projected
> >>    course, or should I plan on dealing with not having this available?
> >
> > We generally do not want to merge apis without any real users, as it's
> > hard to justify them, right?  Also, we don't even know if they work
> > properly or not.
> 
> While null_blk is just a piece of testing infrastructure that you would
> not deploy in a production environment, it is still a "real user". I am
> sure we can agree on the importance of testing.
> 
> The exercise I am undertaking is to produce a drop in replacement of the
> existing C null_blk driver. If all goes well, we are considering to swap
> the C implementation for the Rust implementation in X number of years.
> Granted - a lot of things have to fall into place for that to happen,
> but that is the plan. This plan does not really work well if the two
> modules do not have the same interface.

Why do you have to have the same interface?  Why not do it "properly"
and make it use configfs that way you can have multiple devices and test
them all at the same time?

As this is "just" a testing driver, there should not be any need to keep
the same user/kernel api for setting things up, right?

Again, don't make the mistakes we have in the past, drivers should NOT
be using module parameters.

> I understand that you would like to phase out module parameters, but I
> don't think blocking their use from Rust is the right way to go about
> that task. If you really feel that module parameters have no place in
> new drivers, I would suggest that to be part of review process for each
> individual new driver - not at the stage of enabling module parameters
> for Rust in general.

I'm saying that module parameters do NOT belong in a driver, which is
what you are wanting to do here.  And as for adding new apis, please
only do so when you have a real user, I don't see a real user for module
parameters in rust just yet.  If that changes, I'll reconsider my stance :)

thanks,

greg k-h

Powered by blists - more mailing lists