[PATCH v6 00/18] rnull: add configfs, remote completion to rnull

[PATCH v6 00/18] rnull: add configfs, remote completion to rnull

[Andreas Hindborg]

This series adds configuration via configfs and remote completion to
the rnull driver. The series also includes a set of changes to the
rust block device driver API: a few cleanup patches, and a few features
supporting the rnull changes.

The series removes the raw buffer formatting logic from `kernel::block`
and improves the logic available in `kernel::string` to support the same
use as the removed logic.

This series is a smaller subset of the patches available in the
downstream rnull tree. I hope to minimize the delta between upstream
and downstream over the next few kernel releases.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
Changes in v6:
- Move re-export of `configfs_attrs` next to macro.
- Use implicit coersion to pointer in `kstrtobool`.
- Change the name of `bool_to_bytes` to `kstrtobool_bytes` and improve the implementation.
- Remove a useless pointer cast in "rust: block: add `GenDisk` private data support".
- Get `GFP_KERNEL` from the prelude in "rnull: enable configuration via `configfs`".
- Rebased on v6.17-rc2.
- Link to v5: https://lore.kernel.org/r/20250815-rnull-up-v6-16-v5-0-581453124c15@kernel.org

Changes in v5:
- Re-export `configfs_attrs` from the `configfs` module.
- Remove transient stray dead code `NullTerminatedFormatter::from_array`.
- Use `kstrtobool` when parsing boolean values through configfs.
- Remove a stray space in `pin_init!` macro invocation.
- Remove useless calls to `ptr::cast` when retrieving `queue_data`.
- Rearrange positioning of safety comments.
- Link to v4: https://lore.kernel.org/r/20250812-rnull-up-v6-16-v4-0-ed801dd3ba5c@kernel.org

Changes in v4:
- Rebase on v6.17-rc1
- Merge patches that expose `str::{Formatter, RawFormatter}` publicly.
- Remove `NullTerminatedFormatter::from_array`.
- Change signature of `Formatter::new`.
- Change invariant wording for `NullTerminatedFormatter`.
- Add missing period in comment for `SECTOR_MASK`.
- Rephrase docstring for PAGE_SECTORS_SHIFT.
- Use `write_str` rather than `write_fmt` where applicable.
- Improve boolean parsing in rnull configfs interface.
- Be explicit when intentionally dropping data (GenDisk::drop).
- Add minimum size invariant to `NullTerminatedFormatter`.
- Import `EINVAL` from the prelude.
- Link to v3: https://lore.kernel.org/r/20250711-rnull-up-v6-16-v3-0-3a262b4e2921@kernel.org

Changes in v3:
- Rename `NullBorrowFormatter` as `NullTerminatedFormatter`.
- Remove `pos` from `NullBorrowFormatter`.
- Call into `Self::new` in `NullBorrowFormatter::from_array`.
- Use `Option` return type in `NullBorrowFormatter::from_array`
- Use `Option` return type in `NullBorrowFormatter::new`.
- Remove `BorrowFormatter` and update `Formatter` with a generic lifetime.
- Split visibility change of `str::Formatter` into separate patch.
- Link to v2: https://lore.kernel.org/r/20250708-rnull-up-v6-16-v2-0-ab93c0ff429b@kernel.org

Changes in v2:
- Rework formatter logic. Add two new formatters that write to slices,
  one of which adds a trailing null byte.
- Reorder and split patches so that changes are more clear.
- Fix a typo in soft-irq patch summary.
- Link to v1: https://lore.kernel.org/r/20250616-rnull-up-v6-16-v1-0-a4168b8e76b2@kernel.org

---
Andreas Hindborg (18):
      rust: str: normalize imports in `str.rs`
      rust: str: allow `str::Formatter` to format into `&mut [u8]`.
      rust: str: expose `str::{Formatter, RawFormatter}` publicly.
      rust: str: introduce `NullTerminatedFormatter`
      rust: str: introduce `kstrtobool` function
      rust: str: add `bytes_to_bool` helper function
      rust: configfs: re-export `configfs_attrs` from `configfs` module
      rust: block: normalize imports for `gen_disk.rs`
      rust: block: use `NullTerminatedFormatter`
      rust: block: remove `RawWriter`
      rust: block: remove trait bound from `mq::Request` definition
      rust: block: add block related constants
      rnull: move driver to separate directory
      rnull: enable configuration via `configfs`
      rust: block: add `GenDisk` private data support
      rust: block: mq: fix spelling in a safety comment
      rust: block: add remote completion to `Request`
      rnull: add soft-irq completion support

 MAINTAINERS                        |   2 +-
 drivers/block/Kconfig              |  10 +-
 drivers/block/Makefile             |   4 +-
 drivers/block/rnull.rs             |  80 -----------
 drivers/block/rnull/Kconfig        |  13 ++
 drivers/block/rnull/Makefile       |   3 +
 drivers/block/rnull/configfs.rs    | 262 +++++++++++++++++++++++++++++++++++++
 drivers/block/rnull/rnull.rs       | 104 +++++++++++++++
 rust/kernel/block.rs               |  13 ++
 rust/kernel/block/mq.rs            |  14 +-
 rust/kernel/block/mq/gen_disk.rs   |  54 ++++++--
 rust/kernel/block/mq/operations.rs |  65 +++++++--
 rust/kernel/block/mq/raw_writer.rs |  55 --------
 rust/kernel/block/mq/request.rs    |  21 ++-
 rust/kernel/configfs.rs            |   2 +
 rust/kernel/str.rs                 | 163 +++++++++++++++++++++--
 samples/rust/rust_configfs.rs      |   2 +-
 17 files changed, 675 insertions(+), 192 deletions(-)
---
base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
change-id: 20250616-rnull-up-v6-16-b4571e28feee

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

[PATCH v6 01/18] rust: str: normalize imports in `str.rs`

[Andreas Hindborg]

Clean up imports in `str.rs`. This makes future code manipulation more
manageable.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/str.rs | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 6c892550c0ba..082790b7a621 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -2,12 +2,13 @@
 
 //! String representations.
 
-use crate::alloc::{flags::*, AllocError, KVec};
-use crate::fmt::{self, Write};
+use crate::{
+    alloc::{flags::*, AllocError, KVec},
+    fmt::{self, Write},
+    prelude::*,
+};
 use core::ops::{self, Deref, DerefMut, Index};
 
-use crate::prelude::*;
-
 /// Byte string without UTF-8 validity guarantee.
 #[repr(transparent)]
 pub struct BStr([u8]);

-- 
2.47.2

[PATCH v6 02/18] rust: str: allow `str::Formatter` to format into `&mut [u8]`.

[Andreas Hindborg]

Improve `Formatter` so that it can write to an array or slice buffer.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/str.rs | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 082790b7a621..76632da357a6 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -7,7 +7,10 @@
     fmt::{self, Write},
     prelude::*,
 };
-use core::ops::{self, Deref, DerefMut, Index};
+use core::{
+    marker::PhantomData,
+    ops::{self, Deref, DerefMut, Index},
+};
 
 /// Byte string without UTF-8 validity guarantee.
 #[repr(transparent)]
@@ -825,9 +828,9 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
 /// Allows formatting of [`fmt::Arguments`] into a raw buffer.
 ///
 /// Fails if callers attempt to write more than will fit in the buffer.
-pub(crate) struct Formatter(RawFormatter);
+pub(crate) struct Formatter<'a>(RawFormatter, PhantomData<&'a mut ()>);
 
-impl Formatter {
+impl Formatter<'_> {
     /// Creates a new instance of [`Formatter`] with the given buffer.
     ///
     /// # Safety
@@ -836,11 +839,19 @@ impl Formatter {
     /// for the lifetime of the returned [`Formatter`].
     pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
         // SAFETY: The safety requirements of this function satisfy those of the callee.
-        Self(unsafe { RawFormatter::from_buffer(buf, len) })
+        Self(unsafe { RawFormatter::from_buffer(buf, len) }, PhantomData)
+    }
+
+    /// Create a new [`Self`] instance.
+    #[expect(dead_code)]
+    pub(crate) fn new(buffer: &mut [u8]) -> Self {
+        // SAFETY: `buffer` is valid for writes for the entire length for
+        // the lifetime of `Self`.
+        unsafe { Formatter::from_buffer(buffer.as_mut_ptr(), buffer.len()) }
     }
 }
 
-impl Deref for Formatter {
+impl Deref for Formatter<'_> {
     type Target = RawFormatter;
 
     fn deref(&self) -> &Self::Target {
@@ -848,7 +859,7 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
-impl fmt::Write for Formatter {
+impl fmt::Write for Formatter<'_> {
     fn write_str(&mut self, s: &str) -> fmt::Result {
         self.0.write_str(s)?;
 

-- 
2.47.2

[PATCH v6 03/18] rust: str: expose `str::{Formatter, RawFormatter}` publicly.

[Andreas Hindborg]

rnull is going to make use of `str::Formatter` and `str::RawFormatter`, so
expose them with public visibility.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/str.rs | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 76632da357a6..46cdc85dad63 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -736,7 +736,7 @@ fn test_bstr_debug() -> Result {
 ///
 /// The memory region between `pos` (inclusive) and `end` (exclusive) is valid for writes if `pos`
 /// is less than `end`.
-pub(crate) struct RawFormatter {
+pub struct RawFormatter {
     // Use `usize` to use `saturating_*` functions.
     beg: usize,
     pos: usize,
@@ -794,7 +794,7 @@ pub(crate) fn pos(&self) -> *mut u8 {
     }
 
     /// Returns the number of bytes written to the formatter.
-    pub(crate) fn bytes_written(&self) -> usize {
+    pub fn bytes_written(&self) -> usize {
         self.pos - self.beg
     }
 }
@@ -828,7 +828,7 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
 /// Allows formatting of [`fmt::Arguments`] into a raw buffer.
 ///
 /// Fails if callers attempt to write more than will fit in the buffer.
-pub(crate) struct Formatter<'a>(RawFormatter, PhantomData<&'a mut ()>);
+pub struct Formatter<'a>(RawFormatter, PhantomData<&'a mut ()>);
 
 impl Formatter<'_> {
     /// Creates a new instance of [`Formatter`] with the given buffer.
@@ -843,8 +843,7 @@ pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
     }
 
     /// Create a new [`Self`] instance.
-    #[expect(dead_code)]
-    pub(crate) fn new(buffer: &mut [u8]) -> Self {
+    pub fn new(buffer: &mut [u8]) -> Self {
         // SAFETY: `buffer` is valid for writes for the entire length for
         // the lifetime of `Self`.
         unsafe { Formatter::from_buffer(buffer.as_mut_ptr(), buffer.len()) }

-- 
2.47.2

[PATCH v6 04/18] rust: str: introduce `NullTerminatedFormatter`

[Andreas Hindborg]

Add `NullTerminatedFormatter`, a formatter that writes a null terminated
string to an array or slice buffer. Because this type needs to manage the
trailing null marker, the existing formatters cannot be used to implement
this type.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/str.rs | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 46cdc85dad63..d8326f7bc9c1 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -871,6 +871,55 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
     }
 }
 
+/// A mutable reference to a byte buffer where a string can be written into.
+///
+/// The buffer will be automatically null terminated after the last written character.
+///
+/// # Invariants
+///
+/// * The first byte of `buffer` is always zero.
+/// * The length of `buffer` is at least 1.
+pub(crate) struct NullTerminatedFormatter<'a> {
+    buffer: &'a mut [u8],
+}
+
+impl<'a> NullTerminatedFormatter<'a> {
+    /// Create a new [`Self`] instance.
+    #[expect(dead_code)]
+    pub(crate) fn new(buffer: &'a mut [u8]) -> Option<NullTerminatedFormatter<'a>> {
+        *(buffer.first_mut()?) = 0;
+
+        // INVARIANT:
+        //  - We wrote zero to the first byte above.
+        //  - If buffer was not at least length 1, `buffer.first_mut()` would return None.
+        Some(Self { buffer })
+    }
+}
+
+impl Write for NullTerminatedFormatter<'_> {
+    fn write_str(&mut self, s: &str) -> fmt::Result {
+        let bytes = s.as_bytes();
+        let len = bytes.len();
+
+        // We want space for a zero. By type invariant, buffer length is always at least 1, so no
+        // underflow.
+        if len > self.buffer.len() - 1 {
+            return Err(fmt::Error);
+        }
+
+        let buffer = core::mem::take(&mut self.buffer);
+        // We break the zero start invariant for a short while.
+        buffer[..len].copy_from_slice(bytes);
+        // INVARIANT: We checked above that buffer will have size at least 1 after this assignment.
+        self.buffer = &mut buffer[len..];
+
+        // INVARIANT: We write zero to the first byte of the buffer.
+        self.buffer[0] = 0;
+
+        Ok(())
+    }
+}
+
 /// An owned string that is guaranteed to have exactly one `NUL` byte, which is at the end.
 ///
 /// Used for interoperability with kernel APIs that take C strings.

-- 
2.47.2

[PATCH v6 05/18] rust: str: introduce `kstrtobool` function

[Andreas Hindborg]

Add a Rust wrapper for the kernel's `kstrtobool` function that converts
common user inputs into boolean values.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/str.rs | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index d8326f7bc9c1..d070c0bd86c3 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -4,6 +4,7 @@
 
 use crate::{
     alloc::{flags::*, AllocError, KVec},
+    error::Result,
     fmt::{self, Write},
     prelude::*,
 };
@@ -920,6 +921,62 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
     }
 }
 
+/// Convert common user inputs into boolean values using the kernel's `kstrtobool` function.
+///
+/// This routine returns `Ok(bool)` if the first character is one of 'YyTt1NnFf0', or
+/// \[oO\]\[NnFf\] for "on" and "off". Otherwise it will return `Err(EINVAL)`.
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::{c_str, str::kstrtobool};
+///
+/// // Lowercase
+/// assert_eq!(kstrtobool(c_str!("true")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("tr")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("t")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("twrong")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("false")), Ok(false));
+/// assert_eq!(kstrtobool(c_str!("f")), Ok(false));
+/// assert_eq!(kstrtobool(c_str!("yes")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("no")), Ok(false));
+/// assert_eq!(kstrtobool(c_str!("on")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("off")), Ok(false));
+///
+/// // Camel case
+/// assert_eq!(kstrtobool(c_str!("True")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("False")), Ok(false));
+/// assert_eq!(kstrtobool(c_str!("Yes")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("No")), Ok(false));
+/// assert_eq!(kstrtobool(c_str!("On")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("Off")), Ok(false));
+///
+/// // All caps
+/// assert_eq!(kstrtobool(c_str!("TRUE")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("FALSE")), Ok(false));
+/// assert_eq!(kstrtobool(c_str!("YES")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("NO")), Ok(false));
+/// assert_eq!(kstrtobool(c_str!("ON")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("OFF")), Ok(false));
+///
+/// // Numeric
+/// assert_eq!(kstrtobool(c_str!("1")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("0")), Ok(false));
+///
+/// // Invalid input
+/// assert_eq!(kstrtobool(c_str!("invalid")), Err(EINVAL));
+/// assert_eq!(kstrtobool(c_str!("2")), Err(EINVAL));
+/// ```
+pub fn kstrtobool(string: &CStr) -> Result<bool> {
+    let mut result: bool = false;
+
+    // SAFETY: `string` is a valid null-terminated C string, and `result` is a valid
+    // pointer to a bool that we own.
+    let ret = unsafe { bindings::kstrtobool(string.as_char_ptr(), &mut result) };
+
+    kernel::error::to_result(ret).map(|()| result)
+}
+
 /// An owned string that is guaranteed to have exactly one `NUL` byte, which is at the end.
 ///
 /// Used for interoperability with kernel APIs that take C strings.

-- 
2.47.2

[PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function

[Andreas Hindborg]

Add a convenience function to convert byte slices to boolean values by
wrapping them in a null-terminated C string and delegating to the
existing `kstrtobool` function. Only considers the first two bytes of
the input slice, following the kernel's boolean parsing semantics.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/str.rs | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index d070c0bd86c3..b185262b4851 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -921,6 +921,20 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
     }
 }
 
+/// # Safety
+///
+/// - `string` must point to a null terminated string that is valid for read.
+unsafe fn kstrtobool_raw(string: *const u8) -> Result<bool> {
+    let mut result: bool = false;
+
+    // SAFETY:
+    // - By function safety requirement, `string` is a valid null-terminated string.
+    // - `result` is a valid `bool` that we own.
+    let ret = unsafe { bindings::kstrtobool(string, &mut result) };
+
+    kernel::error::to_result(ret).map(|()| result)
+}
+
 /// Convert common user inputs into boolean values using the kernel's `kstrtobool` function.
 ///
 /// This routine returns `Ok(bool)` if the first character is one of 'YyTt1NnFf0', or
@@ -968,13 +982,22 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
 /// assert_eq!(kstrtobool(c_str!("2")), Err(EINVAL));
 /// ```
 pub fn kstrtobool(string: &CStr) -> Result<bool> {
-    let mut result: bool = false;
-
-    // SAFETY: `string` is a valid null-terminated C string, and `result` is a valid
-    // pointer to a bool that we own.
-    let ret = unsafe { bindings::kstrtobool(string.as_char_ptr(), &mut result) };
+    // SAFETY:
+    // - The pointer returned by `CStr::as_char_ptr` is guaranteed to be
+    //   null terminated.
+    // - `string` is live and thus the string is valid for read.
+    unsafe { kstrtobool_raw(string.as_char_ptr()) }
+}
 
-    kernel::error::to_result(ret).map(|()| result)
+/// Convert `&[u8]` to `bool` by deferring to [`kernel::str::kstrtobool`].
+///
+/// Only considers at most the first two bytes of `bytes`.
+pub fn kstrtobool_bytes(bytes: &[u8]) -> Result<bool> {
+    // `ktostrbool` only considers the first two bytes of the input.
+    let stack_string = [*bytes.first().unwrap_or(&0), *bytes.get(1).unwrap_or(&0), 0];
+    // SAFETY: `stack_string` is null terminated and it is live on the stack so
+    // it is valid for read.
+    unsafe { kstrtobool_raw(stack_string.as_ptr()) }
 }
 
 /// An owned string that is guaranteed to have exactly one `NUL` byte, which is at the end.

-- 
2.47.2

[PATCH v6 07/18] rust: configfs: re-export `configfs_attrs` from `configfs` module

[Andreas Hindborg]

Re-export `configfs_attrs` from `configfs` module, so that users can import
the macro from the `configfs` module rather than the root of the `kernel`
crate.

Also update users to import from the new path.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/configfs.rs       | 2 ++
 samples/rust/rust_configfs.rs | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index 2736b798cdc6..318a2f073d1c 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -1039,3 +1039,5 @@ macro_rules! configfs_attrs {
     };
 
 }
