* [PATCH v6 01/18] rust: str: normalize imports in `str.rs`
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-27 12:25 ` Daniel Almeida
2025-08-22 12:14 ` [PATCH v6 02/18] rust: str: allow `str::Formatter` to format into `&mut [u8]` Andreas Hindborg
` (16 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, 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
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v6 01/18] rust: str: normalize imports in `str.rs`
2025-08-22 12:14 ` [PATCH v6 01/18] rust: str: normalize imports in `str.rs` Andreas Hindborg
@ 2025-08-27 12:25 ` Daniel Almeida
0 siblings, 0 replies; 38+ messages in thread
From: Daniel Almeida @ 2025-08-27 12:25 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Clean up imports in `str.rs`. This makes future code manipulation more
> manageable.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/str.rs | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index 6c892550c0ba..082790b7a621 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -2,12 +2,13 @@
>
> //! String representations.
>
> -use crate::alloc::{flags::*, AllocError, KVec};
> -use crate::fmt::{self, Write};
> +use crate::{
> + alloc::{flags::*, AllocError, KVec},
> + fmt::{self, Write},
> + prelude::*,
> +};
> use core::ops::{self, Deref, DerefMut, Index};
>
> -use crate::prelude::*;
> -
> /// Byte string without UTF-8 validity guarantee.
> #[repr(transparent)]
> pub struct BStr([u8]);
>
> --
> 2.47.2
>
>
>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v6 02/18] rust: str: allow `str::Formatter` to format into `&mut [u8]`.
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
2025-08-22 12:14 ` [PATCH v6 01/18] rust: str: normalize imports in `str.rs` Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-22 12:14 ` [PATCH v6 03/18] rust: str: expose `str::{Formatter, RawFormatter}` publicly Andreas Hindborg
` (15 subsequent siblings)
17 siblings, 0 replies; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg
Improve `Formatter` so that it can write to an array or slice buffer.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/str.rs | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 082790b7a621..76632da357a6 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -7,7 +7,10 @@
fmt::{self, Write},
prelude::*,
};
-use core::ops::{self, Deref, DerefMut, Index};
+use core::{
+ marker::PhantomData,
+ ops::{self, Deref, DerefMut, Index},
+};
/// Byte string without UTF-8 validity guarantee.
#[repr(transparent)]
@@ -825,9 +828,9 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
/// Allows formatting of [`fmt::Arguments`] into a raw buffer.
///
/// Fails if callers attempt to write more than will fit in the buffer.
-pub(crate) struct Formatter(RawFormatter);
+pub(crate) struct Formatter<'a>(RawFormatter, PhantomData<&'a mut ()>);
-impl Formatter {
+impl Formatter<'_> {
/// Creates a new instance of [`Formatter`] with the given buffer.
///
/// # Safety
@@ -836,11 +839,19 @@ impl Formatter {
/// for the lifetime of the returned [`Formatter`].
pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
// SAFETY: The safety requirements of this function satisfy those of the callee.
- Self(unsafe { RawFormatter::from_buffer(buf, len) })
+ Self(unsafe { RawFormatter::from_buffer(buf, len) }, PhantomData)
+ }
+
+ /// Create a new [`Self`] instance.
+ #[expect(dead_code)]
+ pub(crate) fn new(buffer: &mut [u8]) -> Self {
+ // SAFETY: `buffer` is valid for writes for the entire length for
+ // the lifetime of `Self`.
+ unsafe { Formatter::from_buffer(buffer.as_mut_ptr(), buffer.len()) }
}
}
-impl Deref for Formatter {
+impl Deref for Formatter<'_> {
type Target = RawFormatter;
fn deref(&self) -> &Self::Target {
@@ -848,7 +859,7 @@ fn deref(&self) -> &Self::Target {
}
}
-impl fmt::Write for Formatter {
+impl fmt::Write for Formatter<'_> {
fn write_str(&mut self, s: &str) -> fmt::Result {
self.0.write_str(s)?;
--
2.47.2
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v6 03/18] rust: str: expose `str::{Formatter, RawFormatter}` publicly.
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
2025-08-22 12:14 ` [PATCH v6 01/18] rust: str: normalize imports in `str.rs` Andreas Hindborg
2025-08-22 12:14 ` [PATCH v6 02/18] rust: str: allow `str::Formatter` to format into `&mut [u8]` Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-27 12:57 ` Daniel Almeida
2025-08-22 12:14 ` [PATCH v6 04/18] rust: str: introduce `NullTerminatedFormatter` Andreas Hindborg
` (14 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg
rnull is going to make use of `str::Formatter` and `str::RawFormatter`, so
expose them with public visibility.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/str.rs | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 76632da357a6..46cdc85dad63 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -736,7 +736,7 @@ fn test_bstr_debug() -> Result {
///
/// The memory region between `pos` (inclusive) and `end` (exclusive) is valid for writes if `pos`
/// is less than `end`.
-pub(crate) struct RawFormatter {
+pub struct RawFormatter {
// Use `usize` to use `saturating_*` functions.
beg: usize,
pos: usize,
@@ -794,7 +794,7 @@ pub(crate) fn pos(&self) -> *mut u8 {
}
/// Returns the number of bytes written to the formatter.
- pub(crate) fn bytes_written(&self) -> usize {
+ pub fn bytes_written(&self) -> usize {
self.pos - self.beg
}
}
@@ -828,7 +828,7 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
/// Allows formatting of [`fmt::Arguments`] into a raw buffer.
///
/// Fails if callers attempt to write more than will fit in the buffer.
-pub(crate) struct Formatter<'a>(RawFormatter, PhantomData<&'a mut ()>);
+pub struct Formatter<'a>(RawFormatter, PhantomData<&'a mut ()>);
impl Formatter<'_> {
/// Creates a new instance of [`Formatter`] with the given buffer.
@@ -843,8 +843,7 @@ pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
}
/// Create a new [`Self`] instance.
- #[expect(dead_code)]
- pub(crate) fn new(buffer: &mut [u8]) -> Self {
+ pub fn new(buffer: &mut [u8]) -> Self {
// SAFETY: `buffer` is valid for writes for the entire length for
// the lifetime of `Self`.
unsafe { Formatter::from_buffer(buffer.as_mut_ptr(), buffer.len()) }
--
2.47.2
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v6 03/18] rust: str: expose `str::{Formatter, RawFormatter}` publicly.
2025-08-22 12:14 ` [PATCH v6 03/18] rust: str: expose `str::{Formatter, RawFormatter}` publicly Andreas Hindborg
@ 2025-08-27 12:57 ` Daniel Almeida
0 siblings, 0 replies; 38+ messages in thread
From: Daniel Almeida @ 2025-08-27 12:57 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> rnull is going to make use of `str::Formatter` and `str::RawFormatter`, so
> expose them with public visibility.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/str.rs | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index 76632da357a6..46cdc85dad63 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -736,7 +736,7 @@ fn test_bstr_debug() -> Result {
> ///
> /// The memory region between `pos` (inclusive) and `end` (exclusive) is valid for writes if `pos`
> /// is less than `end`.
> -pub(crate) struct RawFormatter {
> +pub struct RawFormatter {
> // Use `usize` to use `saturating_*` functions.
> beg: usize,
> pos: usize,
> @@ -794,7 +794,7 @@ pub(crate) fn pos(&self) -> *mut u8 {
> }
>
> /// Returns the number of bytes written to the formatter.
> - pub(crate) fn bytes_written(&self) -> usize {
> + pub fn bytes_written(&self) -> usize {
> self.pos - self.beg
> }
> }
> @@ -828,7 +828,7 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
> /// Allows formatting of [`fmt::Arguments`] into a raw buffer.
> ///
> /// Fails if callers attempt to write more than will fit in the buffer.
> -pub(crate) struct Formatter<'a>(RawFormatter, PhantomData<&'a mut ()>);
> +pub struct Formatter<'a>(RawFormatter, PhantomData<&'a mut ()>);
>
> impl Formatter<'_> {
> /// Creates a new instance of [`Formatter`] with the given buffer.
> @@ -843,8 +843,7 @@ pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
> }
>
> /// Create a new [`Self`] instance.
> - #[expect(dead_code)]
> - pub(crate) fn new(buffer: &mut [u8]) -> Self {
> + pub fn new(buffer: &mut [u8]) -> Self {
> // SAFETY: `buffer` is valid for writes for the entire length for
> // the lifetime of `Self`.
> unsafe { Formatter::from_buffer(buffer.as_mut_ptr(), buffer.len()) }
>
> --
> 2.47.2
>
>
>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v6 04/18] rust: str: introduce `NullTerminatedFormatter`
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
` (2 preceding siblings ...)
2025-08-22 12:14 ` [PATCH v6 03/18] rust: str: expose `str::{Formatter, RawFormatter}` publicly Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-27 13:02 ` Daniel Almeida
2025-08-22 12:14 ` [PATCH v6 05/18] rust: str: introduce `kstrtobool` function Andreas Hindborg
` (13 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg
Add `NullTerminatedFormatter`, a formatter that writes a null terminated
string to an array or slice buffer. Because this type needs to manage the
trailing null marker, the existing formatters cannot be used to implement
this type.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/str.rs | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 46cdc85dad63..d8326f7bc9c1 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -871,6 +871,55 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
}
}
+/// A mutable reference to a byte buffer where a string can be written into.
+///
+/// The buffer will be automatically null terminated after the last written character.
+///
+/// # Invariants
+///
+/// * The first byte of `buffer` is always zero.
+/// * The length of `buffer` is at least 1.
+pub(crate) struct NullTerminatedFormatter<'a> {
+ buffer: &'a mut [u8],
+}
+
+impl<'a> NullTerminatedFormatter<'a> {
+ /// Create a new [`Self`] instance.
+ #[expect(dead_code)]
+ pub(crate) fn new(buffer: &'a mut [u8]) -> Option<NullTerminatedFormatter<'a>> {
+ *(buffer.first_mut()?) = 0;
+
+ // INVARIANT:
+ // - We wrote zero to the first byte above.
+ // - If buffer was not at least length 1, `buffer.first_mut()` would return None.
+ Some(Self { buffer })
+ }
+}
+
+impl Write for NullTerminatedFormatter<'_> {
+ fn write_str(&mut self, s: &str) -> fmt::Result {
+ let bytes = s.as_bytes();
+ let len = bytes.len();
+
+ // We want space for a zero. By type invariant, buffer length is always at least 1, so no
+ // underflow.
+ if len > self.buffer.len() - 1 {
+ return Err(fmt::Error);
+ }
+
+ let buffer = core::mem::take(&mut self.buffer);
+ // We break the zero start invariant for a short while.
+ buffer[..len].copy_from_slice(bytes);
+ // INVARIANT: We checked above that buffer will have size at least 1 after this assignment.
+ self.buffer = &mut buffer[len..];
+
+ // INVARIANT: We write zero to the first byte of the buffer.
+ self.buffer[0] = 0;
+
+ Ok(())
+ }
+}
+
/// An owned string that is guaranteed to have exactly one `NUL` byte, which is at the end.
///
/// Used for interoperability with kernel APIs that take C strings.
--
2.47.2
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v6 04/18] rust: str: introduce `NullTerminatedFormatter`
2025-08-22 12:14 ` [PATCH v6 04/18] rust: str: introduce `NullTerminatedFormatter` Andreas Hindborg
@ 2025-08-27 13:02 ` Daniel Almeida
0 siblings, 0 replies; 38+ messages in thread
From: Daniel Almeida @ 2025-08-27 13:02 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Add `NullTerminatedFormatter`, a formatter that writes a null terminated
> string to an array or slice buffer. Because this type needs to manage the
> trailing null marker, the existing formatters cannot be used to implement
> this type.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/str.rs | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index 46cdc85dad63..d8326f7bc9c1 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -871,6 +871,55 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
> }
> }
>
> +/// A mutable reference to a byte buffer where a string can be written into.
> +///
> +/// The buffer will be automatically null terminated after the last written character.
> +///
> +/// # Invariants
> +///
> +/// * The first byte of `buffer` is always zero.
> +/// * The length of `buffer` is at least 1.
> +pub(crate) struct NullTerminatedFormatter<'a> {
> + buffer: &'a mut [u8],
> +}
> +
> +impl<'a> NullTerminatedFormatter<'a> {
> + /// Create a new [`Self`] instance.
> + #[expect(dead_code)]
> + pub(crate) fn new(buffer: &'a mut [u8]) -> Option<NullTerminatedFormatter<'a>> {
> + *(buffer.first_mut()?) = 0;
> +
> + // INVARIANT:
> + // - We wrote zero to the first byte above.
> + // - If buffer was not at least length 1, `buffer.first_mut()` would return None.
> + Some(Self { buffer })
> + }
> +}
> +
> +impl Write for NullTerminatedFormatter<'_> {
> + fn write_str(&mut self, s: &str) -> fmt::Result {
> + let bytes = s.as_bytes();
> + let len = bytes.len();
> +
> + // We want space for a zero. By type invariant, buffer length is always at least 1, so no
> + // underflow.
> + if len > self.buffer.len() - 1 {
> + return Err(fmt::Error);
> + }
> +
> + let buffer = core::mem::take(&mut self.buffer);
> + // We break the zero start invariant for a short while.
> + buffer[..len].copy_from_slice(bytes);
> + // INVARIANT: We checked above that buffer will have size at least 1 after this assignment.
> + self.buffer = &mut buffer[len..];
> +
> + // INVARIANT: We write zero to the first byte of the buffer.
> + self.buffer[0] = 0;
> +
> + Ok(())
> + }
> +}
> +
> /// An owned string that is guaranteed to have exactly one `NUL` byte, which is at the end.
> ///
> /// Used for interoperability with kernel APIs that take C strings.
>
> --
> 2.47.2
>
>
>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v6 05/18] rust: str: introduce `kstrtobool` function
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
` (3 preceding siblings ...)
2025-08-22 12:14 ` [PATCH v6 04/18] rust: str: introduce `NullTerminatedFormatter` Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-27 13:10 ` Daniel Almeida
2025-08-22 12:14 ` [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function Andreas Hindborg
` (12 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg
Add a Rust wrapper for the kernel's `kstrtobool` function that converts
common user inputs into boolean values.
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/str.rs | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index d8326f7bc9c1..d070c0bd86c3 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -4,6 +4,7 @@
use crate::{
alloc::{flags::*, AllocError, KVec},
+ error::Result,
fmt::{self, Write},
prelude::*,
};
@@ -920,6 +921,62 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
}
}
+/// Convert common user inputs into boolean values using the kernel's `kstrtobool` function.
+///
+/// This routine returns `Ok(bool)` if the first character is one of 'YyTt1NnFf0', or
+/// \[oO\]\[NnFf\] for "on" and "off". Otherwise it will return `Err(EINVAL)`.
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::{c_str, str::kstrtobool};
+///
+/// // Lowercase
+/// assert_eq!(kstrtobool(c_str!("true")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("tr")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("t")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("twrong")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("false")), Ok(false));
+/// assert_eq!(kstrtobool(c_str!("f")), Ok(false));
+/// assert_eq!(kstrtobool(c_str!("yes")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("no")), Ok(false));
+/// assert_eq!(kstrtobool(c_str!("on")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("off")), Ok(false));
+///
+/// // Camel case
+/// assert_eq!(kstrtobool(c_str!("True")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("False")), Ok(false));
+/// assert_eq!(kstrtobool(c_str!("Yes")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("No")), Ok(false));
+/// assert_eq!(kstrtobool(c_str!("On")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("Off")), Ok(false));
+///
+/// // All caps
+/// assert_eq!(kstrtobool(c_str!("TRUE")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("FALSE")), Ok(false));
+/// assert_eq!(kstrtobool(c_str!("YES")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("NO")), Ok(false));
+/// assert_eq!(kstrtobool(c_str!("ON")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("OFF")), Ok(false));
+///
+/// // Numeric
+/// assert_eq!(kstrtobool(c_str!("1")), Ok(true));
+/// assert_eq!(kstrtobool(c_str!("0")), Ok(false));
+///
+/// // Invalid input
+/// assert_eq!(kstrtobool(c_str!("invalid")), Err(EINVAL));
+/// assert_eq!(kstrtobool(c_str!("2")), Err(EINVAL));
+/// ```
+pub fn kstrtobool(string: &CStr) -> Result<bool> {
+ let mut result: bool = false;
+
+ // SAFETY: `string` is a valid null-terminated C string, and `result` is a valid
+ // pointer to a bool that we own.
+ let ret = unsafe { bindings::kstrtobool(string.as_char_ptr(), &mut result) };
+
+ kernel::error::to_result(ret).map(|()| result)
+}
+
/// An owned string that is guaranteed to have exactly one `NUL` byte, which is at the end.
///
/// Used for interoperability with kernel APIs that take C strings.
--
2.47.2
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v6 05/18] rust: str: introduce `kstrtobool` function
2025-08-22 12:14 ` [PATCH v6 05/18] rust: str: introduce `kstrtobool` function Andreas Hindborg
@ 2025-08-27 13:10 ` Daniel Almeida
0 siblings, 0 replies; 38+ messages in thread
From: Daniel Almeida @ 2025-08-27 13:10 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Add a Rust wrapper for the kernel's `kstrtobool` function that converts
> common user inputs into boolean values.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/str.rs | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index d8326f7bc9c1..d070c0bd86c3 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -4,6 +4,7 @@
>
> use crate::{
> alloc::{flags::*, AllocError, KVec},
> + error::Result,
> fmt::{self, Write},
> prelude::*,
> };
> @@ -920,6 +921,62 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
> }
> }
>
> +/// Convert common user inputs into boolean values using the kernel's `kstrtobool` function.
> +///
> +/// This routine returns `Ok(bool)` if the first character is one of 'YyTt1NnFf0', or
> +/// \[oO\]\[NnFf\] for "on" and "off". Otherwise it will return `Err(EINVAL)`.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// # use kernel::{c_str, str::kstrtobool};
> +///
> +/// // Lowercase
> +/// assert_eq!(kstrtobool(c_str!("true")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("tr")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("t")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("twrong")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("false")), Ok(false));
> +/// assert_eq!(kstrtobool(c_str!("f")), Ok(false));
> +/// assert_eq!(kstrtobool(c_str!("yes")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("no")), Ok(false));
> +/// assert_eq!(kstrtobool(c_str!("on")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("off")), Ok(false));
> +///
> +/// // Camel case
> +/// assert_eq!(kstrtobool(c_str!("True")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("False")), Ok(false));
> +/// assert_eq!(kstrtobool(c_str!("Yes")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("No")), Ok(false));
> +/// assert_eq!(kstrtobool(c_str!("On")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("Off")), Ok(false));
> +///
> +/// // All caps
> +/// assert_eq!(kstrtobool(c_str!("TRUE")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("FALSE")), Ok(false));
> +/// assert_eq!(kstrtobool(c_str!("YES")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("NO")), Ok(false));
> +/// assert_eq!(kstrtobool(c_str!("ON")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("OFF")), Ok(false));
> +///
> +/// // Numeric
> +/// assert_eq!(kstrtobool(c_str!("1")), Ok(true));
> +/// assert_eq!(kstrtobool(c_str!("0")), Ok(false));
> +///
> +/// // Invalid input
> +/// assert_eq!(kstrtobool(c_str!("invalid")), Err(EINVAL));
> +/// assert_eq!(kstrtobool(c_str!("2")), Err(EINVAL));
> +/// ```
> +pub fn kstrtobool(string: &CStr) -> Result<bool> {
> + let mut result: bool = false;
> +
> + // SAFETY: `string` is a valid null-terminated C string, and `result` is a valid
> + // pointer to a bool that we own.
> + let ret = unsafe { bindings::kstrtobool(string.as_char_ptr(), &mut result) };
> +
> + kernel::error::to_result(ret).map(|()| result)
> +}
> +
> /// An owned string that is guaranteed to have exactly one `NUL` byte, which is at the end.
> ///
> /// Used for interoperability with kernel APIs that take C strings.
>
> --
> 2.47.2
>
>
>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
` (4 preceding siblings ...)
2025-08-22 12:14 ` [PATCH v6 05/18] rust: str: introduce `kstrtobool` function Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-27 13:46 ` Daniel Almeida
` (2 more replies)
2025-08-22 12:14 ` [PATCH v6 07/18] rust: configfs: re-export `configfs_attrs` from `configfs` module Andreas Hindborg
` (11 subsequent siblings)
17 siblings, 3 replies; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg
Add a convenience function to convert byte slices to boolean values by
wrapping them in a null-terminated C string and delegating to the
existing `kstrtobool` function. Only considers the first two bytes of
the input slice, following the kernel's boolean parsing semantics.
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/str.rs | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index d070c0bd86c3..b185262b4851 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -921,6 +921,20 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
}
}
+/// # Safety
+///
+/// - `string` must point to a null terminated string that is valid for read.
+unsafe fn kstrtobool_raw(string: *const u8) -> Result<bool> {
+ let mut result: bool = false;
+
+ // SAFETY:
+ // - By function safety requirement, `string` is a valid null-terminated string.
+ // - `result` is a valid `bool` that we own.
+ let ret = unsafe { bindings::kstrtobool(string, &mut result) };
+
+ kernel::error::to_result(ret).map(|()| result)
+}
+
/// Convert common user inputs into boolean values using the kernel's `kstrtobool` function.
///
/// This routine returns `Ok(bool)` if the first character is one of 'YyTt1NnFf0', or
@@ -968,13 +982,22 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
/// assert_eq!(kstrtobool(c_str!("2")), Err(EINVAL));
/// ```
pub fn kstrtobool(string: &CStr) -> Result<bool> {
- let mut result: bool = false;
-
- // SAFETY: `string` is a valid null-terminated C string, and `result` is a valid
- // pointer to a bool that we own.
- let ret = unsafe { bindings::kstrtobool(string.as_char_ptr(), &mut result) };
+ // SAFETY:
+ // - The pointer returned by `CStr::as_char_ptr` is guaranteed to be
+ // null terminated.
+ // - `string` is live and thus the string is valid for read.
+ unsafe { kstrtobool_raw(string.as_char_ptr()) }
+}
- kernel::error::to_result(ret).map(|()| result)
+/// Convert `&[u8]` to `bool` by deferring to [`kernel::str::kstrtobool`].
+///
+/// Only considers at most the first two bytes of `bytes`.
+pub fn kstrtobool_bytes(bytes: &[u8]) -> Result<bool> {
+ // `ktostrbool` only considers the first two bytes of the input.
+ let stack_string = [*bytes.first().unwrap_or(&0), *bytes.get(1).unwrap_or(&0), 0];
+ // SAFETY: `stack_string` is null terminated and it is live on the stack so
+ // it is valid for read.
+ unsafe { kstrtobool_raw(stack_string.as_ptr()) }
}
/// An owned string that is guaranteed to have exactly one `NUL` byte, which is at the end.
--
2.47.2
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function
2025-08-22 12:14 ` [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function Andreas Hindborg
@ 2025-08-27 13:46 ` Daniel Almeida
2025-09-01 8:07 ` Andreas Hindborg
2025-09-01 8:36 ` Alice Ryhl
2025-09-01 8:34 ` Alice Ryhl
2025-09-01 14:40 ` Daniel Almeida
2 siblings, 2 replies; 38+ messages in thread
From: Daniel Almeida @ 2025-08-27 13:46 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Add a convenience function to convert byte slices to boolean values by
> wrapping them in a null-terminated C string and delegating to the
> existing `kstrtobool` function. Only considers the first two bytes of
> the input slice, following the kernel's boolean parsing semantics.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/str.rs | 35 +++++++++++++++++++++++++++++------
> 1 file changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index d070c0bd86c3..b185262b4851 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -921,6 +921,20 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
> }
> }
>
> +/// # Safety
> +///
> +/// - `string` must point to a null terminated string that is valid for read.
> +unsafe fn kstrtobool_raw(string: *const u8) -> Result<bool> {
> + let mut result: bool = false;
> +
> + // SAFETY:
> + // - By function safety requirement, `string` is a valid null-terminated string.
> + // - `result` is a valid `bool` that we own.
> + let ret = unsafe { bindings::kstrtobool(string, &mut result) };
> +
> + kernel::error::to_result(ret).map(|()| result)
> +}
> +
> /// Convert common user inputs into boolean values using the kernel's `kstrtobool` function.
> ///
> /// This routine returns `Ok(bool)` if the first character is one of 'YyTt1NnFf0', or
> @@ -968,13 +982,22 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
> /// assert_eq!(kstrtobool(c_str!("2")), Err(EINVAL));
> /// ```
> pub fn kstrtobool(string: &CStr) -> Result<bool> {
> - let mut result: bool = false;
> -
> - // SAFETY: `string` is a valid null-terminated C string, and `result` is a valid
> - // pointer to a bool that we own.
> - let ret = unsafe { bindings::kstrtobool(string.as_char_ptr(), &mut result) };
> + // SAFETY:
> + // - The pointer returned by `CStr::as_char_ptr` is guaranteed to be
> + // null terminated.
> + // - `string` is live and thus the string is valid for read.
> + unsafe { kstrtobool_raw(string.as_char_ptr()) }
> +}
>
> - kernel::error::to_result(ret).map(|()| result)
> +/// Convert `&[u8]` to `bool` by deferring to [`kernel::str::kstrtobool`].
> +///
> +/// Only considers at most the first two bytes of `bytes`.
> +pub fn kstrtobool_bytes(bytes: &[u8]) -> Result<bool> {
> + // `ktostrbool` only considers the first two bytes of the input.
> + let stack_string = [*bytes.first().unwrap_or(&0), *bytes.get(1).unwrap_or(&0), 0];
Can’t this be CStr::from_bytes_with_nul() ?
This means that kstrtobool_raw could take a &CStr directly and thus not be unsafe IIUC?
> + // SAFETY: `stack_string` is null terminated and it is live on the stack so
> + // it is valid for read.
> + unsafe { kstrtobool_raw(stack_string.as_ptr()) }
> }
>
> /// An owned string that is guaranteed to have exactly one `NUL` byte, which is at the end.
>
> --
> 2.47.2
>
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function
2025-08-27 13:46 ` Daniel Almeida
@ 2025-09-01 8:07 ` Andreas Hindborg
2025-09-01 8:36 ` Alice Ryhl
1 sibling, 0 replies; 38+ messages in thread
From: Andreas Hindborg @ 2025-09-01 8:07 UTC (permalink / raw)
To: Daniel Almeida
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
"Daniel Almeida" <daniel.almeida@collabora.com> writes:
>> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
<cut>
>> - kernel::error::to_result(ret).map(|()| result)
>> +/// Convert `&[u8]` to `bool` by deferring to [`kernel::str::kstrtobool`].
>> +///
>> +/// Only considers at most the first two bytes of `bytes`.
>> +pub fn kstrtobool_bytes(bytes: &[u8]) -> Result<bool> {
>> + // `ktostrbool` only considers the first two bytes of the input.
>> + let stack_string = [*bytes.first().unwrap_or(&0), *bytes.get(1).unwrap_or(&0), 0];
>
> Can’t this be CStr::from_bytes_with_nul() ?
>
> This means that kstrtobool_raw could take a &CStr directly and thus not be unsafe IIUC?
By design, the input to this function need not be null terminated. My
use case is parsing the contents of a configfs file, and I would not
want to change the contents of the file, or allocate to create a null
terminated string, before calling this method.
We could add another function `kstrtobool_cstr` to do what you are
asking, but I think that could be a separate patch.
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function
2025-08-27 13:46 ` Daniel Almeida
2025-09-01 8:07 ` Andreas Hindborg
@ 2025-09-01 8:36 ` Alice Ryhl
1 sibling, 0 replies; 38+ messages in thread
From: Alice Ryhl @ 2025-09-01 8:36 UTC (permalink / raw)
To: Daniel Almeida
Cc: Andreas Hindborg, Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
On Wed, Aug 27, 2025 at 3:46 PM Daniel Almeida
<daniel.almeida@collabora.com> wrote:
>
>
>
> > On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> >
> > Add a convenience function to convert byte slices to boolean values by
> > wrapping them in a null-terminated C string and delegating to the
> > existing `kstrtobool` function. Only considers the first two bytes of
> > the input slice, following the kernel's boolean parsing semantics.
> >
> > Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> > ---
> > rust/kernel/str.rs | 35 +++++++++++++++++++++++++++++------
> > 1 file changed, 29 insertions(+), 6 deletions(-)
> >
> > diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> > index d070c0bd86c3..b185262b4851 100644
> > --- a/rust/kernel/str.rs
> > +++ b/rust/kernel/str.rs
> > @@ -921,6 +921,20 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
> > }
> > }
> >
> > +/// # Safety
> > +///
> > +/// - `string` must point to a null terminated string that is valid for read.
> > +unsafe fn kstrtobool_raw(string: *const u8) -> Result<bool> {
> > + let mut result: bool = false;
> > +
> > + // SAFETY:
> > + // - By function safety requirement, `string` is a valid null-terminated string.
> > + // - `result` is a valid `bool` that we own.
> > + let ret = unsafe { bindings::kstrtobool(string, &mut result) };
> > +
> > + kernel::error::to_result(ret).map(|()| result)
> > +}
> > +
> > /// Convert common user inputs into boolean values using the kernel's `kstrtobool` function.
> > ///
> > /// This routine returns `Ok(bool)` if the first character is one of 'YyTt1NnFf0', or
> > @@ -968,13 +982,22 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
> > /// assert_eq!(kstrtobool(c_str!("2")), Err(EINVAL));
> > /// ```
> > pub fn kstrtobool(string: &CStr) -> Result<bool> {
> > - let mut result: bool = false;
> > -
> > - // SAFETY: `string` is a valid null-terminated C string, and `result` is a valid
> > - // pointer to a bool that we own.
> > - let ret = unsafe { bindings::kstrtobool(string.as_char_ptr(), &mut result) };
> > + // SAFETY:
> > + // - The pointer returned by `CStr::as_char_ptr` is guaranteed to be
> > + // null terminated.
> > + // - `string` is live and thus the string is valid for read.
> > + unsafe { kstrtobool_raw(string.as_char_ptr()) }
> > +}
> >
> > - kernel::error::to_result(ret).map(|()| result)
> > +/// Convert `&[u8]` to `bool` by deferring to [`kernel::str::kstrtobool`].
> > +///
> > +/// Only considers at most the first two bytes of `bytes`.
> > +pub fn kstrtobool_bytes(bytes: &[u8]) -> Result<bool> {
> > + // `ktostrbool` only considers the first two bytes of the input.
> > + let stack_string = [*bytes.first().unwrap_or(&0), *bytes.get(1).unwrap_or(&0), 0];
>
> Can’t this be CStr::from_bytes_with_nul() ?
>
> This means that kstrtobool_raw could take a &CStr directly and thus not be unsafe IIUC?
That's what Andreas did in the previous version. It ended up being
pretty complex because CStr::from_bytes_with_nul() requires that we
compute the length of `stack_string`, which isn't really needed here.
I recommended having a _raw method like this to avoid that complexity.
I don't think this is meaningfully more unsafe the way it is in this
patch.
Alice
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function
2025-08-22 12:14 ` [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function Andreas Hindborg
2025-08-27 13:46 ` Daniel Almeida
@ 2025-09-01 8:34 ` Alice Ryhl
2025-09-01 14:40 ` Daniel Almeida
2 siblings, 0 replies; 38+ messages in thread
From: Alice Ryhl @ 2025-09-01 8:34 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
On Fri, Aug 22, 2025 at 2:15 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Add a convenience function to convert byte slices to boolean values by
> wrapping them in a null-terminated C string and delegating to the
> existing `kstrtobool` function. Only considers the first two bytes of
> the input slice, following the kernel's boolean parsing semantics.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
One nit below, but generally looks good.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> +/// # Safety
> +///
> +/// - `string` must point to a null terminated string that is valid for read.
> +unsafe fn kstrtobool_raw(string: *const u8) -> Result<bool> {
> + let mut result: bool = false;
> +
> + // SAFETY:
> + // - By function safety requirement, `string` is a valid null-terminated string.
> + // - `result` is a valid `bool` that we own.
> + let ret = unsafe { bindings::kstrtobool(string, &mut result) };
> +
> + kernel::error::to_result(ret).map(|()| result)
I think this is easier to read as:
to_result(unsafe { bindings::kstrtobool(string, &mut result) })?;
Ok(result)
Alice
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function
2025-08-22 12:14 ` [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function Andreas Hindborg
2025-08-27 13:46 ` Daniel Almeida
2025-09-01 8:34 ` Alice Ryhl
@ 2025-09-01 14:40 ` Daniel Almeida
2 siblings, 0 replies; 38+ messages in thread
From: Daniel Almeida @ 2025-09-01 14:40 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Add a convenience function to convert byte slices to boolean values by
> wrapping them in a null-terminated C string and delegating to the
> existing `kstrtobool` function. Only considers the first two bytes of
> the input slice, following the kernel's boolean parsing semantics.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/str.rs | 35 +++++++++++++++++++++++++++++------
> 1 file changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index d070c0bd86c3..b185262b4851 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -921,6 +921,20 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
> }
> }
>
> +/// # Safety
> +///
> +/// - `string` must point to a null terminated string that is valid for read.
> +unsafe fn kstrtobool_raw(string: *const u8) -> Result<bool> {
> + let mut result: bool = false;
> +
> + // SAFETY:
> + // - By function safety requirement, `string` is a valid null-terminated string.
> + // - `result` is a valid `bool` that we own.
> + let ret = unsafe { bindings::kstrtobool(string, &mut result) };
> +
> + kernel::error::to_result(ret).map(|()| result)
> +}
> +
> /// Convert common user inputs into boolean values using the kernel's `kstrtobool` function.
> ///
> /// This routine returns `Ok(bool)` if the first character is one of 'YyTt1NnFf0', or
> @@ -968,13 +982,22 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
> /// assert_eq!(kstrtobool(c_str!("2")), Err(EINVAL));
> /// ```
> pub fn kstrtobool(string: &CStr) -> Result<bool> {
> - let mut result: bool = false;
> -
> - // SAFETY: `string` is a valid null-terminated C string, and `result` is a valid
> - // pointer to a bool that we own.
> - let ret = unsafe { bindings::kstrtobool(string.as_char_ptr(), &mut result) };
> + // SAFETY:
> + // - The pointer returned by `CStr::as_char_ptr` is guaranteed to be
> + // null terminated.
> + // - `string` is live and thus the string is valid for read.
> + unsafe { kstrtobool_raw(string.as_char_ptr()) }
> +}
>
> - kernel::error::to_result(ret).map(|()| result)
> +/// Convert `&[u8]` to `bool` by deferring to [`kernel::str::kstrtobool`].
> +///
> +/// Only considers at most the first two bytes of `bytes`.
> +pub fn kstrtobool_bytes(bytes: &[u8]) -> Result<bool> {
> + // `ktostrbool` only considers the first two bytes of the input.
> + let stack_string = [*bytes.first().unwrap_or(&0), *bytes.get(1).unwrap_or(&0), 0];
> + // SAFETY: `stack_string` is null terminated and it is live on the stack so
> + // it is valid for read.
> + unsafe { kstrtobool_raw(stack_string.as_ptr()) }
> }
>
> /// An owned string that is guaranteed to have exactly one `NUL` byte, which is at the end.
>
> --
> 2.47.2
>
>
>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v6 07/18] rust: configfs: re-export `configfs_attrs` from `configfs` module
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
` (5 preceding siblings ...)
2025-08-22 12:14 ` [PATCH v6 06/18] rust: str: add `bytes_to_bool` helper function Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-27 13:48 ` Daniel Almeida
2025-08-22 12:14 ` [PATCH v6 08/18] rust: block: normalize imports for `gen_disk.rs` Andreas Hindborg
` (10 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg
Re-export `configfs_attrs` from `configfs` module, so that users can import
the macro from the `configfs` module rather than the root of the `kernel`
crate.
Also update users to import from the new path.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/configfs.rs | 2 ++
samples/rust/rust_configfs.rs | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index 2736b798cdc6..318a2f073d1c 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -1039,3 +1039,5 @@ macro_rules! configfs_attrs {
};
}
+
+pub use crate::configfs_attrs;
diff --git a/samples/rust/rust_configfs.rs b/samples/rust/rust_configfs.rs
index af04bfa35cb2..ad364fb93e53 100644
--- a/samples/rust/rust_configfs.rs
+++ b/samples/rust/rust_configfs.rs
@@ -5,7 +5,7 @@
use kernel::alloc::flags;
use kernel::c_str;
use kernel::configfs;
-use kernel::configfs_attrs;
+use kernel::configfs::configfs_attrs;
use kernel::new_mutex;
use kernel::page::PAGE_SIZE;
use kernel::prelude::*;
--
2.47.2
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v6 07/18] rust: configfs: re-export `configfs_attrs` from `configfs` module
2025-08-22 12:14 ` [PATCH v6 07/18] rust: configfs: re-export `configfs_attrs` from `configfs` module Andreas Hindborg
@ 2025-08-27 13:48 ` Daniel Almeida
0 siblings, 0 replies; 38+ messages in thread
From: Daniel Almeida @ 2025-08-27 13:48 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Re-export `configfs_attrs` from `configfs` module, so that users can import
> the macro from the `configfs` module rather than the root of the `kernel`
> crate.
>
> Also update users to import from the new path.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/configfs.rs | 2 ++
> samples/rust/rust_configfs.rs | 2 +-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
> index 2736b798cdc6..318a2f073d1c 100644
> --- a/rust/kernel/configfs.rs
> +++ b/rust/kernel/configfs.rs
> @@ -1039,3 +1039,5 @@ macro_rules! configfs_attrs {
> };
>
> }
> +
> +pub use crate::configfs_attrs;
> diff --git a/samples/rust/rust_configfs.rs b/samples/rust/rust_configfs.rs
> index af04bfa35cb2..ad364fb93e53 100644
> --- a/samples/rust/rust_configfs.rs
> +++ b/samples/rust/rust_configfs.rs
> @@ -5,7 +5,7 @@
> use kernel::alloc::flags;
> use kernel::c_str;
> use kernel::configfs;
> -use kernel::configfs_attrs;
> +use kernel::configfs::configfs_attrs;
> use kernel::new_mutex;
> use kernel::page::PAGE_SIZE;
> use kernel::prelude::*;
>
> --
> 2.47.2
>
>
>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v6 08/18] rust: block: normalize imports for `gen_disk.rs`
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
` (6 preceding siblings ...)
2025-08-22 12:14 ` [PATCH v6 07/18] rust: configfs: re-export `configfs_attrs` from `configfs` module Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-22 12:14 ` [PATCH v6 09/18] rust: block: use `NullTerminatedFormatter` Andreas Hindborg
` (9 subsequent siblings)
17 siblings, 0 replies; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg,
Daniel Almeida
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
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v6 09/18] rust: block: use `NullTerminatedFormatter`
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
` (7 preceding siblings ...)
2025-08-22 12:14 ` [PATCH v6 08/18] rust: block: normalize imports for `gen_disk.rs` Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-27 13:50 ` Daniel Almeida
2025-08-22 12:14 ` [PATCH v6 10/18] rust: block: remove `RawWriter` Andreas Hindborg
` (8 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg
Use the new `NullTerminatedFormatter` to write the name of a `GenDisk` to
the name buffer. This new formatter automatically adds a trailing null
marker after the written characters, so we don't need to append that at the
call site any longer.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/block/mq/gen_disk.rs | 12 +++++++-----
rust/kernel/block/mq/raw_writer.rs | 1 +
rust/kernel/str.rs | 1 -
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index 679ee1bb2195..20f1d46c774d 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -7,9 +7,11 @@
use crate::{
bindings,
- block::mq::{raw_writer::RawWriter, Operations, TagSet},
+ block::mq::{Operations, TagSet},
error::{self, from_err_ptr, Result},
+ prelude::*,
static_lock_class,
+ str::NullTerminatedFormatter,
sync::Arc,
};
use core::fmt::{self, Write};
@@ -143,14 +145,14 @@ pub fn build<T: Operations>(
// SAFETY: `gendisk` is a valid pointer as we initialized it above
unsafe { (*gendisk).fops = &TABLE };
- let mut raw_writer = RawWriter::from_array(
+ let mut writer = NullTerminatedFormatter::new(
// SAFETY: `gendisk` points to a valid and initialized instance. We
// have exclusive access, since the disk is not added to the VFS
// yet.
unsafe { &mut (*gendisk).disk_name },
- )?;
- raw_writer.write_fmt(name)?;
- raw_writer.write_char('\0')?;
+ )
+ .ok_or(EINVAL)?;
+ writer.write_fmt(name)?;
// SAFETY: `gendisk` points to a valid and initialized instance of
// `struct gendisk`. `set_capacity` takes a lock to synchronize this
diff --git a/rust/kernel/block/mq/raw_writer.rs b/rust/kernel/block/mq/raw_writer.rs
index 7e2159e4f6a6..0aef55703e71 100644
--- a/rust/kernel/block/mq/raw_writer.rs
+++ b/rust/kernel/block/mq/raw_writer.rs
@@ -24,6 +24,7 @@ fn new(buffer: &'a mut [u8]) -> Result<RawWriter<'a>> {
Ok(Self { buffer, pos: 0 })
}
+ #[expect(dead_code)]
pub(crate) fn from_array<const N: usize>(
a: &'a mut [crate::ffi::c_char; N],
) -> Result<RawWriter<'a>> {
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index b185262b4851..a3e34f566034 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -886,7 +886,6 @@ pub(crate) struct NullTerminatedFormatter<'a> {
impl<'a> NullTerminatedFormatter<'a> {
/// Create a new [`Self`] instance.
- #[expect(dead_code)]
pub(crate) fn new(buffer: &'a mut [u8]) -> Option<NullTerminatedFormatter<'a>> {
*(buffer.first_mut()?) = 0;
--
2.47.2
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v6 09/18] rust: block: use `NullTerminatedFormatter`
2025-08-22 12:14 ` [PATCH v6 09/18] rust: block: use `NullTerminatedFormatter` Andreas Hindborg
@ 2025-08-27 13:50 ` Daniel Almeida
0 siblings, 0 replies; 38+ messages in thread
From: Daniel Almeida @ 2025-08-27 13:50 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Use the new `NullTerminatedFormatter` to write the name of a `GenDisk` to
> the name buffer. This new formatter automatically adds a trailing null
> marker after the written characters, so we don't need to append that at the
> call site any longer.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/block/mq/gen_disk.rs | 12 +++++++-----
> rust/kernel/block/mq/raw_writer.rs | 1 +
> rust/kernel/str.rs | 1 -
> 3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
> index 679ee1bb2195..20f1d46c774d 100644
> --- a/rust/kernel/block/mq/gen_disk.rs
> +++ b/rust/kernel/block/mq/gen_disk.rs
> @@ -7,9 +7,11 @@
>
> use crate::{
> bindings,
> - block::mq::{raw_writer::RawWriter, Operations, TagSet},
> + block::mq::{Operations, TagSet},
> error::{self, from_err_ptr, Result},
> + prelude::*,
> static_lock_class,
> + str::NullTerminatedFormatter,
> sync::Arc,
> };
> use core::fmt::{self, Write};
> @@ -143,14 +145,14 @@ pub fn build<T: Operations>(
> // SAFETY: `gendisk` is a valid pointer as we initialized it above
> unsafe { (*gendisk).fops = &TABLE };
>
> - let mut raw_writer = RawWriter::from_array(
> + let mut writer = NullTerminatedFormatter::new(
> // SAFETY: `gendisk` points to a valid and initialized instance. We
> // have exclusive access, since the disk is not added to the VFS
> // yet.
> unsafe { &mut (*gendisk).disk_name },
> - )?;
> - raw_writer.write_fmt(name)?;
> - raw_writer.write_char('\0')?;
> + )
> + .ok_or(EINVAL)?;
> + writer.write_fmt(name)?;
>
> // SAFETY: `gendisk` points to a valid and initialized instance of
> // `struct gendisk`. `set_capacity` takes a lock to synchronize this
> diff --git a/rust/kernel/block/mq/raw_writer.rs b/rust/kernel/block/mq/raw_writer.rs
> index 7e2159e4f6a6..0aef55703e71 100644
> --- a/rust/kernel/block/mq/raw_writer.rs
> +++ b/rust/kernel/block/mq/raw_writer.rs
> @@ -24,6 +24,7 @@ fn new(buffer: &'a mut [u8]) -> Result<RawWriter<'a>> {
> Ok(Self { buffer, pos: 0 })
> }
>
> + #[expect(dead_code)]
> pub(crate) fn from_array<const N: usize>(
> a: &'a mut [crate::ffi::c_char; N],
> ) -> Result<RawWriter<'a>> {
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index b185262b4851..a3e34f566034 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -886,7 +886,6 @@ pub(crate) struct NullTerminatedFormatter<'a> {
>
> impl<'a> NullTerminatedFormatter<'a> {
> /// Create a new [`Self`] instance.
> - #[expect(dead_code)]
> pub(crate) fn new(buffer: &'a mut [u8]) -> Option<NullTerminatedFormatter<'a>> {
> *(buffer.first_mut()?) = 0;
>
>
> --
> 2.47.2
>
>
>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v6 10/18] rust: block: remove `RawWriter`
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
` (8 preceding siblings ...)
2025-08-22 12:14 ` [PATCH v6 09/18] rust: block: use `NullTerminatedFormatter` Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-22 12:14 ` [PATCH v6 11/18] rust: block: remove trait bound from `mq::Request` definition Andreas Hindborg
` (7 subsequent siblings)
17 siblings, 0 replies; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg,
Daniel Almeida
`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
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v6 11/18] rust: block: remove trait bound from `mq::Request` definition
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
` (9 preceding siblings ...)
2025-08-22 12:14 ` [PATCH v6 10/18] rust: block: remove `RawWriter` Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-22 12:14 ` [PATCH v6 12/18] rust: block: add block related constants Andreas Hindborg
` (6 subsequent siblings)
17 siblings, 0 replies; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg,
Daniel Almeida
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
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v6 12/18] rust: block: add block related constants
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
` (10 preceding siblings ...)
2025-08-22 12:14 ` [PATCH v6 11/18] rust: block: remove trait bound from `mq::Request` definition Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-27 13:52 ` Daniel Almeida
2025-08-22 12:14 ` [PATCH v6 13/18] rnull: move driver to separate directory Andreas Hindborg
` (5 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg
Add a few block subsystem constants to the rust `kernel::block` name space.
This makes it easier to access the constants from rust code.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/block.rs | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/rust/kernel/block.rs b/rust/kernel/block.rs
index 150f710efe5b..32c8d865afb6 100644
--- a/rust/kernel/block.rs
+++ b/rust/kernel/block.rs
@@ -3,3 +3,16 @@
//! Types for working with the block layer.
pub mod mq;
+
+/// Bit mask for masking out [`SECTOR_SIZE`].
+pub const SECTOR_MASK: u32 = bindings::SECTOR_MASK;
+
+/// Sectors are size `1 << SECTOR_SHIFT`.
+pub const SECTOR_SHIFT: u32 = bindings::SECTOR_SHIFT;
+
+/// Size of a sector.
+pub const SECTOR_SIZE: u32 = bindings::SECTOR_SIZE;
+
+/// The difference between the size of a page and the size of a sector,
+/// expressed as a power of two.
+pub const PAGE_SECTORS_SHIFT: u32 = bindings::PAGE_SECTORS_SHIFT;
--
2.47.2
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v6 12/18] rust: block: add block related constants
2025-08-22 12:14 ` [PATCH v6 12/18] rust: block: add block related constants Andreas Hindborg
@ 2025-08-27 13:52 ` Daniel Almeida
0 siblings, 0 replies; 38+ messages in thread
From: Daniel Almeida @ 2025-08-27 13:52 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Add a few block subsystem constants to the rust `kernel::block` name space.
> This makes it easier to access the constants from rust code.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> rust/kernel/block.rs | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/rust/kernel/block.rs b/rust/kernel/block.rs
> index 150f710efe5b..32c8d865afb6 100644
> --- a/rust/kernel/block.rs
> +++ b/rust/kernel/block.rs
> @@ -3,3 +3,16 @@
> //! Types for working with the block layer.
>
> pub mod mq;
> +
> +/// Bit mask for masking out [`SECTOR_SIZE`].
> +pub const SECTOR_MASK: u32 = bindings::SECTOR_MASK;
> +
> +/// Sectors are size `1 << SECTOR_SHIFT`.
> +pub const SECTOR_SHIFT: u32 = bindings::SECTOR_SHIFT;
> +
> +/// Size of a sector.
> +pub const SECTOR_SIZE: u32 = bindings::SECTOR_SIZE;
> +
> +/// The difference between the size of a page and the size of a sector,
> +/// expressed as a power of two.
> +pub const PAGE_SECTORS_SHIFT: u32 = bindings::PAGE_SECTORS_SHIFT;
>
> --
> 2.47.2
>
>
>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v6 13/18] rnull: move driver to separate directory
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
` (11 preceding siblings ...)
2025-08-22 12:14 ` [PATCH v6 12/18] rust: block: add block related constants Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-22 12:14 ` [PATCH v6 14/18] rnull: enable configuration via `configfs` Andreas Hindborg
` (4 subsequent siblings)
17 siblings, 0 replies; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg,
Daniel Almeida
The rust null block driver is about to gain some additional modules. Rather
than pollute the current directory, move the driver to a subdirectory.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
MAINTAINERS | 2 +-
drivers/block/Kconfig | 10 +---------
drivers/block/Makefile | 4 +---
drivers/block/rnull/Kconfig | 13 +++++++++++++
drivers/block/rnull/Makefile | 3 +++
drivers/block/{ => rnull}/rnull.rs | 0
6 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index daf520a13bdf..d5addcc4b132 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4343,7 +4343,7 @@ W: https://rust-for-linux.com
B: https://github.com/Rust-for-Linux/linux/issues
C: https://rust-for-linux.zulipchat.com/#narrow/stream/Block
T: git https://github.com/Rust-for-Linux/linux.git rust-block-next
-F: drivers/block/rnull.rs
+F: drivers/block/rnull/
F: rust/kernel/block.rs
F: rust/kernel/block/
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index df38fb364904..77d694448990 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -17,6 +17,7 @@ menuconfig BLK_DEV
if BLK_DEV
source "drivers/block/null_blk/Kconfig"
+source "drivers/block/rnull/Kconfig"
config BLK_DEV_FD
tristate "Normal floppy disk support"
@@ -311,15 +312,6 @@ config VIRTIO_BLK
This is the virtual block driver for virtio. It can be used with
QEMU based VMMs (like KVM or Xen). Say Y or M.
-config BLK_DEV_RUST_NULL
- tristate "Rust null block driver (Experimental)"
- depends on RUST
- help
- This is the Rust implementation of the null block driver. For now it
- is only a minimal stub.
-
- If unsure, say N.
-
config BLK_DEV_RBD
tristate "Rados block device (RBD)"
depends on INET && BLOCK
diff --git a/drivers/block/Makefile b/drivers/block/Makefile
index a695ce74ef22..2d8096eb8cdf 100644
--- a/drivers/block/Makefile
+++ b/drivers/block/Makefile
@@ -9,9 +9,6 @@
# needed for trace events
ccflags-y += -I$(src)
-obj-$(CONFIG_BLK_DEV_RUST_NULL) += rnull_mod.o
-rnull_mod-y := rnull.o
-
obj-$(CONFIG_MAC_FLOPPY) += swim3.o
obj-$(CONFIG_BLK_DEV_SWIM) += swim_mod.o
obj-$(CONFIG_BLK_DEV_FD) += floppy.o
@@ -38,6 +35,7 @@ obj-$(CONFIG_ZRAM) += zram/
obj-$(CONFIG_BLK_DEV_RNBD) += rnbd/
obj-$(CONFIG_BLK_DEV_NULL_BLK) += null_blk/
+obj-$(CONFIG_BLK_DEV_RUST_NULL) += rnull/
obj-$(CONFIG_BLK_DEV_UBLK) += ublk_drv.o
obj-$(CONFIG_BLK_DEV_ZONED_LOOP) += zloop.o
diff --git a/drivers/block/rnull/Kconfig b/drivers/block/rnull/Kconfig
new file mode 100644
index 000000000000..6dc5aff96bf4
--- /dev/null
+++ b/drivers/block/rnull/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Rust null block device driver configuration
+
+config BLK_DEV_RUST_NULL
+ tristate "Rust null block driver (Experimental)"
+ depends on RUST
+ help
+ This is the Rust implementation of the null block driver. Like
+ the C version, the driver allows the user to create virutal block
+ devices that can be configured via various configuration options.
+
+ If unsure, say N.
diff --git a/drivers/block/rnull/Makefile b/drivers/block/rnull/Makefile
new file mode 100644
index 000000000000..11cfa5e615dc
--- /dev/null
+++ b/drivers/block/rnull/Makefile
@@ -0,0 +1,3 @@
+
+obj-$(CONFIG_BLK_DEV_RUST_NULL) += rnull_mod.o
+rnull_mod-y := rnull.o
diff --git a/drivers/block/rnull.rs b/drivers/block/rnull/rnull.rs
similarity index 100%
rename from drivers/block/rnull.rs
rename to drivers/block/rnull/rnull.rs
--
2.47.2
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v6 14/18] rnull: enable configuration via `configfs`
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
` (12 preceding siblings ...)
2025-08-22 12:14 ` [PATCH v6 13/18] rnull: move driver to separate directory Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-27 14:10 ` Daniel Almeida
2025-08-22 12:14 ` [PATCH v6 15/18] rust: block: add `GenDisk` private data support Andreas Hindborg
` (3 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg
Allow rust null block devices to be configured and instantiated via
`configfs`.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
drivers/block/rnull/Kconfig | 2 +-
drivers/block/rnull/configfs.rs | 207 +++++++++++++++++++++++++++++++++++++++
drivers/block/rnull/rnull.rs | 59 +++++------
rust/kernel/block/mq/gen_disk.rs | 2 +-
4 files changed, 240 insertions(+), 30 deletions(-)
diff --git a/drivers/block/rnull/Kconfig b/drivers/block/rnull/Kconfig
index 6dc5aff96bf4..7bc5b376c128 100644
--- a/drivers/block/rnull/Kconfig
+++ b/drivers/block/rnull/Kconfig
@@ -4,7 +4,7 @@
config BLK_DEV_RUST_NULL
tristate "Rust null block driver (Experimental)"
- depends on RUST
+ depends on RUST && CONFIGFS_FS
help
This is the Rust implementation of the null block driver. Like
the C version, the driver allows the user to create virutal block
diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
new file mode 100644
index 000000000000..46710a1e1af4
--- /dev/null
+++ b/drivers/block/rnull/configfs.rs
@@ -0,0 +1,207 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use super::{NullBlkDevice, THIS_MODULE};
+use core::fmt::Write;
+use kernel::{
+ block::mq::gen_disk::{GenDisk, GenDiskBuilder},
+ c_str,
+ configfs::{self, AttributeOperations},
+ configfs_attrs, new_mutex,
+ page::PAGE_SIZE,
+ prelude::*,
+ str::{kstrtobool_bytes, CString},
+ sync::Mutex,
+};
+use pin_init::PinInit;
+
+pub(crate) fn subsystem() -> impl PinInit<kernel::configfs::Subsystem<Config>, Error> {
+ let item_type = configfs_attrs! {
+ container: configfs::Subsystem<Config>,
+ data: Config,
+ child: DeviceConfig,
+ attributes: [
+ features: 0,
+ ],
+ };
+
+ kernel::configfs::Subsystem::new(c_str!("rnull"), item_type, try_pin_init!(Config {}))
+}
+
+#[pin_data]
+pub(crate) struct Config {}
+
+#[vtable]
+impl AttributeOperations<0> for Config {
+ type Data = Config;
+
+ fn show(_this: &Config, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+ let mut writer = kernel::str::Formatter::new(page);
+ writer.write_str("blocksize,size,rotational\n")?;
+ Ok(writer.bytes_written())
+ }
+}
+
+#[vtable]
+impl configfs::GroupOperations for Config {
+ type Child = DeviceConfig;
+
+ fn make_group(
+ &self,
+ name: &CStr,
+ ) -> Result<impl PinInit<configfs::Group<DeviceConfig>, Error>> {
+ let item_type = configfs_attrs! {
+ container: configfs::Group<DeviceConfig>,
+ data: DeviceConfig,
+ attributes: [
+ // Named for compatibility with C null_blk
+ power: 0,
+ blocksize: 1,
+ rotational: 2,
+ size: 3,
+ ],
+ };
+
+ Ok(configfs::Group::new(
+ name.try_into()?,
+ item_type,
+ // TODO: cannot coerce new_mutex!() to impl PinInit<_, Error>, so put mutex inside
+ try_pin_init!( DeviceConfig {
+ data <- new_mutex!(DeviceConfigInner {
+ powered: false,
+ block_size: 4096,
+ rotational: false,
+ disk: None,
+ capacity_mib: 4096,
+ name: name.try_into()?,
+ }),
+ }),
+ ))
+ }
+}
+
+#[pin_data]
+pub(crate) struct DeviceConfig {
+ #[pin]
+ data: Mutex<DeviceConfigInner>,
+}
+
+#[pin_data]
+struct DeviceConfigInner {
+ powered: bool,
+ name: CString,
+ block_size: u32,
+ rotational: bool,
+ capacity_mib: u64,
+ disk: Option<GenDisk<NullBlkDevice>>,
+}
+
+#[vtable]
+impl configfs::AttributeOperations<0> for DeviceConfig {
+ type Data = DeviceConfig;
+
+ fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+ let mut writer = kernel::str::Formatter::new(page);
+
+ if this.data.lock().powered {
+ writer.write_str("1\n")?;
+ } else {
+ writer.write_str("0\n")?;
+ }
+
+ Ok(writer.bytes_written())
+ }
+
+ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
+ let power_op = kstrtobool_bytes(page)?;
+ let mut guard = this.data.lock();
+
+ if !guard.powered && power_op {
+ guard.disk = Some(NullBlkDevice::new(
+ &guard.name,
+ guard.block_size,
+ guard.rotational,
+ guard.capacity_mib,
+ )?);
+ guard.powered = true;
+ } else if guard.powered && !power_op {
+ drop(guard.disk.take());
+ guard.powered = false;
+ }
+
+ Ok(())
+ }
+}
+
+#[vtable]
+impl configfs::AttributeOperations<1> for DeviceConfig {
+ type Data = DeviceConfig;
+
+ fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+ let mut writer = kernel::str::Formatter::new(page);
+ writer.write_fmt(fmt!("{}\n", this.data.lock().block_size))?;
+ Ok(writer.bytes_written())
+ }
+
+ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
+ if this.data.lock().powered {
+ return Err(EBUSY);
+ }
+
+ let text = core::str::from_utf8(page)?.trim();
+ let value = text.parse::<u32>().map_err(|_| EINVAL)?;
+
+ GenDiskBuilder::validate_block_size(value)?;
+ this.data.lock().block_size = value;
+ Ok(())
+ }
+}
+
+#[vtable]
+impl configfs::AttributeOperations<2> for DeviceConfig {
+ type Data = DeviceConfig;
+
+ fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+ let mut writer = kernel::str::Formatter::new(page);
+
+ if this.data.lock().rotational {
+ writer.write_str("1\n")?;
+ } else {
+ writer.write_str("0\n")?;
+ }
+
+ Ok(writer.bytes_written())
+ }
+
+ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
+ if this.data.lock().powered {
+ return Err(EBUSY);
+ }
+
+ this.data.lock().rotational = kstrtobool_bytes(page)?;
+
+ Ok(())
+ }
+}
+
+#[vtable]
+impl configfs::AttributeOperations<3> for DeviceConfig {
+ type Data = DeviceConfig;
+
+ fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+ let mut writer = kernel::str::Formatter::new(page);
+ writer.write_fmt(fmt!("{}\n", this.data.lock().capacity_mib))?;
+ Ok(writer.bytes_written())
+ }
+
+ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
+ if this.data.lock().powered {
+ return Err(EBUSY);
+ }
+
+ let text = core::str::from_utf8(page)?.trim();
+ let value = text.parse::<u64>().map_err(|_| EINVAL)?;
+
+ this.data.lock().capacity_mib = value;
+ Ok(())
+ }
+}
diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index d07e76ae2c13..8690ff5f974f 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -1,28 +1,25 @@
// SPDX-License-Identifier: GPL-2.0
//! This is a Rust implementation of the C null block driver.
-//!
-//! Supported features:
-//!
-//! - blk-mq interface
-//! - direct completion
-//! - block size 4k
-//!
-//! The driver is not configurable.
+
+mod configfs;
use kernel::{
- alloc::flags,
- block::mq::{
+ block::{
self,
- gen_disk::{self, GenDisk},
- Operations, TagSet,
+ mq::{
+ self,
+ gen_disk::{self, GenDisk},
+ Operations, TagSet,
+ },
},
error::Result,
- new_mutex, pr_info,
+ pr_info,
prelude::*,
- sync::{Arc, Mutex},
+ sync::Arc,
types::ARef,
};
+use pin_init::PinInit;
module! {
type: NullBlkModule,
@@ -35,33 +32,39 @@
#[pin_data]
struct NullBlkModule {
#[pin]
- _disk: Mutex<GenDisk<NullBlkDevice>>,
+ configfs_subsystem: kernel::configfs::Subsystem<configfs::Config>,
}
impl kernel::InPlaceModule for NullBlkModule {
fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
pr_info!("Rust null_blk loaded\n");
- // Use a immediately-called closure as a stable `try` block
- let disk = /* try */ (|| {
- let tagset = Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
-
- gen_disk::GenDiskBuilder::new()
- .capacity_sectors(4096 << 11)
- .logical_block_size(4096)?
- .physical_block_size(4096)?
- .rotational(false)
- .build(format_args!("rnullb{}", 0), tagset)
- })();
-
try_pin_init!(Self {
- _disk <- new_mutex!(disk?, "nullb:disk"),
+ configfs_subsystem <- configfs::subsystem(),
})
}
}
struct NullBlkDevice;
+impl NullBlkDevice {
+ fn new(
+ name: &CStr,
+ block_size: u32,
+ rotational: bool,
+ capacity_mib: u64,
+ ) -> Result<GenDisk<Self>> {
+ let tagset = Arc::pin_init(TagSet::new(1, 256, 1), GFP_KERNEL)?;
+
+ gen_disk::GenDiskBuilder::new()
+ .capacity_sectors(capacity_mib << (20 - block::SECTOR_SHIFT))
+ .logical_block_size(block_size)?
+ .physical_block_size(block_size)?
+ .rotational(rotational)
+ .build(fmt!("{}", name.to_str()?), tagset)
+ }
+}
+
#[vtable]
impl Operations for NullBlkDevice {
#[inline(always)]
diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index 20f1d46c774d..6b1b846874db 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -51,7 +51,7 @@ pub fn rotational(mut self, rotational: bool) -> Self {
/// Validate block size by verifying that it is between 512 and `PAGE_SIZE`,
/// and that it is a power of two.
- fn validate_block_size(size: u32) -> Result {
+ pub fn validate_block_size(size: u32) -> Result {
if !(512..=bindings::PAGE_SIZE as u32).contains(&size) || !size.is_power_of_two() {
Err(error::code::EINVAL)
} else {
--
2.47.2
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v6 14/18] rnull: enable configuration via `configfs`
2025-08-22 12:14 ` [PATCH v6 14/18] rnull: enable configuration via `configfs` Andreas Hindborg
@ 2025-08-27 14:10 ` Daniel Almeida
0 siblings, 0 replies; 38+ messages in thread
From: Daniel Almeida @ 2025-08-27 14:10 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Allow rust null block devices to be configured and instantiated via
> `configfs`.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> drivers/block/rnull/Kconfig | 2 +-
> drivers/block/rnull/configfs.rs | 207 +++++++++++++++++++++++++++++++++++++++
> drivers/block/rnull/rnull.rs | 59 +++++------
> rust/kernel/block/mq/gen_disk.rs | 2 +-
> 4 files changed, 240 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/block/rnull/Kconfig b/drivers/block/rnull/Kconfig
> index 6dc5aff96bf4..7bc5b376c128 100644
> --- a/drivers/block/rnull/Kconfig
> +++ b/drivers/block/rnull/Kconfig
> @@ -4,7 +4,7 @@
>
> config BLK_DEV_RUST_NULL
> tristate "Rust null block driver (Experimental)"
> - depends on RUST
> + depends on RUST && CONFIGFS_FS
> help
> This is the Rust implementation of the null block driver. Like
> the C version, the driver allows the user to create virutal block
> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
> new file mode 100644
> index 000000000000..46710a1e1af4
> --- /dev/null
> +++ b/drivers/block/rnull/configfs.rs
> @@ -0,0 +1,207 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use super::{NullBlkDevice, THIS_MODULE};
> +use core::fmt::Write;
> +use kernel::{
> + block::mq::gen_disk::{GenDisk, GenDiskBuilder},
> + c_str,
> + configfs::{self, AttributeOperations},
> + configfs_attrs, new_mutex,
> + page::PAGE_SIZE,
> + prelude::*,
> + str::{kstrtobool_bytes, CString},
> + sync::Mutex,
> +};
> +use pin_init::PinInit;
> +
> +pub(crate) fn subsystem() -> impl PinInit<kernel::configfs::Subsystem<Config>, Error> {
> + let item_type = configfs_attrs! {
> + container: configfs::Subsystem<Config>,
> + data: Config,
> + child: DeviceConfig,
> + attributes: [
> + features: 0,
> + ],
> + };
> +
> + kernel::configfs::Subsystem::new(c_str!("rnull"), item_type, try_pin_init!(Config {}))
> +}
> +
> +#[pin_data]
> +pub(crate) struct Config {}
> +
> +#[vtable]
> +impl AttributeOperations<0> for Config {
> + type Data = Config;
> +
> + fn show(_this: &Config, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> + let mut writer = kernel::str::Formatter::new(page);
> + writer.write_str("blocksize,size,rotational\n")?;
> + Ok(writer.bytes_written())
> + }
> +}
> +
> +#[vtable]
> +impl configfs::GroupOperations for Config {
> + type Child = DeviceConfig;
> +
> + fn make_group(
> + &self,
> + name: &CStr,
> + ) -> Result<impl PinInit<configfs::Group<DeviceConfig>, Error>> {
> + let item_type = configfs_attrs! {
> + container: configfs::Group<DeviceConfig>,
> + data: DeviceConfig,
> + attributes: [
> + // Named for compatibility with C null_blk
> + power: 0,
> + blocksize: 1,
> + rotational: 2,
> + size: 3,
> + ],
> + };
> +
> + Ok(configfs::Group::new(
> + name.try_into()?,
> + item_type,
> + // TODO: cannot coerce new_mutex!() to impl PinInit<_, Error>, so put mutex inside
> + try_pin_init!( DeviceConfig {
> + data <- new_mutex!(DeviceConfigInner {
> + powered: false,
> + block_size: 4096,
> + rotational: false,
> + disk: None,
> + capacity_mib: 4096,
> + name: name.try_into()?,
> + }),
> + }),
> + ))
> + }
> +}
> +
> +#[pin_data]
> +pub(crate) struct DeviceConfig {
> + #[pin]
> + data: Mutex<DeviceConfigInner>,
> +}
> +
> +#[pin_data]
> +struct DeviceConfigInner {
> + powered: bool,
> + name: CString,
> + block_size: u32,
> + rotational: bool,
> + capacity_mib: u64,
> + disk: Option<GenDisk<NullBlkDevice>>,
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<0> for DeviceConfig {
> + type Data = DeviceConfig;
> +
> + fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> + let mut writer = kernel::str::Formatter::new(page);
> +
> + if this.data.lock().powered {
> + writer.write_str("1\n")?;
> + } else {
> + writer.write_str("0\n")?;
> + }
> +
> + Ok(writer.bytes_written())
> + }
> +
> + fn store(this: &DeviceConfig, page: &[u8]) -> Result {
> + let power_op = kstrtobool_bytes(page)?;
> + let mut guard = this.data.lock();
> +
> + if !guard.powered && power_op {
> + guard.disk = Some(NullBlkDevice::new(
> + &guard.name,
> + guard.block_size,
> + guard.rotational,
> + guard.capacity_mib,
> + )?);
> + guard.powered = true;
> + } else if guard.powered && !power_op {
> + drop(guard.disk.take());
> + guard.powered = false;
> + }
> +
> + Ok(())
> + }
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<1> for DeviceConfig {
> + type Data = DeviceConfig;
> +
> + fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> + let mut writer = kernel::str::Formatter::new(page);
> + writer.write_fmt(fmt!("{}\n", this.data.lock().block_size))?;
> + Ok(writer.bytes_written())
> + }
> +
> + fn store(this: &DeviceConfig, page: &[u8]) -> Result {
> + if this.data.lock().powered {
> + return Err(EBUSY);
> + }
> +
> + let text = core::str::from_utf8(page)?.trim();
> + let value = text.parse::<u32>().map_err(|_| EINVAL)?;
> +
> + GenDiskBuilder::validate_block_size(value)?;
> + this.data.lock().block_size = value;
> + Ok(())
> + }
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<2> for DeviceConfig {
> + type Data = DeviceConfig;
> +
> + fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> + let mut writer = kernel::str::Formatter::new(page);
> +
> + if this.data.lock().rotational {
> + writer.write_str("1\n")?;
> + } else {
> + writer.write_str("0\n")?;
> + }
> +
> + Ok(writer.bytes_written())
> + }
> +
> + fn store(this: &DeviceConfig, page: &[u8]) -> Result {
> + if this.data.lock().powered {
> + return Err(EBUSY);
> + }
> +
> + this.data.lock().rotational = kstrtobool_bytes(page)?;
> +
> + Ok(())
> + }
> +}
> +
> +#[vtable]
> +impl configfs::AttributeOperations<3> for DeviceConfig {
> + type Data = DeviceConfig;
> +
> + fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> + let mut writer = kernel::str::Formatter::new(page);
> + writer.write_fmt(fmt!("{}\n", this.data.lock().capacity_mib))?;
> + Ok(writer.bytes_written())
> + }
> +
> + fn store(this: &DeviceConfig, page: &[u8]) -> Result {
> + if this.data.lock().powered {
> + return Err(EBUSY);
> + }
> +
> + let text = core::str::from_utf8(page)?.trim();
> + let value = text.parse::<u64>().map_err(|_| EINVAL)?;
> +
> + this.data.lock().capacity_mib = value;
> + Ok(())
> + }
> +}
> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
> index d07e76ae2c13..8690ff5f974f 100644
> --- a/drivers/block/rnull/rnull.rs
> +++ b/drivers/block/rnull/rnull.rs
> @@ -1,28 +1,25 @@
> // SPDX-License-Identifier: GPL-2.0
>
> //! This is a Rust implementation of the C null block driver.
> -//!
> -//! Supported features:
> -//!
> -//! - blk-mq interface
> -//! - direct completion
> -//! - block size 4k
> -//!
> -//! The driver is not configurable.
> +
> +mod configfs;
>
> use kernel::{
> - alloc::flags,
> - block::mq::{
> + block::{
> self,
> - gen_disk::{self, GenDisk},
> - Operations, TagSet,
> + mq::{
> + self,
> + gen_disk::{self, GenDisk},
> + Operations, TagSet,
> + },
> },
> error::Result,
> - new_mutex, pr_info,
> + pr_info,
> prelude::*,
> - sync::{Arc, Mutex},
> + sync::Arc,
> types::ARef,
> };
> +use pin_init::PinInit;
>
> module! {
> type: NullBlkModule,
> @@ -35,33 +32,39 @@
> #[pin_data]
> struct NullBlkModule {
> #[pin]
> - _disk: Mutex<GenDisk<NullBlkDevice>>,
> + configfs_subsystem: kernel::configfs::Subsystem<configfs::Config>,
> }
>
> impl kernel::InPlaceModule for NullBlkModule {
> fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
> pr_info!("Rust null_blk loaded\n");
>
> - // Use a immediately-called closure as a stable `try` block
> - let disk = /* try */ (|| {
> - let tagset = Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
> -
> - gen_disk::GenDiskBuilder::new()
> - .capacity_sectors(4096 << 11)
> - .logical_block_size(4096)?
> - .physical_block_size(4096)?
> - .rotational(false)
> - .build(format_args!("rnullb{}", 0), tagset)
> - })();
> -
> try_pin_init!(Self {
> - _disk <- new_mutex!(disk?, "nullb:disk"),
> + configfs_subsystem <- configfs::subsystem(),
> })
> }
> }
>
> struct NullBlkDevice;
>
> +impl NullBlkDevice {
> + fn new(
> + name: &CStr,
> + block_size: u32,
> + rotational: bool,
> + capacity_mib: u64,
> + ) -> Result<GenDisk<Self>> {
> + let tagset = Arc::pin_init(TagSet::new(1, 256, 1), GFP_KERNEL)?;
> +
> + gen_disk::GenDiskBuilder::new()
> + .capacity_sectors(capacity_mib << (20 - block::SECTOR_SHIFT))
> + .logical_block_size(block_size)?
> + .physical_block_size(block_size)?
> + .rotational(rotational)
> + .build(fmt!("{}", name.to_str()?), tagset)
> + }
> +}
> +
> #[vtable]
> impl Operations for NullBlkDevice {
> #[inline(always)]
> diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
> index 20f1d46c774d..6b1b846874db 100644
> --- a/rust/kernel/block/mq/gen_disk.rs
> +++ b/rust/kernel/block/mq/gen_disk.rs
> @@ -51,7 +51,7 @@ pub fn rotational(mut self, rotational: bool) -> Self {
>
> /// Validate block size by verifying that it is between 512 and `PAGE_SIZE`,
> /// and that it is a power of two.
> - fn validate_block_size(size: u32) -> Result {
> + pub fn validate_block_size(size: u32) -> Result {
> if !(512..=bindings::PAGE_SIZE as u32).contains(&size) || !size.is_power_of_two() {
> Err(error::code::EINVAL)
> } else {
>
> --
> 2.47.2
>
>
>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v6 15/18] rust: block: add `GenDisk` private data support
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
` (13 preceding siblings ...)
2025-08-22 12:14 ` [PATCH v6 14/18] rnull: enable configuration via `configfs` Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-27 17:52 ` Daniel Almeida
2025-08-22 12:14 ` [PATCH v6 16/18] rust: block: mq: fix spelling in a safety comment Andreas Hindborg
` (2 subsequent siblings)
17 siblings, 1 reply; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg
Allow users of the rust block device driver API to install private data in
the `GenDisk` structure.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
drivers/block/rnull/rnull.rs | 8 ++++---
rust/kernel/block/mq.rs | 7 +++---
rust/kernel/block/mq/gen_disk.rs | 32 ++++++++++++++++++++++----
rust/kernel/block/mq/operations.rs | 46 ++++++++++++++++++++++++++++++--------
4 files changed, 74 insertions(+), 19 deletions(-)
diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index 8690ff5f974f..8255236bc550 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -61,14 +61,16 @@ fn new(
.logical_block_size(block_size)?
.physical_block_size(block_size)?
.rotational(rotational)
- .build(fmt!("{}", name.to_str()?), tagset)
+ .build(fmt!("{}", name.to_str()?), tagset, ())
}
}
#[vtable]
impl Operations for NullBlkDevice {
+ type QueueData = ();
+
#[inline(always)]
- fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
+ fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
mq::Request::end_ok(rq)
.map_err(|_e| kernel::error::code::EIO)
// We take no refcounts on the request, so we expect to be able to
@@ -79,5 +81,5 @@ fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
Ok(())
}
- fn commit_rqs() {}
+ fn commit_rqs(_queue_data: ()) {}
}
diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
index 98fa0d6bc8f7..6e546f4f3d1c 100644
--- a/rust/kernel/block/mq.rs
+++ b/rust/kernel/block/mq.rs
@@ -69,20 +69,21 @@
//!
//! #[vtable]
//! impl Operations for MyBlkDevice {
+//! type QueueData = ();
//!
-//! fn queue_rq(rq: ARef<Request<Self>>, _is_last: bool) -> Result {
+//! fn queue_rq(_queue_data: (), rq: ARef<Request<Self>>, _is_last: bool) -> Result {
//! Request::end_ok(rq);
//! Ok(())
//! }
//!
-//! fn commit_rqs() {}
+//! fn commit_rqs(_queue_data: ()) {}
//! }
//!
//! let tagset: Arc<TagSet<MyBlkDevice>> =
//! Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
//! let mut disk = gen_disk::GenDiskBuilder::new()
//! .capacity_sectors(4096)
-//! .build(format_args!("myblk"), tagset)?;
+//! .build(format_args!("myblk"), tagset, ())?;
//!
//! # Ok::<(), kernel::error::Error>(())
//! ```
diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index 6b1b846874db..46ec80269970 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -13,6 +13,7 @@
static_lock_class,
str::NullTerminatedFormatter,
sync::Arc,
+ types::{ForeignOwnable, ScopeGuard},
};
use core::fmt::{self, Write};
@@ -98,7 +99,14 @@ pub fn build<T: Operations>(
self,
name: fmt::Arguments<'_>,
tagset: Arc<TagSet<T>>,
+ queue_data: T::QueueData,
) -> Result<GenDisk<T>> {
+ let data = queue_data.into_foreign();
+ let recover_data = ScopeGuard::new(|| {
+ // SAFETY: T::QueueData was created by the call to `into_foreign()` above
+ drop(unsafe { T::QueueData::from_foreign(data) });
+ });
+
// SAFETY: `bindings::queue_limits` contain only fields that are valid when zeroed.
let mut lim: bindings::queue_limits = unsafe { core::mem::zeroed() };
@@ -113,7 +121,7 @@ pub fn build<T: Operations>(
bindings::__blk_mq_alloc_disk(
tagset.raw_tag_set(),
&mut lim,
- core::ptr::null_mut(),
+ data,
static_lock_class!().as_ptr(),
)
})?;
@@ -167,8 +175,12 @@ pub fn build<T: Operations>(
},
)?;
+ recover_data.dismiss();
+
// INVARIANT: `gendisk` was initialized above.
// INVARIANT: `gendisk` was added to the VFS via `device_add_disk` above.
+ // INVARIANT: `gendisk.queue.queue_data` is set to `data` in the call to
+ // `__blk_mq_alloc_disk` above.
Ok(GenDisk {
_tagset: tagset,
gendisk,
@@ -180,9 +192,10 @@ pub fn build<T: Operations>(
///
/// # Invariants
///
-/// - `gendisk` must always point to an initialized and valid `struct gendisk`.
-/// - `gendisk` was added to the VFS through a call to
-/// `bindings::device_add_disk`.
+/// - `gendisk` must always point to an initialized and valid `struct gendisk`.
+/// - `gendisk` was added to the VFS through a call to
+/// `bindings::device_add_disk`.
+/// - `self.gendisk.queue.queuedata` is initialized by a call to `ForeignOwnable::into_foreign`.
pub struct GenDisk<T: Operations> {
_tagset: Arc<TagSet<T>>,
gendisk: *mut bindings::gendisk,
@@ -194,9 +207,20 @@ unsafe impl<T: Operations + Send> Send for GenDisk<T> {}
impl<T: Operations> Drop for GenDisk<T> {
fn drop(&mut self) {
+ // SAFETY: By type invariant of `Self`, `self.gendisk` points to a valid
+ // and initialized instance of `struct gendisk`, and, `queuedata` was
+ // initialized with the result of a call to
+ // `ForeignOwnable::into_foreign`.
+ let queue_data = unsafe { (*(*self.gendisk).queue).queuedata };
+
// SAFETY: By type invariant, `self.gendisk` points to a valid and
// initialized instance of `struct gendisk`, and it was previously added
// to the VFS.
unsafe { bindings::del_gendisk(self.gendisk) };
+
+ // SAFETY: `queue.queuedata` was created by `GenDiskBuilder::build` with
+ // a call to `ForeignOwnable::into_foreign` to create `queuedata`.
+ // `ForeignOwnable::from_foreign` is only called here.
+ drop(unsafe { T::QueueData::from_foreign(queue_data) });
}
}
diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
index c2b98f507bcb..6fb256f55acc 100644
--- a/rust/kernel/block/mq/operations.rs
+++ b/rust/kernel/block/mq/operations.rs
@@ -6,14 +6,15 @@
use crate::{
bindings,
- block::mq::request::RequestDataWrapper,
- block::mq::Request,
+ block::mq::{request::RequestDataWrapper, Request},
error::{from_result, Result},
prelude::*,
- types::ARef,
+ types::{ARef, ForeignOwnable},
};
use core::{marker::PhantomData, sync::atomic::AtomicU64, sync::atomic::Ordering};
+type ForeignBorrowed<'a, T> = <T as ForeignOwnable>::Borrowed<'a>;
+
/// Implement this trait to interface blk-mq as block devices.
///
/// To implement a block device driver, implement this trait as described in the
@@ -26,12 +27,20 @@
/// [module level documentation]: kernel::block::mq
#[macros::vtable]
pub trait Operations: Sized {
+ /// Data associated with the `struct request_queue` that is allocated for
+ /// the `GenDisk` associated with this `Operations` implementation.
+ type QueueData: ForeignOwnable;
+
/// Called by the kernel to queue a request with the driver. If `is_last` is
/// `false`, the driver is allowed to defer committing the request.
- fn queue_rq(rq: ARef<Request<Self>>, is_last: bool) -> Result;
+ fn queue_rq(
+ queue_data: ForeignBorrowed<'_, Self::QueueData>,
+ rq: ARef<Request<Self>>,
+ is_last: bool,
+ ) -> Result;
/// Called by the kernel to indicate that queued requests should be submitted.
- fn commit_rqs();
+ fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>);
/// Called by the kernel to poll the device for completed requests. Only
/// used for poll queues.
@@ -70,7 +79,7 @@ impl<T: Operations> OperationsVTable<T> {
/// promise to not access the request until the driver calls
/// `bindings::blk_mq_end_request` for the request.
unsafe extern "C" fn queue_rq_callback(
- _hctx: *mut bindings::blk_mq_hw_ctx,
+ hctx: *mut bindings::blk_mq_hw_ctx,
bd: *const bindings::blk_mq_queue_data,
) -> bindings::blk_status_t {
// SAFETY: `bd.rq` is valid as required by the safety requirement for
@@ -88,10 +97,20 @@ impl<T: Operations> OperationsVTable<T> {
// reference counted by `ARef` until then.
let rq = unsafe { Request::aref_from_raw((*bd).rq) };
+ // SAFETY: `hctx` is valid as required by this function.
+ let queue_data = unsafe { (*(*hctx).queue).queuedata };
+
+ // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a
+ // call to `ForeignOwnable::into_pointer()` to create `queuedata`.
+ // `ForeignOwnable::from_foreign()` is only called when the tagset is
+ // dropped, which happens after we are dropped.
+ let queue_data = unsafe { T::QueueData::borrow(queue_data) };
+
// SAFETY: We have exclusive access and we just set the refcount above.
unsafe { Request::start_unchecked(&rq) };
let ret = T::queue_rq(
+ queue_data,
rq,
// SAFETY: `bd` is valid as required by the safety requirement for
// this function.
@@ -110,9 +129,18 @@ impl<T: Operations> OperationsVTable<T> {
///
/// # Safety
///
- /// This function may only be called by blk-mq C infrastructure.
- unsafe extern "C" fn commit_rqs_callback(_hctx: *mut bindings::blk_mq_hw_ctx) {
- T::commit_rqs()
+ /// This function may only be called by blk-mq C infrastructure. The caller
+ /// must ensure that `hctx` is valid.
+ unsafe extern "C" fn commit_rqs_callback(hctx: *mut bindings::blk_mq_hw_ctx) {
+ // SAFETY: `hctx` is valid as required by this function.
+ let queue_data = unsafe { (*(*hctx).queue).queuedata };
+
+ // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a
+ // call to `ForeignOwnable::into_pointer()` to create `queuedata`.
+ // `ForeignOwnable::from_foreign()` is only called when the tagset is
+ // dropped, which happens after we are dropped.
+ let queue_data = unsafe { T::QueueData::borrow(queue_data) };
+ T::commit_rqs(queue_data)
}
/// This function is called by the C kernel. It is not currently
--
2.47.2
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v6 15/18] rust: block: add `GenDisk` private data support
2025-08-22 12:14 ` [PATCH v6 15/18] rust: block: add `GenDisk` private data support Andreas Hindborg
@ 2025-08-27 17:52 ` Daniel Almeida
2025-09-01 8:21 ` Andreas Hindborg
0 siblings, 1 reply; 38+ messages in thread
From: Daniel Almeida @ 2025-08-27 17:52 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Allow users of the rust block device driver API to install private data in
> the `GenDisk` structure.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> drivers/block/rnull/rnull.rs | 8 ++++---
> rust/kernel/block/mq.rs | 7 +++---
> rust/kernel/block/mq/gen_disk.rs | 32 ++++++++++++++++++++++----
> rust/kernel/block/mq/operations.rs | 46 ++++++++++++++++++++++++++++++--------
> 4 files changed, 74 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
> index 8690ff5f974f..8255236bc550 100644
> --- a/drivers/block/rnull/rnull.rs
> +++ b/drivers/block/rnull/rnull.rs
> @@ -61,14 +61,16 @@ fn new(
> .logical_block_size(block_size)?
> .physical_block_size(block_size)?
> .rotational(rotational)
> - .build(fmt!("{}", name.to_str()?), tagset)
> + .build(fmt!("{}", name.to_str()?), tagset, ())
> }
> }
>
> #[vtable]
> impl Operations for NullBlkDevice {
> + type QueueData = ();
> +
> #[inline(always)]
> - fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
> + fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
> mq::Request::end_ok(rq)
> .map_err(|_e| kernel::error::code::EIO)
> // We take no refcounts on the request, so we expect to be able to
> @@ -79,5 +81,5 @@ fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
> Ok(())
> }
>
> - fn commit_rqs() {}
> + fn commit_rqs(_queue_data: ()) {}
> }
> diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
> index 98fa0d6bc8f7..6e546f4f3d1c 100644
> --- a/rust/kernel/block/mq.rs
> +++ b/rust/kernel/block/mq.rs
> @@ -69,20 +69,21 @@
> //!
> //! #[vtable]
> //! impl Operations for MyBlkDevice {
> +//! type QueueData = ();
> //!
> -//! fn queue_rq(rq: ARef<Request<Self>>, _is_last: bool) -> Result {
> +//! fn queue_rq(_queue_data: (), rq: ARef<Request<Self>>, _is_last: bool) -> Result {
> //! Request::end_ok(rq);
> //! Ok(())
> //! }
> //!
> -//! fn commit_rqs() {}
> +//! fn commit_rqs(_queue_data: ()) {}
> //! }
> //!
> //! let tagset: Arc<TagSet<MyBlkDevice>> =
> //! Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
> //! let mut disk = gen_disk::GenDiskBuilder::new()
> //! .capacity_sectors(4096)
> -//! .build(format_args!("myblk"), tagset)?;
> +//! .build(format_args!("myblk"), tagset, ())?;
> //!
> //! # Ok::<(), kernel::error::Error>(())
> //! ```
> diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
> index 6b1b846874db..46ec80269970 100644
> --- a/rust/kernel/block/mq/gen_disk.rs
> +++ b/rust/kernel/block/mq/gen_disk.rs
> @@ -13,6 +13,7 @@
> static_lock_class,
> str::NullTerminatedFormatter,
> sync::Arc,
> + types::{ForeignOwnable, ScopeGuard},
> };
> use core::fmt::{self, Write};
>
> @@ -98,7 +99,14 @@ pub fn build<T: Operations>(
> self,
> name: fmt::Arguments<'_>,
> tagset: Arc<TagSet<T>>,
> + queue_data: T::QueueData,
> ) -> Result<GenDisk<T>> {
> + let data = queue_data.into_foreign();
> + let recover_data = ScopeGuard::new(|| {
> + // SAFETY: T::QueueData was created by the call to `into_foreign()` above
> + drop(unsafe { T::QueueData::from_foreign(data) });
> + });
> +
> // SAFETY: `bindings::queue_limits` contain only fields that are valid when zeroed.
> let mut lim: bindings::queue_limits = unsafe { core::mem::zeroed() };
>
> @@ -113,7 +121,7 @@ pub fn build<T: Operations>(
> bindings::__blk_mq_alloc_disk(
> tagset.raw_tag_set(),
> &mut lim,
> - core::ptr::null_mut(),
> + data,
> static_lock_class!().as_ptr(),
> )
> })?;
> @@ -167,8 +175,12 @@ pub fn build<T: Operations>(
> },
> )?;
>
> + recover_data.dismiss();
> +
> // INVARIANT: `gendisk` was initialized above.
> // INVARIANT: `gendisk` was added to the VFS via `device_add_disk` above.
> + // INVARIANT: `gendisk.queue.queue_data` is set to `data` in the call to
> + // `__blk_mq_alloc_disk` above.
> Ok(GenDisk {
> _tagset: tagset,
> gendisk,
> @@ -180,9 +192,10 @@ pub fn build<T: Operations>(
> ///
> /// # Invariants
> ///
> -/// - `gendisk` must always point to an initialized and valid `struct gendisk`.
> -/// - `gendisk` was added to the VFS through a call to
> -/// `bindings::device_add_disk`.
> +/// - `gendisk` must always point to an initialized and valid `struct gendisk`.
> +/// - `gendisk` was added to the VFS through a call to
> +/// `bindings::device_add_disk`.
> +/// - `self.gendisk.queue.queuedata` is initialized by a call to `ForeignOwnable::into_foreign`.
> pub struct GenDisk<T: Operations> {
> _tagset: Arc<TagSet<T>>,
> gendisk: *mut bindings::gendisk,
> @@ -194,9 +207,20 @@ unsafe impl<T: Operations + Send> Send for GenDisk<T> {}
>
> impl<T: Operations> Drop for GenDisk<T> {
> fn drop(&mut self) {
> + // SAFETY: By type invariant of `Self`, `self.gendisk` points to a valid
> + // and initialized instance of `struct gendisk`, and, `queuedata` was
> + // initialized with the result of a call to
> + // `ForeignOwnable::into_foreign`.
> + let queue_data = unsafe { (*(*self.gendisk).queue).queuedata };
> +
> // SAFETY: By type invariant, `self.gendisk` points to a valid and
> // initialized instance of `struct gendisk`, and it was previously added
> // to the VFS.
> unsafe { bindings::del_gendisk(self.gendisk) };
> +
> + // SAFETY: `queue.queuedata` was created by `GenDiskBuilder::build` with
> + // a call to `ForeignOwnable::into_foreign` to create `queuedata`.
> + // `ForeignOwnable::from_foreign` is only called here.
> + drop(unsafe { T::QueueData::from_foreign(queue_data) });
> }
> }
> diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
> index c2b98f507bcb..6fb256f55acc 100644
> --- a/rust/kernel/block/mq/operations.rs
> +++ b/rust/kernel/block/mq/operations.rs
> @@ -6,14 +6,15 @@
>
> use crate::{
> bindings,
> - block::mq::request::RequestDataWrapper,
> - block::mq::Request,
> + block::mq::{request::RequestDataWrapper, Request},
> error::{from_result, Result},
> prelude::*,
> - types::ARef,
> + types::{ARef, ForeignOwnable},
> };
> use core::{marker::PhantomData, sync::atomic::AtomicU64, sync::atomic::Ordering};
>
> +type ForeignBorrowed<'a, T> = <T as ForeignOwnable>::Borrowed<'a>;
> +
> /// Implement this trait to interface blk-mq as block devices.
> ///
> /// To implement a block device driver, implement this trait as described in the
> @@ -26,12 +27,20 @@
> /// [module level documentation]: kernel::block::mq
> #[macros::vtable]
> pub trait Operations: Sized {
> + /// Data associated with the `struct request_queue` that is allocated for
> + /// the `GenDisk` associated with this `Operations` implementation.
> + type QueueData: ForeignOwnable;
> +
> /// Called by the kernel to queue a request with the driver. If `is_last` is
> /// `false`, the driver is allowed to defer committing the request.
> - fn queue_rq(rq: ARef<Request<Self>>, is_last: bool) -> Result;
> + fn queue_rq(
> + queue_data: ForeignBorrowed<'_, Self::QueueData>,
> + rq: ARef<Request<Self>>,
> + is_last: bool,
> + ) -> Result;
>
> /// Called by the kernel to indicate that queued requests should be submitted.
> - fn commit_rqs();
> + fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>);
>
> /// Called by the kernel to poll the device for completed requests. Only
> /// used for poll queues.
> @@ -70,7 +79,7 @@ impl<T: Operations> OperationsVTable<T> {
> /// promise to not access the request until the driver calls
> /// `bindings::blk_mq_end_request` for the request.
> unsafe extern "C" fn queue_rq_callback(
> - _hctx: *mut bindings::blk_mq_hw_ctx,
> + hctx: *mut bindings::blk_mq_hw_ctx,
> bd: *const bindings::blk_mq_queue_data,
> ) -> bindings::blk_status_t {
> // SAFETY: `bd.rq` is valid as required by the safety requirement for
> @@ -88,10 +97,20 @@ impl<T: Operations> OperationsVTable<T> {
> // reference counted by `ARef` until then.
> let rq = unsafe { Request::aref_from_raw((*bd).rq) };
>
> + // SAFETY: `hctx` is valid as required by this function.
> + let queue_data = unsafe { (*(*hctx).queue).queuedata };
> +
> + // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a
isn’t this set on build() ?
> + // call to `ForeignOwnable::into_pointer()` to create `queuedata`.
into_pointer() ?
> + // `ForeignOwnable::from_foreign()` is only called when the tagset is
> + // dropped, which happens after we are dropped.
> + let queue_data = unsafe { T::QueueData::borrow(queue_data) };
> +
> // SAFETY: We have exclusive access and we just set the refcount above.
> unsafe { Request::start_unchecked(&rq) };
>
> let ret = T::queue_rq(
> + queue_data,
> rq,
> // SAFETY: `bd` is valid as required by the safety requirement for
> // this function.
> @@ -110,9 +129,18 @@ impl<T: Operations> OperationsVTable<T> {
> ///
> /// # Safety
> ///
> - /// This function may only be called by blk-mq C infrastructure.
> - unsafe extern "C" fn commit_rqs_callback(_hctx: *mut bindings::blk_mq_hw_ctx) {
> - T::commit_rqs()
> + /// This function may only be called by blk-mq C infrastructure. The caller
> + /// must ensure that `hctx` is valid.
> + unsafe extern "C" fn commit_rqs_callback(hctx: *mut bindings::blk_mq_hw_ctx) {
> + // SAFETY: `hctx` is valid as required by this function.
> + let queue_data = unsafe { (*(*hctx).queue).queuedata };
> +
> + // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a
> + // call to `ForeignOwnable::into_pointer()` to create `queuedata`.
into_foreign()?
> + // `ForeignOwnable::from_foreign()` is only called when the tagset is
> + // dropped, which happens after we are dropped.
> + let queue_data = unsafe { T::QueueData::borrow(queue_data) };
> + T::commit_rqs(queue_data)
> }
>
> /// This function is called by the C kernel. It is not currently
>
> --
> 2.47.2
>
>
>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v6 15/18] rust: block: add `GenDisk` private data support
2025-08-27 17:52 ` Daniel Almeida
@ 2025-09-01 8:21 ` Andreas Hindborg
0 siblings, 0 replies; 38+ messages in thread
From: Andreas Hindborg @ 2025-09-01 8:21 UTC (permalink / raw)
To: Daniel Almeida
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
"Daniel Almeida" <daniel.almeida@collabora.com> writes:
>> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> Allow users of the rust block device driver API to install private data in
>> the `GenDisk` structure.
>>
>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>> drivers/block/rnull/rnull.rs | 8 ++++---
>> rust/kernel/block/mq.rs | 7 +++---
>> rust/kernel/block/mq/gen_disk.rs | 32 ++++++++++++++++++++++----
>> rust/kernel/block/mq/operations.rs | 46 ++++++++++++++++++++++++++++++--------
>> 4 files changed, 74 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
>> index 8690ff5f974f..8255236bc550 100644
>> --- a/drivers/block/rnull/rnull.rs
>> +++ b/drivers/block/rnull/rnull.rs
>> @@ -61,14 +61,16 @@ fn new(
>> .logical_block_size(block_size)?
>> .physical_block_size(block_size)?
>> .rotational(rotational)
>> - .build(fmt!("{}", name.to_str()?), tagset)
>> + .build(fmt!("{}", name.to_str()?), tagset, ())
>> }
>> }
>>
>> #[vtable]
>> impl Operations for NullBlkDevice {
>> + type QueueData = ();
>> +
>> #[inline(always)]
>> - fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
>> + fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
>> mq::Request::end_ok(rq)
>> .map_err(|_e| kernel::error::code::EIO)
>> // We take no refcounts on the request, so we expect to be able to
>> @@ -79,5 +81,5 @@ fn queue_rq(rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
>> Ok(())
>> }
>>
>> - fn commit_rqs() {}
>> + fn commit_rqs(_queue_data: ()) {}
>> }
>> diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
>> index 98fa0d6bc8f7..6e546f4f3d1c 100644
>> --- a/rust/kernel/block/mq.rs
>> +++ b/rust/kernel/block/mq.rs
>> @@ -69,20 +69,21 @@
>> //!
>> //! #[vtable]
>> //! impl Operations for MyBlkDevice {
>> +//! type QueueData = ();
>> //!
>> -//! fn queue_rq(rq: ARef<Request<Self>>, _is_last: bool) -> Result {
>> +//! fn queue_rq(_queue_data: (), rq: ARef<Request<Self>>, _is_last: bool) -> Result {
>> //! Request::end_ok(rq);
>> //! Ok(())
>> //! }
>> //!
>> -//! fn commit_rqs() {}
>> +//! fn commit_rqs(_queue_data: ()) {}
>> //! }
>> //!
>> //! let tagset: Arc<TagSet<MyBlkDevice>> =
>> //! Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
>> //! let mut disk = gen_disk::GenDiskBuilder::new()
>> //! .capacity_sectors(4096)
>> -//! .build(format_args!("myblk"), tagset)?;
>> +//! .build(format_args!("myblk"), tagset, ())?;
>> //!
>> //! # Ok::<(), kernel::error::Error>(())
>> //! ```
>> diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
>> index 6b1b846874db..46ec80269970 100644
>> --- a/rust/kernel/block/mq/gen_disk.rs
>> +++ b/rust/kernel/block/mq/gen_disk.rs
>> @@ -13,6 +13,7 @@
>> static_lock_class,
>> str::NullTerminatedFormatter,
>> sync::Arc,
>> + types::{ForeignOwnable, ScopeGuard},
>> };
>> use core::fmt::{self, Write};
>>
>> @@ -98,7 +99,14 @@ pub fn build<T: Operations>(
>> self,
>> name: fmt::Arguments<'_>,
>> tagset: Arc<TagSet<T>>,
>> + queue_data: T::QueueData,
>> ) -> Result<GenDisk<T>> {
>> + let data = queue_data.into_foreign();
>> + let recover_data = ScopeGuard::new(|| {
>> + // SAFETY: T::QueueData was created by the call to `into_foreign()` above
>> + drop(unsafe { T::QueueData::from_foreign(data) });
>> + });
>> +
>> // SAFETY: `bindings::queue_limits` contain only fields that are valid when zeroed.
>> let mut lim: bindings::queue_limits = unsafe { core::mem::zeroed() };
>>
>> @@ -113,7 +121,7 @@ pub fn build<T: Operations>(
>> bindings::__blk_mq_alloc_disk(
>> tagset.raw_tag_set(),
>> &mut lim,
>> - core::ptr::null_mut(),
>> + data,
>> static_lock_class!().as_ptr(),
>> )
>> })?;
>> @@ -167,8 +175,12 @@ pub fn build<T: Operations>(
>> },
>> )?;
>>
>> + recover_data.dismiss();
>> +
>> // INVARIANT: `gendisk` was initialized above.
>> // INVARIANT: `gendisk` was added to the VFS via `device_add_disk` above.
>> + // INVARIANT: `gendisk.queue.queue_data` is set to `data` in the call to
>> + // `__blk_mq_alloc_disk` above.
>> Ok(GenDisk {
>> _tagset: tagset,
>> gendisk,
>> @@ -180,9 +192,10 @@ pub fn build<T: Operations>(
>> ///
>> /// # Invariants
>> ///
>> -/// - `gendisk` must always point to an initialized and valid `struct gendisk`.
>> -/// - `gendisk` was added to the VFS through a call to
>> -/// `bindings::device_add_disk`.
>> +/// - `gendisk` must always point to an initialized and valid `struct gendisk`.
>> +/// - `gendisk` was added to the VFS through a call to
>> +/// `bindings::device_add_disk`.
>> +/// - `self.gendisk.queue.queuedata` is initialized by a call to `ForeignOwnable::into_foreign`.
>> pub struct GenDisk<T: Operations> {
>> _tagset: Arc<TagSet<T>>,
>> gendisk: *mut bindings::gendisk,
>> @@ -194,9 +207,20 @@ unsafe impl<T: Operations + Send> Send for GenDisk<T> {}
>>
>> impl<T: Operations> Drop for GenDisk<T> {
>> fn drop(&mut self) {
>> + // SAFETY: By type invariant of `Self`, `self.gendisk` points to a valid
>> + // and initialized instance of `struct gendisk`, and, `queuedata` was
>> + // initialized with the result of a call to
>> + // `ForeignOwnable::into_foreign`.
>> + let queue_data = unsafe { (*(*self.gendisk).queue).queuedata };
>> +
>> // SAFETY: By type invariant, `self.gendisk` points to a valid and
>> // initialized instance of `struct gendisk`, and it was previously added
>> // to the VFS.
>> unsafe { bindings::del_gendisk(self.gendisk) };
>> +
>> + // SAFETY: `queue.queuedata` was created by `GenDiskBuilder::build` with
>> + // a call to `ForeignOwnable::into_foreign` to create `queuedata`.
>> + // `ForeignOwnable::from_foreign` is only called here.
>> + drop(unsafe { T::QueueData::from_foreign(queue_data) });
>> }
>> }
>> diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
>> index c2b98f507bcb..6fb256f55acc 100644
>> --- a/rust/kernel/block/mq/operations.rs
>> +++ b/rust/kernel/block/mq/operations.rs
>> @@ -6,14 +6,15 @@
>>
>> use crate::{
>> bindings,
>> - block::mq::request::RequestDataWrapper,
>> - block::mq::Request,
>> + block::mq::{request::RequestDataWrapper, Request},
>> error::{from_result, Result},
>> prelude::*,
>> - types::ARef,
>> + types::{ARef, ForeignOwnable},
>> };
>> use core::{marker::PhantomData, sync::atomic::AtomicU64, sync::atomic::Ordering};
>>
>> +type ForeignBorrowed<'a, T> = <T as ForeignOwnable>::Borrowed<'a>;
>> +
>> /// Implement this trait to interface blk-mq as block devices.
>> ///
>> /// To implement a block device driver, implement this trait as described in the
>> @@ -26,12 +27,20 @@
>> /// [module level documentation]: kernel::block::mq
>> #[macros::vtable]
>> pub trait Operations: Sized {
>> + /// Data associated with the `struct request_queue` that is allocated for
>> + /// the `GenDisk` associated with this `Operations` implementation.
>> + type QueueData: ForeignOwnable;
>> +
>> /// Called by the kernel to queue a request with the driver. If `is_last` is
>> /// `false`, the driver is allowed to defer committing the request.
>> - fn queue_rq(rq: ARef<Request<Self>>, is_last: bool) -> Result;
>> + fn queue_rq(
>> + queue_data: ForeignBorrowed<'_, Self::QueueData>,
>> + rq: ARef<Request<Self>>,
>> + is_last: bool,
>> + ) -> Result;
>>
>> /// Called by the kernel to indicate that queued requests should be submitted.
>> - fn commit_rqs();
>> + fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>);
>>
>> /// Called by the kernel to poll the device for completed requests. Only
>> /// used for poll queues.
>> @@ -70,7 +79,7 @@ impl<T: Operations> OperationsVTable<T> {
>> /// promise to not access the request until the driver calls
>> /// `bindings::blk_mq_end_request` for the request.
>> unsafe extern "C" fn queue_rq_callback(
>> - _hctx: *mut bindings::blk_mq_hw_ctx,
>> + hctx: *mut bindings::blk_mq_hw_ctx,
>> bd: *const bindings::blk_mq_queue_data,
>> ) -> bindings::blk_status_t {
>> // SAFETY: `bd.rq` is valid as required by the safety requirement for
>> @@ -88,10 +97,20 @@ impl<T: Operations> OperationsVTable<T> {
>> // reference counted by `ARef` until then.
>> let rq = unsafe { Request::aref_from_raw((*bd).rq) };
>>
>> + // SAFETY: `hctx` is valid as required by this function.
>> + let queue_data = unsafe { (*(*hctx).queue).queuedata };
>> +
>> + // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a
>
> isn’t this set on build() ?
Yes, you are right. Refactoring happened.
>
>> + // call to `ForeignOwnable::into_pointer()` to create `queuedata`.
>
> into_pointer() ?
Should be `into_foreign`, will fix.
>
>> + // `ForeignOwnable::from_foreign()` is only called when the tagset is
>> + // dropped, which happens after we are dropped.
>> + let queue_data = unsafe { T::QueueData::borrow(queue_data) };
>> +
>> // SAFETY: We have exclusive access and we just set the refcount above.
>> unsafe { Request::start_unchecked(&rq) };
>>
>> let ret = T::queue_rq(
>> + queue_data,
>> rq,
>> // SAFETY: `bd` is valid as required by the safety requirement for
>> // this function.
>> @@ -110,9 +129,18 @@ impl<T: Operations> OperationsVTable<T> {
>> ///
>> /// # Safety
>> ///
>> - /// This function may only be called by blk-mq C infrastructure.
>> - unsafe extern "C" fn commit_rqs_callback(_hctx: *mut bindings::blk_mq_hw_ctx) {
>> - T::commit_rqs()
>> + /// This function may only be called by blk-mq C infrastructure. The caller
>> + /// must ensure that `hctx` is valid.
>> + unsafe extern "C" fn commit_rqs_callback(hctx: *mut bindings::blk_mq_hw_ctx) {
>> + // SAFETY: `hctx` is valid as required by this function.
>> + let queue_data = unsafe { (*(*hctx).queue).queuedata };
>> +
>> + // SAFETY: `queue.queuedata` was created by `GenDisk::try_new()` with a
>> + // call to `ForeignOwnable::into_pointer()` to create `queuedata`.
>
> into_foreign()?
Yep, thanks.
>
>> + // `ForeignOwnable::from_foreign()` is only called when the tagset is
>> + // dropped, which happens after we are dropped.
>> + let queue_data = unsafe { T::QueueData::borrow(queue_data) };
>> + T::commit_rqs(queue_data)
>> }
>>
>> /// This function is called by the C kernel. It is not currently
>>
>> --
>> 2.47.2
>>
>>
>>
>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v6 16/18] rust: block: mq: fix spelling in a safety comment
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
` (14 preceding siblings ...)
2025-08-22 12:14 ` [PATCH v6 15/18] rust: block: add `GenDisk` private data support Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-22 12:14 ` [PATCH v6 17/18] rust: block: add remote completion to `Request` Andreas Hindborg
2025-08-22 12:14 ` [PATCH v6 18/18] rnull: add soft-irq completion support Andreas Hindborg
17 siblings, 0 replies; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg,
Daniel Almeida
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
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v6 17/18] rust: block: add remote completion to `Request`
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
` (15 preceding siblings ...)
2025-08-22 12:14 ` [PATCH v6 16/18] rust: block: mq: fix spelling in a safety comment Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-27 18:00 ` Daniel Almeida
2025-08-22 12:14 ` [PATCH v6 18/18] rnull: add soft-irq completion support Andreas Hindborg
17 siblings, 1 reply; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg,
Daniel Almeida
Allow users of rust block device driver API to schedule completion of
requests via `blk_mq_complete_request_remote`.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
drivers/block/rnull/rnull.rs | 9 +++++++++
rust/kernel/block/mq.rs | 6 ++++++
rust/kernel/block/mq/operations.rs | 19 +++++++++++++++----
rust/kernel/block/mq/request.rs | 17 +++++++++++++++++
4 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index 8255236bc550..a19c55717c4f 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -82,4 +82,13 @@ fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Res
}
fn commit_rqs(_queue_data: ()) {}
+
+ fn complete(rq: ARef<mq::Request<Self>>) {
+ mq::Request::end_ok(rq)
+ .map_err(|_e| kernel::error::code::EIO)
+ // We take no refcounts on the request, so we expect to be able to
+ // end the request. The request reference must be unique at this
+ // point, and so `end_ok` cannot fail.
+ .expect("Fatal error - expected to be able to end request");
+ }
}
diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
index 6e546f4f3d1c..c0ec06b84355 100644
--- a/rust/kernel/block/mq.rs
+++ b/rust/kernel/block/mq.rs
@@ -77,6 +77,12 @@
//! }
//!
//! fn commit_rqs(_queue_data: ()) {}
+//!
+//! fn complete(rq: ARef<Request<Self>>) {
+//! Request::end_ok(rq)
+//! .map_err(|_e| kernel::error::code::EIO)
+//! .expect("Fatal error - expected to be able to end request");
+//! }
//! }
//!
//! let tagset: Arc<TagSet<MyBlkDevice>> =
diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
index 6fb256f55acc..0fece577de7c 100644
--- a/rust/kernel/block/mq/operations.rs
+++ b/rust/kernel/block/mq/operations.rs
@@ -42,6 +42,9 @@ fn queue_rq(
/// Called by the kernel to indicate that queued requests should be submitted.
fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>);
+ /// Called by the kernel when the request is completed.
+ fn complete(rq: ARef<Request<Self>>);
+
/// Called by the kernel to poll the device for completed requests. Only
/// used for poll queues.
fn poll() -> bool {
@@ -143,13 +146,21 @@ impl<T: Operations> OperationsVTable<T> {
T::commit_rqs(queue_data)
}
- /// This function is called by the C kernel. It is not currently
- /// implemented, and there is no way to exercise this code path.
+ /// This function is called by the C kernel. A pointer to this function is
+ /// installed in the `blk_mq_ops` vtable for the driver.
///
/// # Safety
///
- /// This function may only be called by blk-mq C infrastructure.
- unsafe extern "C" fn complete_callback(_rq: *mut bindings::request) {}
+ /// This function may only be called by blk-mq C infrastructure. `rq` must
+ /// point to a valid request that has been marked as completed. The pointee
+ /// of `rq` must be valid for write for the duration of this function.
+ unsafe extern "C" fn complete_callback(rq: *mut bindings::request) {
+ // SAFETY: This function can only be dispatched through
+ // `Request::complete`. We leaked a refcount then which we pick back up
+ // now.
+ let aref = unsafe { Request::aref_from_raw(rq) };
+ T::complete(aref);
+ }
/// This function is called by the C kernel. A pointer to this function is
/// installed in the `blk_mq_ops` vtable for the driver.
diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index 3848cfe63f77..f7f757f7459f 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -135,6 +135,23 @@ pub fn end_ok(this: ARef<Self>) -> Result<(), ARef<Self>> {
Ok(())
}
+ /// Complete the request by scheduling `Operations::complete` for
+ /// execution.
+ ///
+ /// The function may be scheduled locally, via SoftIRQ or remotely via IPMI.
+ /// See `blk_mq_complete_request_remote` in [`blk-mq.c`] for details.
+ ///
+ /// [`blk-mq.c`]: srctree/block/blk-mq.c
+ pub fn complete(this: ARef<Self>) {
+ let ptr = ARef::into_raw(this).cast::<bindings::request>().as_ptr();
+ // SAFETY: By type invariant, `self.0` is a valid `struct request`
+ if !unsafe { bindings::blk_mq_complete_request_remote(ptr) } {
+ // SAFETY: We released a refcount above that we can reclaim here.
+ let this = unsafe { Request::aref_from_raw(ptr) };
+ T::complete(this);
+ }
+ }
+
/// Return a pointer to the [`RequestDataWrapper`] stored in the private area
/// of the request structure.
///
--
2.47.2
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v6 17/18] rust: block: add remote completion to `Request`
2025-08-22 12:14 ` [PATCH v6 17/18] rust: block: add remote completion to `Request` Andreas Hindborg
@ 2025-08-27 18:00 ` Daniel Almeida
2025-08-29 11:12 ` Andreas Hindborg
0 siblings, 1 reply; 38+ messages in thread
From: Daniel Almeida @ 2025-08-27 18:00 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
Hi Andreas,
> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Allow users of rust block device driver API to schedule completion of
> requests via `blk_mq_complete_request_remote`.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> drivers/block/rnull/rnull.rs | 9 +++++++++
> rust/kernel/block/mq.rs | 6 ++++++
> rust/kernel/block/mq/operations.rs | 19 +++++++++++++++----
> rust/kernel/block/mq/request.rs | 17 +++++++++++++++++
> 4 files changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
> index 8255236bc550..a19c55717c4f 100644
> --- a/drivers/block/rnull/rnull.rs
> +++ b/drivers/block/rnull/rnull.rs
> @@ -82,4 +82,13 @@ fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Res
> }
>
> fn commit_rqs(_queue_data: ()) {}
> +
> + fn complete(rq: ARef<mq::Request<Self>>) {
> + mq::Request::end_ok(rq)
> + .map_err(|_e| kernel::error::code::EIO)
> + // We take no refcounts on the request, so we expect to be able to
> + // end the request. The request reference must be unique at this
> + // point, and so `end_ok` cannot fail.
> + .expect("Fatal error - expected to be able to end request");
> + }
> }
> diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
> index 6e546f4f3d1c..c0ec06b84355 100644
> --- a/rust/kernel/block/mq.rs
> +++ b/rust/kernel/block/mq.rs
> @@ -77,6 +77,12 @@
> //! }
> //!
> //! fn commit_rqs(_queue_data: ()) {}
> +//!
> +//! fn complete(rq: ARef<Request<Self>>) {
> +//! Request::end_ok(rq)
> +//! .map_err(|_e| kernel::error::code::EIO)
> +//! .expect("Fatal error - expected to be able to end request");
> +//! }
> //! }
> //!
> //! let tagset: Arc<TagSet<MyBlkDevice>> =
> diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
> index 6fb256f55acc..0fece577de7c 100644
> --- a/rust/kernel/block/mq/operations.rs
> +++ b/rust/kernel/block/mq/operations.rs
> @@ -42,6 +42,9 @@ fn queue_rq(
> /// Called by the kernel to indicate that queued requests should be submitted.
> fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>);
>
> + /// Called by the kernel when the request is completed.
> + fn complete(rq: ARef<Request<Self>>);
> +
> /// Called by the kernel to poll the device for completed requests. Only
> /// used for poll queues.
> fn poll() -> bool {
> @@ -143,13 +146,21 @@ impl<T: Operations> OperationsVTable<T> {
> T::commit_rqs(queue_data)
> }
>
> - /// This function is called by the C kernel. It is not currently
> - /// implemented, and there is no way to exercise this code path.
> + /// This function is called by the C kernel. A pointer to this function is
> + /// installed in the `blk_mq_ops` vtable for the driver.
> ///
> /// # Safety
> ///
> - /// This function may only be called by blk-mq C infrastructure.
> - unsafe extern "C" fn complete_callback(_rq: *mut bindings::request) {}
> + /// This function may only be called by blk-mq C infrastructure. `rq` must
> + /// point to a valid request that has been marked as completed. The pointee
> + /// of `rq` must be valid for write for the duration of this function.
> + unsafe extern "C" fn complete_callback(rq: *mut bindings::request) {
> + // SAFETY: This function can only be dispatched through
> + // `Request::complete`. We leaked a refcount then which we pick back up
> + // now.
> + let aref = unsafe { Request::aref_from_raw(rq) };
> + T::complete(aref);
> + }
>
> /// This function is called by the C kernel. A pointer to this function is
> /// installed in the `blk_mq_ops` vtable for the driver.
> diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
> index 3848cfe63f77..f7f757f7459f 100644
> --- a/rust/kernel/block/mq/request.rs
> +++ b/rust/kernel/block/mq/request.rs
> @@ -135,6 +135,23 @@ pub fn end_ok(this: ARef<Self>) -> Result<(), ARef<Self>> {
> Ok(())
> }
>
> + /// Complete the request by scheduling `Operations::complete` for
> + /// execution.
> + ///
> + /// The function may be scheduled locally, via SoftIRQ or remotely via IPMI.
> + /// See `blk_mq_complete_request_remote` in [`blk-mq.c`] for details.
> + ///
> + /// [`blk-mq.c`]: srctree/block/blk-mq.c
> + pub fn complete(this: ARef<Self>) {
> + let ptr = ARef::into_raw(this).cast::<bindings::request>().as_ptr();
> + // SAFETY: By type invariant, `self.0` is a valid `struct request`
> + if !unsafe { bindings::blk_mq_complete_request_remote(ptr) } {
> + // SAFETY: We released a refcount above that we can reclaim here.
> + let this = unsafe { Request::aref_from_raw(ptr) };
> + T::complete(this);
> + }
> + }
> +
> /// Return a pointer to the [`RequestDataWrapper`] stored in the private area
> /// of the request structure.
> ///
>
> --
> 2.47.2
>
>
I had another look here. While I do trust your reasoning, perhaps we should
remove the call to expect()?
If it is not called ever as you said, great, removing the expect() will not
change the code behavior. If it is, be it because of some minor oversight or
unexpected condition, we should produce some error output instead of crashing
the kernel. Maybe we should use a warn() here instead? Or maybe dev/pr_err as
applicable?
— Daniel
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v6 17/18] rust: block: add remote completion to `Request`
2025-08-27 18:00 ` Daniel Almeida
@ 2025-08-29 11:12 ` Andreas Hindborg
2025-09-01 14:24 ` Daniel Almeida
0 siblings, 1 reply; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-29 11:12 UTC (permalink / raw)
To: Daniel Almeida
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
"Daniel Almeida" <daniel.almeida@collabora.com> writes:
> Hi Andreas,
>
>> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> Allow users of rust block device driver API to schedule completion of
>> requests via `blk_mq_complete_request_remote`.
>>
>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>> drivers/block/rnull/rnull.rs | 9 +++++++++
>> rust/kernel/block/mq.rs | 6 ++++++
>> rust/kernel/block/mq/operations.rs | 19 +++++++++++++++----
>> rust/kernel/block/mq/request.rs | 17 +++++++++++++++++
>> 4 files changed, 47 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
>> index 8255236bc550..a19c55717c4f 100644
>> --- a/drivers/block/rnull/rnull.rs
>> +++ b/drivers/block/rnull/rnull.rs
>> @@ -82,4 +82,13 @@ fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Res
>> }
>>
>> fn commit_rqs(_queue_data: ()) {}
>> +
>> + fn complete(rq: ARef<mq::Request<Self>>) {
>> + mq::Request::end_ok(rq)
>> + .map_err(|_e| kernel::error::code::EIO)
>> + // We take no refcounts on the request, so we expect to be able to
>> + // end the request. The request reference must be unique at this
>> + // point, and so `end_ok` cannot fail.
>> + .expect("Fatal error - expected to be able to end request");
>> + }
>> }
>> diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
>> index 6e546f4f3d1c..c0ec06b84355 100644
>> --- a/rust/kernel/block/mq.rs
>> +++ b/rust/kernel/block/mq.rs
>> @@ -77,6 +77,12 @@
>> //! }
>> //!
>> //! fn commit_rqs(_queue_data: ()) {}
>> +//!
>> +//! fn complete(rq: ARef<Request<Self>>) {
>> +//! Request::end_ok(rq)
>> +//! .map_err(|_e| kernel::error::code::EIO)
>> +//! .expect("Fatal error - expected to be able to end request");
>> +//! }
>> //! }
>> //!
>> //! let tagset: Arc<TagSet<MyBlkDevice>> =
>> diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
>> index 6fb256f55acc..0fece577de7c 100644
>> --- a/rust/kernel/block/mq/operations.rs
>> +++ b/rust/kernel/block/mq/operations.rs
>> @@ -42,6 +42,9 @@ fn queue_rq(
>> /// Called by the kernel to indicate that queued requests should be submitted.
>> fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>);
>>
>> + /// Called by the kernel when the request is completed.
>> + fn complete(rq: ARef<Request<Self>>);
>> +
>> /// Called by the kernel to poll the device for completed requests. Only
>> /// used for poll queues.
>> fn poll() -> bool {
>> @@ -143,13 +146,21 @@ impl<T: Operations> OperationsVTable<T> {
>> T::commit_rqs(queue_data)
>> }
>>
>> - /// This function is called by the C kernel. It is not currently
>> - /// implemented, and there is no way to exercise this code path.
>> + /// This function is called by the C kernel. A pointer to this function is
>> + /// installed in the `blk_mq_ops` vtable for the driver.
>> ///
>> /// # Safety
>> ///
>> - /// This function may only be called by blk-mq C infrastructure.
>> - unsafe extern "C" fn complete_callback(_rq: *mut bindings::request) {}
>> + /// This function may only be called by blk-mq C infrastructure. `rq` must
>> + /// point to a valid request that has been marked as completed. The pointee
>> + /// of `rq` must be valid for write for the duration of this function.
>> + unsafe extern "C" fn complete_callback(rq: *mut bindings::request) {
>> + // SAFETY: This function can only be dispatched through
>> + // `Request::complete`. We leaked a refcount then which we pick back up
>> + // now.
>> + let aref = unsafe { Request::aref_from_raw(rq) };
>> + T::complete(aref);
>> + }
>>
>> /// This function is called by the C kernel. A pointer to this function is
>> /// installed in the `blk_mq_ops` vtable for the driver.
>> diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
>> index 3848cfe63f77..f7f757f7459f 100644
>> --- a/rust/kernel/block/mq/request.rs
>> +++ b/rust/kernel/block/mq/request.rs
>> @@ -135,6 +135,23 @@ pub fn end_ok(this: ARef<Self>) -> Result<(), ARef<Self>> {
>> Ok(())
>> }
>>
>> + /// Complete the request by scheduling `Operations::complete` for
>> + /// execution.
>> + ///
>> + /// The function may be scheduled locally, via SoftIRQ or remotely via IPMI.
>> + /// See `blk_mq_complete_request_remote` in [`blk-mq.c`] for details.
>> + ///
>> + /// [`blk-mq.c`]: srctree/block/blk-mq.c
>> + pub fn complete(this: ARef<Self>) {
>> + let ptr = ARef::into_raw(this).cast::<bindings::request>().as_ptr();
>> + // SAFETY: By type invariant, `self.0` is a valid `struct request`
>> + if !unsafe { bindings::blk_mq_complete_request_remote(ptr) } {
>> + // SAFETY: We released a refcount above that we can reclaim here.
>> + let this = unsafe { Request::aref_from_raw(ptr) };
>> + T::complete(this);
>> + }
>> + }
>> +
>> /// Return a pointer to the [`RequestDataWrapper`] stored in the private area
>> /// of the request structure.
>> ///
>>
>> --
>> 2.47.2
>>
>>
>
> I had another look here. While I do trust your reasoning, perhaps we should
> remove the call to expect()?
>
> If it is not called ever as you said, great, removing the expect() will not
> change the code behavior. If it is, be it because of some minor oversight or
> unexpected condition, we should produce some error output instead of crashing
> the kernel. Maybe we should use a warn() here instead? Or maybe dev/pr_err as
> applicable?
I think for the example, I would like to keep the `expect`. For
demonstration purposes.
We could do `warn!` instead for the rnull driver I guess. But the IO
queue that would hit this code would start to hang pretty fast, since no
IO would complete. I don't think the kernel can recover from this hang.
Best regards,
Andreas Hindborg
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH v6 17/18] rust: block: add remote completion to `Request`
2025-08-29 11:12 ` Andreas Hindborg
@ 2025-09-01 14:24 ` Daniel Almeida
0 siblings, 0 replies; 38+ messages in thread
From: Daniel Almeida @ 2025-09-01 14:24 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
> On 29 Aug 2025, at 08:12, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> "Daniel Almeida" <daniel.almeida@collabora.com> writes:
>
>> Hi Andreas,
>>
>>> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>>
>>> Allow users of rust block device driver API to schedule completion of
>>> requests via `blk_mq_complete_request_remote`.
>>>
>>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>>> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>> ---
>>> drivers/block/rnull/rnull.rs | 9 +++++++++
>>> rust/kernel/block/mq.rs | 6 ++++++
>>> rust/kernel/block/mq/operations.rs | 19 +++++++++++++++----
>>> rust/kernel/block/mq/request.rs | 17 +++++++++++++++++
>>> 4 files changed, 47 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
>>> index 8255236bc550..a19c55717c4f 100644
>>> --- a/drivers/block/rnull/rnull.rs
>>> +++ b/drivers/block/rnull/rnull.rs
>>> @@ -82,4 +82,13 @@ fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Res
>>> }
>>>
>>> fn commit_rqs(_queue_data: ()) {}
>>> +
>>> + fn complete(rq: ARef<mq::Request<Self>>) {
>>> + mq::Request::end_ok(rq)
>>> + .map_err(|_e| kernel::error::code::EIO)
>>> + // We take no refcounts on the request, so we expect to be able to
>>> + // end the request. The request reference must be unique at this
>>> + // point, and so `end_ok` cannot fail.
>>> + .expect("Fatal error - expected to be able to end request");
>>> + }
>>> }
>>> diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
>>> index 6e546f4f3d1c..c0ec06b84355 100644
>>> --- a/rust/kernel/block/mq.rs
>>> +++ b/rust/kernel/block/mq.rs
>>> @@ -77,6 +77,12 @@
>>> //! }
>>> //!
>>> //! fn commit_rqs(_queue_data: ()) {}
>>> +//!
>>> +//! fn complete(rq: ARef<Request<Self>>) {
>>> +//! Request::end_ok(rq)
>>> +//! .map_err(|_e| kernel::error::code::EIO)
>>> +//! .expect("Fatal error - expected to be able to end request");
>>> +//! }
>>> //! }
>>> //!
>>> //! let tagset: Arc<TagSet<MyBlkDevice>> =
>>> diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
>>> index 6fb256f55acc..0fece577de7c 100644
>>> --- a/rust/kernel/block/mq/operations.rs
>>> +++ b/rust/kernel/block/mq/operations.rs
>>> @@ -42,6 +42,9 @@ fn queue_rq(
>>> /// Called by the kernel to indicate that queued requests should be submitted.
>>> fn commit_rqs(queue_data: ForeignBorrowed<'_, Self::QueueData>);
>>>
>>> + /// Called by the kernel when the request is completed.
>>> + fn complete(rq: ARef<Request<Self>>);
>>> +
>>> /// Called by the kernel to poll the device for completed requests. Only
>>> /// used for poll queues.
>>> fn poll() -> bool {
>>> @@ -143,13 +146,21 @@ impl<T: Operations> OperationsVTable<T> {
>>> T::commit_rqs(queue_data)
>>> }
>>>
>>> - /// This function is called by the C kernel. It is not currently
>>> - /// implemented, and there is no way to exercise this code path.
>>> + /// This function is called by the C kernel. A pointer to this function is
>>> + /// installed in the `blk_mq_ops` vtable for the driver.
>>> ///
>>> /// # Safety
>>> ///
>>> - /// This function may only be called by blk-mq C infrastructure.
>>> - unsafe extern "C" fn complete_callback(_rq: *mut bindings::request) {}
>>> + /// This function may only be called by blk-mq C infrastructure. `rq` must
>>> + /// point to a valid request that has been marked as completed. The pointee
>>> + /// of `rq` must be valid for write for the duration of this function.
>>> + unsafe extern "C" fn complete_callback(rq: *mut bindings::request) {
>>> + // SAFETY: This function can only be dispatched through
>>> + // `Request::complete`. We leaked a refcount then which we pick back up
>>> + // now.
>>> + let aref = unsafe { Request::aref_from_raw(rq) };
>>> + T::complete(aref);
>>> + }
>>>
>>> /// This function is called by the C kernel. A pointer to this function is
>>> /// installed in the `blk_mq_ops` vtable for the driver.
>>> diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
>>> index 3848cfe63f77..f7f757f7459f 100644
>>> --- a/rust/kernel/block/mq/request.rs
>>> +++ b/rust/kernel/block/mq/request.rs
>>> @@ -135,6 +135,23 @@ pub fn end_ok(this: ARef<Self>) -> Result<(), ARef<Self>> {
>>> Ok(())
>>> }
>>>
>>> + /// Complete the request by scheduling `Operations::complete` for
>>> + /// execution.
>>> + ///
>>> + /// The function may be scheduled locally, via SoftIRQ or remotely via IPMI.
>>> + /// See `blk_mq_complete_request_remote` in [`blk-mq.c`] for details.
>>> + ///
>>> + /// [`blk-mq.c`]: srctree/block/blk-mq.c
>>> + pub fn complete(this: ARef<Self>) {
>>> + let ptr = ARef::into_raw(this).cast::<bindings::request>().as_ptr();
>>> + // SAFETY: By type invariant, `self.0` is a valid `struct request`
>>> + if !unsafe { bindings::blk_mq_complete_request_remote(ptr) } {
>>> + // SAFETY: We released a refcount above that we can reclaim here.
>>> + let this = unsafe { Request::aref_from_raw(ptr) };
>>> + T::complete(this);
>>> + }
>>> + }
>>> +
>>> /// Return a pointer to the [`RequestDataWrapper`] stored in the private area
>>> /// of the request structure.
>>> ///
>>>
>>> --
>>> 2.47.2
>>>
>>>
>>
>> I had another look here. While I do trust your reasoning, perhaps we should
>> remove the call to expect()?
>>
>> If it is not called ever as you said, great, removing the expect() will not
>> change the code behavior. If it is, be it because of some minor oversight or
>> unexpected condition, we should produce some error output instead of crashing
>> the kernel. Maybe we should use a warn() here instead? Or maybe dev/pr_err as
>> applicable?
>
> I think for the example, I would like to keep the `expect`. For
> demonstration purposes.
I think examples is definitely one place we don’t want to teach people to
use unwrap and expect because that will kill the system IIUC.
>
> We could do `warn!` instead for the rnull driver I guess. But the IO
> queue that would hit this code would start to hang pretty fast, since no
> IO would complete. I don't think the kernel can recover from this hang.
>
No I/O would complete for that device, but that doesn't mean that the system is
unusable IIUC, specially if other devices are available?
>
> Best regards,
> Andreas Hindborg
In any case, perhaps this is only my opinion and others may see this
differently, so I won't complain if you want to keep it.
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v6 18/18] rnull: add soft-irq completion support
2025-08-22 12:14 [PATCH v6 00/18] rnull: add configfs, remote completion to rnull Andreas Hindborg
` (16 preceding siblings ...)
2025-08-22 12:14 ` [PATCH v6 17/18] rust: block: add remote completion to `Request` Andreas Hindborg
@ 2025-08-22 12:14 ` Andreas Hindborg
2025-08-27 18:02 ` Daniel Almeida
17 siblings, 1 reply; 38+ messages in thread
From: Andreas Hindborg @ 2025-08-22 12:14 UTC (permalink / raw)
To: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao
Cc: linux-block, rust-for-linux, linux-kernel, Andreas Hindborg
rnull currently only supports direct completion. Add option for completing
requests across CPU nodes via soft IRQ or IPI.
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
drivers/block/rnull/configfs.rs | 59 +++++++++++++++++++++++++++++++++++++++--
drivers/block/rnull/rnull.rs | 32 ++++++++++++++--------
2 files changed, 78 insertions(+), 13 deletions(-)
diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
index 46710a1e1af4..8498e9bae6fd 100644
--- a/drivers/block/rnull/configfs.rs
+++ b/drivers/block/rnull/configfs.rs
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
use super::{NullBlkDevice, THIS_MODULE};
-use core::fmt::Write;
+use core::fmt::{Display, Write};
use kernel::{
block::mq::gen_disk::{GenDisk, GenDiskBuilder},
c_str,
@@ -36,7 +36,7 @@ impl AttributeOperations<0> for Config {
fn show(_this: &Config, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
let mut writer = kernel::str::Formatter::new(page);
- writer.write_str("blocksize,size,rotational\n")?;
+ writer.write_str("blocksize,size,rotational,irqmode\n")?;
Ok(writer.bytes_written())
}
}
@@ -58,6 +58,7 @@ fn make_group(
blocksize: 1,
rotational: 2,
size: 3,
+ irqmode: 4,
],
};
@@ -72,6 +73,7 @@ fn make_group(
rotational: false,
disk: None,
capacity_mib: 4096,
+ irq_mode: IRQMode::None,
name: name.try_into()?,
}),
}),
@@ -79,6 +81,34 @@ fn make_group(
}
}
+#[derive(Debug, Clone, Copy)]
+pub(crate) enum IRQMode {
+ None,
+ Soft,
+}
+
+impl TryFrom<u8> for IRQMode {
+ type Error = kernel::error::Error;
+
+ fn try_from(value: u8) -> Result<Self> {
+ match value {
+ 0 => Ok(Self::None),
+ 1 => Ok(Self::Soft),
+ _ => Err(EINVAL),
+ }
+ }
+}
+
+impl Display for IRQMode {
+ fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+ match self {
+ Self::None => f.write_str("0")?,
+ Self::Soft => f.write_str("1")?,
+ }
+ Ok(())
+ }
+}
+
#[pin_data]
pub(crate) struct DeviceConfig {
#[pin]
@@ -92,6 +122,7 @@ struct DeviceConfigInner {
block_size: u32,
rotational: bool,
capacity_mib: u64,
+ irq_mode: IRQMode,
disk: Option<GenDisk<NullBlkDevice>>,
}
@@ -121,6 +152,7 @@ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
guard.block_size,
guard.rotational,
guard.capacity_mib,
+ guard.irq_mode,
)?);
guard.powered = true;
} else if guard.powered && !power_op {
@@ -205,3 +237,26 @@ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
Ok(())
}
}
+
+#[vtable]
+impl configfs::AttributeOperations<4> for DeviceConfig {
+ type Data = DeviceConfig;
+
+ fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
+ let mut writer = kernel::str::Formatter::new(page);
+ writer.write_fmt(fmt!("{}\n", this.data.lock().irq_mode))?;
+ Ok(writer.bytes_written())
+ }
+
+ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
+ if this.data.lock().powered {
+ return Err(EBUSY);
+ }
+
+ let text = core::str::from_utf8(page)?.trim();
+ let value = text.parse::<u8>().map_err(|_| EINVAL)?;
+
+ this.data.lock().irq_mode = IRQMode::try_from(value)?;
+ Ok(())
+ }
+}
diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index a19c55717c4f..1ec694d7f1a6 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -4,6 +4,7 @@
mod configfs;
+use configfs::IRQMode;
use kernel::{
block::{
self,
@@ -53,35 +54,44 @@ fn new(
block_size: u32,
rotational: bool,
capacity_mib: u64,
+ irq_mode: IRQMode,
) -> Result<GenDisk<Self>> {
let tagset = Arc::pin_init(TagSet::new(1, 256, 1), GFP_KERNEL)?;
+ let queue_data = Box::new(QueueData { irq_mode }, GFP_KERNEL)?;
+
gen_disk::GenDiskBuilder::new()
.capacity_sectors(capacity_mib << (20 - block::SECTOR_SHIFT))
.logical_block_size(block_size)?
.physical_block_size(block_size)?
.rotational(rotational)
- .build(fmt!("{}", name.to_str()?), tagset, ())
+ .build(fmt!("{}", name.to_str()?), tagset, queue_data)
}
}
+struct QueueData {
+ irq_mode: IRQMode,
+}
+
#[vtable]
impl Operations for NullBlkDevice {
- type QueueData = ();
+ type QueueData = KBox<QueueData>;
#[inline(always)]
- fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
- mq::Request::end_ok(rq)
- .map_err(|_e| kernel::error::code::EIO)
- // We take no refcounts on the request, so we expect to be able to
- // end the request. The request reference must be unique at this
- // point, and so `end_ok` cannot fail.
- .expect("Fatal error - expected to be able to end request");
-
+ fn queue_rq(queue_data: &QueueData, rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
+ match queue_data.irq_mode {
+ IRQMode::None => mq::Request::end_ok(rq)
+ .map_err(|_e| kernel::error::code::EIO)
+ // We take no refcounts on the request, so we expect to be able to
+ // end the request. The request reference must be unique at this
+ // point, and so `end_ok` cannot fail.
+ .expect("Fatal error - expected to be able to end request"),
+ IRQMode::Soft => mq::Request::complete(rq),
+ }
Ok(())
}
- fn commit_rqs(_queue_data: ()) {}
+ fn commit_rqs(_queue_data: &QueueData) {}
fn complete(rq: ARef<mq::Request<Self>>) {
mq::Request::end_ok(rq)
--
2.47.2
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v6 18/18] rnull: add soft-irq completion support
2025-08-22 12:14 ` [PATCH v6 18/18] rnull: add soft-irq completion support Andreas Hindborg
@ 2025-08-27 18:02 ` Daniel Almeida
0 siblings, 0 replies; 38+ messages in thread
From: Daniel Almeida @ 2025-08-27 18:02 UTC (permalink / raw)
To: Andreas Hindborg
Cc: Boqun Feng, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Jens Axboe, Breno Leitao, linux-block,
rust-for-linux, linux-kernel
> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> rnull currently only supports direct completion. Add option for completing
> requests across CPU nodes via soft IRQ or IPI.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> drivers/block/rnull/configfs.rs | 59 +++++++++++++++++++++++++++++++++++++++--
> drivers/block/rnull/rnull.rs | 32 ++++++++++++++--------
> 2 files changed, 78 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
> index 46710a1e1af4..8498e9bae6fd 100644
> --- a/drivers/block/rnull/configfs.rs
> +++ b/drivers/block/rnull/configfs.rs
> @@ -1,7 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
>
> use super::{NullBlkDevice, THIS_MODULE};
> -use core::fmt::Write;
> +use core::fmt::{Display, Write};
> use kernel::{
> block::mq::gen_disk::{GenDisk, GenDiskBuilder},
> c_str,
> @@ -36,7 +36,7 @@ impl AttributeOperations<0> for Config {
>
> fn show(_this: &Config, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> let mut writer = kernel::str::Formatter::new(page);
> - writer.write_str("blocksize,size,rotational\n")?;
> + writer.write_str("blocksize,size,rotational,irqmode\n")?;
> Ok(writer.bytes_written())
> }
> }
> @@ -58,6 +58,7 @@ fn make_group(
> blocksize: 1,
> rotational: 2,
> size: 3,
> + irqmode: 4,
> ],
> };
>
> @@ -72,6 +73,7 @@ fn make_group(
> rotational: false,
> disk: None,
> capacity_mib: 4096,
> + irq_mode: IRQMode::None,
> name: name.try_into()?,
> }),
> }),
> @@ -79,6 +81,34 @@ fn make_group(
> }
> }
>
> +#[derive(Debug, Clone, Copy)]
> +pub(crate) enum IRQMode {
> + None,
> + Soft,
> +}
> +
> +impl TryFrom<u8> for IRQMode {
> + type Error = kernel::error::Error;
> +
> + fn try_from(value: u8) -> Result<Self> {
> + match value {
> + 0 => Ok(Self::None),
> + 1 => Ok(Self::Soft),
> + _ => Err(EINVAL),
> + }
> + }
> +}
> +
> +impl Display for IRQMode {
> + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
> + match self {
> + Self::None => f.write_str("0")?,
> + Self::Soft => f.write_str("1")?,
> + }
> + Ok(())
> + }
> +}
> +
> #[pin_data]
> pub(crate) struct DeviceConfig {
> #[pin]
> @@ -92,6 +122,7 @@ struct DeviceConfigInner {
> block_size: u32,
> rotational: bool,
> capacity_mib: u64,
> + irq_mode: IRQMode,
> disk: Option<GenDisk<NullBlkDevice>>,
> }
>
> @@ -121,6 +152,7 @@ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
> guard.block_size,
> guard.rotational,
> guard.capacity_mib,
> + guard.irq_mode,
> )?);
> guard.powered = true;
> } else if guard.powered && !power_op {
> @@ -205,3 +237,26 @@ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
> Ok(())
> }
> }
> +
> +#[vtable]
> +impl configfs::AttributeOperations<4> for DeviceConfig {
> + type Data = DeviceConfig;
> +
> + fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> + let mut writer = kernel::str::Formatter::new(page);
> + writer.write_fmt(fmt!("{}\n", this.data.lock().irq_mode))?;
> + Ok(writer.bytes_written())
> + }
> +
> + fn store(this: &DeviceConfig, page: &[u8]) -> Result {
> + if this.data.lock().powered {
> + return Err(EBUSY);
> + }
> +
> + let text = core::str::from_utf8(page)?.trim();
> + let value = text.parse::<u8>().map_err(|_| EINVAL)?;
> +
> + this.data.lock().irq_mode = IRQMode::try_from(value)?;
> + Ok(())
> + }
> +}
> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
> index a19c55717c4f..1ec694d7f1a6 100644
> --- a/drivers/block/rnull/rnull.rs
> +++ b/drivers/block/rnull/rnull.rs
> @@ -4,6 +4,7 @@
>
> mod configfs;
>
> +use configfs::IRQMode;
> use kernel::{
> block::{
> self,
> @@ -53,35 +54,44 @@ fn new(
> block_size: u32,
> rotational: bool,
> capacity_mib: u64,
> + irq_mode: IRQMode,
> ) -> Result<GenDisk<Self>> {
> let tagset = Arc::pin_init(TagSet::new(1, 256, 1), GFP_KERNEL)?;
>
> + let queue_data = Box::new(QueueData { irq_mode }, GFP_KERNEL)?;
> +
> gen_disk::GenDiskBuilder::new()
> .capacity_sectors(capacity_mib << (20 - block::SECTOR_SHIFT))
> .logical_block_size(block_size)?
> .physical_block_size(block_size)?
> .rotational(rotational)
> - .build(fmt!("{}", name.to_str()?), tagset, ())
> + .build(fmt!("{}", name.to_str()?), tagset, queue_data)
> }
> }
>
> +struct QueueData {
> + irq_mode: IRQMode,
> +}
> +
> #[vtable]
> impl Operations for NullBlkDevice {
> - type QueueData = ();
> + type QueueData = KBox<QueueData>;
>
> #[inline(always)]
> - fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
> - mq::Request::end_ok(rq)
> - .map_err(|_e| kernel::error::code::EIO)
> - // We take no refcounts on the request, so we expect to be able to
> - // end the request. The request reference must be unique at this
> - // point, and so `end_ok` cannot fail.
> - .expect("Fatal error - expected to be able to end request");
Same comment here as on the last patch. Otherwise LGTM.
> -
> + fn queue_rq(queue_data: &QueueData, rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
> + match queue_data.irq_mode {
> + IRQMode::None => mq::Request::end_ok(rq)
> + .map_err(|_e| kernel::error::code::EIO)
> + // We take no refcounts on the request, so we expect to be able to
> + // end the request. The request reference must be unique at this
> + // point, and so `end_ok` cannot fail.
> + .expect("Fatal error - expected to be able to end request"),
> + IRQMode::Soft => mq::Request::complete(rq),
> + }
> Ok(())
> }
>
> - fn commit_rqs(_queue_data: ()) {}
> + fn commit_rqs(_queue_data: &QueueData) {}
>
> fn complete(rq: ARef<mq::Request<Self>>) {
> mq::Request::end_ok(rq)
>
> --
> 2.47.2
>
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread