[PATCH v4 00/15] rnull: add configfs, remote completion to rnull

[PATCH v4 00/15] 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 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 (15):
      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: 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    | 273 +++++++++++++++++++++++++++++++++++++
 drivers/block/rnull/rnull.rs       | 105 ++++++++++++++
 rust/kernel/block.rs               |  13 ++
 rust/kernel/block/mq.rs            |  14 +-
 rust/kernel/block/mq/gen_disk.rs   |  58 ++++++--
 rust/kernel/block/mq/operations.rs |  65 +++++++--
 rust/kernel/block/mq/raw_writer.rs |  55 --------
 rust/kernel/block/mq/request.rs    |  21 ++-
 rust/kernel/str.rs                 |  83 +++++++++--
 15 files changed, 608 insertions(+), 191 deletions(-)
---
base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
change-id: 20250616-rnull-up-v6-16-b4571e28feee

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

[PATCH v4 01/15] 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 v4 02/15] 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.

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

Re: [PATCH v4 02/15] rust: str: allow `str::Formatter` to format into `&mut [u8]`.

[Alice Ryhl]

On Tue, Aug 12, 2025 at 10:44:20AM +0200, Andreas Hindborg wrote:
> Improve `Formatter` so that it can write to an array or slice buffer.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

[PATCH v4 03/15] 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.

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

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

[Alice Ryhl]

On Tue, Aug 12, 2025 at 10:44:21AM +0200, Andreas Hindborg wrote:
> rnull is going to make use of `str::Formatter` and `str::RawFormatter`, so
> expose them with public visibility.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

[PATCH v4 04/15] 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.

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

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 46cdc85dad63..e84f62c01815 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -871,6 +871,61 @@ 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.
+    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 })
+    }
+
+    #[expect(dead_code)]
+    pub(crate) fn from_array<const N: usize>(
+        buffer: &'a mut [crate::ffi::c_char; N],
+    ) -> Option<NullTerminatedFormatter<'a>> {
+        Self::new(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

Re: [PATCH v4 04/15] rust: str: introduce `NullTerminatedFormatter`

[Alice Ryhl]

On Tue, Aug 12, 2025 at 10:44:22AM +0200, Andreas Hindborg 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.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

> +    #[expect(dead_code)]
> +    pub(crate) fn from_array<const N: usize>(

Don't you delete this function in the next patch?

Alice

Re: [PATCH v4 04/15] rust: str: introduce `NullTerminatedFormatter`

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Tue, Aug 12, 2025 at 10:44:22AM +0200, Andreas Hindborg 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.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>
>> +    #[expect(dead_code)]
>> +    pub(crate) fn from_array<const N: usize>(
>
> Don't you delete this function in the next patch?

It slipped through the cracks. I'll remove it and respin. Thanks.


Best regards,
Andreas Hindborg

[PATCH v4 05/15] 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 v4 06/15] 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.

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                 |  7 -------
 3 files changed, 8 insertions(+), 12 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 e84f62c01815..b8eb50afdb24 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -893,13 +893,6 @@ pub(crate) fn new(buffer: &'a mut [u8]) -> Option<NullTerminatedFormatter<'a>> {
         //  - If buffer was not at least length 1, `buffer.first_mut()` would return None.
         Some(Self { buffer })
     }
-
-    #[expect(dead_code)]
-    pub(crate) fn from_array<const N: usize>(
-        buffer: &'a mut [crate::ffi::c_char; N],
-    ) -> Option<NullTerminatedFormatter<'a>> {
-        Self::new(buffer)
-    }
 }
 
 impl Write for NullTerminatedFormatter<'_> {

-- 
2.47.2

Re: [PATCH v4 06/15] rust: block: use `NullTerminatedFormatter`

[Alice Ryhl]

On Tue, Aug 12, 2025 at 10:44:24AM +0200, Andreas Hindborg 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.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

[PATCH v4 07/15] 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 v4 08/15] 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 v4 09/15] 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.

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

Re: [PATCH v4 09/15] rust: block: add block related constants

[Alice Ryhl]

On Tue, Aug 12, 2025 at 10:44:27AM +0200, Andreas Hindborg 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.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

[PATCH v4 10/15] 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 fe168477caa4..238ff10f537d 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 v4 11/15] rnull: enable configuration via `configfs`