+
+pub use crate::configfs_attrs;
diff --git a/samples/rust/rust_configfs.rs b/samples/rust/rust_configfs.rs
index af04bfa35cb2..ad364fb93e53 100644
--- a/samples/rust/rust_configfs.rs
+++ b/samples/rust/rust_configfs.rs
@@ -5,7 +5,7 @@
 use kernel::alloc::flags;
 use kernel::c_str;
 use kernel::configfs;
-use kernel::configfs_attrs;
+use kernel::configfs::configfs_attrs;
 use kernel::new_mutex;
 use kernel::page::PAGE_SIZE;
 use kernel::prelude::*;

-- 
2.47.2

[PATCH v6 08/18] rust: block: normalize imports for `gen_disk.rs`

[Andreas Hindborg]

Clean up the import statements in `gen_disk.rs` to make the code easier to
maintain.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/block/mq/gen_disk.rs | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index cd54cd64ea88..679ee1bb2195 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -5,9 +5,13 @@
 //! C header: [`include/linux/blkdev.h`](srctree/include/linux/blkdev.h)
 //! C header: [`include/linux/blk_mq.h`](srctree/include/linux/blk_mq.h)
 
-use crate::block::mq::{raw_writer::RawWriter, Operations, TagSet};
-use crate::{bindings, error::from_err_ptr, error::Result, sync::Arc};
-use crate::{error, static_lock_class};
+use crate::{
+    bindings,
+    block::mq::{raw_writer::RawWriter, Operations, TagSet},
+    error::{self, from_err_ptr, Result},
+    static_lock_class,
+    sync::Arc,
+};
 use core::fmt::{self, Write};
 
 /// A builder for [`GenDisk`].

-- 
2.47.2

[PATCH v6 09/18] rust: block: use `NullTerminatedFormatter`

[Andreas Hindborg]

Use the new `NullTerminatedFormatter` to write the name of a `GenDisk` to
the name buffer. This new formatter automatically adds a trailing null
marker after the written characters, so we don't need to append that at the
call site any longer.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/block/mq/gen_disk.rs   | 12 +++++++-----
 rust/kernel/block/mq/raw_writer.rs |  1 +
 rust/kernel/str.rs                 |  1 -
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index 679ee1bb2195..20f1d46c774d 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -7,9 +7,11 @@
 
 use crate::{
     bindings,
-    block::mq::{raw_writer::RawWriter, Operations, TagSet},
+    block::mq::{Operations, TagSet},
     error::{self, from_err_ptr, Result},
+    prelude::*,
     static_lock_class,
+    str::NullTerminatedFormatter,
     sync::Arc,
 };
 use core::fmt::{self, Write};
@@ -143,14 +145,14 @@ pub fn build<T: Operations>(
         // SAFETY: `gendisk` is a valid pointer as we initialized it above
         unsafe { (*gendisk).fops = &TABLE };
 
-        let mut raw_writer = RawWriter::from_array(
+        let mut writer = NullTerminatedFormatter::new(
             // SAFETY: `gendisk` points to a valid and initialized instance. We
             // have exclusive access, since the disk is not added to the VFS
             // yet.
             unsafe { &mut (*gendisk).disk_name },
-        )?;
-        raw_writer.write_fmt(name)?;
-        raw_writer.write_char('\0')?;
+        )
+        .ok_or(EINVAL)?;
+        writer.write_fmt(name)?;
 
         // SAFETY: `gendisk` points to a valid and initialized instance of
         // `struct gendisk`. `set_capacity` takes a lock to synchronize this
diff --git a/rust/kernel/block/mq/raw_writer.rs b/rust/kernel/block/mq/raw_writer.rs
index 7e2159e4f6a6..0aef55703e71 100644
--- a/rust/kernel/block/mq/raw_writer.rs
+++ b/rust/kernel/block/mq/raw_writer.rs
@@ -24,6 +24,7 @@ fn new(buffer: &'a mut [u8]) -> Result<RawWriter<'a>> {
         Ok(Self { buffer, pos: 0 })
     }
 
+    #[expect(dead_code)]
     pub(crate) fn from_array<const N: usize>(
         a: &'a mut [crate::ffi::c_char; N],
     ) -> Result<RawWriter<'a>> {
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index b185262b4851..a3e34f566034 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -886,7 +886,6 @@ pub(crate) struct NullTerminatedFormatter<'a> {
 
 impl<'a> NullTerminatedFormatter<'a> {
     /// Create a new [`Self`] instance.
-    #[expect(dead_code)]
     pub(crate) fn new(buffer: &'a mut [u8]) -> Option<NullTerminatedFormatter<'a>> {
         *(buffer.first_mut()?) = 0;
 

-- 
2.47.2

[PATCH v6 10/18] rust: block: remove `RawWriter`

[Andreas Hindborg]

`RawWriter` is now dead code, so remove it.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/block/mq.rs            |  1 -
 rust/kernel/block/mq/raw_writer.rs | 56 --------------------------------------
 2 files changed, 57 deletions(-)

diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
index 831445d37181..98fa0d6bc8f7 100644
--- a/rust/kernel/block/mq.rs
+++ b/rust/kernel/block/mq.rs
@@ -89,7 +89,6 @@
 
 pub mod gen_disk;
 mod operations;
-mod raw_writer;
 mod request;
 mod tag_set;
 
diff --git a/rust/kernel/block/mq/raw_writer.rs b/rust/kernel/block/mq/raw_writer.rs
deleted file mode 100644
index 0aef55703e71..000000000000
--- a/rust/kernel/block/mq/raw_writer.rs
+++ /dev/null
@@ -1,56 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-use core::fmt::{self, Write};
-
-use crate::error::Result;
-use crate::prelude::EINVAL;
-
-/// A mutable reference to a byte buffer where a string can be written into.
-///
-/// # Invariants
-///
-/// `buffer` is always null terminated.
-pub(crate) struct RawWriter<'a> {
-    buffer: &'a mut [u8],
-    pos: usize,
-}
-
-impl<'a> RawWriter<'a> {
-    /// Create a new `RawWriter` instance.
-    fn new(buffer: &'a mut [u8]) -> Result<RawWriter<'a>> {
-        *(buffer.last_mut().ok_or(EINVAL)?) = 0;
-
-        // INVARIANT: We null terminated the buffer above.
-        Ok(Self { buffer, pos: 0 })
-    }
-
-    #[expect(dead_code)]
-    pub(crate) fn from_array<const N: usize>(
-        a: &'a mut [crate::ffi::c_char; N],
-    ) -> Result<RawWriter<'a>> {
-        Self::new(
-            // SAFETY: the buffer of `a` is valid for read and write as `u8` for
-            // at least `N` bytes.
-            unsafe { core::slice::from_raw_parts_mut(a.as_mut_ptr().cast::<u8>(), N) },
-        )
-    }
-}
-
-impl Write for RawWriter<'_> {
-    fn write_str(&mut self, s: &str) -> fmt::Result {
-        let bytes = s.as_bytes();
-        let len = bytes.len();
-
-        // We do not want to overwrite our null terminator
-        if self.pos + len > self.buffer.len() - 1 {
-            return Err(fmt::Error);
-        }
-
-        // INVARIANT: We are not overwriting the last byte
-        self.buffer[self.pos..self.pos + len].copy_from_slice(bytes);
-
-        self.pos += len;
-
-        Ok(())
-    }
-}

-- 
2.47.2

[PATCH v6 11/18] rust: block: remove trait bound from `mq::Request` definition

[Andreas Hindborg]

Remove the trait bound `T:Operations` from `mq::Request`. The bound is not
required, so remove it to reduce complexity.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/block/mq/request.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index fefd394f064a..f723d74091c1 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -53,7 +53,7 @@
 /// [`struct request`]: srctree/include/linux/blk-mq.h
 ///
 #[repr(transparent)]
-pub struct Request<T: Operations>(Opaque<bindings::request>, PhantomData<T>);
+pub struct Request<T>(Opaque<bindings::request>, PhantomData<T>);
 
 impl<T: Operations> Request<T> {
     /// Create an [`ARef<Request>`] from a [`struct request`] pointer.

-- 
2.47.2

[PATCH v6 12/18] rust: block: add block related constants

[Andreas Hindborg]

Add a few block subsystem constants to the rust `kernel::block` name space.
This makes it easier to access the constants from rust code.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/block.rs | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/rust/kernel/block.rs b/rust/kernel/block.rs
index 150f710efe5b..32c8d865afb6 100644
--- a/rust/kernel/block.rs
+++ b/rust/kernel/block.rs
@@ -3,3 +3,16 @@
 //! Types for working with the block layer.
 
 pub mod mq;
+
+/// Bit mask for masking out [`SECTOR_SIZE`].
+pub const SECTOR_MASK: u32 = bindings::SECTOR_MASK;
+
+/// Sectors are size `1 << SECTOR_SHIFT`.
+pub const SECTOR_SHIFT: u32 = bindings::SECTOR_SHIFT;
+
+/// Size of a sector.
+pub const SECTOR_SIZE: u32 = bindings::SECTOR_SIZE;
+
+/// The difference between the size of a page and the size of a sector,
+/// expressed as a power of two.
+pub const PAGE_SECTORS_SHIFT: u32 = bindings::PAGE_SECTORS_SHIFT;

-- 
2.47.2

[PATCH v6 13/18] rnull: move driver to separate directory

[Andreas Hindborg]

The rust null block driver is about to gain some additional modules. Rather
than pollute the current directory, move the driver to a subdirectory.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 MAINTAINERS                        |  2 +-
 drivers/block/Kconfig              | 10 +---------
 drivers/block/Makefile             |  4 +---
 drivers/block/rnull/Kconfig        | 13 +++++++++++++
 drivers/block/rnull/Makefile       |  3 +++
 drivers/block/{ => rnull}/rnull.rs |  0
 6 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index daf520a13bdf..d5addcc4b132 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4343,7 +4343,7 @@ W:	https://rust-for-linux.com
 B:	https://github.com/Rust-for-Linux/linux/issues
 C:	https://rust-for-linux.zulipchat.com/#narrow/stream/Block
 T:	git https://github.com/Rust-for-Linux/linux.git rust-block-next
-F:	drivers/block/rnull.rs
+F:	drivers/block/rnull/
 F:	rust/kernel/block.rs
 F:	rust/kernel/block/
 
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index df38fb364904..77d694448990 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -17,6 +17,7 @@ menuconfig BLK_DEV
 if BLK_DEV
 
 source "drivers/block/null_blk/Kconfig"
+source "drivers/block/rnull/Kconfig"
 
 config BLK_DEV_FD
 	tristate "Normal floppy disk support"
@@ -311,15 +312,6 @@ config VIRTIO_BLK
 	  This is the virtual block driver for virtio.  It can be used with
           QEMU based VMMs (like KVM or Xen).  Say Y or M.
 
-config BLK_DEV_RUST_NULL
-	tristate "Rust null block driver (Experimental)"
-	depends on RUST
-	help
-	  This is the Rust implementation of the null block driver. For now it
-	  is only a minimal stub.
-
-	  If unsure, say N.
-
 config BLK_DEV_RBD
 	tristate "Rados block device (RBD)"
 	depends on INET && BLOCK
diff --git a/drivers/block/Makefile b/drivers/block/Makefile
index a695ce74ef22..2d8096eb8cdf 100644
--- a/drivers/block/Makefile
+++ b/drivers/block/Makefile
@@ -9,9 +9,6 @@
 # needed for trace events
 ccflags-y				+= -I$(src)
 
-obj-$(CONFIG_BLK_DEV_RUST_NULL) += rnull_mod.o
-rnull_mod-y := rnull.o
-
 obj-$(CONFIG_MAC_FLOPPY)	+= swim3.o
 obj-$(CONFIG_BLK_DEV_SWIM)	+= swim_mod.o
 obj-$(CONFIG_BLK_DEV_FD)	+= floppy.o
@@ -38,6 +35,7 @@ obj-$(CONFIG_ZRAM) += zram/
 obj-$(CONFIG_BLK_DEV_RNBD)	+= rnbd/
 
 obj-$(CONFIG_BLK_DEV_NULL_BLK)	+= null_blk/
+obj-$(CONFIG_BLK_DEV_RUST_NULL) += rnull/
 
 obj-$(CONFIG_BLK_DEV_UBLK)			+= ublk_drv.o
 obj-$(CONFIG_BLK_DEV_ZONED_LOOP) += zloop.o
diff --git a/drivers/block/rnull/Kconfig b/drivers/block/rnull/Kconfig
new file mode 100644
index 000000000000..6dc5aff96bf4
--- /dev/null
+++ b/drivers/block/rnull/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Rust null block device driver configuration
+
+config BLK_DEV_RUST_NULL
+	tristate "Rust null block driver (Experimental)"
+	depends on RUST
+	help
+	  This is the Rust implementation of the null block driver. Like
+	  the C version, the driver allows the user to create virutal block
+	  devices that can be configured via various configuration options.
+
+	  If unsure, say N.
diff --git a/drivers/block/rnull/Makefile b/drivers/block/rnull/Makefile
new file mode 100644
index 000000000000..11cfa5e615dc
--- /dev/null
+++ b/drivers/block/rnull/Makefile
@@ -0,0 +1,3 @@
+
+obj-$(CONFIG_BLK_DEV_RUST_NULL) += rnull_mod.o
+rnull_mod-y := rnull.o
diff --git a/drivers/block/rnull.rs b/drivers/block/rnull/rnull.rs
similarity index 100%
rename from drivers/block/rnull.rs
rename to drivers/block/rnull/rnull.rs

-- 
2.47.2

[PATCH v6 14/18] rnull: enable configuration via `configfs`

[Andreas Hindborg]

Allow rust null block devices to be configured and instantiated via
`configfs`.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 drivers/block/rnull/Kconfig      |   2 +-
 drivers/block/rnull/configfs.rs  | 207 +++++++++++++++++++++++++++++++++++++++
 drivers/block/rnull/rnull.rs     |  59 +++++------
 rust/kernel/block/mq/gen_disk.rs |   2 +-
 4 files changed, 240 insertions(+), 30 deletions(-)

diff --git a/drivers/block/rnull/Kconfig b/drivers/block/rnull/Kconfig
index 6dc5aff96bf4..7bc5b376c128 100644
--- a/drivers/block/rnull/Kconfig
+++ b/drivers/block/rnull/Kconfig
@@ -4,7 +4,7 @@
 
 config BLK_DEV_RUST_NULL
 	tristate "Rust null block driver (Experimental)"
-	depends on RUST
+	depends on RUST && CONFIGFS_FS
 	help
 	  This is the Rust implementation of the null block driver. Like
 	  the C version, the driver allows the user to create virutal block
diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
new file mode 100644
index 000000000000..46710a1e1af4
--- /dev/null
+++ b/drivers/block/rnull/configfs.rs
@@ -0,0 +1,207 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use super::{NullBlkDevice, THIS_MODULE};
+use core::fmt::Write;
+use kernel::{
+    block::mq::gen_disk::{GenDisk, GenDiskBuilder},
+    c_str,
+    configfs::{self, AttributeOperations},
+    configfs_attrs, new_mutex,
+    page::PAGE_SIZE,
+    prelude::*,
+    str::{kstrtobool_bytes, CString},
+    sync::Mutex,
+};
+use pin_init::PinInit;
+
+pub(crate) fn subsystem() -> impl PinInit<kernel::configfs::Subsystem<Config>, Error> {
+    let item_type = configfs_attrs! {
+        container: configfs::Subsystem<Config>,
+        data: Config,
+        child: DeviceConfig,
+        attributes: [
+            features: 0,
+        ],
+    };
+
+    kernel::configfs::Subsystem::new(c_str!("rnull"), item_type, try_pin_init!(Config {}))
+}
+
+#[pin_data]
+pub(crate) struct Config {}
+
+#[vtable]
+impl AttributeOperations<0> for Config {
+    type Data = Config;
+
+    fn show(_this: &Config, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+        let mut writer = kernel::str::Formatter::new(page);
+        writer.write_str("blocksize,size,rotational\n")?;
+        Ok(writer.bytes_written())
+    }
+}
+
+#[vtable]
+impl configfs::GroupOperations for Config {
+    type Child = DeviceConfig;
+
+    fn make_group(
+        &self,
+        name: &CStr,
+    ) -> Result<impl PinInit<configfs::Group<DeviceConfig>, Error>> {
+        let item_type = configfs_attrs! {
+            container: configfs::Group<DeviceConfig>,
+            data: DeviceConfig,
+            attributes: [
+                // Named for compatibility with C null_blk
+                power: 0,
+                blocksize: 1,
+                rotational: 2,
+                size: 3,
+            ],
+        };
+
+        Ok(configfs::Group::new(
+            name.try_into()?,
+            item_type,
+            // TODO: cannot coerce new_mutex!() to impl PinInit<_, Error>, so put mutex inside
+            try_pin_init!( DeviceConfig {
+                data <- new_mutex!(DeviceConfigInner {
+                    powered: false,
+                    block_size: 4096,
+                    rotational: false,
+                    disk: None,
+                    capacity_mib: 4096,
+                    name: name.try_into()?,
+                }),
+            }),
+        ))
+    }
+}
+
+#[pin_data]
+pub(crate) struct DeviceConfig {
+    #[pin]
+    data: Mutex<DeviceConfigInner>,
+}
+
+#[pin_data]
+struct DeviceConfigInner {
+    powered: bool,
+    name: CString,
+    block_size: u32,
+    rotational: bool,
+    capacity_mib: u64,
+    disk: Option<GenDisk<NullBlkDevice>>,
+}
+
+#[vtable]
+impl configfs::AttributeOperations<0> for DeviceConfig {
+    type Data = DeviceConfig;
+
+    fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+        let mut writer = kernel::str::Formatter::new(page);
+
+        if this.data.lock().powered {
+            writer.write_str("1\n")?;
+        } else {
+            writer.write_str("0\n")?;
+        }
+
+        Ok(writer.bytes_written())
+    }
+
+    fn store(this: &DeviceConfig, page: &[u8]) -> Result {
+        let power_op = kstrtobool_bytes(page)?;
+        let mut guard = this.data.lock();
+
+        if !guard.powered && power_op {
+            guard.disk = Some(NullBlkDevice::new(
+                &guard.name,
+                guard.block_size,
+                guard.rotational,
+                guard.capacity_mib,
+            )?);
+            guard.powered = true;
+        } else if guard.powered && !power_op {
+            drop(guard.disk.take());
+            guard.powered = false;
+        }
+
+        Ok(())
+    }
+}
+
+#[vtable]
+impl configfs::AttributeOperations<1> for DeviceConfig {
+    type Data = DeviceConfig;
+
+    fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+        let mut writer = kernel::str::Formatter::new(page);
+        writer.write_fmt(fmt!("{}\n", this.data.lock().block_size))?;
+        Ok(writer.bytes_written())
+    }
+
+    fn store(this: &DeviceConfig, page: &[u8]) -> Result {
+        if this.data.lock().powered {
+            return Err(EBUSY);
+        }
+
+        let text = core::str::from_utf8(page)?.trim();
+        let value = text.parse::<u32>().map_err(|_| EINVAL)?;
+
+        GenDiskBuilder::validate_block_size(value)?;
+        this.data.lock().block_size = value;
+        Ok(())
+    }
+}
+
+#[vtable]
+impl configfs::AttributeOperations<2> for DeviceConfig {
+    type Data = DeviceConfig;
+
+    fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+        let mut writer = kernel::str::Formatter::new(page);
+
+        if this.data.lock().rotational {
+            writer.write_str("1\n")?;
+        } else {
+            writer.write_str("0\n")?;
+        }
+
+        Ok(writer.bytes_written())
+    }
+
+    fn store(this: &DeviceConfig, page: &[u8]) -> Result {
+        if this.data.lock().powered {
+            return Err(EBUSY);
+        }
+
+        this.data.lock().rotational = kstrtobool_bytes(page)?;
+
+        Ok(())
+    }
+}
+
+#[vtable]
+impl configfs::AttributeOperations<3> for DeviceConfig {
+    type Data = DeviceConfig;
+
+    fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+        let mut writer = kernel::str::Formatter::new(page);
+        writer.write_fmt(fmt!("{}\n", this.data.lock().capacity_mib))?;
+        Ok(writer.bytes_written())
+    }
+
+    fn store(this: &DeviceConfig, page: &[u8]) -> Result {
+        if this.data.lock().powered {
+            return Err(EBUSY);
+        }
+
+        let text = core::str::from_utf8(page)?.trim();
+        let value = text.parse::<u64>().map_err(|_| EINVAL)?;
+
+        this.data.lock().capacity_mib = value;
+        Ok(())
+    }
+}
diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index d07e76ae2c13..8690ff5f974f 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -1,28 +1,25 @@
 // SPDX-License-Identifier: GPL-2.0
 
 //! This is a Rust implementation of the C null block driver.
-//!
-//! Supported features:
-//!
-//! - blk-mq interface
-//! - direct completion
-//! - block size 4k
-//!
-//! The driver is not configurable.
+
+mod configfs;
 
 use kernel::{
-    alloc::flags,
-    block::mq::{
+    block::{
         self,
-        gen_disk::{self, GenDisk},
-        Operations, TagSet,
+        mq::{
+            self,
+            gen_disk::{self, GenDisk},
+            Operations, TagSet,
+        },
     },
     error::Result,
-    new_mutex, pr_info,
+    pr_info,
     prelude::*,
-    sync::{Arc, Mutex},
+    sync::Arc,
     types::ARef,
 };
+use pin_init::PinInit;
 
 module! {
     type: NullBlkModule,
@@ -35,33 +32,39 @@
 #[pin_data]
 struct NullBlkModule {
     #[pin]
-    _disk: Mutex<GenDisk<NullBlkDevice>>,
+    configfs_subsystem: kernel::configfs::Subsystem<configfs::Config>,
 }
 
 impl kernel::InPlaceModule for NullBlkModule {
     fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
         pr_info!("Rust null_blk loaded\n");
 
-        // Use a immediately-called closure as a stable `try` block
-        let disk = /* try */ (|| {
-            let tagset = Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
-
-            gen_disk::GenDiskBuilder::new()
-                .capacity_sectors(4096 << 11)
-                .logical_block_size(4096)?
-                .physical_block_size(4096)?
-                .rotational(false)
-                .build(format_args!("rnullb{}", 0), tagset)
-        })();
-
         try_pin_init!(Self {
-            _disk <- new_mutex!(disk?, "nullb:disk"),
+            configfs_subsystem <- configfs::subsystem(),
         })
     }
 }
 
 struct NullBlkDevice;
 
+impl NullBlkDevice {
+    fn new(
+        name: &CStr,
+        block_size: u32,
+        rotational: bool,
+        capacity_mib: u64,
+    ) -> Result<GenDisk<Self>> {
+        let tagset = Arc::pin_init(TagSet::new(1, 256, 1), GFP_KERNEL)?;
+
+        gen_disk::GenDiskBuilder::new()
+            .capacity_sectors(capacity_mib << (20 - block::SECTOR_SHIFT))
+            .logical_block_size(block_size)?
+            .physical_block_size(block_size)?
+            .rotational(rotational)
+            .build(fmt!("{}", name.to_str()?), tagset)
+    }
+}
+
 #[vtable]
 impl Operations for NullBlkDevice {
     #[inline(always)]
diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index 20f1d46c774d..6b1b846874db 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -51,7 +51,7 @@ pub fn rotational(mut self, rotational: bool) -> Self {
 
     /// Validate block size by verifying that it is between 512 and `PAGE_SIZE`,
     /// and that it is a power of two.
-    fn validate_block_size(size: u32) -> Result {
+    pub fn validate_block_size(size: u32) -> Result {
         if !(512..=bindings::PAGE_SIZE as u32).contains(&size) || !size.is_power_of_two() {
             Err(error::code::EINVAL)
         } else {

-- 
2.47.2

[PATCH v6 15/18] rust: block: add `GenDisk` private data support

[Andreas Hindborg]

Allow users of the rust block device driver API to install private data in
the `GenDisk` structure.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 drivers/block/rnull/rnull.rs       |  8 ++++---
 rust/kernel/block/mq.rs            |  7 +++---
 rust/kernel/block/mq/gen_disk.rs   | 32 ++++++++++++++++++++++----
 rust/kernel/block/mq/operations.rs | 46 ++++++++++++++++++++++++++++++--------
 4 files changed, 74 insertions(+), 19 deletions(-)

diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index 8690ff5f974f..8255236bc550 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -61,14 +61,16 @@ fn new(
             .logical_block_size(block_size)?
             .physical_block_size(block_size)?
             .rotational(rotational)
-            .build(fmt!("{}", name.to_str()?), tagset)
+            .build(fmt!("{}", name.to_str()?), tagset, ())
     }
 }
 
 #[vtable]
 impl Operations for NullBlkDevice {
+    type QueueData = ();
+
     #[inline(always)]
-    fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
+    fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
         mq::Request::end_ok(rq)
             .map_err(|_e| kernel::error::code::EIO)
             // We take no refcounts on the request, so we expect to be able to
@@ -79,5 +81,5 @@ fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
         Ok(())
     }
 
-    fn commit_rqs() {}
+    fn commit_rqs(_queue_data: ()) {}
 }
diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
index 98fa0d6bc8f7..6e546f4f3d1c 100644
--- a/rust/kernel/block/mq.rs
+++ b/rust/kernel/block/mq.rs
@@ -69,20 +69,21 @@
 //!
 //! #[vtable]
 //! impl Operations for MyBlkDevice {
+//!     type QueueData = ();
 //!
-//!     fn queue_rq(rq: ARef<Request<Self>>, _is_last: bool) -> Result {
+//!     fn queue_rq(_queue_data: (), rq: ARef<Request<Self>>, _is_last: bool) -> Result {
 //!         Request::end_ok(rq);
 //!         Ok(())
 //!     }
 //!
-//!     fn commit_rqs() {}
+//!     fn commit_rqs(_queue_data: ()) {}
 //! }
 //!
 //! let tagset: Arc<TagSet<MyBlkDevice>> =
 //!     Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
 //! let mut disk = gen_disk::GenDiskBuilder::new()
 //!     .capacity_sectors(4096)
-//!     .build(format_args!("myblk"), tagset)?;
+//!     .build(format_args!("myblk"), tagset, ())?;
 //!
 //! # Ok::<(), kernel::error::Error>(())
 //! ```
diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index 6b1b846874db..46ec80269970 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -13,6 +13,7 @@
     static_lock_class,
     str::NullTerminatedFormatter,
     sync::Arc,
+    types::{ForeignOwnable, ScopeGuard},
 };
 use core::fmt::{self, Write};
 
@@ -98,7 +99,14 @@ pub fn build<T: Operations>(
         self,
         name: fmt::Arguments<'_>,
         tagset: Arc<TagSet<T>>,
+        queue_data: T::QueueData,
     ) -> Result<GenDisk<T>> {
+        let data = queue_data.into_foreign();
+        let recover_data = ScopeGuard::new(|| {
+            // SAFETY: T::QueueData was created by the call to `into_foreign()` above
+            drop(unsafe { T::QueueData::from_foreign(data) });
+        });
+
         // SAFETY: `bindings::queue_limits` contain only fields that are valid when zeroed.
         let mut lim: bindings::queue_limits = unsafe { core::mem::zeroed() };
 
@@ -113,7 +121,7 @@ pub fn build<T: Operations>(
             bindings::__blk_mq_alloc_disk(
                 tagset.raw_tag_set(),
                 &mut lim,
-                core::ptr::null_mut(),
+                data,
                 static_lock_class!().as_ptr(),
             )
         })?;
@@ -167,8 +175,12 @@ pub fn build<T: Operations>(
             },
         )?;
 
+        recover_data.dismiss();
+
         // INVARIANT: `gendisk` was initialized above.
         // INVARIANT: `gendisk` was added to the VFS via `device_add_disk` above.
+        // INVARIANT: `gendisk.queue.queue_data` is set to `data` in the call to
+        // `__blk_mq_alloc_disk` above.
         Ok(GenDisk {
             _tagset: tagset,
             gendisk,
@@ -180,9 +192,10 @@ pub fn build<T: Operations>(
 ///
 /// # Invariants
 ///
-/// - `gendisk` must always point to an initialized and valid `struct gendisk`.
-/// - `gendisk` was added to the VFS through a call to
-///   `bindings::device_add_disk`.
+///  - `gendisk` must always point to an initialized and valid `struct gendisk`.
+///  - `gendisk` was added to the VFS through a call to
+///    `bindings::device_add_disk`.
+///  - `self.gendisk.queue.queuedata` is initialized by a call to `ForeignOwnable::into_foreign`.
 pub struct GenDisk<T: Operations> {
     _tagset: Arc<TagSet<T>>,
     gendisk: *mut bindings::gendisk,
@@ -194,9 +207,20 @@ unsafe impl<T: Operations + Send> Send for GenDisk<T> {}
 
 impl<T: Operations> Drop for GenDisk<T> {
     fn drop(&mut self) {
+        // SAFETY: By type invariant of `Self`, `self.gendisk` points to a valid
+        // and initialized instance of `struct gendisk`, and, `queuedata` was
+        // initialized with the result of a call to
+        // `ForeignOwnable::into_foreign`.
+        let queue_data = unsafe { (*(*self.gendisk).queue).queuedata };
+
         // SAFETY: By type invariant, `self.gendisk` points to a valid and
         // initialized instance of `struct gendisk`, and it was previously added
         // to the VFS.
         unsafe { bindings::del_gendisk(self.gendisk) };
+
+        // SAFETY: `queue.queuedata` was created by `GenDiskBuilder::build` with
+        // a call to `ForeignOwnable::into_foreign` to create `queuedata`.
+        // `ForeignOwnable::from_foreign` is only called here.
+        drop(unsafe { T::QueueData::from_foreign(queue_data) });
     }
 }
diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
index c2b98f507bcb..6fb256f55acc 100644
--- a/rust/kernel/block/mq/operations.rs
+++ b/rust/kernel/block/mq/operations.rs
@@ -6,14 +6,15 @@
 
 use crate::{
     bindings,
-    block::mq::request::RequestDataWrapper,
-    block::mq::Request,
+    block::mq::{request::RequestDataWrapper, Request},
     error::{from_result, Result},
     prelude::*,
-    types::ARef,
+    types::{ARef, ForeignOwnable},
 };
 use core::{marker::PhantomData, sync::atomic::AtomicU64, sync::atomic::Ordering};
 
+type ForeignBorrowed<'a, T> = <T as ForeignOwnable>::Borrowed<'a>;
+
 /// Implement this trait to interface blk-mq as block devices.
 ///
 /// To implement a block device driver, implement this trait as described in the
@@ -26,12 +27,20 @@
 /// [module level documentation]: kernel::block::mq
 #[macros::vtable]
 pub trait Operations: Sized {
+    /// Data associated with the `struct request_queue` that is allocated for
+    /// the `GenDisk` associated with this `Operations` implementation.
+    type QueueData: ForeignOwnable;
+
     /// Called by the kernel to queue a request with the driver. If `is_last` is
     /// `false`, the driver is allowed to defer committing the request.
-    fn queue_rq(rq: ARef<Request<Self>>, is_last: bool) -> Result;
+    fn queue_rq(
+        queue_data: ForeignBorrowed<'_, Self::QueueData>,
+        rq: ARef<Request<Self>>,
+        is_last: bool,
+    ) -> Result;
 
     /// Called by the kernel to indicate that queued requests should be submitted.
-    fn commit_rqs();
+    fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>);
 
     /// Called by the kernel to poll the device for completed requests. Only
     /// used for poll queues.
@@ -70,7 +79,7 @@ impl<T: Operations> OperationsVTable<T> {
     ///   promise to not access the request until the driver calls
     ///   `bindings::blk_mq_end_request` for the request.
     unsafe extern "C" fn queue_rq_callback(
-        _hctx: *mut bindings::blk_mq_hw_ctx,
+        hctx: *mut bindings::blk_mq_hw_ctx,
         bd: *const bindings::blk_mq_queue_data,
     ) -> bindings::blk_status_t {
         // SAFETY: `bd.rq` is valid as required by the safety requirement for
@@ -88,10 +97,20 @@ impl<T: Operations> OperationsVTable<T> {
         //    reference counted by `ARef` until then.
         let rq = unsafe { Request::aref_from_raw((*bd).rq) };
 
+        // SAFETY: `hctx` is valid as required by this function.
+        let queue_data = unsafe { (*(*hctx).queue).queuedata };
+
+        // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a
+        // call to `ForeignOwnable::into_pointer()` to create `queuedata`.
+        // `ForeignOwnable::from_foreign()` is only called when the tagset is
+        // dropped, which happens after we are dropped.
+        let queue_data = unsafe { T::QueueData::borrow(queue_data) };
+
         // SAFETY: We have exclusive access and we just set the refcount above.
         unsafe { Request::start_unchecked(&rq) };
 
         let ret = T::queue_rq(
+            queue_data,
             rq,
             // SAFETY: `bd` is valid as required by the safety requirement for
             // this function.
@@ -110,9 +129,18 @@ impl<T: Operations> OperationsVTable<T> {
     ///
     /// # Safety
     ///
-    /// This function may only be called by blk-mq C infrastructure.
-    unsafe extern "C" fn commit_rqs_callback(_hctx: *mut bindings::blk_mq_hw_ctx) {
-        T::commit_rqs()
+    /// This function may only be called by blk-mq C infrastructure. The caller
+    /// must ensure that `hctx` is valid.
+    unsafe extern "C" fn commit_rqs_callback(hctx: *mut bindings::blk_mq_hw_ctx) {
+        // SAFETY: `hctx` is valid as required by this function.
+        let queue_data = unsafe { (*(*hctx).queue).queuedata };
+
+        // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a
+        // call to `ForeignOwnable::into_pointer()` to create `queuedata`.
+        // `ForeignOwnable::from_foreign()` is only called when the tagset is
+        // dropped, which happens after we are dropped.
+        let queue_data = unsafe { T::QueueData::borrow(queue_data) };
+        T::commit_rqs(queue_data)
     }
 
     /// This function is called by the C kernel. It is not currently

-- 
2.47.2

[PATCH v6 16/18] rust: block: mq: fix spelling in a safety comment

[Andreas Hindborg]

Add code block quotes to a safety comment.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/block/mq/request.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index f723d74091c1..3848cfe63f77 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -148,7 +148,7 @@ pub(crate) unsafe fn wrapper_ptr(this: *mut Self) -> NonNull<RequestDataWrapper>
         // valid allocation.
         let wrapper_ptr =
             unsafe { bindings::blk_mq_rq_to_pdu(request_ptr).cast::<RequestDataWrapper>() };
-        // SAFETY: By C API contract, wrapper_ptr points to a valid allocation
+        // SAFETY: By C API contract, `wrapper_ptr` points to a valid allocation
         // and is not null.
         unsafe { NonNull::new_unchecked(wrapper_ptr) }
     }

-- 
2.47.2

[PATCH v6 17/18] rust: block: add remote completion to `Request`

[Andreas Hindborg]

Allow users of rust block device driver API to schedule completion of
requests via `blk_mq_complete_request_remote`.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 drivers/block/rnull/rnull.rs       |  9 +++++++++
 rust/kernel/block/mq.rs            |  6 ++++++
 rust/kernel/block/mq/operations.rs | 19 +++++++++++++++----
 rust/kernel/block/mq/request.rs    | 17 +++++++++++++++++
 4 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index 8255236bc550..a19c55717c4f 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -82,4 +82,13 @@ fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Res
     }
 
     fn commit_rqs(_queue_data: ()) {}
+
+    fn complete(rq: ARef<mq::Request<Self>>) {
+        mq::Request::end_ok(rq)
+            .map_err(|_e| kernel::error::code::EIO)
+            // We take no refcounts on the request, so we expect to be able to
+            // end the request. The request reference must be unique at this
+            // point, and so `end_ok` cannot fail.
+            .expect("Fatal error - expected to be able to end request");
+    }
 }
diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
index 6e546f4f3d1c..c0ec06b84355 100644
--- a/rust/kernel/block/mq.rs
+++ b/rust/kernel/block/mq.rs
@@ -77,6 +77,12 @@
 //!     }
 //!
 //!     fn commit_rqs(_queue_data: ()) {}
+//!
+//!     fn complete(rq: ARef<Request<Self>>) {
+//!         Request::end_ok(rq)
+//!             .map_err(|_e| kernel::error::code::EIO)
+//!             .expect("Fatal error - expected to be able to end request");
+//!     }
 //! }
 //!
 //! let tagset: Arc<TagSet<MyBlkDevice>> =
diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
index 6fb256f55acc..0fece577de7c 100644
--- a/rust/kernel/block/mq/operations.rs
+++ b/rust/kernel/block/mq/operations.rs
@@ -42,6 +42,9 @@ fn queue_rq(
     /// Called by the kernel to indicate that queued requests should be submitted.
     fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>);
 
+    /// Called by the kernel when the request is completed.
+    fn complete(rq: ARef<Request<Self>>);
+
     /// Called by the kernel to poll the device for completed requests. Only
     /// used for poll queues.
     fn poll() -> bool {
@@ -143,13 +146,21 @@ impl<T: Operations> OperationsVTable<T> {
         T::commit_rqs(queue_data)
     }
 
-    /// This function is called by the C kernel. It is not currently
-    /// implemented, and there is no way to exercise this code path.
+    /// This function is called by the C kernel. A pointer to this function is
+    /// installed in the `blk_mq_ops` vtable for the driver.
     ///
     /// # Safety
     ///
-    /// This function may only be called by blk-mq C infrastructure.
-    unsafe extern "C" fn complete_callback(_rq: *mut bindings::request) {}
+    /// This function may only be called by blk-mq C infrastructure. `rq` must
+    /// point to a valid request that has been marked as completed. The pointee
+    /// of `rq` must be valid for write for the duration of this function.
+    unsafe extern "C" fn complete_callback(rq: *mut bindings::request) {
+        // SAFETY: This function can only be dispatched through
+        // `Request::complete`. We leaked a refcount then which we pick back up
+        // now.
+        let aref = unsafe { Request::aref_from_raw(rq) };
+        T::complete(aref);
+    }
 
     /// This function is called by the C kernel. A pointer to this function is
     /// installed in the `blk_mq_ops` vtable for the driver.
diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index 3848cfe63f77..f7f757f7459f 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -135,6 +135,23 @@ pub fn end_ok(this: ARef<Self>) -> Result<(), ARef<Self>> {
         Ok(())
     }
 
+    /// Complete the request by scheduling `Operations::complete` for
+    /// execution.
+    ///
+    /// The function may be scheduled locally, via SoftIRQ or remotely via IPMI.
+    /// See `blk_mq_complete_request_remote` in [`blk-mq.c`] for details.
+    ///
+    /// [`blk-mq.c`]: srctree/block/blk-mq.c
+    pub fn complete(this: ARef<Self>) {
+        let ptr = ARef::into_raw(this).cast::<bindings::request>().as_ptr();
+        // SAFETY: By type invariant, `self.0` is a valid `struct request`
+        if !unsafe { bindings::blk_mq_complete_request_remote(ptr) } {
+            // SAFETY: We released a refcount above that we can reclaim here.
+            let this = unsafe { Request::aref_from_raw(ptr) };
+            T::complete(this);
+        }
+    }
+
     /// Return a pointer to the [`RequestDataWrapper`] stored in the private area
     /// of the request structure.
     ///

-- 
2.47.2

[PATCH v6 18/18] rnull: add soft-irq completion support

[Andreas Hindborg]

rnull currently only supports direct completion. Add option for completing
requests across CPU nodes via soft IRQ or IPI.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 drivers/block/rnull/configfs.rs | 59 +++++++++++++++++++++++++++++++++++++++--
 drivers/block/rnull/rnull.rs    | 32 ++++++++++++++--------
 2 files changed, 78 insertions(+), 13 deletions(-)

diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
index 46710a1e1af4..8498e9bae6fd 100644
--- a/drivers/block/rnull/configfs.rs
+++ b/drivers/block/rnull/configfs.rs
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 
 use super::{NullBlkDevice, THIS_MODULE};
-use core::fmt::Write;
+use core::fmt::{Display, Write};
 use kernel::{
     block::mq::gen_disk::{GenDisk, GenDiskBuilder},
     c_str,
@@ -36,7 +36,7 @@ impl AttributeOperations<0> for Config {
 
     fn show(_this: &Config, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
         let mut writer = kernel::str::Formatter::new(page);
-        writer.write_str("blocksize,size,rotational\n")?;
+        writer.write_str("blocksize,size,rotational,irqmode\n")?;
         Ok(writer.bytes_written())
     }
 }
@@ -58,6 +58,7 @@ fn make_group(
                 blocksize: 1,
                 rotational: 2,
                 size: 3,
+                irqmode: 4,
             ],
         };
 
@@ -72,6 +73,7 @@ fn make_group(
                     rotational: false,
                     disk: None,
                     capacity_mib: 4096,
+                    irq_mode: IRQMode::None,
                     name: name.try_into()?,
                 }),
             }),
@@ -79,6 +81,34 @@ fn make_group(
     }
 }
 
+#[derive(Debug, Clone, Copy)]
+pub(crate) enum IRQMode {
+    None,
+    Soft,
+}
+
+impl TryFrom<u8> for IRQMode {
+    type Error = kernel::error::Error;
+
+    fn try_from(value: u8) -> Result<Self> {
+        match value {
+            0 => Ok(Self::None),
+            1 => Ok(Self::Soft),
+            _ => Err(EINVAL),
+        }
+    }
+}
+
+impl Display for IRQMode {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        match self {
+            Self::None => f.write_str("0")?,
+            Self::Soft => f.write_str("1")?,
+        }
+        Ok(())
+    }
+}
+
 #[pin_data]
 pub(crate) struct DeviceConfig {
     #[pin]
@@ -92,6 +122,7 @@ struct DeviceConfigInner {
     block_size: u32,
     rotational: bool,
     capacity_mib: u64,
+    irq_mode: IRQMode,
     disk: Option<GenDisk<NullBlkDevice>>,
 }
 
@@ -121,6 +152,7 @@ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
                 guard.block_size,
                 guard.rotational,
                 guard.capacity_mib,
+                guard.irq_mode,
             )?);
             guard.powered = true;
         } else if guard.powered && !power_op {
@@ -205,3 +237,26 @@ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
         Ok(())
     }
 }
+
+#[vtable]
+impl configfs::AttributeOperations<4> for DeviceConfig {
+    type Data = DeviceConfig;
+
+    fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+        let mut writer = kernel::str::Formatter::new(page);
+        writer.write_fmt(fmt!("{}\n", this.data.lock().irq_mode))?;
+        Ok(writer.bytes_written())
+    }
+
+    fn store(this: &DeviceConfig, page: &[u8]) -> Result {
+        if this.data.lock().powered {
+            return Err(EBUSY);
+        }
+
+        let text = core::str::from_utf8(page)?.trim();
+        let value = text.parse::<u8>().map_err(|_| EINVAL)?;
+
+        this.data.lock().irq_mode = IRQMode::try_from(value)?;
+        Ok(())
+    }
+}
diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index a19c55717c4f..1ec694d7f1a6 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -4,6 +4,7 @@
 
 mod configfs;
 
+use configfs::IRQMode;
 use kernel::{
     block::{
         self,
@@ -53,35 +54,44 @@ fn new(
         block_size: u32,
         rotational: bool,
         capacity_mib: u64,
+        irq_mode: IRQMode,
     ) -> Result<GenDisk<Self>> {
         let tagset = Arc::pin_init(TagSet::new(1, 256, 1), GFP_KERNEL)?;
 
+        let queue_data = Box::new(QueueData { irq_mode }, GFP_KERNEL)?;
+
         gen_disk::GenDiskBuilder::new()
             .capacity_sectors(capacity_mib << (20 - block::SECTOR_SHIFT))
             .logical_block_size(block_size)?
             .physical_block_size(block_size)?
             .rotational(rotational)
-            .build(fmt!("{}", name.to_str()?), tagset, ())
+            .build(fmt!("{}", name.to_str()?), tagset, queue_data)
     }
 }
 
+struct QueueData {
+    irq_mode: IRQMode,
+}
+
 #[vtable]
 impl Operations for NullBlkDevice {
-    type QueueData = ();
+    type QueueData = KBox<QueueData>;
 
     #[inline(always)]
-    fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
-        mq::Request::end_ok(rq)
-            .map_err(|_e| kernel::error::code::EIO)
-            // We take no refcounts on the request, so we expect to be able to
-            // end the request. The request reference must be unique at this
-            // point, and so `end_ok` cannot fail.
-            .expect("Fatal error - expected to be able to end request");
-
+    fn queue_rq(queue_data: &QueueData, rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
+        match queue_data.irq_mode {
+            IRQMode::None => mq::Request::end_ok(rq)
+                .map_err(|_e| kernel::error::code::EIO)
+                // We take no refcounts on the request, so we expect to be able to
+                // end the request. The request reference must be unique at this
+                // point, and so `end_ok` cannot fail.
+                .expect("Fatal error - expected to be able to end request"),
+            IRQMode::Soft => mq::Request::complete(rq),
+        }
         Ok(())
     }
 
-    fn commit_rqs(_queue_data: ()) {}
+    fn commit_rqs(_queue_data: &QueueData) {}
 
     fn complete(rq: ARef<mq::Request<Self>>) {
         mq::Request::end_ok(rq)

-- 
2.47.2

Re: [PATCH v6 01/18] rust: str: normalize imports in `str.rs`

[Daniel Almeida]

> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> Clean up imports in `str.rs`. This makes future code manipulation more
> manageable.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/str.rs | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index 6c892550c0ba..082790b7a621 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -2,12 +2,13 @@
> 
> //! String representations.
> 
> -use crate::alloc::{flags::*, AllocError, KVec};
> -use crate::fmt::{self, Write};
> +use crate::{
> +    alloc::{flags::*, AllocError, KVec},
> +    fmt::{self, Write},
> +    prelude::*,
> +};
> use core::ops::{self, Deref, DerefMut, Index};
> 
> -use crate::prelude::*;
> -
> /// Byte string without UTF-8 validity guarantee.
> #[repr(transparent)]
> pub struct BStr([u8]);
> 
> -- 
> 2.47.2
> 
> 
> 

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

Re: [PATCH v6 03/18] rust: str: expose `str::{Formatter, RawFormatter}` publicly.

[Daniel Almeida]

> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> rnull is going to make use of `str::Formatter` and `str::RawFormatter`, so
> expose them with public visibility.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/str.rs | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index 76632da357a6..46cdc85dad63 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -736,7 +736,7 @@ fn test_bstr_debug() -> Result {
> ///
> /// The memory region between `pos` (inclusive) and `end` (exclusive) is valid for writes if `pos`
> /// is less than `end`.
> -pub(crate) struct RawFormatter {
> +pub struct RawFormatter {
>     // Use `usize` to use `saturating_*` functions.
>     beg: usize,
>     pos: usize,
> @@ -794,7 +794,7 @@ pub(crate) fn pos(&self) -> *mut u8 {
>     }
> 
>     /// Returns the number of bytes written to the formatter.
> -    pub(crate) fn bytes_written(&self) -> usize {
> +    pub fn bytes_written(&self) -> usize {
>         self.pos - self.beg
>     }
> }
> @@ -828,7 +828,7 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
> /// Allows formatting of [`fmt::Arguments`] into a raw buffer.
> ///
> /// Fails if callers attempt to write more than will fit in the buffer.
> -pub(crate) struct Formatter<'a>(RawFormatter, PhantomData<&'a mut ()>);
> +pub struct Formatter<'a>(RawFormatter, PhantomData<&'a mut ()>);
> 
> impl Formatter<'_> {
>     /// Creates a new instance of [`Formatter`] with the given buffer.
> @@ -843,8 +843,7 @@ pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
>     }
> 
>     /// Create a new [`Self`] instance.
> -    #[expect(dead_code)]
> -    pub(crate) fn new(buffer: &mut [u8]) -> Self {
> +    pub fn new(buffer: &mut [u8]) -> Self {
>         // SAFETY: `buffer` is valid for writes for the entire length for
>         // the lifetime of `Self`.
>         unsafe { Formatter::from_buffer(buffer.as_mut_ptr(), buffer.len()) }
> 
> -- 
> 2.47.2
> 
> 
> 

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

Re: [PATCH v6 04/18] rust: str: introduce `NullTerminatedFormatter`

[Daniel Almeida]

> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> Add `NullTerminatedFormatter`, a formatter that writes a null terminated
> string to an array or slice buffer. Because this type needs to manage the
> trailing null marker, the existing formatters cannot be used to implement
> this type.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/str.rs | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
> 
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index 46cdc85dad63..d8326f7bc9c1 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -871,6 +871,55 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
>     }
> }
> 
> +/// A mutable reference to a byte buffer where a string can be written into.
> +///
> +/// The buffer will be automatically null terminated after the last written character.
> +///
> +/// # Invariants
> +///
> +/// * The first byte of `buffer` is always zero.
> +/// * The length of `buffer` is at least 1.
> +pub(crate) struct NullTerminatedFormatter<'a> {
> +    buffer: &'a mut [u8],
> +}
> +
> +impl<'a> NullTerminatedFormatter<'a> {
> +    /// Create a new [`Self`] instance.
> +    #[expect(dead_code)]
> +    pub(crate) fn new(buffer: &'a mut [u8]) -> Option<NullTerminatedFormatter<'a>> {
> +        *(buffer.first_mut()?) = 0;
> +
> +        // INVARIANT:
> +        //  - We wrote zero to the first byte above.
> +        //  - If buffer was not at least length 1, `buffer.first_mut()` would return None.
> +        Some(Self { buffer })
> +    }
> +}
> +
> +impl Write for NullTerminatedFormatter<'_> {
> +    fn write_str(&mut self, s: &str) -> fmt::Result {
> +        let bytes = s.as_bytes();
> +        let len = bytes.len();
> +
> +        // We want space for a zero. By type invariant, buffer length is always at least 1, so no
> +        // underflow.
> +        if len > self.buffer.len() - 1 {
> +            return Err(fmt::Error);
> +        }
> +
> +        let buffer = core::mem::take(&mut self.buffer);
> +        // We break the zero start invariant for a short while.
> +        buffer[..len].copy_from_slice(bytes);
> +        // INVARIANT: We checked above that buffer will have size at least 1 after this assignment.
> +        self.buffer = &mut buffer[len..];
> +
> +        // INVARIANT: We write zero to the first byte of the buffer.
> +        self.buffer[0] = 0;
> +
> +        Ok(())
> +    }
> +}
> +
> /// An owned string that is guaranteed to have exactly one `NUL` byte, which is at the end.
> ///
> /// Used for interoperability with kernel APIs that take C strings.
> 
> -- 
> 2.47.2
> 
> 
> 


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

Re: [PATCH v6 05/18] rust: str: introduce `kstrtobool` function

[Daniel Almeida]

> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> Add a Rust wrapper for the kernel's `kstrtobool` function that converts
> common user inputs into boolean values.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/str.rs | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
> 
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index d8326f7bc9c1..d070c0bd86c3 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -4,6 +4,7 @@
> 
> use crate::{
>     alloc::{flags::*, AllocError, KVec},
> +    error::Result,
>     fmt::{self, Write},
>     prelude::*,
> };
> @@ -920,6 +921,62 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
>     }
> }
> 
> +/// Convert common user inputs into boolean values using the kernel's `kstrtobool` function.
> +///
> +/// This routine returns `Ok(bool)` if the first character is one of 'YyTt1NnFf0', or
> +/// \[oO\]\[NnFf\] for "on" and "off". Otherwise it will return `Err(EINVAL)`.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// # use kernel::{c_str, str::kstrtobool};
> +///
> +/// // Lowercase
> +/// assert_eq!(kstrtobool(c_str!("true")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("tr")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("t")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("twrong")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("false")), Ok(false));
> +/// assert_eq!(kstrtobool(c_str!("f")), Ok(false));
> +/// assert_eq!(kstrtobool(c_str!("yes")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("no")), Ok(false));
> +/// assert_eq!(kstrtobool(c_str!("on")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("off")), Ok(false));
> +///
> +/// // Camel case
> +/// assert_eq!(kstrtobool(c_str!("True")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("False")), Ok(false));
> +/// assert_eq!(kstrtobool(c_str!("Yes")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("No")), Ok(false));
> +/// assert_eq!(kstrtobool(c_str!("On")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("Off")), Ok(false));
> +///
> +/// // All caps
> +/// assert_eq!(kstrtobool(c_str!("TRUE")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("FALSE")), Ok(false));
> +/// assert_eq!(kstrtobool(c_str!("YES")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("NO")), Ok(false));
> +/// assert_eq!(kstrtobool(c_str!("ON")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("OFF")), Ok(false));
> +///
> +/// // Numeric
> +/// assert_eq!(kstrtobool(c_str!("1")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("0")), Ok(false));
> +///
> +/// // Invalid input
> +/// assert_eq!(kstrtobool(c_str!("invalid")), Err(EINVAL));
> +/// assert_eq!(kstrtobool(c_str!("2")), Err(EINVAL));
> +/// ```
> +pub fn kstrtobool(string: &CStr) -> Result<bool> {
> +    let mut result: bool = false;
> +
> +    // SAFETY: `string` is a valid null-terminated C string, and `result` is a valid
> +    // pointer to a bool that we own.
> +    let ret = unsafe { bindings::kstrtobool(string.as_char_ptr(), &mut result) };
> +
> +    kernel::error::to_result(ret).map(|()| result)
> +}
> +
> /// An owned string that is guaranteed to have exactly one `NUL` byte, which is at the end.
> ///
> /// Used for interoperability with kernel APIs that take C strings.
> 
> -- 
> 2.47.2
> 
> 
> 

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

Re: [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function

[Daniel Almeida]

> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> Add a convenience function to convert byte slices to boolean values by
> wrapping them in a null-terminated C string and delegating to the
> existing `kstrtobool` function. Only considers the first two bytes of
> the input slice, following the kernel's boolean parsing semantics.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/str.rs | 35 +++++++++++++++++++++++++++++------
> 1 file changed, 29 insertions(+), 6 deletions(-)
> 
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index d070c0bd86c3..b185262b4851 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -921,6 +921,20 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
>     }
> }
> 
> +/// # Safety
> +///
> +/// - `string` must point to a null terminated string that is valid for read.
> +unsafe fn kstrtobool_raw(string: *const u8) -> Result<bool> {
> +    let mut result: bool = false;
> +
> +    // SAFETY:
> +    // - By function safety requirement, `string` is a valid null-terminated string.
> +    // - `result` is a valid `bool` that we own.
> +    let ret = unsafe { bindings::kstrtobool(string, &mut result) };
> +
> +    kernel::error::to_result(ret).map(|()| result)
> +}
> +
> /// Convert common user inputs into boolean values using the kernel's `kstrtobool` function.
> ///
> /// This routine returns `Ok(bool)` if the first character is one of 'YyTt1NnFf0', or
> @@ -968,13 +982,22 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
> /// assert_eq!(kstrtobool(c_str!("2")), Err(EINVAL));
> /// ```
> pub fn kstrtobool(string: &CStr) -> Result<bool> {
> -    let mut result: bool = false;
> -
> -    // SAFETY: `string` is a valid null-terminated C string, and `result` is a valid
> -    // pointer to a bool that we own.
> -    let ret = unsafe { bindings::kstrtobool(string.as_char_ptr(), &mut result) };
> +    // SAFETY:
> +    // - The pointer returned by `CStr::as_char_ptr` is guaranteed to be
> +    //   null terminated.
> +    // - `string` is live and thus the string is valid for read.
> +    unsafe { kstrtobool_raw(string.as_char_ptr()) }
> +}
> 
> -    kernel::error::to_result(ret).map(|()| result)
> +/// Convert `&[u8]` to `bool` by deferring to [`kernel::str::kstrtobool`].
> +///
> +/// Only considers at most the first two bytes of `bytes`.
> +pub fn kstrtobool_bytes(bytes: &[u8]) -> Result<bool> {
> +    // `ktostrbool` only considers the first two bytes of the input.
> +    let stack_string = [*bytes.first().unwrap_or(&0), *bytes.get(1).unwrap_or(&0), 0];

Can’t this be CStr::from_bytes_with_nul() ?

This means that kstrtobool_raw could take a &CStr directly and thus not be unsafe IIUC?

> +    // SAFETY: `stack_string` is null terminated and it is live on the stack so
> +    // it is valid for read.
> +    unsafe { kstrtobool_raw(stack_string.as_ptr()) }
> }
> 
> /// An owned string that is guaranteed to have exactly one `NUL` byte, which is at the end.
> 
> -- 
> 2.47.2
> 
> 
> 

Re: [PATCH v6 07/18] rust: configfs: re-export `configfs_attrs` from `configfs` module

[Daniel Almeida]

> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> Re-export `configfs_attrs` from `configfs` module, so that users can import
> the macro from the `configfs` module rather than the root of the `kernel`
> crate.
> 
> Also update users to import from the new path.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/configfs.rs       | 2 ++
> samples/rust/rust_configfs.rs | 2 +-
> 2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
> index 2736b798cdc6..318a2f073d1c 100644
> --- a/rust/kernel/configfs.rs
> +++ b/rust/kernel/configfs.rs
> @@ -1039,3 +1039,5 @@ macro_rules! configfs_attrs {
>     };
> 
> }
> +
> +pub use crate::configfs_attrs;
> diff --git a/samples/rust/rust_configfs.rs b/samples/rust/rust_configfs.rs
> index af04bfa35cb2..ad364fb93e53 100644
> --- a/samples/rust/rust_configfs.rs
> +++ b/samples/rust/rust_configfs.rs
> @@ -5,7 +5,7 @@
> use kernel::alloc::flags;
> use kernel::c_str;
> use kernel::configfs;
> -use kernel::configfs_attrs;
> +use kernel::configfs::configfs_attrs;
> use kernel::new_mutex;
> use kernel::page::PAGE_SIZE;
> use kernel::prelude::*;
> 
> -- 
> 2.47.2
> 
> 
> 

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

