This series extends the `module!` macro with support module parameters. It
also adds some string to integer parsing functions and updates `BStr` with
a method to strip a string prefix.

This series stated out as code by Adam Bratschi-Kaye lifted from the original
`rust` branch [1].

After a bit of discussion on v3 about whether or not module parameters
is a good idea, it seems that module parameters in Rust has a place
in the kernel for now. This series is a dependency for `rnull`, the Rust
null block driver [2].

---
Module subsystem people: how do you want to handle merging of the patches and
subsequent maintenance of code?

I think we discussed you guys taking this under the current module maintainer
entry? If that is correct, will you add the new files to your entry yourself, or
should I include an update to MAINTAINERS in this series?

If prefer another solution, let me know and we can figure that out.

To: Miguel Ojeda <ojeda@kernel.org>
To: Alex Gaynor <alex.gaynor@gmail.com>
To: Boqun Feng <boqun.feng@gmail.com>
To: Gary Guo <gary@garyguo.net>
To: Björn Roy Baron <bjorn3_gh@protonmail.com>
To: Benno Lossin <benno.lossin@proton.me>
To: Alice Ryhl <aliceryhl@google.com>
To: Masahiro Yamada <masahiroy@kernel.org>
To: Nathan Chancellor <nathan@kernel.org>
To: Nicolas Schier <nicolas@fjasle.eu>
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: Trevor Gross <tmgross@umich.edu>
Cc: Adam Bratschi-Kaye <ark.email@gmail.com>
Cc: rust-for-linux@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-kbuild@vger.kernel.org
Cc: Petr Pavlu <petr.pavlu@suse.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Daniel Gomez <da.gomez@samsung.com>
Cc: Simona Vetter <simona.vetter@ffwll.ch>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: linux-modules@vger.kernel.org
Link: https://github.com/Rust-for-Linux/linux/tree/bc22545f38d74473cfef3e9fd65432733435b79f [1]
Link: https://git.kernel.org/pub/scm/linux/kernel/git/a.hindborg/linux.git/log/?h=rnull [2]
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

---
Changes in v7:
- Remove dependency on `pr_warn_once` patches, replace with TODO.
- Rework `ParseInt::from_str` to avoid allocating.
- Add a comment explaining how we parse "0".
- Change trait bound on `Index` impl for `BStr` to match std library approach.
- Link to v6: https://lore.kernel.org/r/20250211-module-params-v3-v6-0-24b297ddc43d@kernel.org

Changes in v6:
- Fix a bug that prevented parsing of negative default values for
  parameters in the `module!` macro.
- Fix a bug that prevented parsing zero in `strip_radix`. Also add a
  test case for this.
- Add `AsRef<BStr>` for `[u8]` and `BStr`.
- Use `impl AsRef<BStr>` as type of prefix in `BStr::strip_prefix`.
- Link to v5: https://lore.kernel.org/r/20250204-module-params-v3-v5-0-bf5ec2041625@kernel.org

Changes in v5:
- Fix a typo in a safety comment in `set_param`.
- Use a match statement in `parse_int::strip_radix`.
- Add an implementation of `Index` for `BStr`.
- Fix a logic inversion bug where parameters would not be parsed.
- Use `kernel::ffi::c_char` in `set_param` rather than the one in `core`.
- Use `kernel::c_str!` rather than `c"..."` literal in module macro.
- Rebase on v6.14-rc1.
- Link to v4: https://lore.kernel.org/r/20250109-module-params-v3-v4-0-c208bcfbe11f@kernel.org

Changes in v4:
- Add module maintainers to Cc list (sorry)
- Add a few missing [`doc_links`]
- Add panic section to `expect_string_field`
- Fix a typo in safety requirement of `module_params::free`
- Change `assert!` to `pr_warn_once!` in `module_params::set_param`
- Remove `module_params::get_param` and install null pointer instead
- Remove use of the unstable feature `sync_unsafe_cell`
- Link to v3: https://lore.kernel.org/r/20241213-module-params-v3-v3-0-485a015ac2cf@kernel.org

Changes in v3:
- use `SyncUnsafeCell` rather than `static mut` and simplify parameter access
- remove `Display` bound from `ModuleParam`
- automatically generate documentation for `PARAM_OPS_.*`
- remove `as *const _ as *mut_` phrasing
- inline parameter name in struct instantiation in  `emit_params`
- move `RacyKernelParam` out of macro template
- use C string literals rather than byte string literals with explicit null
- template out `__{name}_{param_name}` in `emit_param`
- indent template in `emit_params`
- use let-else expression in `emit_params` to get rid of an indentation level
- document `expect_string_field`
- move invication of `impl_int_module_param` to be closer to macro def
- move attributes after docs in `make_param_ops`
- rename `impl_module_param` to impl_int_module_param`
- use `ty` instead of `ident` in `impl_parse_int`
- use `BStr` instead of `&str` for string manipulation
- move string parsing functions to seperate patch and add examples, fix bugs
- degrade comment about future support from doc comment to regular comment
- remove std lib path from `Sized` marker
- update documentation for `trait ModuleParam`
- Link to v2: https://lore.kernel.org/all/20240819133345.3438739-1-nmi@metaspace.dk/

Changes in v2:
- Remove support for params without values (`NOARG_ALLOWED`).
- Improve documentation for `try_from_param_arg`.
- Use prelude import.
- Refactor `try_from_param_arg` to return `Result`.
- Refactor `ParseInt::from_str` to return `Result`.
- Move C callable functions out of `ModuleParam` trait.
- Rename literal string field parser to `expect_string_field`.
- Move parameter parsing from generation to parsing stage.
- Use absolute type paths in macro code.
- Inline `kparam`and `read_func` values.
- Resolve TODO regarding alignment attributes.
- Remove unnecessary unsafe blocks in macro code.
- Improve error message for unrecognized parameter types.
- Do not use `self` receiver when reading parameter value.
- Add parameter documentation to `module!` macro.
- Use empty `enum` for parameter type.
- Use `addr_of_mut` to get address of parameter value variable.
- Enabled building of docs for for `module_param` module.
- Link to v1: https://lore.kernel.org/rust-for-linux/20240705111455.142790-1-nmi@metaspace.dk/

---
Andreas Hindborg (6):
      rust: str: implement `PartialEq` for `BStr`
      rust: str: implement `Index` for `BStr`
      rust: str: implement `AsRef<BStr>` for `[u8]` and `BStr`
      rust: str: implement `strip_prefix` for `BStr`
      rust: str: add radix prefixed integer parsing functions
      rust: add parameter support to the `module!` macro

 rust/kernel/lib.rs           |   1 +
 rust/kernel/module_param.rs  | 226 +++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/str.rs           | 163 +++++++++++++++++++++++++++++++
 rust/macros/helpers.rs       |  25 +++++
 rust/macros/lib.rs           |  31 ++++++
 rust/macros/module.rs        | 191 ++++++++++++++++++++++++++++++++----
 samples/rust/rust_minimal.rs |  10 ++
 7 files changed, 629 insertions(+), 18 deletions(-)
---
base-commit: 379487e17ca406b47392e7ab6cf35d1c3bacb371
change-id: 20241211-module-params-v3-ae7e5c8d8b5a

Best regards,
-- 
Andreas Hindborg <a.hindborg@kernel.org>

Hi Petr,

"Andreas Hindborg" <a.hindborg@kernel.org> writes:

> This series extends the `module!` macro with support module parameters. It
> also adds some string to integer parsing functions and updates `BStr` with
> a method to strip a string prefix.
>
> This series stated out as code by Adam Bratschi-Kaye lifted from the original
> `rust` branch [1].
>
> After a bit of discussion on v3 about whether or not module parameters
> is a good idea, it seems that module parameters in Rust has a place
> in the kernel for now. This series is a dependency for `rnull`, the Rust
> null block driver [2].


Luis told me you are the one wearing the modules hat for the moment. How
do you want to handle merging of patch 6 and subsequent maintenance of
the code?

I think we discussed you guys taking this under the current module
maintainer entry? If that is correct, will you add the new files to your
entry yourself, or should I include an update to MAINTAINERS in the next
version of this series?

If prefer another solution, let me know and we can figure that out.


Best regards,
Andreas Hindborg

On 2/24/25 12:27, Andreas Hindborg wrote:
> Hi Petr,
> 
> "Andreas Hindborg" <a.hindborg@kernel.org> writes:
> 
>> This series extends the `module!` macro with support module parameters. It
>> also adds some string to integer parsing functions and updates `BStr` with
>> a method to strip a string prefix.
>>
>> This series stated out as code by Adam Bratschi-Kaye lifted from the original
>> `rust` branch [1].
>>
>> After a bit of discussion on v3 about whether or not module parameters
>> is a good idea, it seems that module parameters in Rust has a place
>> in the kernel for now. This series is a dependency for `rnull`, the Rust
>> null block driver [2].
> 
> 
> Luis told me you are the one wearing the modules hat for the moment. How
> do you want to handle merging of patch 6 and subsequent maintenance of
> the code?

I'd say the easiest is for the entire series to go through the Rust
tree. I'd also propose that any updates go primarily through that tree
as well.

> 
> I think we discussed you guys taking this under the current module
> maintainer entry? If that is correct, will you add the new files to your
> entry yourself, or should I include an update to MAINTAINERS in the next
> version of this series?

Makes sense, I think it is useful for all changes to this code to be
looked at by both Rust and modules people. To that effect, we could add
the following under the MODULES SUPPORT entry:
F:	rust/kernel/module_param.rs
F:	rust/macros/module.rs

My slight worry is that this might suggest the entirety of maintenance
is on the modules folks. Fortunately, adding the above and running
get_maintainer.pl on rust/kernel/module_param.rs gives me a list of
people for both Rust and modules, so this could be ok?

-- 
Thanks,
Petr

On Tue, Feb 25, 2025 at 11:22 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> I'd say the easiest is for the entire series to go through the Rust
> tree. I'd also propose that any updates go primarily through that tree
> as well.
>
> Makes sense, I think it is useful for all changes to this code to be
> looked at by both Rust and modules people. To that effect, we could add
> the following under the MODULES SUPPORT entry:
> F:      rust/kernel/module_param.rs
> F:      rust/macros/module.rs
>
> My slight worry is that this might suggest the entirety of maintenance
> is on the modules folks. Fortunately, adding the above and running
> get_maintainer.pl on rust/kernel/module_param.rs gives me a list of
> people for both Rust and modules, so this could be ok?

Up to you, of course -- a couple days ago (in the context of the
hrtimer thread) I wrote a summary of the usual discussion we have
around this (copy-pasting here for convenience):

    So far, what we have been doing is ask maintainers, first, if they
    would be willing take the patches themselves -- they are the experts
    of the subsystem, know what changes are incoming, etc. Some subsystems
    have done this (e.g. KUnit). That is ideal, because the goal is to
    scale and allows maintainers to be in full control.

    Of course, sometimes maintainers are not fully comfortable doing that,
    since they may not have the bandwidth, or the setup, or the Rust
    knowledge. In those cases, we typically ask if they would be willing
    to have a co-maintainer (i.e. in their entry, e.g. like locking did),
    or a sub-maintainer (i.e. in a new entry, e.g. like block did), that
    would take care of the bulk of the work from them.

    I think that is a nice middle-ground -- the advantage of doing it like
    that is that you get the benefits of knowing best what is going on
    without too much work (hopefully), and it may allow you to get more
    and more involved over time and confident on what is going on with the
    Rust callers, typical issues that appear, etc. Plus the sub-maintainer
    gets to learn more about the subsystem, its timelines, procedures,
    etc., which you may welcome (if you are looking for new people to get
    involved).

    I think that would be a nice middle-ground. As far as I understand,
    Andreas would be happy to commit to maintain the Rust side as a
    sub-maintainer (for instance). He would also need to make sure the
    tree builds properly with Rust enabled and so on. He already does
    something similar for Jens. Would that work for you?

    You could take the patches directly with his RoBs or Acked-bys, for
    instance. Or perhaps it makes more sense to take PRs from him (on the
    Rust code only, of course), to save you more work. Andreas does not
    send PRs to anyone yet, but I think it would be a good time for him to
    start learning how to apply patches himself etc.

    If not, then the last fallback would be putting it in the Rust tree as
    a sub-entry or similar.

    I hope that clarifies (and thanks whatever you decide!).

    https://lore.kernel.org/rust-for-linux/CANiq72mpYoig2Ro76K0E-sUtP31fW+0403zYWd6MumCgFKfTDQ@mail.gmail.com/

Would any of those other options work for you?

Thanks!

Cheers,
Miguel

On 2/25/25 12:54, Miguel Ojeda wrote:
> On Tue, Feb 25, 2025 at 11:22 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>>
>> I'd say the easiest is for the entire series to go through the Rust
>> tree. I'd also propose that any updates go primarily through that tree
>> as well.
>>
>> Makes sense, I think it is useful for all changes to this code to be
>> looked at by both Rust and modules people. To that effect, we could add
>> the following under the MODULES SUPPORT entry:
>> F:      rust/kernel/module_param.rs
>> F:      rust/macros/module.rs
>>
>> My slight worry is that this might suggest the entirety of maintenance
>> is on the modules folks. Fortunately, adding the above and running
>> get_maintainer.pl on rust/kernel/module_param.rs gives me a list of
>> people for both Rust and modules, so this could be ok?
> 
> Up to you, of course -- a couple days ago (in the context of the
> hrtimer thread) I wrote a summary of the usual discussion we have
> around this (copy-pasting here for convenience):
> 
>     So far, what we have been doing is ask maintainers, first, if they
>     would be willing take the patches themselves -- they are the experts
>     of the subsystem, know what changes are incoming, etc. Some subsystems
>     have done this (e.g. KUnit). That is ideal, because the goal is to
>     scale and allows maintainers to be in full control.
> 
>     Of course, sometimes maintainers are not fully comfortable doing that,
>     since they may not have the bandwidth, or the setup, or the Rust
>     knowledge. In those cases, we typically ask if they would be willing
>     to have a co-maintainer (i.e. in their entry, e.g. like locking did),
>     or a sub-maintainer (i.e. in a new entry, e.g. like block did), that
>     would take care of the bulk of the work from them.
> 
>     I think that is a nice middle-ground -- the advantage of doing it like
>     that is that you get the benefits of knowing best what is going on
>     without too much work (hopefully), and it may allow you to get more
>     and more involved over time and confident on what is going on with the
>     Rust callers, typical issues that appear, etc. Plus the sub-maintainer
>     gets to learn more about the subsystem, its timelines, procedures,
>     etc., which you may welcome (if you are looking for new people to get
>     involved).
> 
>     I think that would be a nice middle-ground. As far as I understand,
>     Andreas would be happy to commit to maintain the Rust side as a
>     sub-maintainer (for instance). He would also need to make sure the
>     tree builds properly with Rust enabled and so on. He already does
>     something similar for Jens. Would that work for you?
> 
>     You could take the patches directly with his RoBs or Acked-bys, for
>     instance. Or perhaps it makes more sense to take PRs from him (on the
>     Rust code only, of course), to save you more work. Andreas does not
>     send PRs to anyone yet, but I think it would be a good time for him to
>     start learning how to apply patches himself etc.
> 
>     If not, then the last fallback would be putting it in the Rust tree as
>     a sub-entry or similar.
> 
>     I hope that clarifies (and thanks whatever you decide!).
> 
>     https://lore.kernel.org/rust-for-linux/CANiq72mpYoig2Ro76K0E-sUtP31fW+0403zYWd6MumCgFKfTDQ@mail.gmail.com/
> 
> Would any of those other options work for you?

From what I can see in this series, the bindings required adding
a number of generic functions to the Rust support code and also most
discussion revolved around that. I'm worried this might be the case also
for foreseeable future updates, until more building blocks are in place.
It then makes me think most changes to this code will need to go through
the Rust tree for now.

On the other hand, if the changes are reasonably limited to mostly
rust/kernel/module_param.rs and rust/macros/module.rs, then yes, I'd say
it should be ok to take the patches through the modules tree.

@Luis, Sami, Daniel, please shout if you see any problem with this.

-- 
Thanks,
Petr

On Thu, Feb 27, 2025 at 3:55 PM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> From what I can see in this series, the bindings required adding
> a number of generic functions to the Rust support code and also most
> discussion revolved around that. I'm worried this might be the case also
> for foreseeable future updates, until more building blocks are in place.
> It then makes me think most changes to this code will need to go through
> the Rust tree for now.

Those would normally go through the Rust tree, yeah. Since we don't
have many users yet, and we avoid adding dead code in general, things
are fairly barebones.

If you prefer, we can take the non-module related dependencies through
the Rust tree this cycle, and then you can pick the modules parts in
the next one.

> On the other hand, if the changes are reasonably limited to mostly
> rust/kernel/module_param.rs and rust/macros/module.rs, then yes, I'd say
> it should be ok to take the patches through the modules tree.

Yeah, that should be the case. Worst case, we can do the delay-a-cycle
thing, or if urgent create a small branch, etc.

Cheers,
Miguel

Hi Andreas,

> On 18 Feb 2025, at 10:00, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> This series extends the `module!` macro with support module parameters. It
> also adds some string to integer parsing functions and updates `BStr` with
> a method to strip a string prefix.
> 
> This series stated out as code by Adam Bratschi-Kaye lifted from the original
> `rust` branch [1].
> 
> After a bit of discussion on v3 about whether or not module parameters
> is a good idea, it seems that module parameters in Rust has a place
> in the kernel for now. This series is a dependency for `rnull`, the Rust
> null block driver [2].
> 

```
$ sudo modprobe rust_minimal test_parameter=2
[  251.384125] rust_minimal: Rust minimal sample (init)
[  251.384600] rust_minimal: Am I built-in? false
[  251.385010] rust_minimal: My parameter: 2
```

Tested-by: Daniel Almeida <daniel.almeida@collabora.com>

IMHO, this is slightly confusing, since the parameter is named
“test_parameter”, but you’re printing “My parameter”.

This is of course very minor. Overall, congrats on getting this to work :)

— Daniel 

"Daniel Almeida" <daniel.almeida@collabora.com> writes:

> Hi Andreas,
>
>> On 18 Feb 2025, at 10:00, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> This series extends the `module!` macro with support module parameters. It
>> also adds some string to integer parsing functions and updates `BStr` with
>> a method to strip a string prefix.
>>
>> This series stated out as code by Adam Bratschi-Kaye lifted from the original
>> `rust` branch [1].
>>
>> After a bit of discussion on v3 about whether or not module parameters
>> is a good idea, it seems that module parameters in Rust has a place
>> in the kernel for now. This series is a dependency for `rnull`, the Rust
>> null block driver [2].
>>
>
> ```
> $ sudo modprobe rust_minimal test_parameter=2
> [  251.384125] rust_minimal: Rust minimal sample (init)
> [  251.384600] rust_minimal: Am I built-in? false
> [  251.385010] rust_minimal: My parameter: 2
> ```
>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
>
> IMHO, this is slightly confusing, since the parameter is named
> “test_parameter”, but you’re printing “My parameter”.

You are right, let's change that to "test_parameter: ".


Best regards,
Andreas Hindborg