[Andreas Hindborg]

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

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 drivers/block/rnull/Kconfig      |   2 +-
 drivers/block/rnull/configfs.rs  | 218 +++++++++++++++++++++++++++++++++++++++
 drivers/block/rnull/rnull.rs     |  58 ++++++-----
 rust/kernel/block/mq/gen_disk.rs |   2 +-
 4 files changed, 251 insertions(+), 29 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..8d469c046a39
--- /dev/null
+++ b/drivers/block/rnull/configfs.rs
@@ -0,0 +1,218 @@
+// 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::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_str = core::str::from_utf8(page)?.trim();
+
+        let power_op = match power_op_str {
+            "0" => Ok(false),
+            "1" => Ok(true),
+            _ => Err(EINVAL),
+        }?;
+
+        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 = match core::str::from_utf8(page)?.trim() {
+            "0" => false,
+            "1" => true,
+            _ => return Err(EINVAL),
+        };
+
+        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..d09bc77861e4 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -1,28 +1,26 @@
 // 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 +33,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), flags::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

Re: [PATCH v4 11/15] rnull: enable configuration via `configfs`

[Alice Ryhl]

On Tue, Aug 12, 2025 at 10:44:29AM +0200, Andreas Hindborg wrote:
> Allow rust null block devices to be configured and instantiated via
> `configfs`.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Overall LGTM, but a few comments below:

> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
> new file mode 100644
> index 000000000000..8d469c046a39
> --- /dev/null
> +++ b/drivers/block/rnull/configfs.rs
> @@ -0,0 +1,218 @@
> +// 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,

It would be nice to add

	pub use configfs_attrs;

to the configfs module so that you can import the macro from the
configfs module instead of the root.

> +            try_pin_init!( DeviceConfig {
> +                data <- new_mutex!( DeviceConfigInner {

Extra spaces in these macros.

> +        let power_op_str = core::str::from_utf8(page)?.trim();
> +
> +        let power_op = match power_op_str {
> +            "0" => Ok(false),
> +            "1" => Ok(true),
> +            _ => Err(EINVAL),
> +        }?;

We probably want kstrtobool here instead of manually parsing the
boolean.

Alice

Re: [PATCH v4 11/15] rnull: enable configuration via `configfs`

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Tue, Aug 12, 2025 at 10:44:29AM +0200, Andreas Hindborg wrote:
>> Allow rust null block devices to be configured and instantiated via
>> `configfs`.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> Overall LGTM, but a few comments below:
>
>> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
>> new file mode 100644
>> index 000000000000..8d469c046a39
>> --- /dev/null
>> +++ b/drivers/block/rnull/configfs.rs
>> @@ -0,0 +1,218 @@
>> +// 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,
>
> It would be nice to add
>
> 	pub use configfs_attrs;
>
> to the configfs module so that you can import the macro from the
> configfs module instead of the root.

OK, I'll do that.

>
>> +            try_pin_init!( DeviceConfig {
>> +                data <- new_mutex!( DeviceConfigInner {
>
> Extra spaces in these macros.

Thanks. I subconsciously like the space in that location, so when
rustfmt is bailing, I get these things in my code.

>> +        let power_op_str = core::str::from_utf8(page)?.trim();
>> +
>> +        let power_op = match power_op_str {
>> +            "0" => Ok(false),
>> +            "1" => Ok(true),
>> +            _ => Err(EINVAL),
>> +        }?;
>
> We probably want kstrtobool here instead of manually parsing the
> boolean.

Yea, I was debating on this a bit. I did want to consolidate this code,
but I don't particularly like ktostrbool. But I guess in the name of
consistency across the kernel it is the right choice.

I'll add it to next spin.


Best regards,
Andreas Hindborg

Re: [PATCH v4 11/15] rnull: enable configuration via `configfs`

[Alice Ryhl]

On Wed, Aug 13, 2025 at 3:47 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> "Alice Ryhl" <aliceryhl@google.com> writes:
>
> > On Tue, Aug 12, 2025 at 10:44:29AM +0200, Andreas Hindborg wrote:
> >> Allow rust null block devices to be configured and instantiated via
> >> `configfs`.
> >>
> >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> >
> > Overall LGTM, but a few comments below:
> >
> >> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
> >> new file mode 100644
> >> index 000000000000..8d469c046a39
> >> --- /dev/null
> >> +++ b/drivers/block/rnull/configfs.rs
> >> @@ -0,0 +1,218 @@
> >> +// 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,
> >
> > It would be nice to add
> >
> >       pub use configfs_attrs;
> >
> > to the configfs module so that you can import the macro from the
> > configfs module instead of the root.
>
> OK, I'll do that.
>
> >
> >> +            try_pin_init!( DeviceConfig {
> >> +                data <- new_mutex!( DeviceConfigInner {
> >
> > Extra spaces in these macros.
>
> Thanks. I subconsciously like the space in that location, so when
> rustfmt is bailing, I get these things in my code.
>
> >> +        let power_op_str = core::str::from_utf8(page)?.trim();
> >> +
> >> +        let power_op = match power_op_str {
> >> +            "0" => Ok(false),
> >> +            "1" => Ok(true),
> >> +            _ => Err(EINVAL),
> >> +        }?;
> >
> > We probably want kstrtobool here instead of manually parsing the
> > boolean.
>
> Yea, I was debating on this a bit. I did want to consolidate this code,
> but I don't particularly like ktostrbool. But I guess in the name of
> consistency across the kernel it is the right choice.
>
> I'll add it to next spin.

For your convenience, I already wrote a safe wrapper of kstrtobool for
an out-of-tree driver. You're welcome to copy-paste this:

fn kstrtobool(kstr: &CStr) -> Result<bool> {
    let mut res = false;
    to_result(unsafe {
kernel::bindings::kstrtobool(kstr.as_char_ptr(), &mut res) })?;
    Ok(res)
}

Alice

Re: [PATCH v4 11/15] rnull: enable configuration via `configfs`

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Wed, Aug 13, 2025 at 3:47 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> "Alice Ryhl" <aliceryhl@google.com> writes:
>>
>> > On Tue, Aug 12, 2025 at 10:44:29AM +0200, Andreas Hindborg wrote:
>> >> Allow rust null block devices to be configured and instantiated via
>> >> `configfs`.
>> >>
>> >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> >
>> > Overall LGTM, but a few comments below:
>> >
>> >> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
>> >> new file mode 100644
>> >> index 000000000000..8d469c046a39
>> >> --- /dev/null
>> >> +++ b/drivers/block/rnull/configfs.rs
>> >> @@ -0,0 +1,218 @@
>> >> +// 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,
>> >
>> > It would be nice to add
>> >
>> >       pub use configfs_attrs;
>> >
>> > to the configfs module so that you can import the macro from the
>> > configfs module instead of the root.
>>
>> OK, I'll do that.
>>
>> >
>> >> +            try_pin_init!( DeviceConfig {
>> >> +                data <- new_mutex!( DeviceConfigInner {
>> >
>> > Extra spaces in these macros.
>>
>> Thanks. I subconsciously like the space in that location, so when
>> rustfmt is bailing, I get these things in my code.
>>
>> >> +        let power_op_str = core::str::from_utf8(page)?.trim();
>> >> +
>> >> +        let power_op = match power_op_str {
>> >> +            "0" => Ok(false),
>> >> +            "1" => Ok(true),
>> >> +            _ => Err(EINVAL),
>> >> +        }?;
>> >
>> > We probably want kstrtobool here instead of manually parsing the
>> > boolean.
>>
>> Yea, I was debating on this a bit. I did want to consolidate this code,
>> but I don't particularly like ktostrbool. But I guess in the name of
>> consistency across the kernel it is the right choice.
>>
>> I'll add it to next spin.
>
> For your convenience, I already wrote a safe wrapper of kstrtobool for
> an out-of-tree driver. You're welcome to copy-paste this:
>
> fn kstrtobool(kstr: &CStr) -> Result<bool> {
>     let mut res = false;
>     to_result(unsafe {
> kernel::bindings::kstrtobool(kstr.as_char_ptr(), &mut res) })?;
>     Ok(res)
> }

Thanks, I did one as well today, accepting `&str` instead. The examples
highlight why it is not great:


  /// 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::str::kstrtobool;
  ///
  /// // Lowercase
  /// assert_eq!(kstrtobool("true"), Ok(true));
  /// assert_eq!(kstrtobool("tr"), Ok(true));
  /// assert_eq!(kstrtobool("t"), Ok(true));
  /// assert_eq!(kstrtobool("twrong"), Ok(true)); // <-- 🤷
  /// assert_eq!(kstrtobool("false"), Ok(false));
  /// assert_eq!(kstrtobool("f"), Ok(false));
  /// assert_eq!(kstrtobool("yes"), Ok(true));
  /// assert_eq!(kstrtobool("no"), Ok(false));
  /// assert_eq!(kstrtobool("on"), Ok(true));
  /// assert_eq!(kstrtobool("off"), Ok(false));
  ///
  /// // Camel case
  /// assert_eq!(kstrtobool("True"), Ok(true));
  /// assert_eq!(kstrtobool("False"), Ok(false));
  /// assert_eq!(kstrtobool("Yes"), Ok(true));
  /// assert_eq!(kstrtobool("No"), Ok(false));
  /// assert_eq!(kstrtobool("On"), Ok(true));
  /// assert_eq!(kstrtobool("Off"), Ok(false));
  ///
  /// // All caps
  /// assert_eq!(kstrtobool("TRUE"), Ok(true));
  /// assert_eq!(kstrtobool("FALSE"), Ok(false));
  /// assert_eq!(kstrtobool("YES"), Ok(true));
  /// assert_eq!(kstrtobool("NO"), Ok(false));
  /// assert_eq!(kstrtobool("ON"), Ok(true));
  /// assert_eq!(kstrtobool("OFF"), Ok(false));
  ///
  /// // Numeric
  /// assert_eq!(kstrtobool("1"), Ok(true));
  /// assert_eq!(kstrtobool("0"), Ok(false));
  ///
  /// // Invalid input
  /// assert_eq!(kstrtobool("invalid"), Err(EINVAL));
  /// assert_eq!(kstrtobool("2"), Err(EINVAL));
  /// ```
  pub fn kstrtobool(input: &str) -> Result<bool> {
      let mut result: bool = false;
      let c_str = CString::try_from_fmt(fmt!("{input}"))?;

      // SAFETY: `c_str` points to a valid null-terminated C string, and `result` is a valid
      // pointer to a bool that we own.
      let ret = unsafe { bindings::kstrtobool(c_str.as_char_ptr(), &mut result as *mut bool) };

      kernel::error::to_result(ret).map(|_| result)
  }

Not sure if we should take `CStr` or `str`, what do you think?


Best regards,
Andreas Hindborg

Re: [PATCH v4 11/15] rnull: enable configuration via `configfs`

[Alice Ryhl]

On Wed, Aug 13, 2025 at 7:36 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> "Alice Ryhl" <aliceryhl@google.com> writes:
>
> > For your convenience, I already wrote a safe wrapper of kstrtobool for
> > an out-of-tree driver. You're welcome to copy-paste this:
> >
> > fn kstrtobool(kstr: &CStr) -> Result<bool> {
> >     let mut res = false;
> >     to_result(unsafe {
> > kernel::bindings::kstrtobool(kstr.as_char_ptr(), &mut res) })?;
> >     Ok(res)
> > }
>
> Thanks, I did one as well today, accepting `&str` instead. The examples
> highlight why it is not great:

Yeah, well, I think we should still use it for consistency.

>   /// 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::str::kstrtobool;
>   ///
>   /// // Lowercase
>   /// assert_eq!(kstrtobool("true"), Ok(true));
>   /// assert_eq!(kstrtobool("tr"), Ok(true));
>   /// assert_eq!(kstrtobool("t"), Ok(true));
>   /// assert_eq!(kstrtobool("twrong"), Ok(true)); // <-- 🤷
>   /// assert_eq!(kstrtobool("false"), Ok(false));
>   /// assert_eq!(kstrtobool("f"), Ok(false));
>   /// assert_eq!(kstrtobool("yes"), Ok(true));
>   /// assert_eq!(kstrtobool("no"), Ok(false));
>   /// assert_eq!(kstrtobool("on"), Ok(true));
>   /// assert_eq!(kstrtobool("off"), Ok(false));
>   ///
>   /// // Camel case
>   /// assert_eq!(kstrtobool("True"), Ok(true));
>   /// assert_eq!(kstrtobool("False"), Ok(false));
>   /// assert_eq!(kstrtobool("Yes"), Ok(true));
>   /// assert_eq!(kstrtobool("No"), Ok(false));
>   /// assert_eq!(kstrtobool("On"), Ok(true));
>   /// assert_eq!(kstrtobool("Off"), Ok(false));
>   ///
>   /// // All caps
>   /// assert_eq!(kstrtobool("TRUE"), Ok(true));
>   /// assert_eq!(kstrtobool("FALSE"), Ok(false));
>   /// assert_eq!(kstrtobool("YES"), Ok(true));
>   /// assert_eq!(kstrtobool("NO"), Ok(false));
>   /// assert_eq!(kstrtobool("ON"), Ok(true));
>   /// assert_eq!(kstrtobool("OFF"), Ok(false));
>   ///
>   /// // Numeric
>   /// assert_eq!(kstrtobool("1"), Ok(true));
>   /// assert_eq!(kstrtobool("0"), Ok(false));
>   ///
>   /// // Invalid input
>   /// assert_eq!(kstrtobool("invalid"), Err(EINVAL));
>   /// assert_eq!(kstrtobool("2"), Err(EINVAL));
>   /// ```
>   pub fn kstrtobool(input: &str) -> Result<bool> {
>       let mut result: bool = false;
>       let c_str = CString::try_from_fmt(fmt!("{input}"))?;
>
>       // SAFETY: `c_str` points to a valid null-terminated C string, and `result` is a valid
>       // pointer to a bool that we own.
>       let ret = unsafe { bindings::kstrtobool(c_str.as_char_ptr(), &mut result as *mut bool) };
>
>       kernel::error::to_result(ret).map(|_| result)
>   }
>
> Not sure if we should take `CStr` or `str`, what do you think?

Using CStr makes sense, since it avoids having the caller perform a
useless utf-8 check.

Alice

Re: [PATCH v4 11/15] rnull: enable configuration via `configfs`

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Wed, Aug 13, 2025 at 7:36 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> "Alice Ryhl" <aliceryhl@google.com> writes:
>>
>> > For your convenience, I already wrote a safe wrapper of kstrtobool for
>> > an out-of-tree driver. You're welcome to copy-paste this:
>> >
>> > fn kstrtobool(kstr: &CStr) -> Result<bool> {
>> >     let mut res = false;
>> >     to_result(unsafe {
>> > kernel::bindings::kstrtobool(kstr.as_char_ptr(), &mut res) })?;
>> >     Ok(res)
>> > }
>>
>> Thanks, I did one as well today, accepting `&str` instead. The examples
>> highlight why it is not great:
>
> Yeah, well, I think we should still use it for consistency.
>
>>   /// 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::str::kstrtobool;
>>   ///
>>   /// // Lowercase
>>   /// assert_eq!(kstrtobool("true"), Ok(true));
>>   /// assert_eq!(kstrtobool("tr"), Ok(true));
>>   /// assert_eq!(kstrtobool("t"), Ok(true));
>>   /// assert_eq!(kstrtobool("twrong"), Ok(true)); // <-- 🤷
>>   /// assert_eq!(kstrtobool("false"), Ok(false));
>>   /// assert_eq!(kstrtobool("f"), Ok(false));
>>   /// assert_eq!(kstrtobool("yes"), Ok(true));
>>   /// assert_eq!(kstrtobool("no"), Ok(false));
>>   /// assert_eq!(kstrtobool("on"), Ok(true));
>>   /// assert_eq!(kstrtobool("off"), Ok(false));
>>   ///
>>   /// // Camel case
>>   /// assert_eq!(kstrtobool("True"), Ok(true));
>>   /// assert_eq!(kstrtobool("False"), Ok(false));
>>   /// assert_eq!(kstrtobool("Yes"), Ok(true));
>>   /// assert_eq!(kstrtobool("No"), Ok(false));
>>   /// assert_eq!(kstrtobool("On"), Ok(true));
>>   /// assert_eq!(kstrtobool("Off"), Ok(false));
>>   ///
>>   /// // All caps
>>   /// assert_eq!(kstrtobool("TRUE"), Ok(true));
>>   /// assert_eq!(kstrtobool("FALSE"), Ok(false));
>>   /// assert_eq!(kstrtobool("YES"), Ok(true));
>>   /// assert_eq!(kstrtobool("NO"), Ok(false));
>>   /// assert_eq!(kstrtobool("ON"), Ok(true));
>>   /// assert_eq!(kstrtobool("OFF"), Ok(false));
>>   ///
>>   /// // Numeric
>>   /// assert_eq!(kstrtobool("1"), Ok(true));
>>   /// assert_eq!(kstrtobool("0"), Ok(false));
>>   ///
>>   /// // Invalid input
>>   /// assert_eq!(kstrtobool("invalid"), Err(EINVAL));
>>   /// assert_eq!(kstrtobool("2"), Err(EINVAL));
>>   /// ```
>>   pub fn kstrtobool(input: &str) -> Result<bool> {
>>       let mut result: bool = false;
>>       let c_str = CString::try_from_fmt(fmt!("{input}"))?;
>>
>>       // SAFETY: `c_str` points to a valid null-terminated C string, and `result` is a valid
>>       // pointer to a bool that we own.
>>       let ret = unsafe { bindings::kstrtobool(c_str.as_char_ptr(), &mut result as *mut bool) };
>>
>>       kernel::error::to_result(ret).map(|_| result)
>>   }
>>
>> Not sure if we should take `CStr` or `str`, what do you think?
>
> Using CStr makes sense, since it avoids having the caller perform a
> useless utf-8 check.

If we re-implement the entire function in rust, we can do the processing
on a `&str`. That way, we can skip the allocation to enforce null
termination. At least for this use case. I would rather do a utf8 check
than allocate and copy.


Best regards,
Andreas Hindborg

Re: [PATCH v4 11/15] rnull: enable configuration via `configfs`

[Alice Ryhl]

On Thu, Aug 14, 2025 at 9:40 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> "Alice Ryhl" <aliceryhl@google.com> writes:
>
> > On Wed, Aug 13, 2025 at 7:36 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
> >>
> >> "Alice Ryhl" <aliceryhl@google.com> writes:
> >>
> >> > For your convenience, I already wrote a safe wrapper of kstrtobool for
> >> > an out-of-tree driver. You're welcome to copy-paste this:
> >> >
> >> > fn kstrtobool(kstr: &CStr) -> Result<bool> {
> >> >     let mut res = false;
> >> >     to_result(unsafe {
> >> > kernel::bindings::kstrtobool(kstr.as_char_ptr(), &mut res) })?;
> >> >     Ok(res)
> >> > }
> >>
> >> Thanks, I did one as well today, accepting `&str` instead. The examples
> >> highlight why it is not great:
> >
> > Yeah, well, I think we should still use it for consistency.
> >
> >>   /// 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::str::kstrtobool;
> >>   ///
> >>   /// // Lowercase
> >>   /// assert_eq!(kstrtobool("true"), Ok(true));
> >>   /// assert_eq!(kstrtobool("tr"), Ok(true));
> >>   /// assert_eq!(kstrtobool("t"), Ok(true));
> >>   /// assert_eq!(kstrtobool("twrong"), Ok(true)); // <-- 🤷
> >>   /// assert_eq!(kstrtobool("false"), Ok(false));
> >>   /// assert_eq!(kstrtobool("f"), Ok(false));
> >>   /// assert_eq!(kstrtobool("yes"), Ok(true));
> >>   /// assert_eq!(kstrtobool("no"), Ok(false));
> >>   /// assert_eq!(kstrtobool("on"), Ok(true));
> >>   /// assert_eq!(kstrtobool("off"), Ok(false));
> >>   ///
> >>   /// // Camel case
> >>   /// assert_eq!(kstrtobool("True"), Ok(true));
> >>   /// assert_eq!(kstrtobool("False"), Ok(false));
> >>   /// assert_eq!(kstrtobool("Yes"), Ok(true));
> >>   /// assert_eq!(kstrtobool("No"), Ok(false));
> >>   /// assert_eq!(kstrtobool("On"), Ok(true));
> >>   /// assert_eq!(kstrtobool("Off"), Ok(false));
> >>   ///
> >>   /// // All caps
> >>   /// assert_eq!(kstrtobool("TRUE"), Ok(true));
> >>   /// assert_eq!(kstrtobool("FALSE"), Ok(false));
> >>   /// assert_eq!(kstrtobool("YES"), Ok(true));
> >>   /// assert_eq!(kstrtobool("NO"), Ok(false));
> >>   /// assert_eq!(kstrtobool("ON"), Ok(true));
> >>   /// assert_eq!(kstrtobool("OFF"), Ok(false));
> >>   ///
> >>   /// // Numeric
> >>   /// assert_eq!(kstrtobool("1"), Ok(true));
> >>   /// assert_eq!(kstrtobool("0"), Ok(false));
> >>   ///
> >>   /// // Invalid input
> >>   /// assert_eq!(kstrtobool("invalid"), Err(EINVAL));
> >>   /// assert_eq!(kstrtobool("2"), Err(EINVAL));
> >>   /// ```
> >>   pub fn kstrtobool(input: &str) -> Result<bool> {
> >>       let mut result: bool = false;
> >>       let c_str = CString::try_from_fmt(fmt!("{input}"))?;
> >>
> >>       // SAFETY: `c_str` points to a valid null-terminated C string, and `result` is a valid
> >>       // pointer to a bool that we own.
> >>       let ret = unsafe { bindings::kstrtobool(c_str.as_char_ptr(), &mut result as *mut bool) };
> >>
> >>       kernel::error::to_result(ret).map(|_| result)
> >>   }
> >>
> >> Not sure if we should take `CStr` or `str`, what do you think?
> >
> > Using CStr makes sense, since it avoids having the caller perform a
> > useless utf-8 check.
>
> If we re-implement the entire function in rust, we can do the processing
> on a `&str`. That way, we can skip the allocation to enforce null
> termination. At least for this use case. I would rather do a utf8 check
> than allocate and copy.

You can copy to an array on the stack, so allocations aren't necessary
even if the string is not nul-terminated. I don't think we should
duplicate this logic.

And if we do add a Rust method that does not enforce a nul-check, then
I'd say it should use &[u8] as the argument rather than &str. (Or
possibly &Bstr.)

Alice

[PATCH v4 12/15] 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.

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   | 36 +++++++++++++++++++++++++----
 rust/kernel/block/mq/operations.rs | 46 ++++++++++++++++++++++++++++++--------
 4 files changed, 78 insertions(+), 19 deletions(-)

diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index d09bc77861e4..a012c59ecb3c 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -62,14 +62,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
@@ -80,5 +82,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..988e0ba63693 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,16 @@ 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(|| {
+            drop(
+                // SAFETY: T::QueueData was created by the call to `into_foreign()` above
+                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 +123,7 @@ pub fn build<T: Operations>(
             bindings::__blk_mq_alloc_disk(
                 tagset.raw_tag_set(),
                 &mut lim,
-                core::ptr::null_mut(),
+                data.cast(),
                 static_lock_class!().as_ptr(),
             )
         })?;
@@ -167,8 +177,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 +194,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 +209,22 @@ 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) };
+
+        drop(
+            // 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.
+            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..a6c1ee2190a1 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.cast()) };
+
         // 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.cast()) };
+        T::commit_rqs(queue_data)
     }
 
     /// This function is called by the C kernel. It is not currently

-- 
2.47.2

Re: [PATCH v4 12/15] rust: block: add `GenDisk` private data support

[Alice Ryhl]

On Tue, Aug 12, 2025 at 10:44:30AM +0200, Andreas Hindborg wrote:
> Allow users of the rust block device driver API to install private data in
> the `GenDisk` structure.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Overall LGTM.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>

>          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(|| {
> +            drop(
> +                // SAFETY: T::QueueData was created by the call to `into_foreign()` above
> +                unsafe { T::QueueData::from_foreign(data) },
> +            );

This is usually formatted as:

// SAFETY: T::QueueData was created by the call to `into_foreign()` above
drop(unsafe { T::QueueData::from_foreign(data) });

>  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) };
> +
> +        drop(
> +            // 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.
> +            unsafe { T::QueueData::from_foreign(queue_data) },
> +        );

Ditto here.

>          //    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.cast()) };

Is this cast necessary? Is it not a void pointer?

> @@ -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.cast()) };

Ditto here.

Alice

Re: [PATCH v4 12/15] rust: block: add `GenDisk` private data support

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Tue, Aug 12, 2025 at 10:44:30AM +0200, Andreas Hindborg wrote:
>> Allow users of the rust block device driver API to install private data in
>> the `GenDisk` structure.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> Overall LGTM.
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>
>>          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(|| {
>> +            drop(
>> +                // SAFETY: T::QueueData was created by the call to `into_foreign()` above
>> +                unsafe { T::QueueData::from_foreign(data) },
>> +            );
>
> This is usually formatted as:
>
> // SAFETY: T::QueueData was created by the call to `into_foreign()` above
> drop(unsafe { T::QueueData::from_foreign(data) });

I don't really have a preference, my optimization function was to
minimize distance to the unsafe block. Are there any rust guidelines on this?

>
>>  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) };
>> +
>> +        drop(
>> +            // 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.
>> +            unsafe { T::QueueData::from_foreign(queue_data) },
>> +        );
>
> Ditto here.
>
>>          //    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.cast()) };
>
> Is this cast necessary? Is it not a void pointer?

Leftover from old `ForeignOwnable` I think. I'll remove it.


Best regards,
Andreas Hindborg

Re: [PATCH v4 12/15] rust: block: add `GenDisk` private data support

[Alice Ryhl]

On Wed, Aug 13, 2025 at 3:47 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> "Alice Ryhl" <aliceryhl@google.com> writes:
>
> > On Tue, Aug 12, 2025 at 10:44:30AM +0200, Andreas Hindborg wrote:
> >> Allow users of the rust block device driver API to install private data in
> >> the `GenDisk` structure.
> >>
> >> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> >
> > Overall LGTM.
> > Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> >
> >>          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(|| {
> >> +            drop(
> >> +                // SAFETY: T::QueueData was created by the call to `into_foreign()` above
> >> +                unsafe { T::QueueData::from_foreign(data) },
> >> +            );
> >
> > This is usually formatted as:
> >
> > // SAFETY: T::QueueData was created by the call to `into_foreign()` above
> > drop(unsafe { T::QueueData::from_foreign(data) });
>
> I don't really have a preference, my optimization function was to
> minimize distance to the unsafe block. Are there any rust guidelines on this?

I would say that the unsafe keyword just has to be on the next line
from the safety comment. Optimizing further than that leads to wonky
formatting. A similar example that I also think is going too far:

let var =
    // SAFETY: bla bla
    unsafe { value };

Alice

[PATCH v4 13/15] 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 v4 14/15] 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 a012c59ecb3c..371786be7f47 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -83,4 +83,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 a6c1ee2190a1..fe516ca52ef9 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 v4 15/15] 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.

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 8d469c046a39..c4f6131eb9da 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>>,
 }
 
@@ -128,6 +159,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 {
@@ -216,3 +248,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 371786be7f47..85b1509a3106 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -4,6 +4,7 @@
 
 mod configfs;
 
+use configfs::IRQMode;
 use kernel::{
     alloc::flags,
     block::{
@@ -54,35 +55,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), flags::GFP_KERNEL)?;
 
+        let queue_data = Box::new(QueueData { irq_mode }, flags::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 v4 15/15] rnull: add soft-irq completion support

[Alice Ryhl]

On Tue, Aug 12, 2025 at 10:44:33AM +0200, Andreas Hindborg wrote:
> rnull currently only supports direct completion. Add option for completing
> requests across CPU nodes via soft IRQ or IPI.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

> +        let text = core::str::from_utf8(page)?.trim();
> +        let value = text.parse::<u8>().map_err(|_| EINVAL)?;

Same comment about kstrtobool here as on the other patch.

Alice

Re: [PATCH v4 15/15] rnull: add soft-irq completion support

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> On Tue, Aug 12, 2025 at 10:44:33AM +0200, Andreas Hindborg wrote:
>> rnull currently only supports direct completion. Add option for completing
>> requests across CPU nodes via soft IRQ or IPI.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
>> +        let text = core::str::from_utf8(page)?.trim();
>> +        let value = text.parse::<u8>().map_err(|_| EINVAL)?;
>
> Same comment about kstrtobool here as on the other patch.

This one does not parse a bool, it parses the `IRQMode` enum. I don't
think we need to use kernel C integer parsing functions here, if that is
what you suggest.


Best regards,
Andreas Hindborg