Re: [PATCH v6 09/18] rust: block: use `NullTerminatedFormatter`

[Daniel Almeida]

> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> Use the new `NullTerminatedFormatter` to write the name of a `GenDisk` to
> the name buffer. This new formatter automatically adds a trailing null
> marker after the written characters, so we don't need to append that at the
> call site any longer.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/block/mq/gen_disk.rs   | 12 +++++++-----
> rust/kernel/block/mq/raw_writer.rs |  1 +
> rust/kernel/str.rs                 |  1 -
> 3 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
> index 679ee1bb2195..20f1d46c774d 100644
> --- a/rust/kernel/block/mq/gen_disk.rs
> +++ b/rust/kernel/block/mq/gen_disk.rs
> @@ -7,9 +7,11 @@
> 
> use crate::{
>     bindings,
> -    block::mq::{raw_writer::RawWriter, Operations, TagSet},
> +    block::mq::{Operations, TagSet},
>     error::{self, from_err_ptr, Result},
> +    prelude::*,
>     static_lock_class,
> +    str::NullTerminatedFormatter,
>     sync::Arc,
> };
> use core::fmt::{self, Write};
> @@ -143,14 +145,14 @@ pub fn build<T: Operations>(
>         // SAFETY: `gendisk` is a valid pointer as we initialized it above
>         unsafe { (*gendisk).fops = &TABLE };
> 
> -        let mut raw_writer = RawWriter::from_array(
> +        let mut writer = NullTerminatedFormatter::new(
>             // SAFETY: `gendisk` points to a valid and initialized instance. We
>             // have exclusive access, since the disk is not added to the VFS
>             // yet.
>             unsafe { &mut (*gendisk).disk_name },
> -        )?;
> -        raw_writer.write_fmt(name)?;
> -        raw_writer.write_char('\0')?;
> +        )
> +        .ok_or(EINVAL)?;
> +        writer.write_fmt(name)?;
> 
>         // SAFETY: `gendisk` points to a valid and initialized instance of
>         // `struct gendisk`. `set_capacity` takes a lock to synchronize this
> diff --git a/rust/kernel/block/mq/raw_writer.rs b/rust/kernel/block/mq/raw_writer.rs
> index 7e2159e4f6a6..0aef55703e71 100644
> --- a/rust/kernel/block/mq/raw_writer.rs
> +++ b/rust/kernel/block/mq/raw_writer.rs
> @@ -24,6 +24,7 @@ fn new(buffer: &'a mut [u8]) -> Result<RawWriter<'a>> {
>         Ok(Self { buffer, pos: 0 })
>     }
> 
> +    #[expect(dead_code)]
>     pub(crate) fn from_array<const N: usize>(
>         a: &'a mut [crate::ffi::c_char; N],
>     ) -> Result<RawWriter<'a>> {
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index b185262b4851..a3e34f566034 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -886,7 +886,6 @@ pub(crate) struct NullTerminatedFormatter<'a> {
> 
> impl<'a> NullTerminatedFormatter<'a> {
>     /// Create a new [`Self`] instance.
> -    #[expect(dead_code)]
>     pub(crate) fn new(buffer: &'a mut [u8]) -> Option<NullTerminatedFormatter<'a>> {
>         *(buffer.first_mut()?) = 0;
> 
> 
> -- 
> 2.47.2
> 
> 
> 

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

Re: [PATCH v6 12/18] rust: block: add block related constants

[Daniel Almeida]

> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> Add a few block subsystem constants to the rust `kernel::block` name space.
> This makes it easier to access the constants from rust code.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/block.rs | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
> 
> diff --git a/rust/kernel/block.rs b/rust/kernel/block.rs
> index 150f710efe5b..32c8d865afb6 100644
> --- a/rust/kernel/block.rs
> +++ b/rust/kernel/block.rs
> @@ -3,3 +3,16 @@
> //! Types for working with the block layer.
> 
> pub mod mq;
> +
> +/// Bit mask for masking out [`SECTOR_SIZE`].
> +pub const SECTOR_MASK: u32 = bindings::SECTOR_MASK;
> +
> +/// Sectors are size `1 << SECTOR_SHIFT`.
> +pub const SECTOR_SHIFT: u32 = bindings::SECTOR_SHIFT;
> +
> +/// Size of a sector.
> +pub const SECTOR_SIZE: u32 = bindings::SECTOR_SIZE;
> +
> +/// The difference between the size of a page and the size of a sector,
> +/// expressed as a power of two.
> +pub const PAGE_SECTORS_SHIFT: u32 = bindings::PAGE_SECTORS_SHIFT;
> 
> -- 
> 2.47.2
> 
> 
> 

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

Re: [PATCH v6 14/18] rnull: enable configuration via `configfs`

[Daniel Almeida]

> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> Allow rust null block devices to be configured and instantiated via
> `configfs`.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> drivers/block/rnull/Kconfig      |   2 +-
> drivers/block/rnull/configfs.rs  | 207 +++++++++++++++++++++++++++++++++++++++
> drivers/block/rnull/rnull.rs     |  59 +++++------
> rust/kernel/block/mq/gen_disk.rs |   2 +-
> 4 files changed, 240 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/block/rnull/Kconfig b/drivers/block/rnull/Kconfig
> index 6dc5aff96bf4..7bc5b376c128 100644
> --- a/drivers/block/rnull/Kconfig
> +++ b/drivers/block/rnull/Kconfig
> @@ -4,7 +4,7 @@
> 
> config BLK_DEV_RUST_NULL
> tristate "Rust null block driver (Experimental)"
> - depends on RUST
> + depends on RUST && CONFIGFS_FS
> help
>  This is the Rust implementation of the null block driver. Like
>  the C version, the driver allows the user to create virutal block
> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
> new file mode 100644
> index 000000000000..46710a1e1af4
> --- /dev/null
> +++ b/drivers/block/rnull/configfs.rs
> @@ -0,0 +1,207 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use super::{NullBlkDevice, THIS_MODULE};
> +use core::fmt::Write;
> +use kernel::{
> +    block::mq::gen_disk::{GenDisk, GenDiskBuilder},
> +    c_str,
> +    configfs::{self, AttributeOperations},
> +    configfs_attrs, new_mutex,
> +    page::PAGE_SIZE,
> +    prelude::*,
> +    str::{kstrtobool_bytes, CString},
> +    sync::Mutex,
> +};
> +use pin_init::PinInit;
> +
> +pub(crate) fn subsystem() -> impl PinInit<kernel::configfs::Subsystem<Config>, Error> {
> +    let item_type = configfs_attrs! {
> +        container: configfs::Subsystem<Config>,
> +        data: Config,
> +        child: DeviceConfig,
> +        attributes: [
> +            features: 0,
> +        ],
> +    };
> +
> +    kernel::configfs::Subsystem::new(c_str!("rnull"), item_type, try_pin_init!(Config {}))
> +}
> +
> +#[pin_data]
> +pub(crate) struct Config {}
> +
> +#[vtable]
> +impl AttributeOperations<0> for Config {
> +    type Data = Config;
> +
> +    fn show(_this: &Config, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> +        let mut writer = kernel::str::Formatter::new(page);
> +        writer.write_str("blocksize,size,rotational\n")?;
> +        Ok(writer.bytes_written())
> +    }
> +}
> +
> +#[vtable]
> +impl configfs::GroupOperations for Config {
> +    type Child = DeviceConfig;
> +
> +    fn make_group(
> +        &self,
> +        name: &CStr,
> +    ) -> Result<impl PinInit<configfs::Group<DeviceConfig>, Error>> {
> +        let item_type = configfs_attrs! {
> +            container: configfs::Group<DeviceConfig>,
> +            data: DeviceConfig,
> +            attributes: [
> +                // Named for compatibility with C null_blk
> +                power: 0,
> +                blocksize: 1,
> +                rotational: 2,
> +                size: 3,
> +            ],
> +        };
> +
> +        Ok(configfs::Group::new(
> +            name.try_into()?,
> +            item_type,
> +            // TODO: cannot coerce new_mutex!() to impl PinInit<_, Error>, so put mutex inside
> +            try_pin_init!( DeviceConfig {
> +                data <- new_mutex!(DeviceConfigInner {
> +                    powered: false,
> +                    block_size: 4096,
> +                    rotational: false,
> +                    disk: None,
> +                    capacity_mib: 4096,
> +                    name: name.try_into()?,
> +                }),
> +            }),
> +        ))
> +    }
> +}
> +
> +#[pin_data]
> +pub(crate) struct DeviceConfig {
> +    #[pin]
> +    data: Mutex<DeviceConfigInner>,
> +}
> +
> +#[pin_data]
> +struct DeviceConfigInner {
> +    powered: bool,
> +    name: CString,
> +    block_size: u32,
> +    rotational: bool,
> +    capacity_mib: u64,
> +    disk: Option<GenDisk<NullBlkDevice>>,
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<0> for DeviceConfig {
> +    type Data = DeviceConfig;
> +
> +    fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> +        let mut writer = kernel::str::Formatter::new(page);
> +
> +        if this.data.lock().powered {
> +            writer.write_str("1\n")?;
> +        } else {
> +            writer.write_str("0\n")?;
> +        }
> +
> +        Ok(writer.bytes_written())
> +    }
> +
> +    fn store(this: &DeviceConfig, page: &[u8]) -> Result {
> +        let power_op = kstrtobool_bytes(page)?;
> +        let mut guard = this.data.lock();
> +
> +        if !guard.powered && power_op {
> +            guard.disk = Some(NullBlkDevice::new(
> +                &guard.name,
> +                guard.block_size,
> +                guard.rotational,
> +                guard.capacity_mib,
> +            )?);
> +            guard.powered = true;
> +        } else if guard.powered && !power_op {
> +            drop(guard.disk.take());
> +            guard.powered = false;
> +        }
> +
> +        Ok(())
> +    }
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<1> for DeviceConfig {
> +    type Data = DeviceConfig;
> +
> +    fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> +        let mut writer = kernel::str::Formatter::new(page);
> +        writer.write_fmt(fmt!("{}\n", this.data.lock().block_size))?;
> +        Ok(writer.bytes_written())
> +    }
> +
> +    fn store(this: &DeviceConfig, page: &[u8]) -> Result {
> +        if this.data.lock().powered {
> +            return Err(EBUSY);
> +        }
> +
> +        let text = core::str::from_utf8(page)?.trim();
> +        let value = text.parse::<u32>().map_err(|_| EINVAL)?;
> +
> +        GenDiskBuilder::validate_block_size(value)?;
> +        this.data.lock().block_size = value;
> +        Ok(())
> +    }
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<2> for DeviceConfig {
> +    type Data = DeviceConfig;
> +
> +    fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> +        let mut writer = kernel::str::Formatter::new(page);
> +
> +        if this.data.lock().rotational {
> +            writer.write_str("1\n")?;
> +        } else {
> +            writer.write_str("0\n")?;
> +        }
> +
> +        Ok(writer.bytes_written())
> +    }
> +
> +    fn store(this: &DeviceConfig, page: &[u8]) -> Result {
> +        if this.data.lock().powered {
> +            return Err(EBUSY);
> +        }
> +
> +        this.data.lock().rotational = kstrtobool_bytes(page)?;
> +
> +        Ok(())
> +    }
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<3> for DeviceConfig {
> +    type Data = DeviceConfig;
> +
> +    fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> +        let mut writer = kernel::str::Formatter::new(page);
> +        writer.write_fmt(fmt!("{}\n", this.data.lock().capacity_mib))?;
> +        Ok(writer.bytes_written())
> +    }
> +
> +    fn store(this: &DeviceConfig, page: &[u8]) -> Result {
> +        if this.data.lock().powered {
> +            return Err(EBUSY);
> +        }
> +
> +        let text = core::str::from_utf8(page)?.trim();
> +        let value = text.parse::<u64>().map_err(|_| EINVAL)?;
> +
> +        this.data.lock().capacity_mib = value;
> +        Ok(())
> +    }
> +}
> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
> index d07e76ae2c13..8690ff5f974f 100644
> --- a/drivers/block/rnull/rnull.rs
> +++ b/drivers/block/rnull/rnull.rs
> @@ -1,28 +1,25 @@
> // SPDX-License-Identifier: GPL-2.0
> 
> //! This is a Rust implementation of the C null block driver.
> -//!
> -//! Supported features:
> -//!
> -//! - blk-mq interface
> -//! - direct completion
> -//! - block size 4k
> -//!
> -//! The driver is not configurable.
> +
> +mod configfs;
> 
> use kernel::{
> -    alloc::flags,
> -    block::mq::{
> +    block::{
>         self,
> -        gen_disk::{self, GenDisk},
> -        Operations, TagSet,
> +        mq::{
> +            self,
> +            gen_disk::{self, GenDisk},
> +            Operations, TagSet,
> +        },
>     },
>     error::Result,
> -    new_mutex, pr_info,
> +    pr_info,
>     prelude::*,
> -    sync::{Arc, Mutex},
> +    sync::Arc,
>     types::ARef,
> };
> +use pin_init::PinInit;
> 
> module! {
>     type: NullBlkModule,
> @@ -35,33 +32,39 @@
> #[pin_data]
> struct NullBlkModule {
>     #[pin]
> -    _disk: Mutex<GenDisk<NullBlkDevice>>,
> +    configfs_subsystem: kernel::configfs::Subsystem<configfs::Config>,
> }
> 
> impl kernel::InPlaceModule for NullBlkModule {
>     fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
>         pr_info!("Rust null_blk loaded\n");
> 
> -        // Use a immediately-called closure as a stable `try` block
> -        let disk = /* try */ (|| {
> -            let tagset = Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
> -
> -            gen_disk::GenDiskBuilder::new()
> -                .capacity_sectors(4096 << 11)
> -                .logical_block_size(4096)?
> -                .physical_block_size(4096)?
> -                .rotational(false)
> -                .build(format_args!("rnullb{}", 0), tagset)
> -        })();
> -
>         try_pin_init!(Self {
> -            _disk <- new_mutex!(disk?, "nullb:disk"),
> +            configfs_subsystem <- configfs::subsystem(),
>         })
>     }
> }
> 
> struct NullBlkDevice;
> 
> +impl NullBlkDevice {
> +    fn new(
> +        name: &CStr,
> +        block_size: u32,
> +        rotational: bool,
> +        capacity_mib: u64,
> +    ) -> Result<GenDisk<Self>> {
> +        let tagset = Arc::pin_init(TagSet::new(1, 256, 1), GFP_KERNEL)?;
> +
> +        gen_disk::GenDiskBuilder::new()
> +            .capacity_sectors(capacity_mib << (20 - block::SECTOR_SHIFT))
> +            .logical_block_size(block_size)?
> +            .physical_block_size(block_size)?
> +            .rotational(rotational)
> +            .build(fmt!("{}", name.to_str()?), tagset)
> +    }
> +}
> +
> #[vtable]
> impl Operations for NullBlkDevice {
>     #[inline(always)]
> diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
> index 20f1d46c774d..6b1b846874db 100644
> --- a/rust/kernel/block/mq/gen_disk.rs
> +++ b/rust/kernel/block/mq/gen_disk.rs
> @@ -51,7 +51,7 @@ pub fn rotational(mut self, rotational: bool) -> Self {
> 
>     /// Validate block size by verifying that it is between 512 and `PAGE_SIZE`,
>     /// and that it is a power of two.
> -    fn validate_block_size(size: u32) -> Result {
> +    pub fn validate_block_size(size: u32) -> Result {
>         if !(512..=bindings::PAGE_SIZE as u32).contains(&size) || !size.is_power_of_two() {
>             Err(error::code::EINVAL)
>         } else {
> 
> -- 
> 2.47.2
> 
> 
> 

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

Re: [PATCH v6 15/18] rust: block: add `GenDisk` private data support

[Daniel Almeida]

> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> Allow users of the rust block device driver API to install private data in
> the `GenDisk` structure.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> drivers/block/rnull/rnull.rs       |  8 ++++---
> rust/kernel/block/mq.rs            |  7 +++---
> rust/kernel/block/mq/gen_disk.rs   | 32 ++++++++++++++++++++++----
> rust/kernel/block/mq/operations.rs | 46 ++++++++++++++++++++++++++++++--------
> 4 files changed, 74 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
> index 8690ff5f974f..8255236bc550 100644
> --- a/drivers/block/rnull/rnull.rs
> +++ b/drivers/block/rnull/rnull.rs
> @@ -61,14 +61,16 @@ fn new(
>             .logical_block_size(block_size)?
>             .physical_block_size(block_size)?
>             .rotational(rotational)
> -            .build(fmt!("{}", name.to_str()?), tagset)
> +            .build(fmt!("{}", name.to_str()?), tagset, ())
>     }
> }
> 
> #[vtable]
> impl Operations for NullBlkDevice {
> +    type QueueData = ();
> +
>     #[inline(always)]
> -    fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
> +    fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
>         mq::Request::end_ok(rq)
>             .map_err(|_e| kernel::error::code::EIO)
>             // We take no refcounts on the request, so we expect to be able to
> @@ -79,5 +81,5 @@ fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
>         Ok(())
>     }
> 
> -    fn commit_rqs() {}
> +    fn commit_rqs(_queue_data: ()) {}
> }
> diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
> index 98fa0d6bc8f7..6e546f4f3d1c 100644
> --- a/rust/kernel/block/mq.rs
> +++ b/rust/kernel/block/mq.rs
> @@ -69,20 +69,21 @@
> //!
> //! #[vtable]
> //! impl Operations for MyBlkDevice {
> +//!     type QueueData = ();
> //!
> -//!     fn queue_rq(rq: ARef<Request<Self>>, _is_last: bool) -> Result {
> +//!     fn queue_rq(_queue_data: (), rq: ARef<Request<Self>>, _is_last: bool) -> Result {
> //!         Request::end_ok(rq);
> //!         Ok(())
> //!     }
> //!
> -//!     fn commit_rqs() {}
> +//!     fn commit_rqs(_queue_data: ()) {}
> //! }
> //!
> //! let tagset: Arc<TagSet<MyBlkDevice>> =
> //!     Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
> //! let mut disk = gen_disk::GenDiskBuilder::new()
> //!     .capacity_sectors(4096)
> -//!     .build(format_args!("myblk"), tagset)?;
> +//!     .build(format_args!("myblk"), tagset, ())?;
> //!
> //! # Ok::<(), kernel::error::Error>(())
> //! ```
> diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
> index 6b1b846874db..46ec80269970 100644
> --- a/rust/kernel/block/mq/gen_disk.rs
> +++ b/rust/kernel/block/mq/gen_disk.rs
> @@ -13,6 +13,7 @@
>     static_lock_class,
>     str::NullTerminatedFormatter,
>     sync::Arc,
> +    types::{ForeignOwnable, ScopeGuard},
> };
> use core::fmt::{self, Write};
> 
> @@ -98,7 +99,14 @@ pub fn build<T: Operations>(
>         self,
>         name: fmt::Arguments<'_>,
>         tagset: Arc<TagSet<T>>,
> +        queue_data: T::QueueData,
>     ) -> Result<GenDisk<T>> {
> +        let data = queue_data.into_foreign();
> +        let recover_data = ScopeGuard::new(|| {
> +            // SAFETY: T::QueueData was created by the call to `into_foreign()` above
> +            drop(unsafe { T::QueueData::from_foreign(data) });
> +        });
> +
>         // SAFETY: `bindings::queue_limits` contain only fields that are valid when zeroed.
>         let mut lim: bindings::queue_limits = unsafe { core::mem::zeroed() };
> 
> @@ -113,7 +121,7 @@ pub fn build<T: Operations>(
>             bindings::__blk_mq_alloc_disk(
>                 tagset.raw_tag_set(),
>                 &mut lim,
> -                core::ptr::null_mut(),
> +                data,
>                 static_lock_class!().as_ptr(),
>             )
>         })?;
> @@ -167,8 +175,12 @@ pub fn build<T: Operations>(
>             },
>         )?;
> 
> +        recover_data.dismiss();
> +
>         // INVARIANT: `gendisk` was initialized above.
>         // INVARIANT: `gendisk` was added to the VFS via `device_add_disk` above.
> +        // INVARIANT: `gendisk.queue.queue_data` is set to `data` in the call to
> +        // `__blk_mq_alloc_disk` above.
>         Ok(GenDisk {
>             _tagset: tagset,
>             gendisk,
> @@ -180,9 +192,10 @@ pub fn build<T: Operations>(
> ///
> /// # Invariants
> ///
> -/// - `gendisk` must always point to an initialized and valid `struct gendisk`.
> -/// - `gendisk` was added to the VFS through a call to
> -///   `bindings::device_add_disk`.
> +///  - `gendisk` must always point to an initialized and valid `struct gendisk`.
> +///  - `gendisk` was added to the VFS through a call to
> +///    `bindings::device_add_disk`.
> +///  - `self.gendisk.queue.queuedata` is initialized by a call to `ForeignOwnable::into_foreign`.
> pub struct GenDisk<T: Operations> {
>     _tagset: Arc<TagSet<T>>,
>     gendisk: *mut bindings::gendisk,
> @@ -194,9 +207,20 @@ unsafe impl<T: Operations + Send> Send for GenDisk<T> {}
> 
> impl<T: Operations> Drop for GenDisk<T> {
>     fn drop(&mut self) {
> +        // SAFETY: By type invariant of `Self`, `self.gendisk` points to a valid
> +        // and initialized instance of `struct gendisk`, and, `queuedata` was
> +        // initialized with the result of a call to
> +        // `ForeignOwnable::into_foreign`.
> +        let queue_data = unsafe { (*(*self.gendisk).queue).queuedata };
> +
>         // SAFETY: By type invariant, `self.gendisk` points to a valid and
>         // initialized instance of `struct gendisk`, and it was previously added
>         // to the VFS.
>         unsafe { bindings::del_gendisk(self.gendisk) };
> +
> +        // SAFETY: `queue.queuedata` was created by `GenDiskBuilder::build` with
> +        // a call to `ForeignOwnable::into_foreign` to create `queuedata`.
> +        // `ForeignOwnable::from_foreign` is only called here.
> +        drop(unsafe { T::QueueData::from_foreign(queue_data) });
>     }
> }
> diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
> index c2b98f507bcb..6fb256f55acc 100644
> --- a/rust/kernel/block/mq/operations.rs
> +++ b/rust/kernel/block/mq/operations.rs
> @@ -6,14 +6,15 @@
> 
> use crate::{
>     bindings,
> -    block::mq::request::RequestDataWrapper,
> -    block::mq::Request,
> +    block::mq::{request::RequestDataWrapper, Request},
>     error::{from_result, Result},
>     prelude::*,
> -    types::ARef,
> +    types::{ARef, ForeignOwnable},
> };
> use core::{marker::PhantomData, sync::atomic::AtomicU64, sync::atomic::Ordering};
> 
> +type ForeignBorrowed<'a, T> = <T as ForeignOwnable>::Borrowed<'a>;
> +
> /// Implement this trait to interface blk-mq as block devices.
> ///
> /// To implement a block device driver, implement this trait as described in the
> @@ -26,12 +27,20 @@
> /// [module level documentation]: kernel::block::mq
> #[macros::vtable]
> pub trait Operations: Sized {
> +    /// Data associated with the `struct request_queue` that is allocated for
> +    /// the `GenDisk` associated with this `Operations` implementation.
> +    type QueueData: ForeignOwnable;
> +
>     /// Called by the kernel to queue a request with the driver. If `is_last` is
>     /// `false`, the driver is allowed to defer committing the request.
> -    fn queue_rq(rq: ARef<Request<Self>>, is_last: bool) -> Result;
> +    fn queue_rq(
> +        queue_data: ForeignBorrowed<'_, Self::QueueData>,
> +        rq: ARef<Request<Self>>,
> +        is_last: bool,
> +    ) -> Result;
> 
>     /// Called by the kernel to indicate that queued requests should be submitted.
> -    fn commit_rqs();
> +    fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>);
> 
>     /// Called by the kernel to poll the device for completed requests. Only
>     /// used for poll queues.
> @@ -70,7 +79,7 @@ impl<T: Operations> OperationsVTable<T> {
>     ///   promise to not access the request until the driver calls
>     ///   `bindings::blk_mq_end_request` for the request.
>     unsafe extern "C" fn queue_rq_callback(
> -        _hctx: *mut bindings::blk_mq_hw_ctx,
> +        hctx: *mut bindings::blk_mq_hw_ctx,
>         bd: *const bindings::blk_mq_queue_data,
>     ) -> bindings::blk_status_t {
>         // SAFETY: `bd.rq` is valid as required by the safety requirement for
> @@ -88,10 +97,20 @@ impl<T: Operations> OperationsVTable<T> {
>         //    reference counted by `ARef` until then.
>         let rq = unsafe { Request::aref_from_raw((*bd).rq) };
> 
> +        // SAFETY: `hctx` is valid as required by this function.
> +        let queue_data = unsafe { (*(*hctx).queue).queuedata };
> +
> +        // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a

isn’t this set on build() ?

> +        // call to `ForeignOwnable::into_pointer()` to create `queuedata`.

into_pointer() ?

> +        // `ForeignOwnable::from_foreign()` is only called when the tagset is
> +        // dropped, which happens after we are dropped.
> +        let queue_data = unsafe { T::QueueData::borrow(queue_data) };
> +
>         // SAFETY: We have exclusive access and we just set the refcount above.
>         unsafe { Request::start_unchecked(&rq) };
> 
>         let ret = T::queue_rq(
> +            queue_data,
>             rq,
>             // SAFETY: `bd` is valid as required by the safety requirement for
>             // this function.
> @@ -110,9 +129,18 @@ impl<T: Operations> OperationsVTable<T> {
>     ///
>     /// # Safety
>     ///
> -    /// This function may only be called by blk-mq C infrastructure.
> -    unsafe extern "C" fn commit_rqs_callback(_hctx: *mut bindings::blk_mq_hw_ctx) {
> -        T::commit_rqs()
> +    /// This function may only be called by blk-mq C infrastructure. The caller
> +    /// must ensure that `hctx` is valid.
> +    unsafe extern "C" fn commit_rqs_callback(hctx: *mut bindings::blk_mq_hw_ctx) {
> +        // SAFETY: `hctx` is valid as required by this function.
> +        let queue_data = unsafe { (*(*hctx).queue).queuedata };
> +
> +        // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a
> +        // call to `ForeignOwnable::into_pointer()` to create `queuedata`.

into_foreign()?

> +        // `ForeignOwnable::from_foreign()` is only called when the tagset is
> +        // dropped, which happens after we are dropped.
> +        let queue_data = unsafe { T::QueueData::borrow(queue_data) };
> +        T::commit_rqs(queue_data)
>     }
> 
>     /// This function is called by the C kernel. It is not currently
> 
> -- 
> 2.47.2
> 
> 
> 

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

Re: [PATCH v6 17/18] rust: block: add remote completion to `Request`

[Daniel Almeida]

Hi Andreas,

> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> Allow users of rust block device driver API to schedule completion of
> requests via `blk_mq_complete_request_remote`.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> drivers/block/rnull/rnull.rs       |  9 +++++++++
> rust/kernel/block/mq.rs            |  6 ++++++
> rust/kernel/block/mq/operations.rs | 19 +++++++++++++++----
> rust/kernel/block/mq/request.rs    | 17 +++++++++++++++++
> 4 files changed, 47 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
> index 8255236bc550..a19c55717c4f 100644
> --- a/drivers/block/rnull/rnull.rs
> +++ b/drivers/block/rnull/rnull.rs
> @@ -82,4 +82,13 @@ fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Res
>     }
> 
>     fn commit_rqs(_queue_data: ()) {}
> +
> +    fn complete(rq: ARef<mq::Request<Self>>) {
> +        mq::Request::end_ok(rq)
> +            .map_err(|_e| kernel::error::code::EIO)
> +            // We take no refcounts on the request, so we expect to be able to
> +            // end the request. The request reference must be unique at this
> +            // point, and so `end_ok` cannot fail.
> +            .expect("Fatal error - expected to be able to end request");
> +    }
> }
> diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
> index 6e546f4f3d1c..c0ec06b84355 100644
> --- a/rust/kernel/block/mq.rs
> +++ b/rust/kernel/block/mq.rs
> @@ -77,6 +77,12 @@
> //!     }
> //!
> //!     fn commit_rqs(_queue_data: ()) {}
> +//!
> +//!     fn complete(rq: ARef<Request<Self>>) {
> +//!         Request::end_ok(rq)
> +//!             .map_err(|_e| kernel::error::code::EIO)
> +//!             .expect("Fatal error - expected to be able to end request");
> +//!     }
> //! }
> //!
> //! let tagset: Arc<TagSet<MyBlkDevice>> =
> diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
> index 6fb256f55acc..0fece577de7c 100644
> --- a/rust/kernel/block/mq/operations.rs
> +++ b/rust/kernel/block/mq/operations.rs
> @@ -42,6 +42,9 @@ fn queue_rq(
>     /// Called by the kernel to indicate that queued requests should be submitted.
>     fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>);
> 
> +    /// Called by the kernel when the request is completed.
> +    fn complete(rq: ARef<Request<Self>>);
> +
>     /// Called by the kernel to poll the device for completed requests. Only
>     /// used for poll queues.
>     fn poll() -> bool {
> @@ -143,13 +146,21 @@ impl<T: Operations> OperationsVTable<T> {
>         T::commit_rqs(queue_data)
>     }
> 
> -    /// This function is called by the C kernel. It is not currently
> -    /// implemented, and there is no way to exercise this code path.
> +    /// This function is called by the C kernel. A pointer to this function is
> +    /// installed in the `blk_mq_ops` vtable for the driver.
>     ///
>     /// # Safety
>     ///
> -    /// This function may only be called by blk-mq C infrastructure.
> -    unsafe extern "C" fn complete_callback(_rq: *mut bindings::request) {}
> +    /// This function may only be called by blk-mq C infrastructure. `rq` must
> +    /// point to a valid request that has been marked as completed. The pointee
> +    /// of `rq` must be valid for write for the duration of this function.
> +    unsafe extern "C" fn complete_callback(rq: *mut bindings::request) {
> +        // SAFETY: This function can only be dispatched through
> +        // `Request::complete`. We leaked a refcount then which we pick back up
> +        // now.
> +        let aref = unsafe { Request::aref_from_raw(rq) };
> +        T::complete(aref);
> +    }
> 
>     /// This function is called by the C kernel. A pointer to this function is
>     /// installed in the `blk_mq_ops` vtable for the driver.
> diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
> index 3848cfe63f77..f7f757f7459f 100644
> --- a/rust/kernel/block/mq/request.rs
> +++ b/rust/kernel/block/mq/request.rs
> @@ -135,6 +135,23 @@ pub fn end_ok(this: ARef<Self>) -> Result<(), ARef<Self>> {
>         Ok(())
>     }
> 
> +    /// Complete the request by scheduling `Operations::complete` for
> +    /// execution.
> +    ///
> +    /// The function may be scheduled locally, via SoftIRQ or remotely via IPMI.
> +    /// See `blk_mq_complete_request_remote` in [`blk-mq.c`] for details.
> +    ///
> +    /// [`blk-mq.c`]: srctree/block/blk-mq.c
> +    pub fn complete(this: ARef<Self>) {
> +        let ptr = ARef::into_raw(this).cast::<bindings::request>().as_ptr();
> +        // SAFETY: By type invariant, `self.0` is a valid `struct request`
> +        if !unsafe { bindings::blk_mq_complete_request_remote(ptr) } {
> +            // SAFETY: We released a refcount above that we can reclaim here.
> +            let this = unsafe { Request::aref_from_raw(ptr) };
> +            T::complete(this);
> +        }
> +    }
> +
>     /// Return a pointer to the [`RequestDataWrapper`] stored in the private area
>     /// of the request structure.
>     ///
> 
> -- 
> 2.47.2
> 
> 

I had another look here. While I do trust your reasoning, perhaps we should
remove the call to expect()?

If it is not called ever as you said, great, removing the expect() will not
change the code behavior. If it is, be it because of some minor oversight or
unexpected condition, we should produce some error output instead of crashing
the kernel. Maybe we should use a warn() here instead? Or maybe dev/pr_err as
applicable?

— Daniel

Re: [PATCH v6 18/18] rnull: add soft-irq completion support

[Daniel Almeida]

> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> rnull currently only supports direct completion. Add option for completing
> requests across CPU nodes via soft IRQ or IPI.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> drivers/block/rnull/configfs.rs | 59 +++++++++++++++++++++++++++++++++++++++--
> drivers/block/rnull/rnull.rs    | 32 ++++++++++++++--------
> 2 files changed, 78 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
> index 46710a1e1af4..8498e9bae6fd 100644
> --- a/drivers/block/rnull/configfs.rs
> +++ b/drivers/block/rnull/configfs.rs
> @@ -1,7 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> 
> use super::{NullBlkDevice, THIS_MODULE};
> -use core::fmt::Write;
> +use core::fmt::{Display, Write};
> use kernel::{
>     block::mq::gen_disk::{GenDisk, GenDiskBuilder},
>     c_str,
> @@ -36,7 +36,7 @@ impl AttributeOperations<0> for Config {
> 
>     fn show(_this: &Config, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
>         let mut writer = kernel::str::Formatter::new(page);
> -        writer.write_str("blocksize,size,rotational\n")?;
> +        writer.write_str("blocksize,size,rotational,irqmode\n")?;
>         Ok(writer.bytes_written())
>     }
> }
> @@ -58,6 +58,7 @@ fn make_group(
>                 blocksize: 1,
>                 rotational: 2,
>                 size: 3,
> +                irqmode: 4,
>             ],
>         };
> 
> @@ -72,6 +73,7 @@ fn make_group(
>                     rotational: false,
>                     disk: None,
>                     capacity_mib: 4096,
> +                    irq_mode: IRQMode::None,
>                     name: name.try_into()?,
>                 }),
>             }),
> @@ -79,6 +81,34 @@ fn make_group(
>     }
> }
> 
> +#[derive(Debug, Clone, Copy)]
> +pub(crate) enum IRQMode {
> +    None,
> +    Soft,
> +}
> +
> +impl TryFrom<u8> for IRQMode {
> +    type Error = kernel::error::Error;
> +
> +    fn try_from(value: u8) -> Result<Self> {
> +        match value {
> +            0 => Ok(Self::None),
> +            1 => Ok(Self::Soft),
> +            _ => Err(EINVAL),
> +        }
> +    }
> +}
> +
> +impl Display for IRQMode {
> +    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
> +        match self {
> +            Self::None => f.write_str("0")?,
> +            Self::Soft => f.write_str("1")?,
> +        }
> +        Ok(())
> +    }
> +}
> +
> #[pin_data]
> pub(crate) struct DeviceConfig {
>     #[pin]
> @@ -92,6 +122,7 @@ struct DeviceConfigInner {
>     block_size: u32,
>     rotational: bool,
>     capacity_mib: u64,
> +    irq_mode: IRQMode,
>     disk: Option<GenDisk<NullBlkDevice>>,
> }
> 
> @@ -121,6 +152,7 @@ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
>                 guard.block_size,
>                 guard.rotational,
>                 guard.capacity_mib,
> +                guard.irq_mode,
>             )?);
>             guard.powered = true;
>         } else if guard.powered && !power_op {
> @@ -205,3 +237,26 @@ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
>         Ok(())
>     }
> }
> +
> +#[vtable]
> +impl configfs::AttributeOperations<4> for DeviceConfig {
> +    type Data = DeviceConfig;
> +
> +    fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> +        let mut writer = kernel::str::Formatter::new(page);
> +        writer.write_fmt(fmt!("{}\n", this.data.lock().irq_mode))?;
> +        Ok(writer.bytes_written())
> +    }
> +
> +    fn store(this: &DeviceConfig, page: &[u8]) -> Result {
> +        if this.data.lock().powered {
> +            return Err(EBUSY);
> +        }
> +
> +        let text = core::str::from_utf8(page)?.trim();
> +        let value = text.parse::<u8>().map_err(|_| EINVAL)?;
> +
> +        this.data.lock().irq_mode = IRQMode::try_from(value)?;
> +        Ok(())
> +    }
> +}
> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
> index a19c55717c4f..1ec694d7f1a6 100644
> --- a/drivers/block/rnull/rnull.rs
> +++ b/drivers/block/rnull/rnull.rs
> @@ -4,6 +4,7 @@
> 
> mod configfs;
> 
> +use configfs::IRQMode;
> use kernel::{
>     block::{
>         self,
> @@ -53,35 +54,44 @@ fn new(
>         block_size: u32,
>         rotational: bool,
>         capacity_mib: u64,
> +        irq_mode: IRQMode,
>     ) -> Result<GenDisk<Self>> {
>         let tagset = Arc::pin_init(TagSet::new(1, 256, 1), GFP_KERNEL)?;
> 
> +        let queue_data = Box::new(QueueData { irq_mode }, GFP_KERNEL)?;
> +
>         gen_disk::GenDiskBuilder::new()
>             .capacity_sectors(capacity_mib << (20 - block::SECTOR_SHIFT))
>             .logical_block_size(block_size)?
>             .physical_block_size(block_size)?
>             .rotational(rotational)
> -            .build(fmt!("{}", name.to_str()?), tagset, ())
> +            .build(fmt!("{}", name.to_str()?), tagset, queue_data)
>     }
> }
> 
> +struct QueueData {
> +    irq_mode: IRQMode,
> +}
> +
> #[vtable]
> impl Operations for NullBlkDevice {
> -    type QueueData = ();
> +    type QueueData = KBox<QueueData>;
> 
>     #[inline(always)]
> -    fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
> -        mq::Request::end_ok(rq)
> -            .map_err(|_e| kernel::error::code::EIO)
> -            // We take no refcounts on the request, so we expect to be able to
> -            // end the request. The request reference must be unique at this
> -            // point, and so `end_ok` cannot fail.
> -            .expect("Fatal error - expected to be able to end request");

Same comment here as on the last patch. Otherwise LGTM.

> -
> +    fn queue_rq(queue_data: &QueueData, rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
> +        match queue_data.irq_mode {
> +            IRQMode::None => mq::Request::end_ok(rq)
> +                .map_err(|_e| kernel::error::code::EIO)
> +                // We take no refcounts on the request, so we expect to be able to
> +                // end the request. The request reference must be unique at this
> +                // point, and so `end_ok` cannot fail.
> +                .expect("Fatal error - expected to be able to end request"),
> +            IRQMode::Soft => mq::Request::complete(rq),
> +        }
>         Ok(())
>     }
> 
> -    fn commit_rqs(_queue_data: ()) {}
> +    fn commit_rqs(_queue_data: &QueueData) {}
> 
>     fn complete(rq: ARef<mq::Request<Self>>) {
>         mq::Request::end_ok(rq)
> 
> -- 
> 2.47.2
> 
> 
> 

Re: [PATCH v6 17/18] rust: block: add remote completion to `Request`

[Andreas Hindborg]

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

> Hi Andreas,
>
>> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> Allow users of rust block device driver API to schedule completion of
>> requests via `blk_mq_complete_request_remote`.
>>
>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>> drivers/block/rnull/rnull.rs       |  9 +++++++++
>> rust/kernel/block/mq.rs            |  6 ++++++
>> rust/kernel/block/mq/operations.rs | 19 +++++++++++++++----
>> rust/kernel/block/mq/request.rs    | 17 +++++++++++++++++
>> 4 files changed, 47 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
>> index 8255236bc550..a19c55717c4f 100644
>> --- a/drivers/block/rnull/rnull.rs
>> +++ b/drivers/block/rnull/rnull.rs
>> @@ -82,4 +82,13 @@ fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Res
>>     }
>>
>>     fn commit_rqs(_queue_data: ()) {}
>> +
>> +    fn complete(rq: ARef<mq::Request<Self>>) {
>> +        mq::Request::end_ok(rq)
>> +            .map_err(|_e| kernel::error::code::EIO)
>> +            // We take no refcounts on the request, so we expect to be able to
>> +            // end the request. The request reference must be unique at this
>> +            // point, and so `end_ok` cannot fail.
>> +            .expect("Fatal error - expected to be able to end request");
>> +    }
>> }
>> diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
>> index 6e546f4f3d1c..c0ec06b84355 100644
>> --- a/rust/kernel/block/mq.rs
>> +++ b/rust/kernel/block/mq.rs
>> @@ -77,6 +77,12 @@
>> //!     }
>> //!
>> //!     fn commit_rqs(_queue_data: ()) {}
>> +//!
>> +//!     fn complete(rq: ARef<Request<Self>>) {
>> +//!         Request::end_ok(rq)
>> +//!             .map_err(|_e| kernel::error::code::EIO)
>> +//!             .expect("Fatal error - expected to be able to end request");
>> +//!     }
>> //! }
>> //!
>> //! let tagset: Arc<TagSet<MyBlkDevice>> =
>> diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
>> index 6fb256f55acc..0fece577de7c 100644
>> --- a/rust/kernel/block/mq/operations.rs
>> +++ b/rust/kernel/block/mq/operations.rs
>> @@ -42,6 +42,9 @@ fn queue_rq(
>>     /// Called by the kernel to indicate that queued requests should be submitted.
>>     fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>);
>>
>> +    /// Called by the kernel when the request is completed.
>> +    fn complete(rq: ARef<Request<Self>>);
>> +
>>     /// Called by the kernel to poll the device for completed requests. Only
>>     /// used for poll queues.
>>     fn poll() -> bool {
>> @@ -143,13 +146,21 @@ impl<T: Operations> OperationsVTable<T> {
>>         T::commit_rqs(queue_data)
>>     }
>>
>> -    /// This function is called by the C kernel. It is not currently
>> -    /// implemented, and there is no way to exercise this code path.
>> +    /// This function is called by the C kernel. A pointer to this function is
>> +    /// installed in the `blk_mq_ops` vtable for the driver.
>>     ///
>>     /// # Safety
>>     ///
>> -    /// This function may only be called by blk-mq C infrastructure.
>> -    unsafe extern "C" fn complete_callback(_rq: *mut bindings::request) {}
>> +    /// This function may only be called by blk-mq C infrastructure. `rq` must
>> +    /// point to a valid request that has been marked as completed. The pointee
>> +    /// of `rq` must be valid for write for the duration of this function.
>> +    unsafe extern "C" fn complete_callback(rq: *mut bindings::request) {
>> +        // SAFETY: This function can only be dispatched through
>> +        // `Request::complete`. We leaked a refcount then which we pick back up
>> +        // now.
>> +        let aref = unsafe { Request::aref_from_raw(rq) };
>> +        T::complete(aref);
>> +    }
>>
>>     /// This function is called by the C kernel. A pointer to this function is
>>     /// installed in the `blk_mq_ops` vtable for the driver.
>> diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
>> index 3848cfe63f77..f7f757f7459f 100644
>> --- a/rust/kernel/block/mq/request.rs
>> +++ b/rust/kernel/block/mq/request.rs
>> @@ -135,6 +135,23 @@ pub fn end_ok(this: ARef<Self>) -> Result<(), ARef<Self>> {
>>         Ok(())
>>     }
>>
>> +    /// Complete the request by scheduling `Operations::complete` for
>> +    /// execution.
>> +    ///
>> +    /// The function may be scheduled locally, via SoftIRQ or remotely via IPMI.
>> +    /// See `blk_mq_complete_request_remote` in [`blk-mq.c`] for details.
>> +    ///
>> +    /// [`blk-mq.c`]: srctree/block/blk-mq.c
>> +    pub fn complete(this: ARef<Self>) {
>> +        let ptr = ARef::into_raw(this).cast::<bindings::request>().as_ptr();
>> +        // SAFETY: By type invariant, `self.0` is a valid `struct request`
>> +        if !unsafe { bindings::blk_mq_complete_request_remote(ptr) } {
>> +            // SAFETY: We released a refcount above that we can reclaim here.
>> +            let this = unsafe { Request::aref_from_raw(ptr) };
>> +            T::complete(this);
>> +        }
>> +    }
>> +
>>     /// Return a pointer to the [`RequestDataWrapper`] stored in the private area
>>     /// of the request structure.
>>     ///
>>
>> --
>> 2.47.2
>>
>>
>
> I had another look here. While I do trust your reasoning, perhaps we should
> remove the call to expect()?
>
> If it is not called ever as you said, great, removing the expect() will not
> change the code behavior. If it is, be it because of some minor oversight or
> unexpected condition, we should produce some error output instead of crashing
> the kernel. Maybe we should use a warn() here instead? Or maybe dev/pr_err as
> applicable?

I think for the example, I would like to keep the `expect`. For
demonstration purposes.

We could do `warn!` instead for the rnull driver I guess. But the IO
queue that would hit this code would start to hang pretty fast, since no
IO would complete. I don't think the kernel can recover from this hang.


Best regards,
Andreas Hindborg

Re: [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function

[Andreas Hindborg]

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

>> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:

<cut>

>> -    kernel::error::to_result(ret).map(|()| result)
>> +/// Convert `&[u8]` to `bool` by deferring to [`kernel::str::kstrtobool`].
>> +///
>> +/// Only considers at most the first two bytes of `bytes`.
>> +pub fn kstrtobool_bytes(bytes: &[u8]) -> Result<bool> {
>> +    // `ktostrbool` only considers the first two bytes of the input.
>> +    let stack_string = [*bytes.first().unwrap_or(&0), *bytes.get(1).unwrap_or(&0), 0];
>
> Can’t this be CStr::from_bytes_with_nul() ?
>
> This means that kstrtobool_raw could take a &CStr directly and thus not be unsafe IIUC?

By design, the input to this function need not be null terminated. My
use case is parsing the contents of a configfs file, and I would not
want to change the contents of the file, or allocate to create a null
terminated string, before calling this method.

We could add another function `kstrtobool_cstr` to do what you are
asking, but I think that could be a separate patch.


Best regards,
Andreas Hindborg

Re: [PATCH v6 15/18] rust: block: add `GenDisk` private data support

[Andreas Hindborg]

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

>> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> Allow users of the rust block device driver API to install private data in
>> the `GenDisk` structure.
>>
>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>> drivers/block/rnull/rnull.rs       |  8 ++++---
>> rust/kernel/block/mq.rs            |  7 +++---
>> rust/kernel/block/mq/gen_disk.rs   | 32 ++++++++++++++++++++++----
>> rust/kernel/block/mq/operations.rs | 46 ++++++++++++++++++++++++++++++--------
>> 4 files changed, 74 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
>> index 8690ff5f974f..8255236bc550 100644
>> --- a/drivers/block/rnull/rnull.rs
>> +++ b/drivers/block/rnull/rnull.rs
>> @@ -61,14 +61,16 @@ fn new(
>>             .logical_block_size(block_size)?
>>             .physical_block_size(block_size)?
>>             .rotational(rotational)
>> -            .build(fmt!("{}", name.to_str()?), tagset)
>> +            .build(fmt!("{}", name.to_str()?), tagset, ())
>>     }
>> }
>>
>> #[vtable]
>> impl Operations for NullBlkDevice {
>> +    type QueueData = ();
>> +
>>     #[inline(always)]
>> -    fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
>> +    fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
>>         mq::Request::end_ok(rq)
>>             .map_err(|_e| kernel::error::code::EIO)
>>             // We take no refcounts on the request, so we expect to be able to
>> @@ -79,5 +81,5 @@ fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
>>         Ok(())
>>     }
>>
>> -    fn commit_rqs() {}
>> +    fn commit_rqs(_queue_data: ()) {}
>> }
>> diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
>> index 98fa0d6bc8f7..6e546f4f3d1c 100644
>> --- a/rust/kernel/block/mq.rs
>> +++ b/rust/kernel/block/mq.rs
>> @@ -69,20 +69,21 @@
>> //!
>> //! #[vtable]
>> //! impl Operations for MyBlkDevice {
>> +//!     type QueueData = ();
>> //!
>> -//!     fn queue_rq(rq: ARef<Request<Self>>, _is_last: bool) -> Result {
>> +//!     fn queue_rq(_queue_data: (), rq: ARef<Request<Self>>, _is_last: bool) -> Result {
>> //!         Request::end_ok(rq);
>> //!         Ok(())
>> //!     }
>> //!
>> -//!     fn commit_rqs() {}
>> +//!     fn commit_rqs(_queue_data: ()) {}
>> //! }
>> //!
>> //! let tagset: Arc<TagSet<MyBlkDevice>> =
>> //!     Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
>> //! let mut disk = gen_disk::GenDiskBuilder::new()
>> //!     .capacity_sectors(4096)
>> -//!     .build(format_args!("myblk"), tagset)?;
>> +//!     .build(format_args!("myblk"), tagset, ())?;
>> //!
>> //! # Ok::<(), kernel::error::Error>(())
>> //! ```
>> diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
>> index 6b1b846874db..46ec80269970 100644
>> --- a/rust/kernel/block/mq/gen_disk.rs
>> +++ b/rust/kernel/block/mq/gen_disk.rs
>> @@ -13,6 +13,7 @@
>>     static_lock_class,
>>     str::NullTerminatedFormatter,
>>     sync::Arc,
>> +    types::{ForeignOwnable, ScopeGuard},
>> };
>> use core::fmt::{self, Write};
>>
>> @@ -98,7 +99,14 @@ pub fn build<T: Operations>(
>>         self,
>>         name: fmt::Arguments<'_>,
>>         tagset: Arc<TagSet<T>>,
>> +        queue_data: T::QueueData,
>>     ) -> Result<GenDisk<T>> {
>> +        let data = queue_data.into_foreign();
>> +        let recover_data = ScopeGuard::new(|| {
>> +            // SAFETY: T::QueueData was created by the call to `into_foreign()` above
>> +            drop(unsafe { T::QueueData::from_foreign(data) });
>> +        });
>> +
>>         // SAFETY: `bindings::queue_limits` contain only fields that are valid when zeroed.
>>         let mut lim: bindings::queue_limits = unsafe { core::mem::zeroed() };
>>
>> @@ -113,7 +121,7 @@ pub fn build<T: Operations>(
>>             bindings::__blk_mq_alloc_disk(
>>                 tagset.raw_tag_set(),
>>                 &mut lim,
>> -                core::ptr::null_mut(),
>> +                data,
>>                 static_lock_class!().as_ptr(),
>>             )
>>         })?;
>> @@ -167,8 +175,12 @@ pub fn build<T: Operations>(
>>             },
>>         )?;
>>
>> +        recover_data.dismiss();
>> +
>>         // INVARIANT: `gendisk` was initialized above.
>>         // INVARIANT: `gendisk` was added to the VFS via `device_add_disk` above.
>> +        // INVARIANT: `gendisk.queue.queue_data` is set to `data` in the call to
>> +        // `__blk_mq_alloc_disk` above.
>>         Ok(GenDisk {
>>             _tagset: tagset,
>>             gendisk,
>> @@ -180,9 +192,10 @@ pub fn build<T: Operations>(
>> ///
>> /// # Invariants
>> ///
>> -/// - `gendisk` must always point to an initialized and valid `struct gendisk`.
>> -/// - `gendisk` was added to the VFS through a call to
>> -///   `bindings::device_add_disk`.
>> +///  - `gendisk` must always point to an initialized and valid `struct gendisk`.
>> +///  - `gendisk` was added to the VFS through a call to
>> +///    `bindings::device_add_disk`.
>> +///  - `self.gendisk.queue.queuedata` is initialized by a call to `ForeignOwnable::into_foreign`.
>> pub struct GenDisk<T: Operations> {
>>     _tagset: Arc<TagSet<T>>,
>>     gendisk: *mut bindings::gendisk,
>> @@ -194,9 +207,20 @@ unsafe impl<T: Operations + Send> Send for GenDisk<T> {}
>>
>> impl<T: Operations> Drop for GenDisk<T> {
>>     fn drop(&mut self) {
>> +        // SAFETY: By type invariant of `Self`, `self.gendisk` points to a valid
>> +        // and initialized instance of `struct gendisk`, and, `queuedata` was
>> +        // initialized with the result of a call to
>> +        // `ForeignOwnable::into_foreign`.
>> +        let queue_data = unsafe { (*(*self.gendisk).queue).queuedata };
>> +
>>         // SAFETY: By type invariant, `self.gendisk` points to a valid and
>>         // initialized instance of `struct gendisk`, and it was previously added
>>         // to the VFS.
>>         unsafe { bindings::del_gendisk(self.gendisk) };
>> +
>> +        // SAFETY: `queue.queuedata` was created by `GenDiskBuilder::build` with
>> +        // a call to `ForeignOwnable::into_foreign` to create `queuedata`.
>> +        // `ForeignOwnable::from_foreign` is only called here.
>> +        drop(unsafe { T::QueueData::from_foreign(queue_data) });
>>     }
>> }
>> diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
>> index c2b98f507bcb..6fb256f55acc 100644
>> --- a/rust/kernel/block/mq/operations.rs
>> +++ b/rust/kernel/block/mq/operations.rs
>> @@ -6,14 +6,15 @@
>>
>> use crate::{
>>     bindings,
>> -    block::mq::request::RequestDataWrapper,
>> -    block::mq::Request,
>> +    block::mq::{request::RequestDataWrapper, Request},
>>     error::{from_result, Result},
>>     prelude::*,
>> -    types::ARef,
>> +    types::{ARef, ForeignOwnable},
>> };
>> use core::{marker::PhantomData, sync::atomic::AtomicU64, sync::atomic::Ordering};
>>
>> +type ForeignBorrowed<'a, T> = <T as ForeignOwnable>::Borrowed<'a>;
>> +
>> /// Implement this trait to interface blk-mq as block devices.
>> ///
>> /// To implement a block device driver, implement this trait as described in the
>> @@ -26,12 +27,20 @@
>> /// [module level documentation]: kernel::block::mq
>> #[macros::vtable]
>> pub trait Operations: Sized {
>> +    /// Data associated with the `struct request_queue` that is allocated for
>> +    /// the `GenDisk` associated with this `Operations` implementation.
>> +    type QueueData: ForeignOwnable;
>> +
>>     /// Called by the kernel to queue a request with the driver. If `is_last` is
>>     /// `false`, the driver is allowed to defer committing the request.
>> -    fn queue_rq(rq: ARef<Request<Self>>, is_last: bool) -> Result;
>> +    fn queue_rq(
>> +        queue_data: ForeignBorrowed<'_, Self::QueueData>,
>> +        rq: ARef<Request<Self>>,
>> +        is_last: bool,
>> +    ) -> Result;
>>
>>     /// Called by the kernel to indicate that queued requests should be submitted.
>> -    fn commit_rqs();
>> +    fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>);
>>
>>     /// Called by the kernel to poll the device for completed requests. Only
>>     /// used for poll queues.
>> @@ -70,7 +79,7 @@ impl<T: Operations> OperationsVTable<T> {
>>     ///   promise to not access the request until the driver calls
>>     ///   `bindings::blk_mq_end_request` for the request.
>>     unsafe extern "C" fn queue_rq_callback(
>> -        _hctx: *mut bindings::blk_mq_hw_ctx,
>> +        hctx: *mut bindings::blk_mq_hw_ctx,
>>         bd: *const bindings::blk_mq_queue_data,
>>     ) -> bindings::blk_status_t {
>>         // SAFETY: `bd.rq` is valid as required by the safety requirement for
>> @@ -88,10 +97,20 @@ impl<T: Operations> OperationsVTable<T> {
>>         //    reference counted by `ARef` until then.
>>         let rq = unsafe { Request::aref_from_raw((*bd).rq) };
>>
>> +        // SAFETY: `hctx` is valid as required by this function.
>> +        let queue_data = unsafe { (*(*hctx).queue).queuedata };
>> +
>> +        // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a
>
> isn’t this set on build() ?

Yes, you are right. Refactoring happened.

>
>> +        // call to `ForeignOwnable::into_pointer()` to create `queuedata`.
>
> into_pointer() ?

Should be `into_foreign`, will fix.

>
>> +        // `ForeignOwnable::from_foreign()` is only called when the tagset is
>> +        // dropped, which happens after we are dropped.
>> +        let queue_data = unsafe { T::QueueData::borrow(queue_data) };
>> +
>>         // SAFETY: We have exclusive access and we just set the refcount above.
>>         unsafe { Request::start_unchecked(&rq) };
>>
>>         let ret = T::queue_rq(
>> +            queue_data,
>>             rq,
>>             // SAFETY: `bd` is valid as required by the safety requirement for
>>             // this function.
>> @@ -110,9 +129,18 @@ impl<T: Operations> OperationsVTable<T> {
>>     ///
>>     /// # Safety
>>     ///
>> -    /// This function may only be called by blk-mq C infrastructure.
>> -    unsafe extern "C" fn commit_rqs_callback(_hctx: *mut bindings::blk_mq_hw_ctx) {
>> -        T::commit_rqs()
>> +    /// This function may only be called by blk-mq C infrastructure. The caller
>> +    /// must ensure that `hctx` is valid.
>> +    unsafe extern "C" fn commit_rqs_callback(hctx: *mut bindings::blk_mq_hw_ctx) {
>> +        // SAFETY: `hctx` is valid as required by this function.
>> +        let queue_data = unsafe { (*(*hctx).queue).queuedata };
>> +
>> +        // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a
>> +        // call to `ForeignOwnable::into_pointer()` to create `queuedata`.
>
> into_foreign()?

Yep, thanks.

>
>> +        // `ForeignOwnable::from_foreign()` is only called when the tagset is
>> +        // dropped, which happens after we are dropped.
>> +        let queue_data = unsafe { T::QueueData::borrow(queue_data) };
>> +        T::commit_rqs(queue_data)
>>     }
>>
>>     /// This function is called by the C kernel. It is not currently
>>
>> --
>> 2.47.2
>>
>>
>>
>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

Re: [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function

[Alice Ryhl]

On Fri, Aug 22, 2025 at 2:15 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Add a convenience function to convert byte slices to boolean values by
> wrapping them in a null-terminated C string and delegating to the
> existing `kstrtobool` function. Only considers the first two bytes of
> the input slice, following the kernel's boolean parsing semantics.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

One nit below, but generally looks good.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>

> +/// # Safety
> +///
> +/// - `string` must point to a null terminated string that is valid for read.
> +unsafe fn kstrtobool_raw(string: *const u8) -> Result<bool> {
> +    let mut result: bool = false;
> +
> +    // SAFETY:
> +    // - By function safety requirement, `string` is a valid null-terminated string.
> +    // - `result` is a valid `bool` that we own.
> +    let ret = unsafe { bindings::kstrtobool(string, &mut result) };
> +
> +    kernel::error::to_result(ret).map(|()| result)

I think this is easier to read as:

to_result(unsafe { bindings::kstrtobool(string, &mut result) })?;
Ok(result)

Alice

Re: [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function

[Alice Ryhl]

On Wed, Aug 27, 2025 at 3:46 PM Daniel Almeida
<daniel.almeida@collabora.com> wrote:
>
>
>
> > On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> >
> > Add a convenience function to convert byte slices to boolean values by
> > wrapping them in a null-terminated C string and delegating to the
> > existing `kstrtobool` function. Only considers the first two bytes of
> > the input slice, following the kernel's boolean parsing semantics.
> >
> > Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> > ---
> > rust/kernel/str.rs | 35 +++++++++++++++++++++++++++++------
> > 1 file changed, 29 insertions(+), 6 deletions(-)
> >
> > diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> > index d070c0bd86c3..b185262b4851 100644
> > --- a/rust/kernel/str.rs
> > +++ b/rust/kernel/str.rs
> > @@ -921,6 +921,20 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
> >     }
> > }
> >
> > +/// # Safety
> > +///
> > +/// - `string` must point to a null terminated string that is valid for read.
> > +unsafe fn kstrtobool_raw(string: *const u8) -> Result<bool> {
> > +    let mut result: bool = false;
> > +
> > +    // SAFETY:
> > +    // - By function safety requirement, `string` is a valid null-terminated string.
> > +    // - `result` is a valid `bool` that we own.
> > +    let ret = unsafe { bindings::kstrtobool(string, &mut result) };
> > +
> > +    kernel::error::to_result(ret).map(|()| result)
> > +}
> > +
> > /// Convert common user inputs into boolean values using the kernel's `kstrtobool` function.
> > ///
> > /// This routine returns `Ok(bool)` if the first character is one of 'YyTt1NnFf0', or
> > @@ -968,13 +982,22 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
> > /// assert_eq!(kstrtobool(c_str!("2")), Err(EINVAL));
> > /// ```
> > pub fn kstrtobool(string: &CStr) -> Result<bool> {
> > -    let mut result: bool = false;
> > -
> > -    // SAFETY: `string` is a valid null-terminated C string, and `result` is a valid
> > -    // pointer to a bool that we own.
> > -    let ret = unsafe { bindings::kstrtobool(string.as_char_ptr(), &mut result) };
> > +    // SAFETY:
> > +    // - The pointer returned by `CStr::as_char_ptr` is guaranteed to be
> > +    //   null terminated.
> > +    // - `string` is live and thus the string is valid for read.
> > +    unsafe { kstrtobool_raw(string.as_char_ptr()) }
> > +}
> >
> > -    kernel::error::to_result(ret).map(|()| result)
> > +/// Convert `&[u8]` to `bool` by deferring to [`kernel::str::kstrtobool`].
> > +///
> > +/// Only considers at most the first two bytes of `bytes`.
> > +pub fn kstrtobool_bytes(bytes: &[u8]) -> Result<bool> {
> > +    // `ktostrbool` only considers the first two bytes of the input.
> > +    let stack_string = [*bytes.first().unwrap_or(&0), *bytes.get(1).unwrap_or(&0), 0];
>
> Can’t this be CStr::from_bytes_with_nul() ?
>
> This means that kstrtobool_raw could take a &CStr directly and thus not be unsafe IIUC?

That's what Andreas did in the previous version. It ended up being
pretty complex because CStr::from_bytes_with_nul() requires that we
compute the length of `stack_string`, which isn't really needed here.
I recommended having a _raw method like this to avoid that complexity.
I don't think this is meaningfully more unsafe the way it is in this
patch.

Alice

Re: [PATCH v6 17/18] rust: block: add remote completion to `Request`

[Daniel Almeida]

> On 29 Aug 2025, at 08:12, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> "Daniel Almeida" <daniel.almeida@collabora.com> writes:
> 
>> Hi Andreas,
>> 
>>> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>> 
>>> Allow users of rust block device driver API to schedule completion of
>>> requests via `blk_mq_complete_request_remote`.
>>> 
>>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>> ---
>>> drivers/block/rnull/rnull.rs       |  9 +++++++++
>>> rust/kernel/block/mq.rs            |  6 ++++++
>>> rust/kernel/block/mq/operations.rs | 19 +++++++++++++++----
>>> rust/kernel/block/mq/request.rs    | 17 +++++++++++++++++
>>> 4 files changed, 47 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
>>> index 8255236bc550..a19c55717c4f 100644
>>> --- a/drivers/block/rnull/rnull.rs
>>> +++ b/drivers/block/rnull/rnull.rs
>>> @@ -82,4 +82,13 @@ fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Res
>>>    }
>>> 
>>>    fn commit_rqs(_queue_data: ()) {}
>>> +
>>> +    fn complete(rq: ARef<mq::Request<Self>>) {
>>> +        mq::Request::end_ok(rq)
>>> +            .map_err(|_e| kernel::error::code::EIO)
>>> +            // We take no refcounts on the request, so we expect to be able to
>>> +            // end the request. The request reference must be unique at this
>>> +            // point, and so `end_ok` cannot fail.
>>> +            .expect("Fatal error - expected to be able to end request");
>>> +    }
>>> }
>>> diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
>>> index 6e546f4f3d1c..c0ec06b84355 100644
>>> --- a/rust/kernel/block/mq.rs
>>> +++ b/rust/kernel/block/mq.rs
>>> @@ -77,6 +77,12 @@
>>> //!     }
>>> //!
>>> //!     fn commit_rqs(_queue_data: ()) {}
>>> +//!
>>> +//!     fn complete(rq: ARef<Request<Self>>) {
>>> +//!         Request::end_ok(rq)
>>> +//!             .map_err(|_e| kernel::error::code::EIO)
>>> +//!             .expect("Fatal error - expected to be able to end request");
>>> +//!     }
>>> //! }
>>> //!
>>> //! let tagset: Arc<TagSet<MyBlkDevice>> =
>>> diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
>>> index 6fb256f55acc..0fece577de7c 100644
>>> --- a/rust/kernel/block/mq/operations.rs
>>> +++ b/rust/kernel/block/mq/operations.rs
>>> @@ -42,6 +42,9 @@ fn queue_rq(
>>>    /// Called by the kernel to indicate that queued requests should be submitted.
>>>    fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>);
>>> 
>>> +    /// Called by the kernel when the request is completed.
>>> +    fn complete(rq: ARef<Request<Self>>);
>>> +
>>>    /// Called by the kernel to poll the device for completed requests. Only
>>>    /// used for poll queues.
>>>    fn poll() -> bool {
>>> @@ -143,13 +146,21 @@ impl<T: Operations> OperationsVTable<T> {
>>>        T::commit_rqs(queue_data)
>>>    }
>>> 
>>> -    /// This function is called by the C kernel. It is not currently
>>> -    /// implemented, and there is no way to exercise this code path.
>>> +    /// This function is called by the C kernel. A pointer to this function is
>>> +    /// installed in the `blk_mq_ops` vtable for the driver.
>>>    ///
>>>    /// # Safety
>>>    ///
>>> -    /// This function may only be called by blk-mq C infrastructure.
>>> -    unsafe extern "C" fn complete_callback(_rq: *mut bindings::request) {}
>>> +    /// This function may only be called by blk-mq C infrastructure. `rq` must
>>> +    /// point to a valid request that has been marked as completed. The pointee
>>> +    /// of `rq` must be valid for write for the duration of this function.
>>> +    unsafe extern "C" fn complete_callback(rq: *mut bindings::request) {
>>> +        // SAFETY: This function can only be dispatched through
>>> +        // `Request::complete`. We leaked a refcount then which we pick back up
>>> +        // now.
>>> +        let aref = unsafe { Request::aref_from_raw(rq) };
>>> +        T::complete(aref);
>>> +    }
>>> 
>>>    /// This function is called by the C kernel. A pointer to this function is
>>>    /// installed in the `blk_mq_ops` vtable for the driver.
>>> diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
>>> index 3848cfe63f77..f7f757f7459f 100644
>>> --- a/rust/kernel/block/mq/request.rs
>>> +++ b/rust/kernel/block/mq/request.rs
>>> @@ -135,6 +135,23 @@ pub fn end_ok(this: ARef<Self>) -> Result<(), ARef<Self>> {
>>>        Ok(())
>>>    }
>>> 
>>> +    /// Complete the request by scheduling `Operations::complete` for
>>> +    /// execution.
>>> +    ///
>>> +    /// The function may be scheduled locally, via SoftIRQ or remotely via IPMI.
>>> +    /// See `blk_mq_complete_request_remote` in [`blk-mq.c`] for details.
>>> +    ///
>>> +    /// [`blk-mq.c`]: srctree/block/blk-mq.c
>>> +    pub fn complete(this: ARef<Self>) {
>>> +        let ptr = ARef::into_raw(this).cast::<bindings::request>().as_ptr();
>>> +        // SAFETY: By type invariant, `self.0` is a valid `struct request`
>>> +        if !unsafe { bindings::blk_mq_complete_request_remote(ptr) } {
>>> +            // SAFETY: We released a refcount above that we can reclaim here.
>>> +            let this = unsafe { Request::aref_from_raw(ptr) };
>>> +            T::complete(this);
>>> +        }
>>> +    }
>>> +
>>>    /// Return a pointer to the [`RequestDataWrapper`] stored in the private area
>>>    /// of the request structure.
>>>    ///
>>> 
>>> --
>>> 2.47.2
>>> 
>>> 
>> 
>> I had another look here. While I do trust your reasoning, perhaps we should
>> remove the call to expect()?
>> 
>> If it is not called ever as you said, great, removing the expect() will not
>> change the code behavior. If it is, be it because of some minor oversight or
>> unexpected condition, we should produce some error output instead of crashing
>> the kernel. Maybe we should use a warn() here instead? Or maybe dev/pr_err as
>> applicable?
> 
> I think for the example, I would like to keep the `expect`. For
> demonstration purposes.

I think examples is definitely one place we don’t want to teach people to
use unwrap and expect because that will kill the system IIUC.

> 
> We could do `warn!` instead for the rnull driver I guess. But the IO
> queue that would hit this code would start to hang pretty fast, since no
> IO would complete. I don't think the kernel can recover from this hang.
> 

No I/O would complete for that device, but that doesn't mean that the system is
unusable IIUC, specially if other devices are available?

> 
> Best regards,
> Andreas Hindborg

In any case, perhaps this is only my opinion and others may see this
differently, so I won't complain if you want to keep it.


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

Re: [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function

[Daniel Almeida]

> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> Add a convenience function to convert byte slices to boolean values by
> wrapping them in a null-terminated C string and delegating to the
> existing `kstrtobool` function. Only considers the first two bytes of
> the input slice, following the kernel's boolean parsing semantics.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/str.rs | 35 +++++++++++++++++++++++++++++------
> 1 file changed, 29 insertions(+), 6 deletions(-)
> 
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index d070c0bd86c3..b185262b4851 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -921,6 +921,20 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
>     }
> }
> 
> +/// # Safety
> +///
> +/// - `string` must point to a null terminated string that is valid for read.
> +unsafe fn kstrtobool_raw(string: *const u8) -> Result<bool> {
> +    let mut result: bool = false;
> +
> +    // SAFETY:
> +    // - By function safety requirement, `string` is a valid null-terminated string.
> +    // - `result` is a valid `bool` that we own.
> +    let ret = unsafe { bindings::kstrtobool(string, &mut result) };
> +
> +    kernel::error::to_result(ret).map(|()| result)
> +}
> +
> /// Convert common user inputs into boolean values using the kernel's `kstrtobool` function.
> ///
> /// This routine returns `Ok(bool)` if the first character is one of 'YyTt1NnFf0', or
> @@ -968,13 +982,22 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
> /// assert_eq!(kstrtobool(c_str!("2")), Err(EINVAL));
> /// ```
> pub fn kstrtobool(string: &CStr) -> Result<bool> {
> -    let mut result: bool = false;
> -
> -    // SAFETY: `string` is a valid null-terminated C string, and `result` is a valid
> -    // pointer to a bool that we own.
> -    let ret = unsafe { bindings::kstrtobool(string.as_char_ptr(), &mut result) };
> +    // SAFETY:
> +    // - The pointer returned by `CStr::as_char_ptr` is guaranteed to be
> +    //   null terminated.
> +    // - `string` is live and thus the string is valid for read.
> +    unsafe { kstrtobool_raw(string.as_char_ptr()) }
> +}
> 
> -    kernel::error::to_result(ret).map(|()| result)
> +/// Convert `&[u8]` to `bool` by deferring to [`kernel::str::kstrtobool`].
> +///
> +/// Only considers at most the first two bytes of `bytes`.
> +pub fn kstrtobool_bytes(bytes: &[u8]) -> Result<bool> {
> +    // `ktostrbool` only considers the first two bytes of the input.
> +    let stack_string = [*bytes.first().unwrap_or(&0), *bytes.get(1).unwrap_or(&0), 0];
> +    // SAFETY: `stack_string` is null terminated and it is live on the stack so
> +    // it is valid for read.
> +    unsafe { kstrtobool_raw(stack_string.as_ptr()) }
> }
> 
> /// An owned string that is guaranteed to have exactly one `NUL` byte, which is at the end.
> 
> -- 
> 2.47.2
> 
> 
> 

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