[v8,0/7] rust: extend `module!` macro with integer parameter support

Message

Andreas Hindborg Feb. 27, 2025, 2:38 p.m. UTC

Extend the `module!` macro with support module parameters. Also add some string
to integer parsing functions and updates `BStr` with a method to strip a string
prefix.

Based on code by Adam Bratschi-Kaye lifted from the original `rust` branch [1].

Link: https://github.com/Rust-for-Linux/linux/tree/bc22545f38d74473cfef3e9fd65432733435b79f [1]
Signed-off-by: Andreas Hindborg <[e-mail]>
---
Changes in v8:
- Change print statement in sample to better communicate parameter name.
- Use imperative mode in commit messages.
- Remove prefix path from `EINVAL`.
- Change `try_from_param_arg` to accept `&BStr` rather than `&[u8]`.
- Parse integers without 128 bit integer types.
- Seal trait `FromStrRadix`.
- Strengthen safety requirement of `set_param`.
- Remove comment about Display and `PAGE_SIZE`.
- Add note describing why `ModuleParamAccess` is pub.
- Typo and grammar fixes for documentation.
- Update MAINTAINERS with rust module files.
- Link to v7: https://lore.kernel.org/r/[e-mail]

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/[e-mail]

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/[e-mail]

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/[e-mail]

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/[e-mail]

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/[e-mail]/

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/[e-mail]/

---
Andreas Hindborg (7):
      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
      modules: add rust modules files to MAINTAINERS

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

Best regards,

Comments

Daniel Gomez March 20, 2025, 8:41 a.m. UTC | #1

Hi, 
On Thu, Feb 27, 2025 at 03:38:06PM +0100, Andreas Hindborg wrote:
> Extend the `module!` macro with support module parameters. Also add some string
> to integer parsing functions and updates `BStr` with a method to strip a string
> prefix.
> 
> Based on code by Adam Bratschi-Kaye lifted from the original `rust` branch [1].
> 
> Link: https://github.com/Rust-for-Linux/linux/tree/bc22545f38d74473cfef3e9fd65432733435b79f [1]
> Signed-off-by: Andreas Hindborg <[e-mail]>

I've tested this series including the following patches from Andreas' tree [1]
as dependency for module testing parameters with the Rust Null Block driver:

[1] https://web.git.kernel.org/pub/scm/linux/kernel/git/a.hindborg/linux.git/log/?h=rnull-v6.14-rc1

51d304103b7f rust: refactor: rename `RawWriter` to `BufferWriter`
544811b24574 LIST: [PATCH v2 1/3] rust: sync: change `<Arc<T> as ForeignOwnable>::PointedTo` to `T`
3f097abd58de LIST: [PATCH v15 1/3] rust: types: add `ForeignOwnable::PointedTo`
0525eda0ff8d LIST: [PATCH v7 3/14] rust: sync: add `Arc::as_ptr`
ce7343b48e63 LIST: [PATCH v2 2/3] rust: configfs: introduce rust support for configfs
6efae1a9a226 rust: rnull: add module parameter support
b6545e0eaf94 rust: rnull: enable configuration via `configfs`
6a3bc0dc31d0 rust: rnull: move driver to separate directory

* modinfo
sudo modinfo rnull_mod
filename:       /lib/modules/6.14.0-rc6-00015-g51d304103b7f/kernel/drivers/block/rnull/rnull_mod.ko
author:         Andreas Hindborg
description:    Rust implementation of the C null block driver
license:        GPL v2
name:           rnull_mod
intree:         Y
depends:
vermagic:       6.14.0-rc6-00015-g51d304103b7f mod_unload modversions
parm:           nr_devices:Number of devices to register (u64)
parm:           bs:Block size (in bytes) (u32)
parm:           rotational:Set the rotational feature for the device (0 for false, 1 for true). Default: 0 (u8)
parm:           gb:Device capacity in MiB (u64)

* Testing nr_devices parameter:
sudo modprobe rnull_mod nr_devices=100

sudo ls /dev/rnullb* | wc -l
100

* Testing block size and capacity parameters:
sudo rmmod rnull_mod
sudo modprobe rnull_mod nr_devices=1 bs=512 gb=1024

sudo fdisk -l /dev/rnullb0
Disk /dev/rnullb0: 1 GiB, 1073741824 bytes, 2097152 sectors
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes

* Testing block size with fio and blkalgn [1] (tool for validating driver block
size):

[1] blkalgn is an eBPF-based tool for tracing block operations that also reports
block granularity and alignment histograms:
https://github.com/dkruces/bcc/tree/blkalgn

Install:
https://github.com/dkruces/bcc/releases/latest/download/blkalgn-$(uname -m) \
--output /usr/sbin/blkalgn \
&& sudo chmod +x /usr/sbin/blkalgn

sudo modprobe rnull_mod nr_devices=1 bs=1024 gb=1024
sudo curl --location \

sudo blkalgn --disk=rnullb0 --ops=Write
sudo fio --name=test --direct=1 --rw=write --bs=1024 --size=512k \
--filename=/dev/rnullb0 --loop=1000

I/O Granularity Histogram for Device rnullb0 (lbads: 10 - 1024 bytes)
Total I/Os: 10748
     Bytes         : count    distribution
        1024       : 10748    |****************************************|

I/O Alignment Histogram for Device rnullb0
     Bytes               : count    distribution
         0 -> 1          : 0        |                                        |
         2 -> 3          : 0        |                                        |
         4 -> 7          : 0        |                                        |
         8 -> 15         : 0        |                                        |
        16 -> 31         : 0        |                                        |
        32 -> 63         : 0        |                                        |
        64 -> 127        : 0        |                                        |
       128 -> 255        : 0        |                                        |
       256 -> 511        : 0        |                                        |
       512 -> 1023       : 0        |                                        |
      1024 -> 2047       : 10748    |****************************************|

Tested-by: Daniel Gomez <[e-mail]>


Andreas, Petr, Miguel,

Based on the discussion in v7, it seems that all these patches will go through
the Rust tree. Is that correct? What would be missing from the module's side?

I agree with Petr in that thread that if the changes are mostly limited to
rust-module files, they can go through the module's tree. However, that is not
the case yet.

Daniel

> ---
> Changes in v8:
> - Change print statement in sample to better communicate parameter name.
> - Use imperative mode in commit messages.
> - Remove prefix path from `EINVAL`.
> - Change `try_from_param_arg` to accept `&BStr` rather than `&[u8]`.
> - Parse integers without 128 bit integer types.
> - Seal trait `FromStrRadix`.
> - Strengthen safety requirement of `set_param`.
> - Remove comment about Display and `PAGE_SIZE`.
> - Add note describing why `ModuleParamAccess` is pub.
> - Typo and grammar fixes for documentation.
> - Update MAINTAINERS with rust module files.
> - Link to v7: https://lore.kernel.org/r/[e-mail]
> 
> 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/[e-mail]
> 
> 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/[e-mail]
> 
> 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/[e-mail]
> 
> 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/[e-mail]
> 
> 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/[e-mail]/
> 
> 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/[e-mail]/
> 
> ---
> Andreas Hindborg (7):
>       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
>       modules: add rust modules files to MAINTAINERS
> 
>  MAINTAINERS                  |   2 +
>  rust/kernel/lib.rs           |   1 +
>  rust/kernel/module_param.rs  | 221 +++++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/str.rs           | 200 +++++++++++++++++++++++++++++++++++++++
>  rust/macros/helpers.rs       |  25 +++++
>  rust/macros/lib.rs           |  31 ++++++
>  rust/macros/module.rs        | 191 +++++++++++++++++++++++++++++++++----
>  samples/rust/rust_minimal.rs |  10 ++
>  8 files changed, 663 insertions(+), 18 deletions(-)
> ---
> base-commit: 379487e17ca406b47392e7ab6cf35d1c3bacb371
> change-id: 20241211-module-params-v3-ae7e5c8d8b5a
> 
> Best regards,
> -- 
> Andreas Hindborg <[e-mail]>
> 
>

Andreas Hindborg March 20, 2025, 10:26 a.m. UTC | #2

"Daniel Gomez" <[e-mail]> writes:

> Hi,
> On Thu, Feb 27, 2025 at 03:38:06PM +0100, Andreas Hindborg wrote:
>> Extend the `module!` macro with support module parameters. Also add some string
>> to integer parsing functions and updates `BStr` with a method to strip a string
>> prefix.
>>
>> Based on code by Adam Bratschi-Kaye lifted from the original `rust` branch [1].
>>
>> Link: https://github.com/Rust-for-Linux/linux/tree/bc22545f38d74473cfef3e9fd65432733435b79f [1]
>> Signed-off-by: Andreas Hindborg <[e-mail]>
>
> I've tested this series including the following patches from Andreas' tree [1]
> as dependency for module testing parameters with the Rust Null Block driver:
>
> [1] https://web.git.kernel.org/pub/scm/linux/kernel/git/a.hindborg/linux.git/log/?h=rnull-v6.14-rc1
>
> 51d304103b7f rust: refactor: rename `RawWriter` to `BufferWriter`
> 544811b24574 LIST: [PATCH v2 1/3] rust: sync: change `<Arc<T> as ForeignOwnable>::PointedTo` to `T`
> 3f097abd58de LIST: [PATCH v15 1/3] rust: types: add `ForeignOwnable::PointedTo`
> 0525eda0ff8d LIST: [PATCH v7 3/14] rust: sync: add `Arc::as_ptr`
> ce7343b48e63 LIST: [PATCH v2 2/3] rust: configfs: introduce rust support for configfs
> 6efae1a9a226 rust: rnull: add module parameter support
> b6545e0eaf94 rust: rnull: enable configuration via `configfs`
> 6a3bc0dc31d0 rust: rnull: move driver to separate directory
>
> * modinfo
> sudo modinfo rnull_mod
> filename:       /lib/modules/6.14.0-rc6-00015-g51d304103b7f/kernel/drivers/block/rnull/rnull_mod.ko
> author:         Andreas Hindborg
> description:    Rust implementation of the C null block driver
> license:        GPL v2
> name:           rnull_mod
> intree:         Y
> depends:
> vermagic:       6.14.0-rc6-00015-g51d304103b7f mod_unload modversions
> parm:           nr_devices:Number of devices to register (u64)
> parm:           bs:Block size (in bytes) (u32)
> parm:           rotational:Set the rotational feature for the device (0 for false, 1 for true). Default: 0 (u8)
> parm:           gb:Device capacity in MiB (u64)
>
> * Testing nr_devices parameter:
> sudo modprobe rnull_mod nr_devices=100
>
> sudo ls /dev/rnullb* | wc -l
> 100
>
> * Testing block size and capacity parameters:
> sudo rmmod rnull_mod
> sudo modprobe rnull_mod nr_devices=1 bs=512 gb=1024
>
> sudo fdisk -l /dev/rnullb0
> Disk /dev/rnullb0: 1 GiB, 1073741824 bytes, 2097152 sectors
> Units: sectors of 1 * 512 = 512 bytes
> Sector size (logical/physical): 512 bytes / 512 bytes
> I/O size (minimum/optimal): 512 bytes / 512 bytes
>
> * Testing block size with fio and blkalgn [1] (tool for validating driver block
> size):
>
> [1] blkalgn is an eBPF-based tool for tracing block operations that also reports
> block granularity and alignment histograms:
> https://github.com/dkruces/bcc/tree/blkalgn
>
> Install:
> https://github.com/dkruces/bcc/releases/latest/download/blkalgn-$(uname -m) \
> --output /usr/sbin/blkalgn \
> && sudo chmod +x /usr/sbin/blkalgn
>
> sudo modprobe rnull_mod nr_devices=1 bs=1024 gb=1024
> sudo curl --location \
>
> sudo blkalgn --disk=rnullb0 --ops=Write
> sudo fio --name=test --direct=1 --rw=write --bs=1024 --size=512k \
> --filename=/dev/rnullb0 --loop=1000
>
> I/O Granularity Histogram for Device rnullb0 (lbads: 10 - 1024 bytes)
> Total I/Os: 10748
>      Bytes         : count    distribution
>         1024       : 10748    |****************************************|
>
> I/O Alignment Histogram for Device rnullb0
>      Bytes               : count    distribution
>          0 -> 1          : 0        |                                        |
>          2 -> 3          : 0        |                                        |
>          4 -> 7          : 0        |                                        |
>          8 -> 15         : 0        |                                        |
>         16 -> 31         : 0        |                                        |
>         32 -> 63         : 0        |                                        |
>         64 -> 127        : 0        |                                        |
>        128 -> 255        : 0        |                                        |
>        256 -> 511        : 0        |                                        |
>        512 -> 1023       : 0        |                                        |
>       1024 -> 2047       : 10748    |****************************************|
>
> Tested-by: Daniel Gomez <[e-mail]>
>
>
> Andreas, Petr, Miguel,
>
> Based on the discussion in v7, it seems that all these patches will go through
> the Rust tree. Is that correct? What would be missing from the module's side?
>
> I agree with Petr in that thread that if the changes are mostly limited to
> rust-module files, they can go through the module's tree. However, that is not
> the case yet.

As far as I understand, Miguel would take patch 1-5 for v6.15 and
modules would take patch 6-7 for v6.16. At least that is my
understanding from [1], @Petr and @Miguel please correct me if I am
wrong.


Best regards,
Andreas Hindborg

[1] https://lore.kernel.org/all/[e-mail]

Miguel Ojeda March 20, 2025, noon UTC | #3

On Thu, Mar 20, 2025 at 11:26 AM Andreas Hindborg <[e-mail]> wrote:
>
> As far as I understand, Miguel would take patch 1-5 for v6.15 and
> modules would take patch 6-7 for v6.16. At least that is my
> understanding from [1], @Petr and @Miguel please correct me if I am
> wrong.

So I offered that as an option -- I assume it is OK since nobody said
anything (please correct me if I am wrong), and it would help get
things moving.

So I will take 1-5 later today or tomorrow unless someone shouts.

Thanks!

Cheers,
Miguel

Petr Pavlu March 20, 2025, 12:47 p.m. UTC | #4

On 3/20/25 13:00, Miguel Ojeda wrote:
> On Thu, Mar 20, 2025 at 11:26 AM Andreas Hindborg <[e-mail]> wrote:
>>
>> As far as I understand, Miguel would take patch 1-5 for v6.15 and
>> modules would take patch 6-7 for v6.16. At least that is my
>> understanding from [1], @Petr and @Miguel please correct me if I am
>> wrong.
> 
> So I offered that as an option -- I assume it is OK since nobody said
> anything (please correct me if I am wrong), and it would help get
> things moving.
> 
> So I will take 1-5 later today or tomorrow unless someone shouts.

Yep, looks good to me.

Miguel Ojeda March 20, 2025, 11:23 p.m. UTC | #5

On Thu, Feb 27, 2025 at 3:39 PM Andreas Hindborg <[e-mail]> wrote:
>
> Extend the `module!` macro with support module parameters. Also add some string
> to integer parsing functions and updates `BStr` with a method to strip a string
> prefix.
>
> Based on code by Adam Bratschi-Kaye lifted from the original `rust` branch [1].
>
> Link: https://github.com/Rust-for-Linux/linux/tree/bc22545f38d74473cfef3e9fd65432733435b79f [1]
> Signed-off-by: Andreas Hindborg <[e-mail]>

Applied (1-4) to `rust-next` -- thanks everyone!

    [ Pluralized section name. Hid `use`. - Miguel ]

For the moment, I skipped 5 due to the UB I found.

Cheers,
Miguel