* Re: [PATCH 28/79] block: rnull: add partial I/O support for bad blocks
From: Andreas Hindborg @ 2026-06-03 9:44 UTC (permalink / raw)
To: Alice Ryhl
Cc: Boqun Feng, Jens Axboe, Miguel Ojeda, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, FUJITA Tomonori, Frederic Weisbecker,
Lyude Paul, Thomas Gleixner, Anna-Maria Behnsen, John Stultz,
Stephen Boyd, Lorenzo Stoakes, Liam R. Howlett, linux-block,
rust-for-linux, linux-kernel, linux-mm
In-Reply-To: <abf0FHWkQzbnOtqD@google.com>
Alice Ryhl <aliceryhl@google.com> writes:
> On Mon, Feb 16, 2026 at 12:35:15AM +0100, Andreas Hindborg wrote:
<cut>
>> +fn is_power_of_two<T>(value: T) -> bool
>> +where
>> + T: core::ops::Sub<T, Output = T>,
>> + T: core::ops::BitAnd<Output = T>,
>> + T: core::cmp::PartialOrd<T>,
>> + T: Copy,
>> + T: From<u8>,
>> +{
>> + (value > 0u8.into()) && (value & (value - 1u8.into())) == 0u8.into()
>> +}
>
> This is in the standard library.
But there is no trait for this, the method is defined for concrete
integer types. I would still need to define a trait and macro implement
it for all integers.
We could do this in `kernel::num`?
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [PATCH 24/79] block: rust: add `BadBlocks` for bad block tracking
From: Andreas Hindborg @ 2026-06-03 10:15 UTC (permalink / raw)
To: Alice Ryhl
Cc: Boqun Feng, Jens Axboe, Miguel Ojeda, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, FUJITA Tomonori, Frederic Weisbecker,
Lyude Paul, Thomas Gleixner, Anna-Maria Behnsen, John Stultz,
Stephen Boyd, Lorenzo Stoakes, Liam R. Howlett, linux-block,
rust-for-linux, linux-kernel, linux-mm
In-Reply-To: <abfvrh0ObKhtsR8I@google.com>
Alice Ryhl <aliceryhl@google.com> writes:
> On Mon, Feb 16, 2026 at 12:35:11AM +0100, Andreas Hindborg wrote:
>> Add a safe Rust wrapper around the Linux kernel's badblocks infrastructure
>> to track and manage defective sectors on block devices. The BadBlocks type
>> provides methods to:
>>
>> - Mark sectors as bad or good (set_bad/set_good)
>> - Check if sector ranges contain bad blocks (check)
>> - Automatically handle memory management with PinnedDrop
>>
>> The implementation includes comprehensive documentation with examples for
>> block device drivers that need to avoid known bad sectors to maintain
>> data integrity. Bad blocks information is used by device drivers,
>> filesystem layers, and device management tools.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
>> diff --git a/rust/kernel/block/badblocks.rs b/rust/kernel/block/badblocks.rs
>> new file mode 100644
>> index 0000000000000..a5fe0fde2e755
>> --- /dev/null
>> +++ b/rust/kernel/block/badblocks.rs
>> @@ -0,0 +1,721 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Bad blocks tracking for block devices.
>> +//!
>> +//! This module provides a safe Rust wrapper around the badblocks
>> +//! infrastructure, which is used to track and manage bad sectors on block
>> +//! devices. Bad blocks are sectors that cannot reliably store data and should
>> +//! be avoided during I/O operations.
>
> Could use a srctree link to badblocks.h here.
Will add.
>
>> +/// # Examples
>> +///
>> +/// Basic usage:
>> +///
>> +/// ```rust
>> +/// # use kernel::block::badblocks::{BadBlocks, BlockStatus};
>> +/// # use kernel::prelude::*;
>> +///
>
> Unhide the imports or remove this empty line.
Ok.
>
>> +/// // Create a new bad blocks tracker
>> +/// let bad_blocks = KBox::pin_init(BadBlocks::new(true), GFP_KERNEL)?;
>> +///
>> +/// // Mark sectors 100-109 as bad (unacknowledged)
>> +/// bad_blocks.set_bad(100..110, false)?;
>> +///
>> +/// // Check if sector range 95-104 contains bad blocks
>> +/// match bad_blocks.check(95..105) {
>> +/// BlockStatus::None => pr_info!("No bad blocks found"),
>> +/// BlockStatus::Acknowledged(range) => pr_warn!("Acknowledged bad blocks: {:?}", range),
>> +/// BlockStatus::Unacknowledged(range) => pr_err!("Unacknowledged bad blocks: {:?}", range),
>> +/// }
>> +/// # Ok::<(), kernel::error::Error>(())
>> +/// ```
>> +/// # Invariants
>> +///
>> +/// - `self.blocks` is a valid `bindings::badblocks` struct.
>> +#[pin_data(PinnedDrop)]
>> +pub struct BadBlocks {
>> + #[pin]
>> + blocks: Opaque<bindings::badblocks>,
>> +}
>> +
>> +impl BadBlocks {
>> + /// Creates a new bad blocks tracker.
>> + ///
>> + /// Initializes an empty bad blocks tracker that can manage defective sectors
>> + /// on a block device. The tracker starts with no bad blocks recorded and
>> + /// allocates a single page for storing bad block entries.
>> + ///
>> + /// # Returns
>> + ///
>> + /// Returns a [`PinInit`] that can be used to initialize a [`BadBlocks`] instance.
>> + /// Initialization may fail with `ENOMEM` if memory allocation fails.
>> + ///
>> + /// # Examples
>> + ///
>> + /// ```rust
>> + /// # use kernel::block::badblocks::{BadBlocks, BlockStatus};
>> + /// # use kernel::prelude::*;
>> + ///
>
> Ditto. (Many times throughout file.)
>
>> + /// // Create and initialize a bad blocks tracker
>> + /// let bad_blocks = KBox::pin_init(BadBlocks::new(true), GFP_KERNEL)?;
>> + ///
>> + /// // The tracker is ready to use with no bad blocks initially
>> + /// match bad_blocks.check(0..100) {
>> + /// BlockStatus::None => pr_info!("No bad blocks found initially"),
>> + /// _ => unreachable!(),
>> + /// }
>> + /// # Ok::<(), kernel::error::Error>(())
>> + /// ```
>> + pub fn new(enable: bool) -> impl PinInit<Self, Error> {
>> + // INVARIANT: We initialize `self.blocks` below. If initialization fails, an error is
>> + // returned.
>> + try_pin_init!(Self {
>> + blocks <- Opaque::try_ffi_init(|slot| {
>> + // SAFETY: `slot` is a valid pointer to uninitialized memory
>> + // allocated by the Opaque type. `badblocks_init` is safe to
>> + // call with uninitialized memory.
>> + to_result(unsafe {bindings::badblocks_init(slot, if enable {1} else {0})})
>
> I think you can just cast the boolean to an integer.
Ok.
>
> Also, formatting here is off (but ignored by rustfmt due to macro.)
Will try to fix.
>
>> + /// Enables the bad blocks tracker if it was previously disabled.
>> + ///
>> + /// Attempts to enable bad block tracking by transitioning the tracker from
>> + /// a disabled state to an enabled state.
>> + ///
>> + /// # Behavior
>> + ///
>> + /// - If the tracker is disabled, it will be enabled.
>> + /// - If the tracker is already enabled, this operation has no effect.
>> + /// - The operation is atomic and thread-safe.
>> + ///
>> + /// # Usage
>> + ///
>> + /// Bad blocks trackers can be created in a disabled state and enabled later
>> + /// when needed. This is useful for conditional bad block tracking or for
>> + /// deferring activation until the device is fully initialized.
>> + ///
>> + /// # Examples
>> + ///
>> + /// ```rust
>> + /// # use kernel::block::badblocks::BadBlocks;
>> + /// # use kernel::prelude::*;
>> + ///
>> + /// // Create a disabled bad blocks tracker
>> + /// let bad_blocks = KBox::pin_init(BadBlocks::new(false), GFP_KERNEL)?;
>> + /// assert!(!bad_blocks.enabled());
>> + ///
>> + /// // Enable it when needed
>> + /// bad_blocks.enable();
>> + /// assert!(bad_blocks.enabled());
>> + ///
>> + /// // Subsequent enable calls have no effect
>> + /// bad_blocks.enable();
>> + /// assert!(bad_blocks.enabled());
>> + /// # Ok::<(), kernel::error::Error>(())
>> + /// ```
>> + pub fn enable(&self) {
>> + let _ = self.shift_ref().cmpxchg(-1, 0, ordering::Relaxed);
>
> Is there not a C function you can call here? It would be simpler that
> way. Surely drivers don't do this directly.
No, this is the canonical way [1].
[1] https://github.com/torvalds/linux/blob/ba3e43a9e601636f5edb54e259a74f96ca3b8fd8/drivers/block/null_blk/main.c#L563
>
>> + }
>> +
>> + /// Checks whether the bad blocks tracker is currently enabled.
>> + ///
>> + /// Returns `true` if bad block tracking is active, `false` if it is disabled.
>> + /// When disabled, the tracker will not perform bad block checks or operations.
>> + ///
>> + /// # Returns
>> + ///
>> + /// - `true` - Bad block tracking is enabled and operational
>> + /// - `false` - Bad block tracking is disabled
>
> You explain the meaning of return values twice here. Just drop the
> 'Returns' section.
Ok.
>
>> + /// # Thread Safety
>> + ///
>> + /// This method is thread-safe and uses atomic operations to check the
>> + /// tracker's state without requiring external synchronization.
>
> This is implicit from the signature of the function.
Will remove.
>
>> + pub fn set_good(&self, range: impl RangeBounds<u64>) -> Result {
>> + let range = Self::range(range);
>> + // SAFETY: By type invariant `self.blocks` is valid. The C function
>> + // `badblocks_clear` handles synchronization internally.
>> + unsafe {
>> + bindings::badblocks_clear(self.blocks.get(), range.start, range.end - range.start)
>> + }
>> + .then_some(())
>> + .ok_or(EINVAL)
>
> then_some() is quite obscure. I would recommend if/else here.
Ok.
Thanks,
Andreas
^ permalink raw reply
* Re: [PATCH 06/79] block: rust: add `Request` private data support
From: Andreas Hindborg @ 2026-06-03 10:40 UTC (permalink / raw)
To: Alice Ryhl
Cc: Boqun Feng, Jens Axboe, Miguel Ojeda, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, FUJITA Tomonori, Frederic Weisbecker,
Lyude Paul, Thomas Gleixner, Anna-Maria Behnsen, John Stultz,
Stephen Boyd, Lorenzo Stoakes, Liam R. Howlett, linux-block,
rust-for-linux, linux-kernel, linux-mm
In-Reply-To: <abfXIqlT9qRhXssT@google.com>
Alice Ryhl <aliceryhl@google.com> writes:
> On Mon, Feb 16, 2026 at 12:34:53AM +0100, Andreas Hindborg wrote:
>> C block device drivers can attach private data to a `struct request`. This
>> data is stored next to the request structure and is part of the request
>> allocation set up during driver initialization.
>>
>> Expose this private request data area to Rust block device drivers.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> Overall looks ok. A few nits below:
>
>> ---
>> drivers/block/rnull/rnull.rs | 5 +++++
>> rust/kernel/block/mq.rs | 6 ++++++
>> rust/kernel/block/mq/operations.rs | 24 +++++++++++++++++++++++-
>> rust/kernel/block/mq/request.rs | 24 +++++++++++++++++++-----
>> rust/kernel/block/mq/tag_set.rs | 2 +-
>> 5 files changed, 54 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
>> index 6a7f660d31998..065639fc4f941 100644
>> --- a/drivers/block/rnull/rnull.rs
>> +++ b/drivers/block/rnull/rnull.rs
>> @@ -134,6 +134,11 @@ struct QueueData {
>> #[vtable]
>> impl Operations for NullBlkDevice {
>> type QueueData = KBox<QueueData>;
>> + type RequestData = ();
>> +
>> + fn new_request_data() -> impl PinInit<Self::RequestData> {
>> + pin_init::zeroed::<Self::RequestData>()
>
> Simpler to just return Ok(()) here.
Not sure why I picked zeroed. Will change.
>
>> + }
>>
>> #[inline(always)]
>> fn queue_rq(queue_data: &QueueData, rq: Owned<mq::Request<Self>>, _is_last: bool) -> Result {
>> diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
>> index b8ecd69abe980..a285b753ada88 100644
>> --- a/rust/kernel/block/mq.rs
>> +++ b/rust/kernel/block/mq.rs
>> @@ -69,8 +69,14 @@
>> //!
>> //! #[vtable]
>> //! impl Operations for MyBlkDevice {
>> +//! type RequestData = ();
>> //! type QueueData = ();
>> //!
>> +//! fn new_request_data(
>> +//! ) -> impl PinInit<()> {
>> +//! pin_init::zeroed::<()>()
>
> Simpler to just return Ok(()) here.
Will change.
>
>> +//! }
>> +//!
>> //! fn queue_rq(_queue_data: (), rq: Owned<Request<Self>>, _is_last: bool) -> Result {
>> //! rq.end_ok();
>> //! Ok(())
>> diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
>> index 3dea79d647ff7..cd37b939bbf30 100644
>> --- a/rust/kernel/block/mq/operations.rs
>> +++ b/rust/kernel/block/mq/operations.rs
>> @@ -13,6 +13,7 @@
>> types::{ForeignOwnable, Owned},
>> };
>> use core::{marker::PhantomData, ptr::NonNull};
>> +use pin_init::PinInit;
>>
>> type ForeignBorrowed<'a, T> = <T as ForeignOwnable>::Borrowed<'a>;
>>
>> @@ -28,10 +29,24 @@
>> /// [module level documentation]: kernel::block::mq
>> #[macros::vtable]
>> pub trait Operations: Sized {
>> + /// Data associated with a request. This data is located next to the request
>> + /// structure.
>> + ///
>> + /// To be able to handle accessing this data from interrupt context, this
>> + /// data must be `Sync`.
>> + ///
>> + /// The `RequestData` object is initialized when the requests are allocated
>> + /// during queue initialization, and it is are dropped when the requests are
>> + /// dropped during queue teardown.
>> + type RequestData: Sized + Sync;
>
> It was surprising to me that `request` is reused over many requests. I
> think it could be a bit more obvious in the wording.
I will add a bit of info in the `Request` type docs.
>
>> /// 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 get an initializer for a `Pin<&mut RequestData>`.
>> + fn new_request_data() -> impl PinInit<Self::RequestData>;
>> +
>> /// 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(
>> @@ -236,6 +251,13 @@ impl<T: Operations> OperationsVTable<T> {
>> // it is valid for writes.
>> unsafe { RequestDataWrapper::refcount_ptr(pdu.as_ptr()).write(Refcount::new(0)) };
>>
>> + let initializer = T::new_request_data();
>> +
>> + // SAFETY: `pdu` is a valid pointer as established above. We do not
>> + // touch `pdu` if `__pinned_init` returns an error. We promise ot to
>
> typo
>
>
>> diff --git a/rust/kernel/block/mq/tag_set.rs b/rust/kernel/block/mq/tag_set.rs
>> index c3cf56d52beec..46481754b1335 100644
>> --- a/rust/kernel/block/mq/tag_set.rs
>> +++ b/rust/kernel/block/mq/tag_set.rs
>> @@ -41,7 +41,7 @@ pub fn new(
>> // SAFETY: `blk_mq_tag_set` only contains integers and pointers, which
>> // all are allowed to be 0.
>> let tag_set: bindings::blk_mq_tag_set = unsafe { core::mem::zeroed() };
>> - let tag_set: Result<_> = core::mem::size_of::<RequestDataWrapper>()
>> + let tag_set: Result<_> = core::mem::size_of::<RequestDataWrapper<T>>()
>
> size_of in prelude.
Fixed.
Thanks,
Andreas
^ permalink raw reply
* Re: [PATCH 09/79] block: rust: introduce `kernel::block::bio` module
From: Andreas Hindborg @ 2026-06-03 11:29 UTC (permalink / raw)
To: Alice Ryhl
Cc: Boqun Feng, Jens Axboe, Miguel Ojeda, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, FUJITA Tomonori, Frederic Weisbecker,
Lyude Paul, Thomas Gleixner, Anna-Maria Behnsen, John Stultz,
Stephen Boyd, Lorenzo Stoakes, Liam R. Howlett, linux-block,
rust-for-linux, linux-kernel, linux-mm
In-Reply-To: <abfbfV6-FXnVT9Ud@google.com>
Alice Ryhl <aliceryhl@google.com> writes:
> On Mon, Feb 16, 2026 at 12:34:56AM +0100, Andreas Hindborg wrote:
>> Add Rust abstractions for working with `struct bio`, the core IO command
>> descriptor for the block layer.
>>
>> The `Bio` type wraps `struct bio` and provides safe access to the IO
>> vector describing the data buffers associated with the IO command. The
>> data buffers are represented as a vector of `Segment`s, where each
>> segment is a contiguous region of physical memory backed by `Page`.
>>
>> The `BioSegmentIterator` provides iteration over segments in a single
>> bio, while `BioIterator` allows traversing a chain of bios. The
>> `Segment` type offers methods for copying data to and from pages, as
>> well as zeroing page contents, which are the fundamental operations
>> needed by block device drivers to process IO requests.
>>
>> The `Request` type is extended with methods to access the bio chain
>> associated with a request, allowing drivers to iterate over all data
>> buffers that need to be processed.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>> rust/helpers/blk.c | 8 +
>> rust/kernel/block.rs | 1 +
>> rust/kernel/block/bio.rs | 143 +++++++++++++++
>> rust/kernel/block/bio/vec.rs | 389 ++++++++++++++++++++++++++++++++++++++++
>> rust/kernel/block/mq/request.rs | 46 +++++
>> rust/kernel/lib.rs | 2 +
>> rust/kernel/page.rs | 2 +-
>> 7 files changed, 590 insertions(+), 1 deletion(-)
>>
>> diff --git a/rust/helpers/blk.c b/rust/helpers/blk.c
>> index cc9f4e6a2d234..53beba8c7782d 100644
>> --- a/rust/helpers/blk.c
>> +++ b/rust/helpers/blk.c
>> @@ -1,5 +1,6 @@
>> // SPDX-License-Identifier: GPL-2.0
>>
>> +#include <linux/bio.h>
>> #include <linux/blk-mq.h>
>> #include <linux/blkdev.h>
>>
>> @@ -12,3 +13,10 @@ struct request *rust_helper_blk_mq_rq_from_pdu(void *pdu)
>> {
>> return blk_mq_rq_from_pdu(pdu);
>> }
>> +
>> +void rust_helper_bio_advance_iter_single(const struct bio *bio,
>> + struct bvec_iter *iter,
>> + unsigned int bytes)
>> +{
>> + bio_advance_iter_single(bio, iter, bytes);
>> +}
>
> __rust_helper
Thanks.
>
>> diff --git a/rust/kernel/block/bio.rs b/rust/kernel/block/bio.rs
>> new file mode 100644
>> index 0000000000000..94062ea5281e6
>> --- /dev/null
>> +++ b/rust/kernel/block/bio.rs
>> @@ -0,0 +1,143 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Types for working with the bio layer.
>> +//!
>> +//! C header: [`include/linux/blk_types.h`](../../include/linux/blk_types.h)
>
> srctree/
Ok.
>
>> +/// A block device IO descriptor (`struct bio`).
>> +///
>> +/// A `Bio` is the main unit of IO for the block layer. It describes an IO command and associated
>> +/// data buffers.
>> +///
>> +/// The data buffers associated with a `Bio` are represented by a vector of [`Segment`]s. These
>> +/// segments represent physically contiguous regions of memory. The memory is represented by
>> +/// [`Page`] descriptors internally.
>> +///
>> +/// The vector of [`Segment`]s can be iterated by obtaining a [`SegmentIterator`].
>> +///
>> +/// # Invariants
>> +///
>> +/// Instances of this type is always reference counted. A call to
>> +/// `bindings::bio_get()` ensures that the instance is valid for read at least
>> +/// until a matching call to `bindings :bio_put()`.
>
> Refcounted? None of these methods are called anywhere, and you do not
> implement AlwaysRefcounted.
This is stale info, I will remove it.
>
>> +#[repr(transparent)]
>> +pub struct Bio(Opaque<bindings::bio>);
>> +
>> +impl Bio {
>> + /// Returns an iterator over segments in this `Bio`. Does not consider
>> + /// segments of other bios in this bio chain.
>> + #[inline(always)]
>> + pub fn segment_iter(&mut self) -> BioSegmentIterator<'_> {
>
> Not `self: Pin<&mut Self>` here?
It definitely must be pinned, thanks.
>
>> + /// Create an instance of `Bio` from a raw pointer.
>> + ///
>> + /// # Safety
>> + ///
>> + /// Caller must ensure that the `ptr` is valid for use as a reference to
>> + /// `Bio` for the duration of `'a`.
>> + #[inline(always)]
>> + pub(crate) unsafe fn from_raw<'a>(ptr: *mut bindings::bio) -> Option<&'a Self> {
>> + Some(
>> + // SAFETY: by the safety requirement of this funciton, `ptr` is
>> + // valid for read for the duration of the returned lifetime
>> + unsafe { &*NonNull::new(ptr)?.as_ptr().cast::<Bio>() },
>> + )
>> + }
>> +
>> + /// Create an instance of `Bio` from a raw pointer.
>> + ///
>> + /// # Safety
>> + ///
>> + /// Caller must ensure that the `ptr` is valid for use as a unique reference
>> + /// to `Bio` for the duration of `'a`.
>> + #[inline(always)]
>> + pub(crate) unsafe fn from_raw_mut<'a>(ptr: *mut bindings::bio) -> Option<&'a mut Self> {
>> + Some(
>> + // SAFETY: by the safety requirement of this funciton, `ptr` is
>> + // valid for read for the duration of the returned lifetime
>> + unsafe { &mut *NonNull::new(ptr)?.as_ptr().cast::<Bio>() },
>
> Why the Option? I imagine every caller has a non-null pointert
It felt more streamlined to have the check here than at the call site:
/// Get a mutable reference to the first [`Bio`] in this request.
#[inline(always)]
pub fn bio_mut(&mut self) -> Option<&mut Bio> {
// SAFETY: By type invariant of `Self`, `self.0` is valid and the deref
// is safe.
let ptr = unsafe { (*self.0 .0.get()).bio };
// SAFETY: By C API contract, if `bio` is not null it will have a
// positive refcount at least for the duration of the lifetime of
// `&self`.
unsafe { Bio::from_raw_mut(ptr) }
}
Yes, this also needs to take Pin<&mut Self>.
>
>> + )
>> + }
>> +}
>> +
>> +impl core::fmt::Display for Bio {
>
> We have our own fmt trait now, right?
Will switch to the kernel one.
>
>> + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>> + write!(
>> + f,
>> + "Bio({:?}, vcnt: {}, idx: {}, size: 0x{:x}, completed: 0x{:x})",
>> + self.0.get(),
>> + self.io_vec_count(),
>> + self.raw_iter().bi_idx,
>> + self.raw_iter().bi_size,
>> + self.raw_iter().bi_bvec_done
>
> This reads the entire `bi_iter` field three separate times. A local
> variable may be a good idea.
Ok.
>
>> +/// An iterator over `Segment`
>> +///
>> +/// # Invariants
>> +///
>> +/// If `iter.bi_size` > 0, `iter` must always index a valid `bio_vec` in `bio.io_vec()`.
>> +pub struct BioSegmentIterator<'a> {
>> + bio: &'a mut Bio,
>> + iter: bindings::bvec_iter,
>> +}
>> +
>> +impl<'a> BioSegmentIterator<'a> {
>> + /// Creeate a new segemnt iterator for iterating the segments of `bio`. The
>
> typo
Thanks.
>
>> +impl<'a> core::iter::Iterator for BioSegmentIterator<'a> {
>> + type Item = Segment<'a>;
>> +
>> + #[inline(always)]
>> + fn next(&mut self) -> Option<Self::Item> {
>> + if self.iter.bi_size == 0 {
>> + return None;
>> + }
>> +
>> + // SAFETY: We checked that `self.iter.bi_size` > 0 above.
>> + let bio_vec_ret = unsafe { self.io_vec() };
>> +
>> + // SAFETY: By existence of reference `&bio`, `bio.0` contains a valid
>> + // `struct bio`. By type invariant of `BioSegmentItarator` `self.iter`
>> + // indexes into a valid `bio_vec` entry. By C API contracit, `bv_len`
>> + // does not exceed the size of the bio.
>> + unsafe {
>> + bindings::bio_advance_iter_single(
>> + self.bio.0.get(),
>> + core::ptr::from_mut(&mut self.iter),
>
> Creating this BioSegmentItarator copies the bvec_iter from the Bio, and
> then here you modify the copy. Is that the intent? Is the C type such
> that copying it is always okay?
Yes. I can see if I can document this better.
> Also, is the C type such that moves are ok? It's playsible that the
> answer is yes - the same applies in rust/kernel/iov.rs but it could be
> clearer in e.g. "Invariants" that this is the case.
Yes, it is so. I'll update docs.
> Nit: core::ptr::from_mut(&mut self.iter) -> &raw mut self.iter
Ok.
>
>> + /// Get a mutable reference to the first [`Bio`] in this request.
>> + #[inline(always)]
>> + pub fn bio_mut(&mut self) -> Option<&mut Bio> {
>> + // SAFETY: By type invariant of `Self`, `self.0` is valid and the deref
>> + // is safe.
>> + let ptr = unsafe { (*self.0.get()).bio };
>> + // SAFETY: By C API contract, if `bio` is not null it will have a
>> + // positive refcount at least for the duration of the lifetime of
>> + // `&self`.
>> + unsafe { Bio::from_raw_mut(ptr) }
>
> Surely &mut requires refcount == 1, not just positive refcount?
No, that is not how the C refcount works. Upper layers of the IO stack
will hold refcounts on the bio, even though lower layers are allowed to
mutate the data buffers of the bio.
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [PATCH 10/79] block: rust: add `command` getter to `Request`
From: Andreas Hindborg @ 2026-06-03 11:50 UTC (permalink / raw)
To: Alice Ryhl
Cc: Boqun Feng, Jens Axboe, Miguel Ojeda, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, FUJITA Tomonori, Frederic Weisbecker,
Lyude Paul, Thomas Gleixner, Anna-Maria Behnsen, John Stultz,
Stephen Boyd, Lorenzo Stoakes, Liam R. Howlett, linux-block,
rust-for-linux, linux-kernel, linux-mm
In-Reply-To: <abfbuJVbPtIEyweC@google.com>
Alice Ryhl <aliceryhl@google.com> writes:
> On Mon, Feb 16, 2026 at 12:34:57AM +0100, Andreas Hindborg wrote:
>> Add a method to extract the command operation code from a request. The
>> command is obtained by masking the lower bits of `cmd_flags` as defined by
>> `REQ_OP_BITS`. This allows Rust block drivers to determine the type of
>> operation being requested.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> With the nit below fixed:
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>
>> rust/kernel/block/mq/request.rs | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
>> index b49197a0c66d7..0dd329ae93dfc 100644
>> --- a/rust/kernel/block/mq/request.rs
>> +++ b/rust/kernel/block/mq/request.rs
>> @@ -78,6 +78,12 @@ pub(crate) unsafe fn aref_from_raw(ptr: *mut bindings::request) -> ARef<Self> {
>> unsafe { ARef::from_raw(NonNull::new_unchecked(ptr.cast())) }
>> }
>>
>> + /// Get the command identifier for the request
>> + pub fn command(&self) -> u32 {
>> + // SAFETY: By C API contract and type invariant, `cmd_flags` is valid for read
>> + unsafe { (*self.0.get()).cmd_flags & ((1 << bindings::REQ_OP_BITS) - 1) }
>
> Nit: scope of unsafe
>
> unsafe { (*self.0.get()).cmd_flags } & ((1 << bindings::REQ_OP_BITS) - 1)
The `&` is parsed as reference operator with this change. But we can do
this:
use core::ops::BitAnd;
// SAFETY: By C API contract and type invariant, `cmd_flags` is valid for read
unsafe { (*self.0.get()).cmd_flags }.bitand((1u32 << bindings::REQ_OP_BITS) - 1)
Best regards,
Andreas Hindborg
^ permalink raw reply
* [PATCH v3] block: assign caller-specific lockdep class to disk->open_mutex
From: Tetsuo Handa @ 2026-06-03 11:54 UTC (permalink / raw)
To: Christoph Hellwig, Bart Van Assche, Jens Axboe
Cc: linux-block, LKML, Andrew Morton, Ming Lei, Damien Le Moal,
Qu Wenruo, Hillf Danton, Miguel Ojeda
In-Reply-To: <420f723a-8168-4f56-b84a-2a36ecd87fea@I-love.SAKURA.ne.jp>
The block core currently allocates a single monolithic lockdep key for
disk->open_mutex across all callers. This single key conflates locking
hierarchies between independent block streams. For example, if a stacked
driver like loop flushes its internal workqueues inside lo_release() while
holding its own open_mutex, lockdep views this as a potential ABBA deadlock
against the underlying storage stack, leading to numerous circular
dependency splats.
To structurally reduce false positives, this patch splits the global
monolithic lock class into distinct, per-caller instances during disk
allocation. This is done by replacing "struct lock_class_key" with
"struct gendisk_lkclass", which contains two instances of
"struct lock_class_key" for the legacy "(bio completion)" map and
disk->open_mutex respectively.
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
Changes in v3:
- Adjusted Rust part for safe pointer passing, pointed out by sashiko
( https://sashiko.dev/#/patchset/420f723a-8168-4f56-b84a-2a36ecd87fea%40I-love.SAKURA.ne.jp ) .
Changes in v2:
- Replaced a two-element array with a struct with two named members, suggested by Bart Van Assche
( https://lore.kernel.org/all/4cf7ecc7-932c-4589-9d0f-3e025e83e27c@acm.org/ ).
- Added changes needed by Rust block layer bindings and rnull module, pointed out by sashiko
( https://sashiko.dev/#/patchset/147ed056-03d9-4214-b925-0f10fc00cf27%40I-love.SAKURA.ne.jp ).
Testing result of v1:
- I kept v1 patch in linux-next for several more days, but result was that
some of circular dependency splats which I thought this change succeeded to
eliminate are still getting reported. That is, we need to determine whether
we should make this change without example syzbot reports that demonstrates
difference. But in general, keeping locking chains simpler and shorter
should be a good change.
Acknowledgment:
Since I have no experience with Rust, changes needed by Rust block layer
bindings and rnull module are made based on conversation with the Gemini
AI collaborator.
block/blk-mq.c | 4 ++--
block/blk.h | 2 +-
block/genhd.c | 8 +++----
drivers/block/rnull/rnull.rs | 2 +-
drivers/scsi/sd.c | 2 +-
drivers/scsi/sr.c | 2 +-
include/linux/blk-mq.h | 6 ++---
include/linux/blkdev.h | 9 +++++--
rust/kernel/block/mq.rs | 2 +-
rust/kernel/block/mq/gen_disk.rs | 41 ++++++++++++++++++++++++++++++--
10 files changed, 60 insertions(+), 18 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index a24175441380..5203e8cc6a28 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -4492,7 +4492,7 @@ EXPORT_SYMBOL(blk_mq_destroy_queue);
struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set,
struct queue_limits *lim, void *queuedata,
- struct lock_class_key *lkclass)
+ struct gendisk_lkclass *lkclass)
{
struct request_queue *q;
struct gendisk *disk;
@@ -4513,7 +4513,7 @@ struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set,
EXPORT_SYMBOL(__blk_mq_alloc_disk);
struct gendisk *blk_mq_alloc_disk_for_queue(struct request_queue *q,
- struct lock_class_key *lkclass)
+ struct gendisk_lkclass *lkclass)
{
struct gendisk *disk;
diff --git a/block/blk.h b/block/blk.h
index b998a7761faf..611bcd655357 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -614,7 +614,7 @@ void drop_partition(struct block_device *part);
void bdev_set_nr_sectors(struct block_device *bdev, sector_t sectors);
struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
- struct lock_class_key *lkclass);
+ struct gendisk_lkclass *lkclass);
struct request_queue *blk_alloc_queue(struct queue_limits *lim, int node_id);
int disk_scan_partitions(struct gendisk *disk, blk_mode_t mode);
diff --git a/block/genhd.c b/block/genhd.c
index 7d6854fd28e9..8f4a3d8ca15e 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1444,7 +1444,7 @@ dev_t part_devt(struct gendisk *disk, u8 partno)
}
struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
- struct lock_class_key *lkclass)
+ struct gendisk_lkclass *lkclass)
{
struct gendisk *disk;
@@ -1467,7 +1467,7 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
goto out_free_bdi;
disk->node_id = node_id;
- mutex_init(&disk->open_mutex);
+ mutex_init_with_key(&disk->open_mutex, &lkclass->open_mutex_lkclass);
xa_init(&disk->part_tbl);
if (xa_insert(&disk->part_tbl, 0, disk->part0, GFP_KERNEL))
goto out_destroy_part_tbl;
@@ -1482,7 +1482,7 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
device_initialize(disk_to_dev(disk));
inc_diskseq(disk);
q->disk = disk;
- lockdep_init_map(&disk->lockdep_map, "(bio completion)", lkclass, 0);
+ lockdep_init_map(&disk->lockdep_map, "(bio completion)", &lkclass->bio_lkclass, 0);
#ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
INIT_LIST_HEAD(&disk->slave_bdevs);
#endif
@@ -1506,7 +1506,7 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
}
struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
- struct lock_class_key *lkclass)
+ struct gendisk_lkclass *lkclass)
{
struct queue_limits default_lim = { };
struct request_queue *q;
diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index 0ca8715febe8..476a8910c432 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -61,7 +61,7 @@ fn new(
.logical_block_size(block_size)?
.physical_block_size(block_size)?
.rotational(rotational)
- .build(fmt!("{}", name.to_str()?), tagset, queue_data)
+ .build(fmt!("{}", name.to_str()?), tagset, queue_data, kernel::my_gendisk_lkclass!())
}
}
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 599e75f33334..63fe8c86606a 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -112,7 +112,7 @@ static DEFINE_MUTEX(sd_mutex_lock);
static mempool_t *sd_page_pool;
static mempool_t *sd_large_page_pool;
static atomic_t sd_large_page_pool_users = ATOMIC_INIT(0);
-static struct lock_class_key sd_bio_compl_lkclass;
+static struct gendisk_lkclass sd_bio_compl_lkclass;
static const char *sd_cache_types[] = {
"write through", "none", "write back",
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index c36c54ecd354..734567ae0e43 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -106,7 +106,7 @@ static struct scsi_driver sr_template = {
static unsigned long sr_index_bits[SR_DISKS / BITS_PER_LONG];
static DEFINE_SPINLOCK(sr_index_lock);
-static struct lock_class_key sr_bio_compl_lkclass;
+static struct gendisk_lkclass sr_bio_compl_lkclass;
static int sr_open(struct cdrom_device_info *, int);
static void sr_release(struct cdrom_device_info *);
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 18a2388ba581..5aa17e82c3ba 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -726,15 +726,15 @@ enum {
struct gendisk *__blk_mq_alloc_disk(struct blk_mq_tag_set *set,
struct queue_limits *lim, void *queuedata,
- struct lock_class_key *lkclass);
+ struct gendisk_lkclass *lkclass);
#define blk_mq_alloc_disk(set, lim, queuedata) \
({ \
- static struct lock_class_key __key; \
+ static struct gendisk_lkclass __key; \
\
__blk_mq_alloc_disk(set, lim, queuedata, &__key); \
})
struct gendisk *blk_mq_alloc_disk_for_queue(struct request_queue *q,
- struct lock_class_key *lkclass);
+ struct gendisk_lkclass *lkclass);
struct request_queue *blk_mq_alloc_queue(struct blk_mq_tag_set *set,
struct queue_limits *lim, void *queuedata);
int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 890128cdea1c..28b0aee6b3ba 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -49,6 +49,11 @@ extern const struct device_type disk_type;
extern const struct device_type part_type;
extern const struct class block_class;
+struct gendisk_lkclass {
+ struct lock_class_key bio_lkclass;
+ struct lock_class_key open_mutex_lkclass;
+};
+
/*
* Maximum number of blkcg policies allowed to be registered concurrently.
* Defined here to simplify include dependency.
@@ -974,7 +979,7 @@ int bdev_disk_changed(struct gendisk *disk, bool invalidate);
void put_disk(struct gendisk *disk);
struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
- struct lock_class_key *lkclass);
+ struct gendisk_lkclass *lkclass);
/**
* blk_alloc_disk - allocate a gendisk structure
@@ -990,7 +995,7 @@ struct gendisk *__blk_alloc_disk(struct queue_limits *lim, int node,
*/
#define blk_alloc_disk(lim, node_id) \
({ \
- static struct lock_class_key __key; \
+ static struct gendisk_lkclass __key; \
\
__blk_alloc_disk(lim, node_id, &__key); \
})
diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
index 1fd0d54dd549..10f22b200567 100644
--- a/rust/kernel/block/mq.rs
+++ b/rust/kernel/block/mq.rs
@@ -88,7 +88,7 @@
//! Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
//! let mut disk = gen_disk::GenDiskBuilder::new()
//! .capacity_sectors(4096)
-//! .build(fmt!("myblk"), tagset, ())?;
+//! .build(fmt!("myblk"), tagset, (), kernel::my_gendisk_lkclass!())?;
//!
//! # Ok::<(), kernel::error::Error>(())
//! ```
diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index 912cb805caf5..7e669ca5c032 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -11,7 +11,6 @@
error::{self, from_err_ptr, Result},
fmt::{self, Write},
prelude::*,
- static_lock_class,
str::NullTerminatedFormatter,
sync::Arc,
types::{ForeignOwnable, ScopeGuard},
@@ -38,6 +37,43 @@ fn default() -> Self {
}
}
+/// A wrapper type for safely passing "struct gendisk_lkclass" argument.
+///
+/// This type can only be instantiated via the [`my_gendisk_lkclass!`] macro.
+pub struct GenDiskLockClass(pub(crate) *mut bindings::gendisk_lkclass);
+
+impl GenDiskLockClass {
+ /// Retrieve the underlying raw pointer.
+ pub(crate) fn as_ptr(&self) -> *mut bindings::gendisk_lkclass {
+ self.0
+ }
+}
+
+#[doc(hidden)]
+pub mod __internal {
+ use super::*;
+
+ /// Internal constructor used ONLY by the `my_gendisk_lkclass!` macro.
+ ///
+ /// SAFETY: `ptr` must point to a valid static `gendisk_lkclass` instance.
+ pub const unsafe fn new_lock_class(ptr: *mut bindings::gendisk_lkclass) -> GenDiskLockClass {
+ GenDiskLockClass(ptr)
+ }
+}
+
+/// Helper macro to generate a unique caller-local static lock class struct
+#[macro_export]
+macro_rules! my_gendisk_lkclass {
+ () => {{
+ static mut LKCLASS: $crate::bindings::gendisk_lkclass = $crate::bindings::gendisk_lkclass {
+ bio_lkclass: const { unsafe { ::core::mem::zeroed() } },
+ open_mutex_lkclass: const { unsafe { ::core::mem::zeroed() } },
+ };
+
+ unsafe { $crate::block::mq::gen_disk::__internal::new_lock_class(&raw mut LKCLASS) }
+ }};
+}
+
impl GenDiskBuilder {
/// Create a new instance.
pub fn new() -> Self {
@@ -100,6 +136,7 @@ pub fn build<T: Operations>(
name: fmt::Arguments<'_>,
tagset: Arc<TagSet<T>>,
queue_data: T::QueueData,
+ lkclass: GenDiskLockClass,
) -> Result<GenDisk<T>> {
let data = queue_data.into_foreign();
let recover_data = ScopeGuard::new(|| {
@@ -121,7 +158,7 @@ pub fn build<T: Operations>(
tagset.raw_tag_set(),
&mut lim,
data,
- static_lock_class!().as_ptr(),
+ lkclass.as_ptr(),
)
})?;
--
2.54.0
^ permalink raw reply related
* Re: [PATCH 15/79] block: rnull: add `use_per_node_hctx` config option
From: Andreas Hindborg @ 2026-06-03 12:09 UTC (permalink / raw)
To: Miguel Ojeda
Cc: Alice Ryhl, Boqun Feng, Jens Axboe, Miguel Ojeda, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, FUJITA Tomonori, Frederic Weisbecker,
Lyude Paul, Thomas Gleixner, Anna-Maria Behnsen, John Stultz,
Stephen Boyd, Lorenzo Stoakes, Liam R. Howlett, linux-block,
rust-for-linux, linux-kernel, linux-mm
In-Reply-To: <CANiq72n5+JadJUW-GO0cwQrq9eJW7cB3ZK7zx4J484RzjxmYEA@mail.gmail.com>
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> writes:
> On Mon, Mar 16, 2026 at 2:58 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> This is intentional. The line gets too long if I pull it up, and I don't
>
> Do you mean you don't want to go over the limit or, otherwise, that
> something else complains?
>
> i.e. the limit is not a hard requirement -- exceptions can be made
> where reasonable (as long as `rustfmt` is clean), and moving the line
> to an odd position doesn't sound worth it.
Yes, checkpatch was complaining. I'll keep the long line then.
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [PATCH 14/79] block: rnull: add submit queue count config option
From: Andreas Hindborg @ 2026-06-03 12:00 UTC (permalink / raw)
To: Alice Ryhl
Cc: Boqun Feng, Jens Axboe, Miguel Ojeda, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, FUJITA Tomonori, Frederic Weisbecker,
Lyude Paul, Thomas Gleixner, Anna-Maria Behnsen, John Stultz,
Stephen Boyd, Lorenzo Stoakes, Liam R. Howlett, linux-block,
rust-for-linux, linux-kernel, linux-mm
In-Reply-To: <abfeIpz8HP_Pr5Yv@google.com>
Alice Ryhl <aliceryhl@google.com> writes:
> On Mon, Feb 16, 2026 at 12:35:01AM +0100, Andreas Hindborg wrote:
>> Allow user space to control the number of submission queues when creating
>> null block devices.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>> drivers/block/rnull/configfs.rs | 56 +++++++++++++++++++++++++++++++++--------
>> drivers/block/rnull/rnull.rs | 56 +++++++++++++++++++++++++++--------------
>> 2 files changed, 83 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
>> index b5dc30c5d3e20..fd3cbf7aa012e 100644
>> --- a/drivers/block/rnull/configfs.rs
>> +++ b/drivers/block/rnull/configfs.rs
>> @@ -59,7 +59,10 @@ 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,irqmode,completion_nsec,memory_backed\n")?;
>> + writer.write_str(
>> + "blocksize,size,rotational,irqmode,completion_nsec,memory_backed\
>> + submit_queues\n",
>> + )?;
>
> Missing comma? If so, this may indicate missing test coverage :)
Indeed :o
Best regards,
Andreas Hindborg
^ permalink raw reply
* Re: [PATCH 26/79] block: rnull: add badblocks support
From: Andreas Hindborg @ 2026-06-03 12:43 UTC (permalink / raw)
To: Alice Ryhl
Cc: Boqun Feng, Jens Axboe, Miguel Ojeda, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, FUJITA Tomonori, Frederic Weisbecker,
Lyude Paul, Thomas Gleixner, Anna-Maria Behnsen, John Stultz,
Stephen Boyd, Lorenzo Stoakes, Liam R. Howlett, linux-block,
rust-for-linux, linux-kernel, linux-mm
In-Reply-To: <abfxSTL2qTMtpnY7@google.com>
Alice Ryhl <aliceryhl@google.com> writes:
> On Mon, Feb 16, 2026 at 12:35:13AM +0100, Andreas Hindborg wrote:
>> Add badblocks support to the rnull driver with a configfs interface for
>> managing bad sectors.
>>
>> - Configfs attribute for adding/removing bad blocks via "+start-end" and
>> "-start-end" syntax.
>> - Request handling that checks for bad blocks and returns IO errors.
>> - Updated request completion to handle error status properly.
>>
>> The badblocks functionality is disabled by default and is enabled when
>> first bad block is added.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
>> + fn store(this: &DeviceConfig, page: &[u8]) -> Result {
>> + // This attribute can be set while device is powered.
>> +
>> + for line in core::str::from_utf8(page)?.lines() {
>> + let mut chars = line.chars();
>> + match chars.next() {
>> + Some(sign @ '+' | sign @ '-') => {
>> + if let Some((start, end)) = chars.as_str().split_once('-') {
>> + let start: u64 = start.parse().map_err(|_| EINVAL)?;
>> + let end: u64 = end.parse().map_err(|_| EINVAL)?;
>> +
>> + if start > end {
>> + return Err(EINVAL);
>> + }
>> +
>> + this.data.lock().bad_blocks.enable();
>> +
>> + if sign == '+' {
>> + this.data.lock().bad_blocks.set_bad(start..=end, true)?;
>> + } else {
>> + this.data.lock().bad_blocks.set_good(start..=end)?;
>
> Taking lock twice: TOCTOU.
I fixed all of these, thanks for reporting.
>
>> @@ -118,6 +125,7 @@ fn make_group(
>> home_node: bindings::NUMA_NO_NODE,
>> discard: false,
>> no_sched: false,
>> + bad_blocks: Arc::pin_init(BadBlocks::new(false), GFP_KERNEL)?,
>> }),
>> }),
>> core::iter::empty(),
>
> [..]
>
>> @@ -155,6 +160,7 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
>> home_node: *module_parameters::home_node.value(),
>> discard: *module_parameters::discard.value() != 0,
>> no_sched: *module_parameters::no_sched.value() != 0,
>> + bad_blocks: Arc::pin_init(BadBlocks::new(false), GFP_KERNEL)?,
>
> It seems weird to construct this Arc in two places. Is it shared or not?
In the case where the device is created via configfs, the `BadBlocks`
instance is shared. When the device is constructed via module
parameters, the `BadBlocks` instance is not shared. In this case it is
actually not used, as there is no way to enable the code path. But we
need to provide an instance anyway.
I did not want to use an Option, because I did not want the checks on
access.
Best regards,
Andreas Hindborg
^ permalink raw reply
* [PATCH 0/5] blk-cgroup: fix blkg list and policy data races
From: Yu Kuai @ 2026-06-03 13:27 UTC (permalink / raw)
To: Jens Axboe
Cc: Tejun Heo, Josef Bacik, Ming Lei, Nilay Shroff, Bart Van Assche,
linux-block, cgroups, linux-kernel
This small series fixes races between blkg destruction, q->blkg_list
iteration, and blkcg policy activation.
The first two patches serialize q->blkg_list walks in blkg_destroy_all()
and BFQ writeback weight-raising teardown with blkcg_mutex. The next two
patches close policy activation races with concurrent blkg destruction,
including skipping blkgs that are already dying. The final patch factors
the common policy data teardown loop.
This uses blkcg_mutex rather than extending queue_lock coverage because
the races are about blkg list visibility and policy-data lifetime, not
request-queue dispatch state. blkg_free_workfn() already uses
blkcg_mutex to serialize policy-data freeing with policy deactivation
and removes blkgs from q->blkg_list only after that teardown. Taking the
same mutex around the remaining q->blkg_list walkers gives one sleepable
serialization point for blkg lifetime, avoids adding more queue_lock
nesting, and prepares the follow-up conversion that removes queue_lock
from blkcg list protection entirely.
Yu Kuai (2):
blk-cgroup: protect q->blkg_list iteration in blkg_destroy_all() with
blkcg_mutex
bfq: protect q->blkg_list iteration in bfq_end_wr_async() with
blkcg_mutex
Zheng Qixing (3):
blk-cgroup: fix race between policy activation and blkg destruction
blk-cgroup: skip dying blkg in blkcg_activate_policy()
blk-cgroup: factor policy pd teardown loop into helper
block/bfq-cgroup.c | 3 ++-
block/bfq-iosched.c | 6 +++++
block/blk-cgroup.c | 65 ++++++++++++++++++++++++---------------------
3 files changed, 43 insertions(+), 31 deletions(-)
--
2.51.0
^ permalink raw reply
* [PATCH 1/5] blk-cgroup: protect q->blkg_list iteration in blkg_destroy_all() with blkcg_mutex
From: Yu Kuai @ 2026-06-03 13:27 UTC (permalink / raw)
To: Jens Axboe
Cc: Tejun Heo, Josef Bacik, Ming Lei, Nilay Shroff, Bart Van Assche,
linux-block, cgroups, linux-kernel
In-Reply-To: <cover.1780492756.git.yukuai@fygo.io>
blkg_destroy_all() iterates q->blkg_list without holding blkcg_mutex,
which can race with blkg_free_workfn() that removes blkgs from the list
while holding blkcg_mutex.
Add blkcg_mutex protection around the q->blkg_list iteration to prevent
potential list corruption or use-after-free issues.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 554c87bb4a86..a98a22e06fd1 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -573,10 +573,11 @@ static void blkg_destroy_all(struct gendisk *disk)
struct blkcg_gq *blkg;
int count = BLKG_DESTROY_BATCH_SIZE;
int i;
restart:
+ mutex_lock(&q->blkcg_mutex);
spin_lock_irq(&q->queue_lock);
list_for_each_entry(blkg, &q->blkg_list, q_node) {
struct blkcg *blkcg = blkg->blkcg;
if (hlist_unhashed(&blkg->blkcg_node))
@@ -591,10 +592,11 @@ static void blkg_destroy_all(struct gendisk *disk)
* it when a batch of blkgs are destroyed.
*/
if (!(--count)) {
count = BLKG_DESTROY_BATCH_SIZE;
spin_unlock_irq(&q->queue_lock);
+ mutex_unlock(&q->blkcg_mutex);
cond_resched();
goto restart;
}
}
@@ -610,10 +612,11 @@ static void blkg_destroy_all(struct gendisk *disk)
__clear_bit(pol->plid, q->blkcg_pols);
}
q->root_blkg = NULL;
spin_unlock_irq(&q->queue_lock);
+ mutex_unlock(&q->blkcg_mutex);
wake_up_var(&q->root_blkg);
}
static void blkg_iostat_set(struct blkg_iostat *dst, struct blkg_iostat *src)
--
2.51.0
^ permalink raw reply related
* [PATCH 2/5] bfq: protect q->blkg_list iteration in bfq_end_wr_async() with blkcg_mutex
From: Yu Kuai @ 2026-06-03 13:27 UTC (permalink / raw)
To: Jens Axboe
Cc: Tejun Heo, Josef Bacik, Ming Lei, Nilay Shroff, Bart Van Assche,
linux-block, cgroups, linux-kernel
In-Reply-To: <cover.1780492756.git.yukuai@fygo.io>
bfq_end_wr_async() iterates q->blkg_list while only holding bfqd->lock,
but not blkcg_mutex. This can race with blkg_free_workfn() that removes
blkgs from the list while holding blkcg_mutex.
Add blkcg_mutex protection in bfq_end_wr() before taking bfqd->lock to
ensure proper synchronization when iterating q->blkg_list.
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/bfq-cgroup.c | 3 ++-
block/bfq-iosched.c | 6 ++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c
index 37ab70930c8d..f765e767d36a 100644
--- a/block/bfq-cgroup.c
+++ b/block/bfq-cgroup.c
@@ -939,11 +939,12 @@ void bfq_end_wr_async(struct bfq_data *bfqd)
struct blkcg_gq *blkg;
list_for_each_entry(blkg, &bfqd->queue->blkg_list, q_node) {
struct bfq_group *bfqg = blkg_to_bfqg(blkg);
- bfq_end_wr_async_queues(bfqd, bfqg);
+ if (bfqg)
+ bfq_end_wr_async_queues(bfqd, bfqg);
}
bfq_end_wr_async_queues(bfqd, bfqd->root_group);
}
static int bfq_io_show_weight_legacy(struct seq_file *sf, void *v)
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 141c602d5e85..42ccfd0c6140 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -2643,10 +2643,13 @@ void bfq_end_wr_async_queues(struct bfq_data *bfqd,
static void bfq_end_wr(struct bfq_data *bfqd)
{
struct bfq_queue *bfqq;
int i;
+#ifdef CONFIG_BFQ_GROUP_IOSCHED
+ mutex_lock(&bfqd->queue->blkcg_mutex);
+#endif
spin_lock_irq(&bfqd->lock);
for (i = 0; i < bfqd->num_actuators; i++) {
list_for_each_entry(bfqq, &bfqd->active_list[i], bfqq_list)
bfq_bfqq_end_wr(bfqq);
@@ -2654,10 +2657,13 @@ static void bfq_end_wr(struct bfq_data *bfqd)
list_for_each_entry(bfqq, &bfqd->idle_list, bfqq_list)
bfq_bfqq_end_wr(bfqq);
bfq_end_wr_async(bfqd);
spin_unlock_irq(&bfqd->lock);
+#ifdef CONFIG_BFQ_GROUP_IOSCHED
+ mutex_unlock(&bfqd->queue->blkcg_mutex);
+#endif
}
static sector_t bfq_io_struct_pos(void *io_struct, bool request)
{
if (request)
--
2.51.0
^ permalink raw reply related
* [PATCH 3/5] blk-cgroup: fix race between policy activation and blkg destruction
From: Yu Kuai @ 2026-06-03 13:27 UTC (permalink / raw)
To: Jens Axboe
Cc: Tejun Heo, Josef Bacik, Ming Lei, Nilay Shroff, Bart Van Assche,
linux-block, cgroups, linux-kernel
In-Reply-To: <cover.1780492756.git.yukuai@fygo.io>
From: Zheng Qixing <zhengqixing@huawei.com>
When switching an IO scheduler on a block device, blkcg_activate_policy()
allocates blkg_policy_data (pd) for all blkgs attached to the queue.
However, blkcg_activate_policy() may race with concurrent blkcg deletion,
leading to use-after-free and memory leak issues.
The use-after-free occurs in the following race:
T1 (blkcg_activate_policy):
- Successfully allocates pd for blkg1 (loop0->queue, blkcgA)
- Fails to allocate pd for blkg2 (loop0->queue, blkcgB)
- Enters the enomem rollback path to release blkg1 resources
T2 (blkcg deletion):
- blkcgA is deleted concurrently
- blkg1 is freed via blkg_free_workfn()
- blkg1->pd is freed
T1 (continued):
- Rollback path accesses blkg1->pd->online after pd is freed
- Triggers use-after-free
In addition, blkg_free_workfn() frees pd before removing the blkg from
q->blkg_list. This allows blkcg_activate_policy() to allocate a new pd
for a blkg that is being destroyed, leaving the newly allocated pd
unreachable when the blkg is finally freed.
Fix these races by extending blkcg_mutex coverage to serialize
blkcg_activate_policy() rollback and blkg destruction, ensuring pd
lifecycle is synchronized with blkg list visibility.
Fixes: f1c006f1c685 ("blk-cgroup: synchronize pd_free_fn() from blkg_free_workfn() and blkcg_deactivate_policy()")
Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index a98a22e06fd1..007dfc4f9c59 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1612,10 +1612,12 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
if (WARN_ON_ONCE(!pol->pd_alloc_fn || !pol->pd_free_fn))
return -EINVAL;
if (queue_is_mq(q))
memflags = blk_mq_freeze_queue(q);
+
+ mutex_lock(&q->blkcg_mutex);
retry:
spin_lock_irq(&q->queue_lock);
/* blkg_list is pushed at the head, reverse walk to initialize parents first */
list_for_each_entry_reverse(blkg, &q->blkg_list, q_node) {
@@ -1674,10 +1676,11 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
__set_bit(pol->plid, q->blkcg_pols);
ret = 0;
spin_unlock_irq(&q->queue_lock);
out:
+ mutex_unlock(&q->blkcg_mutex);
if (queue_is_mq(q))
blk_mq_unfreeze_queue(q, memflags);
if (pinned_blkg)
blkg_put(pinned_blkg);
if (pd_prealloc)
--
2.51.0
^ permalink raw reply related
* [PATCH 4/5] blk-cgroup: skip dying blkg in blkcg_activate_policy()
From: Yu Kuai @ 2026-06-03 13:27 UTC (permalink / raw)
To: Jens Axboe
Cc: Tejun Heo, Josef Bacik, Ming Lei, Nilay Shroff, Bart Van Assche,
linux-block, cgroups, linux-kernel
In-Reply-To: <cover.1780492756.git.yukuai@fygo.io>
From: Zheng Qixing <zhengqixing@huawei.com>
When switching IO schedulers on a block device, blkcg_activate_policy()
can race with concurrent blkcg deletion, leading to a use-after-free in
rcu_accelerate_cbs.
T1: T2:
blkg_destroy
kill(&blkg->refcnt) // blkg->refcnt=1->0
blkg_release // call_rcu(__blkg_release)
...
blkg_free_workfn
->pd_free_fn(pd)
elv_iosched_store
elevator_switch
...
iterate blkg list
blkg_get(blkg) // blkg->refcnt=0->1
list_del_init(&blkg->q_node)
blkg_put(pinned_blkg) // blkg->refcnt=1->0
blkg_release // call_rcu again
rcu_accelerate_cbs // uaf
Fix this by checking hlist_unhashed(&blkg->blkcg_node) before getting
a reference to the blkg. This is the same check used in blkg_destroy()
to detect if a blkg has already been destroyed. If the blkg is already
unhashed, skip processing it since it's being destroyed.
Fixes: f1c006f1c685 ("blk-cgroup: synchronize pd_free_fn() from blkg_free_workfn() and blkcg_deactivate_policy()")
Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 007dfc4f9c59..b479a9796fb3 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1623,10 +1623,12 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
list_for_each_entry_reverse(blkg, &q->blkg_list, q_node) {
struct blkg_policy_data *pd;
if (blkg->pd[pol->plid])
continue;
+ if (hlist_unhashed(&blkg->blkcg_node))
+ continue;
/* If prealloc matches, use it; otherwise try GFP_NOWAIT */
if (blkg == pinned_blkg) {
pd = pd_prealloc;
pd_prealloc = NULL;
--
2.51.0
^ permalink raw reply related
* [PATCH 5/5] blk-cgroup: factor policy pd teardown loop into helper
From: Yu Kuai @ 2026-06-03 13:27 UTC (permalink / raw)
To: Jens Axboe
Cc: Tejun Heo, Josef Bacik, Ming Lei, Nilay Shroff, Bart Van Assche,
linux-block, cgroups, linux-kernel
In-Reply-To: <cover.1780492756.git.yukuai@fygo.io>
From: Zheng Qixing <zhengqixing@huawei.com>
Move the teardown sequence which offlines and frees per-policy
blkg_policy_data (pd) into a helper for readability.
No functional change intended.
Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Yu Kuai <yukuai@fygo.io>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
---
block/blk-cgroup.c | 57 ++++++++++++++++++++++------------------------
1 file changed, 27 insertions(+), 30 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index b479a9796fb3..c75b2a103bbc 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1575,10 +1575,35 @@ struct cgroup_subsys io_cgrp_subsys = {
.depends_on = 1 << memory_cgrp_id,
#endif
};
EXPORT_SYMBOL_GPL(io_cgrp_subsys);
+/*
+ * Tear down per-blkg policy data for @pol on @q.
+ */
+static void blkcg_policy_teardown_pds(struct request_queue *q,
+ const struct blkcg_policy *pol)
+{
+ struct blkcg_gq *blkg;
+
+ list_for_each_entry(blkg, &q->blkg_list, q_node) {
+ struct blkcg *blkcg = blkg->blkcg;
+ struct blkg_policy_data *pd;
+
+ spin_lock(&blkcg->lock);
+ pd = blkg->pd[pol->plid];
+ if (pd) {
+ if (pd->online && pol->pd_offline_fn)
+ pol->pd_offline_fn(pd);
+ pd->online = false;
+ pol->pd_free_fn(pd);
+ blkg->pd[pol->plid] = NULL;
+ }
+ spin_unlock(&blkcg->lock);
+ }
+}
+
/**
* blkcg_activate_policy - activate a blkcg policy on a gendisk
* @disk: gendisk of interest
* @pol: blkcg policy to activate
*
@@ -1690,25 +1715,11 @@ int blkcg_activate_policy(struct gendisk *disk, const struct blkcg_policy *pol)
return ret;
enomem:
/* alloc failed, take down everything */
spin_lock_irq(&q->queue_lock);
- list_for_each_entry(blkg, &q->blkg_list, q_node) {
- struct blkcg *blkcg = blkg->blkcg;
- struct blkg_policy_data *pd;
-
- spin_lock(&blkcg->lock);
- pd = blkg->pd[pol->plid];
- if (pd) {
- if (pd->online && pol->pd_offline_fn)
- pol->pd_offline_fn(pd);
- pd->online = false;
- pol->pd_free_fn(pd);
- blkg->pd[pol->plid] = NULL;
- }
- spin_unlock(&blkcg->lock);
- }
+ blkcg_policy_teardown_pds(q, pol);
spin_unlock_irq(&q->queue_lock);
ret = -ENOMEM;
goto out;
}
EXPORT_SYMBOL_GPL(blkcg_activate_policy);
@@ -1723,11 +1734,10 @@ EXPORT_SYMBOL_GPL(blkcg_activate_policy);
*/
void blkcg_deactivate_policy(struct gendisk *disk,
const struct blkcg_policy *pol)
{
struct request_queue *q = disk->queue;
- struct blkcg_gq *blkg;
unsigned int memflags;
if (!blkcg_policy_enabled(q, pol))
return;
@@ -1736,24 +1746,11 @@ void blkcg_deactivate_policy(struct gendisk *disk,
mutex_lock(&q->blkcg_mutex);
spin_lock_irq(&q->queue_lock);
__clear_bit(pol->plid, q->blkcg_pols);
-
- list_for_each_entry(blkg, &q->blkg_list, q_node) {
- struct blkcg *blkcg = blkg->blkcg;
-
- spin_lock(&blkcg->lock);
- if (blkg->pd[pol->plid]) {
- if (blkg->pd[pol->plid]->online && pol->pd_offline_fn)
- pol->pd_offline_fn(blkg->pd[pol->plid]);
- pol->pd_free_fn(blkg->pd[pol->plid]);
- blkg->pd[pol->plid] = NULL;
- }
- spin_unlock(&blkcg->lock);
- }
-
+ blkcg_policy_teardown_pds(q, pol);
spin_unlock_irq(&q->queue_lock);
mutex_unlock(&q->blkcg_mutex);
if (queue_is_mq(q))
blk_mq_unfreeze_queue(q, memflags);
--
2.51.0
^ permalink raw reply related
* Re: [PATCH RFC 7/8] erofs: open via dedicated fs bdev helpers
From: Christian Brauner @ 2026-06-03 13:42 UTC (permalink / raw)
To: Gao Xiang
Cc: Jens Axboe, Alexander Viro, linux-block, linux-kernel,
linux-fsdevel, Carlos Maiolino, linux-xfs, Chris Mason,
David Sterba, linux-btrfs, Theodore Ts'o, linux-ext4,
Gao Xiang, linux-erofs, Christoph Hellwig, Jan Kara
In-Reply-To: <7c5bfcf0-36a3-4cc6-bf31-6af4fc901c37@linux.alibaba.com>
> May I ask if it's an urgent 7.2 work? If not, I could
No no, it's way too late for that this cycle.
> make a preparation patch for the upcoming 7.2 cycle
> to handle erofs_map_dev() failure here so you don't
> need to bother with this in this patchset.
Sounds good. I take it you can just do this yourself without me.
> I will seek more time to resolve the recent todos
Thanks!
> yet always intercepted by other unrelated stuffs.
:)
^ permalink raw reply
* [PATCH v2] rust: add procedural macro for declaring configfs attributes
From: Malte Wechter @ 2026-06-03 14:08 UTC (permalink / raw)
To: Andreas Hindborg, Breno Leitao, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jens Axboe
Cc: linux-kernel, rust-for-linux, linux-block, Malte Wechter
Implement `configfs_attrs!` as a procedural macro using `syn`, this
improves readability and maintainability. Remove the old macro and
replace all uses with the new macro. Add the new macro implementation
file to MAINTAINERS.
Signed-off-by: Malte Wechter <maltewechter@gmail.com>
---
Changes in v2:
- Add a try_parse helper function to macros/helpers.rs
- Fix bug where 'child' configuration gets dropped if trailing comma is missing (sashiko)
- Link to v1: https://lore.kernel.org/r/20260520-configfs-syn-v1-1-6c5b80a9cef2@gmail.com
---
MAINTAINERS | 1 +
drivers/block/rnull/configfs.rs | 2 +-
rust/kernel/configfs.rs | 251 ----------------------------------------
rust/macros/configfs_attrs.rs | 182 +++++++++++++++++++++++++++++
rust/macros/helpers.rs | 15 +++
rust/macros/lib.rs | 85 ++++++++++++++
samples/rust/rust_configfs.rs | 2 +-
7 files changed, 285 insertions(+), 253 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 2fb1c75afd163..45f7a1ec93b45 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6464,6 +6464,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/a.hindborg/linux.git config
F: fs/configfs/
F: include/linux/configfs.h
F: rust/kernel/configfs.rs
+F: rust/macros/configfs_attrs.rs
F: samples/configfs/
F: samples/rust/rust_configfs.rs
diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
index 7c2eb5c0b7228..f28ec69d79846 100644
--- a/drivers/block/rnull/configfs.rs
+++ b/drivers/block/rnull/configfs.rs
@@ -4,8 +4,8 @@
use kernel::{
block::mq::gen_disk::{GenDisk, GenDiskBuilder},
configfs::{self, AttributeOperations},
- configfs_attrs,
fmt::{self, Write as _},
+ macros::configfs_attrs,
new_mutex,
page::PAGE_SIZE,
prelude::*,
diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index 2339c6467325d..7a91e36677f52 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -791,254 +791,3 @@ fn as_ptr(&self) -> *const bindings::config_item_type {
self.item_type.get()
}
}
-
-/// Define a list of configfs attributes statically.
-///
-/// Invoking the macro in the following manner:
-///
-/// ```ignore
-/// let item_type = configfs_attrs! {
-/// container: configfs::Subsystem<Configuration>,
-/// data: Configuration,
-/// child: Child,
-/// attributes: [
-/// message: 0,
-/// bar: 1,
-/// ],
-/// };
-/// ```
-///
-/// Expands the following output:
-///
-/// ```ignore
-/// let item_type = {
-/// static CONFIGURATION_MESSAGE_ATTR: kernel::configfs::Attribute<
-/// 0,
-/// Configuration,
-/// Configuration,
-/// > = unsafe {
-/// kernel::configfs::Attribute::new({
-/// const S: &str = "message\u{0}";
-/// const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
-/// S.as_bytes()
-/// ) {
-/// Ok(v) => v,
-/// Err(_) => {
-/// core::panicking::panic_fmt(core::const_format_args!(
-/// "string contains interior NUL"
-/// ));
-/// }
-/// };
-/// C
-/// })
-/// };
-///
-/// static CONFIGURATION_BAR_ATTR: kernel::configfs::Attribute<
-/// 1,
-/// Configuration,
-/// Configuration
-/// > = unsafe {
-/// kernel::configfs::Attribute::new({
-/// const S: &str = "bar\u{0}";
-/// const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
-/// S.as_bytes()
-/// ) {
-/// Ok(v) => v,
-/// Err(_) => {
-/// core::panicking::panic_fmt(core::const_format_args!(
-/// "string contains interior NUL"
-/// ));
-/// }
-/// };
-/// C
-/// })
-/// };
-///
-/// const N: usize = (1usize + (1usize + 0usize)) + 1usize;
-///
-/// static CONFIGURATION_ATTRS: kernel::configfs::AttributeList<N, Configuration> =
-/// unsafe { kernel::configfs::AttributeList::new() };
-///
-/// {
-/// const N: usize = 0usize;
-/// unsafe { CONFIGURATION_ATTRS.add::<N, 0, _>(&CONFIGURATION_MESSAGE_ATTR) };
-/// }
-///
-/// {
-/// const N: usize = (1usize + 0usize);
-/// unsafe { CONFIGURATION_ATTRS.add::<N, 1, _>(&CONFIGURATION_BAR_ATTR) };
-/// }
-///
-/// static CONFIGURATION_TPE:
-/// kernel::configfs::ItemType<configfs::Subsystem<Configuration> ,Configuration>
-/// = kernel::configfs::ItemType::<
-/// configfs::Subsystem<Configuration>,
-/// Configuration
-/// >::new_with_child_ctor::<N,Child>(
-/// &THIS_MODULE,
-/// &CONFIGURATION_ATTRS
-/// );
-///
-/// &CONFIGURATION_TPE
-/// }
-/// ```
-#[macro_export]
-macro_rules! configfs_attrs {
- (
- container: $container:ty,
- data: $data:ty,
- attributes: [
- $($name:ident: $attr:literal),* $(,)?
- ] $(,)?
- ) => {
- $crate::configfs_attrs!(
- count:
- @container($container),
- @data($data),
- @child(),
- @no_child(x),
- @attrs($($name $attr)*),
- @eat($($name $attr,)*),
- @assign(),
- @cnt(0usize),
- )
- };
- (
- container: $container:ty,
- data: $data:ty,
- child: $child:ty,
- attributes: [
- $($name:ident: $attr:literal),* $(,)?
- ] $(,)?
- ) => {
- $crate::configfs_attrs!(
- count:
- @container($container),
- @data($data),
- @child($child),
- @no_child(),
- @attrs($($name $attr)*),
- @eat($($name $attr,)*),
- @assign(),
- @cnt(0usize),
- )
- };
- (count:
- @container($container:ty),
- @data($data:ty),
- @child($($child:ty)?),
- @no_child($($no_child:ident)?),
- @attrs($($aname:ident $aattr:literal)*),
- @eat($name:ident $attr:literal, $($rname:ident $rattr:literal,)*),
- @assign($($assign:block)*),
- @cnt($cnt:expr),
- ) => {
- $crate::configfs_attrs!(
- count:
- @container($container),
- @data($data),
- @child($($child)?),
- @no_child($($no_child)?),
- @attrs($($aname $aattr)*),
- @eat($($rname $rattr,)*),
- @assign($($assign)* {
- const N: usize = $cnt;
- // The following macro text expands to a call to `Attribute::add`.
-
- // SAFETY: By design of this macro, the name of the variable we
- // invoke the `add` method on below, is not visible outside of
- // the macro expansion. The macro does not operate concurrently
- // on this variable, and thus we have exclusive access to the
- // variable.
- unsafe {
- $crate::macros::paste!(
- [< $data:upper _ATTRS >]
- .add::<N, $attr, _>(&[< $data:upper _ $name:upper _ATTR >])
- )
- };
- }),
- @cnt(1usize + $cnt),
- )
- };
- (count:
- @container($container:ty),
- @data($data:ty),
- @child($($child:ty)?),
- @no_child($($no_child:ident)?),
- @attrs($($aname:ident $aattr:literal)*),
- @eat(),
- @assign($($assign:block)*),
- @cnt($cnt:expr),
- ) =>
- {
- $crate::configfs_attrs!(
- final:
- @container($container),
- @data($data),
- @child($($child)?),
- @no_child($($no_child)?),
- @attrs($($aname $aattr)*),
- @assign($($assign)*),
- @cnt($cnt),
- )
- };
- (final:
- @container($container:ty),
- @data($data:ty),
- @child($($child:ty)?),
- @no_child($($no_child:ident)?),
- @attrs($($name:ident $attr:literal)*),
- @assign($($assign:block)*),
- @cnt($cnt:expr),
- ) =>
- {
- $crate::macros::paste!{
- {
- $(
- // SAFETY: We are expanding `configfs_attrs`.
- static [< $data:upper _ $name:upper _ATTR >]:
- $crate::configfs::Attribute<$attr, $data, $data> =
- unsafe {
- $crate::configfs::Attribute::new(
- $crate::c_str!(::core::stringify!($name)),
- )
- };
- )*
-
-
- // We need space for a null terminator.
- const N: usize = $cnt + 1usize;
-
- // SAFETY: We are expanding `configfs_attrs`.
- static [< $data:upper _ATTRS >]:
- $crate::configfs::AttributeList<N, $data> =
- unsafe { $crate::configfs::AttributeList::new() };
-
- $($assign)*
-
- $(
- const [<$no_child:upper>]: bool = true;
-
- static [< $data:upper _TPE >] : $crate::configfs::ItemType<$container, $data> =
- $crate::configfs::ItemType::<$container, $data>::new::<N>(
- &THIS_MODULE, &[<$ data:upper _ATTRS >]
- );
- )?
-
- $(
- static [< $data:upper _TPE >]:
- $crate::configfs::ItemType<$container, $data> =
- $crate::configfs::ItemType::<$container, $data>::
- new_with_child_ctor::<N, $child>(
- &THIS_MODULE, &[<$ data:upper _ATTRS >]
- );
- )?
-
- & [< $data:upper _TPE >]
- }
- }
- };
-
-}
-
-pub use crate::configfs_attrs;
diff --git a/rust/macros/configfs_attrs.rs b/rust/macros/configfs_attrs.rs
new file mode 100644
index 0000000000000..a7fc75cdebcc0
--- /dev/null
+++ b/rust/macros/configfs_attrs.rs
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use quote::{
+ quote, //
+ ToTokens,
+};
+
+use proc_macro2::{
+ Span, //
+};
+
+use syn::{
+ bracketed,
+ parse::{
+ Parse,
+ ParseStream, //
+ },
+ punctuated::Punctuated,
+ spanned::Spanned,
+ Ident,
+ LitInt,
+ Token,
+ Type, //
+};
+
+use crate::helpers::try_parse;
+
+pub(crate) struct ConfigfsAttrs {
+ container: Type,
+ data: Type,
+ child: Option<Type>,
+ attributes: Vec<(Ident, LitInt)>,
+}
+
+fn parse_attribute_field(stream: ParseStream<'_>) -> syn::Result<(Ident, LitInt)> {
+ let id = stream.parse::<syn::Ident>()?;
+ let _colon = stream.parse::<Token![:]>()?;
+ let v = stream.parse::<LitInt>()?;
+ Ok((id, v))
+}
+
+fn parse_named_field(stream: ParseStream<'_>, name: &str) -> syn::Result<Type> {
+ let name_id = stream.parse::<Ident>()?;
+ if name_id != name {
+ return Err(parse_field_error(name_id.span(), &name_id, name));
+ }
+ let _colon = stream.parse::<Token![:]>()?;
+ let v = stream.parse::<Type>()?;
+ stream.parse::<Token![,]>()?;
+ Ok(v)
+}
+
+fn parse_attributes(stream: ParseStream<'_>) -> syn::Result<Vec<(Ident, LitInt)>> {
+ let attribute_id = stream.parse::<Ident>()?;
+ if attribute_id != "attributes" {
+ return Err(syn::Error::new(
+ attribute_id.span(),
+ format!(
+ "unexpected identifier: {}, expected: 'attributes'",
+ attribute_id
+ ),
+ ));
+ }
+ stream.parse::<Token![:]>()?;
+ let attr_stream;
+ let _bracket = bracketed!(attr_stream in stream);
+ let attributes = Punctuated::<(Ident, LitInt), Token![,]>::parse_terminated_with(
+ &attr_stream,
+ parse_attribute_field,
+ )?;
+ let attributes = attributes.into_iter().collect::<Vec<_>>();
+
+ stream.parse::<Option<Token![,]>>()?;
+ Ok(attributes)
+}
+
+fn parse_field_error(span: Span, name: &Ident, expected_name: &str) -> syn::Error {
+ syn::Error::new(
+ span,
+ format!("Unexpected field name, got: {name} expected: {expected_name}"),
+ )
+}
+
+impl Parse for ConfigfsAttrs {
+ fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
+ let container = try_parse(input, |s| parse_named_field(s, "container"))?;
+ let data = try_parse(input, |s| parse_named_field(s, "data"))?;
+ let child = try_parse(input, |s| parse_named_field(s, "child")).ok();
+ let attributes = parse_attributes(input)?;
+
+ Ok(ConfigfsAttrs {
+ container,
+ data,
+ child,
+ attributes,
+ })
+ }
+}
+
+fn make_static_ident<T: ToTokens>(ty: &T, suffix: &str) -> syn::Ident {
+ let raw_id = quote! { #ty }.to_string();
+
+ // Sanitizing syn::Type::Path, this is safe since it is
+ // only used as the identifier.
+ let normalized = raw_id
+ .split("::")
+ .map(|s| String::from(s.trim()))
+ .reduce(|a, b| format!("{a}_{b}"))
+ .expect("Cannot be empty")
+ .to_uppercase()
+ .replace(|c: char| !c.is_alphanumeric(), "_");
+
+ Ident::new(&format!("{}_{}", normalized, suffix), ty.span())
+}
+
+pub(crate) fn configfs_attrs(cfs_attrs: ConfigfsAttrs) -> proc_macro2::TokenStream {
+ let (container_ty, data_ty) = (&cfs_attrs.container, &cfs_attrs.data);
+
+ let data_tp_ident = make_static_ident(data_ty, "TPE");
+ let data_attr_ident = make_static_ident(data_ty, "ATTR_LIST");
+
+ let n = cfs_attrs.attributes.len() + 1;
+
+ let attr_list = quote! {
+ static #data_attr_ident: kernel::configfs::AttributeList<#n, #data_ty> =
+ // SAFETY: We are expanding `configfs_attrs`.
+ unsafe { kernel::configfs::AttributeList::new() };
+ };
+
+ let mut attrs = Vec::new();
+ for (attr_idx, (name, id)) in cfs_attrs.attributes.iter().enumerate() {
+ let name_with_attr = make_static_ident(name, "ATTR");
+
+ let id: u64 = match id.base10_parse::<u64>() {
+ Ok(v) => v,
+ Err(_) => {
+ return syn::Error::new(id.span(), "Could not parse attribute ID as a u64")
+ .to_compile_error();
+ }
+ };
+
+ attrs.push(quote! {
+ static #name_with_attr: kernel::configfs::Attribute<#id, #data_ty, #data_ty> =
+ // SAFETY: We are expanding `configfs_attrs`.
+ unsafe {
+ kernel::configfs::Attribute::new(kernel::c_str!(::core::stringify!(#name)))
+ };
+
+ // SAFETY: By design of this macro, the name of the variable we
+ // invoke the `add` method on below, is not visible outside of
+ // the macro expansion. The macro does not operate concurrently
+ // on this variable, and thus we have exclusive access to the
+ // variable.
+ unsafe { #data_attr_ident.add::<#attr_idx, #id, _>(&#name_with_attr) }
+ });
+ }
+
+ let has_child_code = if let Some(child) = cfs_attrs.child {
+ quote! { new_with_child_ctor::<#n, #child>}
+ } else {
+ quote! { new::<#n> }
+ };
+
+ let data_type = quote! {
+ {
+ static #data_tp_ident:
+ kernel::configfs::ItemType<#container_ty, #data_ty> =
+ kernel::configfs::ItemType::<#container_ty, #data_ty>::#has_child_code(
+ &THIS_MODULE, &#data_attr_ident
+ );
+ &#data_tp_ident
+ }
+ };
+
+ quote! {
+ {
+ #attr_list
+ #(#attrs)*
+ #data_type
+ }
+ }
+}
diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
index d18fbf4daa0a5..fdab8804e1ba9 100644
--- a/rust/macros/helpers.rs
+++ b/rust/macros/helpers.rs
@@ -4,6 +4,7 @@
use quote::ToTokens;
use syn::{
parse::{
+ discouraged::Speculative,
Parse,
ParseStream, //
},
@@ -54,6 +55,20 @@ pub(crate) fn file() -> String {
}
}
+pub(crate) fn try_parse<T>(
+ input: ParseStream<'_>,
+ parser: impl FnOnce(ParseStream<'_>) -> Result<T>,
+) -> Result<T> {
+ let fork = input.fork();
+ match parser(&fork) {
+ Ok(value) => {
+ input.advance_to(&fork);
+ Ok(value)
+ }
+ Err(e) => Err(e),
+ }
+}
+
/// Obtain all `#[cfg]` attributes.
pub(crate) fn gather_cfg_attrs(attr: &[Attribute]) -> impl Iterator<Item = &Attribute> + '_ {
attr.iter().filter(|a| a.path().is_ident("cfg"))
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 2cfd59e0f9e7c..745ba83c828b9 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -15,6 +15,8 @@
#![cfg_attr(not(CONFIG_RUSTC_HAS_SPAN_FILE), feature(proc_macro_span))]
mod concat_idents;
+#[cfg(CONFIG_CONFIGFS_FS)]
+mod configfs_attrs;
mod export;
mod fmt;
mod helpers;
@@ -489,3 +491,86 @@ pub fn kunit_tests(attr: TokenStream, input: TokenStream) -> TokenStream {
.unwrap_or_else(|e| e.into_compile_error())
.into()
}
+
+/// Define a list of configfs attributes statically.
+///
+/// # Examples
+///
+/// ```ignore
+/// let item_type = configfs_attrs! {
+/// container: configfs::Subsystem<Configuration>,
+/// data: Configuration,
+/// child: Child,
+/// attributes: [
+/// message: 0,
+/// bar: 1,
+/// ],
+/// };
+///```
+///
+/// Expands the following output:
+/// let item_type = {
+/// static CONFIGURATION_ATTR_LIST: kernel::configfs::AttributeList<
+/// 3usize,
+/// Configuration,
+/// > = unsafe { kernel::configfs::AttributeList::new() };
+/// static MESSAGE_ATTR: kernel::configfs::Attribute<
+/// 0u64,
+/// Configuration,
+/// Configuration,
+/// > = unsafe {
+/// kernel::configfs::Attribute::new({
+/// const S: &str = "message\u{0}";
+/// const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
+/// S.as_bytes(),
+/// ) {
+/// Ok(v) => v,
+/// Err(_) => {
+/// ::core::panicking::panic_fmt(
+/// format_args!("string contains interior NUL"),
+/// );
+/// }
+/// };
+/// C
+/// })
+/// };
+/// unsafe { CONFIGURATION_ATTR_LIST.add::<0usize, 0u64, _>(&MESSAGE_ATTR) }
+/// static BAR_ATTR: kernel::configfs::Attribute<
+/// 1u64,
+/// Configuration,
+/// Configuration,
+/// > = unsafe {
+/// kernel::configfs::Attribute::new({
+/// const S: &str = "bar\u{0}";
+/// const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
+/// S.as_bytes(),
+/// ) {
+/// Ok(v) => v,
+/// Err(_) => {
+/// ::core::panicking::panic_fmt(
+/// format_args!("string contains interior NUL"),
+/// );
+/// }
+/// };
+/// C
+/// })
+/// };
+/// unsafe { CONFIGURATION_ATTR_LIST.add::<1usize, 1u64, _>(&BAR_ATTR) }
+/// {
+/// static CONFIGURATION_TPE: kernel::configfs::ItemType<
+/// Subsystem<Configuration>,
+/// Configuration,
+/// > = kernel::configfs::ItemType::<
+/// Subsystem<Configuration>,
+/// Configuration,
+/// >::new_with_child_ctor::<3usize, Child>(&THIS_MODULE, &CONFIGURATION_ATTR_LIST);
+/// &CONFIGURATION_TPE
+/// }
+/// };
+///
+#[cfg(CONFIG_CONFIGFS_FS)]
+#[proc_macro]
+pub fn configfs_attrs(input: TokenStream) -> TokenStream {
+ configfs_attrs::configfs_attrs(parse_macro_input!(input as configfs_attrs::ConfigfsAttrs))
+ .into()
+}
diff --git a/samples/rust/rust_configfs.rs b/samples/rust/rust_configfs.rs
index a1bd9db6010da..876462f7789d1 100644
--- a/samples/rust/rust_configfs.rs
+++ b/samples/rust/rust_configfs.rs
@@ -4,7 +4,7 @@
use kernel::alloc::flags;
use kernel::configfs;
-use kernel::configfs::configfs_attrs;
+use kernel::macros::configfs_attrs;
use kernel::new_mutex;
use kernel::page::PAGE_SIZE;
use kernel::prelude::*;
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260417-configfs-syn-191e07130027
Best regards,
--
Malte Wechter <maltewechter@gmail.com>
^ permalink raw reply related
* Re: [PATCH 10/79] block: rust: add `command` getter to `Request`
From: Gary Guo @ 2026-06-03 14:21 UTC (permalink / raw)
To: Andreas Hindborg, Alice Ryhl
Cc: Boqun Feng, Jens Axboe, Miguel Ojeda, Gary Guo,
Björn Roy Baron, Benno Lossin, Trevor Gross,
Danilo Krummrich, FUJITA Tomonori, Frederic Weisbecker,
Lyude Paul, Thomas Gleixner, Anna-Maria Behnsen, John Stultz,
Stephen Boyd, Lorenzo Stoakes, Liam R. Howlett, linux-block,
rust-for-linux, linux-kernel, linux-mm
In-Reply-To: <87mrxbkfme.fsf@t14s.mail-host-address-is-not-set>
On Wed Jun 3, 2026 at 12:50 PM BST, Andreas Hindborg wrote:
> Alice Ryhl <aliceryhl@google.com> writes:
>
>> On Mon, Feb 16, 2026 at 12:34:57AM +0100, Andreas Hindborg wrote:
>>> Add a method to extract the command operation code from a request. The
>>> command is obtained by masking the lower bits of `cmd_flags` as defined by
>>> `REQ_OP_BITS`. This allows Rust block drivers to determine the type of
>>> operation being requested.
>>>
>>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>
>> With the nit below fixed:
>> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
>>
>>> rust/kernel/block/mq/request.rs | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
>>> index b49197a0c66d7..0dd329ae93dfc 100644
>>> --- a/rust/kernel/block/mq/request.rs
>>> +++ b/rust/kernel/block/mq/request.rs
>>> @@ -78,6 +78,12 @@ pub(crate) unsafe fn aref_from_raw(ptr: *mut bindings::request) -> ARef<Self> {
>>> unsafe { ARef::from_raw(NonNull::new_unchecked(ptr.cast())) }
>>> }
>>>
>>> + /// Get the command identifier for the request
>>> + pub fn command(&self) -> u32 {
>>> + // SAFETY: By C API contract and type invariant, `cmd_flags` is valid for read
>>> + unsafe { (*self.0.get()).cmd_flags & ((1 << bindings::REQ_OP_BITS) - 1) }
>>
>> Nit: scope of unsafe
>>
>> unsafe { (*self.0.get()).cmd_flags } & ((1 << bindings::REQ_OP_BITS) - 1)
>
> The `&` is parsed as reference operator with this change. But we can do
> this:
>
> use core::ops::BitAnd;
> // SAFETY: By C API contract and type invariant, `cmd_flags` is valid for read
> unsafe { (*self.0.get()).cmd_flags }.bitand((1u32 << bindings::REQ_OP_BITS) - 1)
Wrapping the unsafe block in a parenthesis should do the trick.
Best,
Gary
^ permalink raw reply
* Re: [PATCH v2] rust: add procedural macro for declaring configfs attributes
From: Gary Guo @ 2026-06-03 14:32 UTC (permalink / raw)
To: Malte Wechter, Andreas Hindborg, Breno Leitao, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Jens Axboe
Cc: linux-kernel, rust-for-linux, linux-block
In-Reply-To: <20260603-configfs-syn-v2-1-cb58489c2647@gmail.com>
On Wed Jun 3, 2026 at 3:08 PM BST, Malte Wechter wrote:
> Implement `configfs_attrs!` as a procedural macro using `syn`, this
> improves readability and maintainability. Remove the old macro and
> replace all uses with the new macro. Add the new macro implementation
> file to MAINTAINERS.
>
> Signed-off-by: Malte Wechter <maltewechter@gmail.com>
> ---
> Changes in v2:
> - Add a try_parse helper function to macros/helpers.rs
> - Fix bug where 'child' configuration gets dropped if trailing comma is missing (sashiko)
> - Link to v1: https://lore.kernel.org/r/20260520-configfs-syn-v1-1-6c5b80a9cef2@gmail.com
> ---
> MAINTAINERS | 1 +
> drivers/block/rnull/configfs.rs | 2 +-
> rust/kernel/configfs.rs | 251 ----------------------------------------
> rust/macros/configfs_attrs.rs | 182 +++++++++++++++++++++++++++++
> rust/macros/helpers.rs | 15 +++
> rust/macros/lib.rs | 85 ++++++++++++++
> samples/rust/rust_configfs.rs | 2 +-
> 7 files changed, 285 insertions(+), 253 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2fb1c75afd163..45f7a1ec93b45 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6464,6 +6464,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/a.hindborg/linux.git config
> F: fs/configfs/
> F: include/linux/configfs.h
> F: rust/kernel/configfs.rs
> +F: rust/macros/configfs_attrs.rs
> F: samples/configfs/
> F: samples/rust/rust_configfs.rs
>
> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
> index 7c2eb5c0b7228..f28ec69d79846 100644
> --- a/drivers/block/rnull/configfs.rs
> +++ b/drivers/block/rnull/configfs.rs
> @@ -4,8 +4,8 @@
> use kernel::{
> block::mq::gen_disk::{GenDisk, GenDiskBuilder},
> configfs::{self, AttributeOperations},
> - configfs_attrs,
> fmt::{self, Write as _},
> + macros::configfs_attrs,
> new_mutex,
> page::PAGE_SIZE,
> prelude::*,
> diff --git a/rust/macros/configfs_attrs.rs b/rust/macros/configfs_attrs.rs
> new file mode 100644
> index 0000000000000..a7fc75cdebcc0
> --- /dev/null
> +++ b/rust/macros/configfs_attrs.rs
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use quote::{
> + quote, //
> + ToTokens,
> +};
> +
> +use proc_macro2::{
> + Span, //
> +};
> +
> +use syn::{
> + bracketed,
> + parse::{
> + Parse,
> + ParseStream, //
> + },
> + punctuated::Punctuated,
> + spanned::Spanned,
> + Ident,
> + LitInt,
> + Token,
> + Type, //
> +};
> +
> +use crate::helpers::try_parse;
> +
> +pub(crate) struct ConfigfsAttrs {
> + container: Type,
> + data: Type,
> + child: Option<Type>,
> + attributes: Vec<(Ident, LitInt)>,
> +}
> +
> +fn parse_attribute_field(stream: ParseStream<'_>) -> syn::Result<(Ident, LitInt)> {
> + let id = stream.parse::<syn::Ident>()?;
> + let _colon = stream.parse::<Token![:]>()?;
> + let v = stream.parse::<LitInt>()?;
> + Ok((id, v))
> +}
> +
> +fn parse_named_field(stream: ParseStream<'_>, name: &str) -> syn::Result<Type> {
> + let name_id = stream.parse::<Ident>()?;
> + if name_id != name {
> + return Err(parse_field_error(name_id.span(), &name_id, name));
> + }
> + let _colon = stream.parse::<Token![:]>()?;
> + let v = stream.parse::<Type>()?;
> + stream.parse::<Token![,]>()?;
> + Ok(v)
> +}
> +
> +fn parse_attributes(stream: ParseStream<'_>) -> syn::Result<Vec<(Ident, LitInt)>> {
> + let attribute_id = stream.parse::<Ident>()?;
> + if attribute_id != "attributes" {
> + return Err(syn::Error::new(
> + attribute_id.span(),
> + format!(
> + "unexpected identifier: {}, expected: 'attributes'",
> + attribute_id
> + ),
> + ));
> + }
> + stream.parse::<Token![:]>()?;
> + let attr_stream;
> + let _bracket = bracketed!(attr_stream in stream);
> + let attributes = Punctuated::<(Ident, LitInt), Token![,]>::parse_terminated_with(
> + &attr_stream,
> + parse_attribute_field,
> + )?;
> + let attributes = attributes.into_iter().collect::<Vec<_>>();
> +
> + stream.parse::<Option<Token![,]>>()?;
> + Ok(attributes)
> +}
> +
> +fn parse_field_error(span: Span, name: &Ident, expected_name: &str) -> syn::Error {
> + syn::Error::new(
> + span,
> + format!("Unexpected field name, got: {name} expected: {expected_name}"),
> + )
> +}
> +
> +impl Parse for ConfigfsAttrs {
> + fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
> + let container = try_parse(input, |s| parse_named_field(s, "container"))?;
> + let data = try_parse(input, |s| parse_named_field(s, "data"))?;
> + let child = try_parse(input, |s| parse_named_field(s, "child")).ok();
> + let attributes = parse_attributes(input)?;
I have a `parse_ordered_fields!()` macro in module.rs, please extract it to
helpers and use it instead of implementing the parsing in an ad-hoc manner.
> +
> + Ok(ConfigfsAttrs {
> + container,
> + data,
> + child,
> + attributes,
> + })
> + }
> +}
> +
> +fn make_static_ident<T: ToTokens>(ty: &T, suffix: &str) -> syn::Ident {
> + let raw_id = quote! { #ty }.to_string();
> +
> + // Sanitizing syn::Type::Path, this is safe since it is
> + // only used as the identifier.
> + let normalized = raw_id
> + .split("::")
> + .map(|s| String::from(s.trim()))
> + .reduce(|a, b| format!("{a}_{b}"))
> + .expect("Cannot be empty")
> + .to_uppercase()
> + .replace(|c: char| !c.is_alphanumeric(), "_");
> +
> + Ident::new(&format!("{}_{}", normalized, suffix), ty.span())
> +}
> +
> +pub(crate) fn configfs_attrs(cfs_attrs: ConfigfsAttrs) -> proc_macro2::TokenStream {
> + let (container_ty, data_ty) = (&cfs_attrs.container, &cfs_attrs.data);
> +
> + let data_tp_ident = make_static_ident(data_ty, "TPE");
> + let data_attr_ident = make_static_ident(data_ty, "ATTR_LIST");
Instead of creating identifers like this, just scope them properly so that
a fixed identifier is used without colliding.
> +
> + let n = cfs_attrs.attributes.len() + 1;
> +
> + let attr_list = quote! {
> + static #data_attr_ident: kernel::configfs::AttributeList<#n, #data_ty> =
> + // SAFETY: We are expanding `configfs_attrs`.
> + unsafe { kernel::configfs::AttributeList::new() };
> + };
> +
> + let mut attrs = Vec::new();
> + for (attr_idx, (name, id)) in cfs_attrs.attributes.iter().enumerate() {
> + let name_with_attr = make_static_ident(name, "ATTR");
> +
> + let id: u64 = match id.base10_parse::<u64>() {
> + Ok(v) => v,
> + Err(_) => {
> + return syn::Error::new(id.span(), "Could not parse attribute ID as a u64")
> + .to_compile_error();
> + }
> + };
Why parsing? The ID looks like it's just substituted as is.
Best,
Gary
> +
> + attrs.push(quote! {
> + static #name_with_attr: kernel::configfs::Attribute<#id, #data_ty, #data_ty> =
> + // SAFETY: We are expanding `configfs_attrs`.
> + unsafe {
> + kernel::configfs::Attribute::new(kernel::c_str!(::core::stringify!(#name)))
> + };
> +
> + // SAFETY: By design of this macro, the name of the variable we
> + // invoke the `add` method on below, is not visible outside of
> + // the macro expansion. The macro does not operate concurrently
> + // on this variable, and thus we have exclusive access to the
> + // variable.
> + unsafe { #data_attr_ident.add::<#attr_idx, #id, _>(&#name_with_attr) }
> + });
> + }
> +
> + let has_child_code = if let Some(child) = cfs_attrs.child {
> + quote! { new_with_child_ctor::<#n, #child>}
> + } else {
> + quote! { new::<#n> }
> + };
> +
> + let data_type = quote! {
> + {
> + static #data_tp_ident:
> + kernel::configfs::ItemType<#container_ty, #data_ty> =
> + kernel::configfs::ItemType::<#container_ty, #data_ty>::#has_child_code(
> + &THIS_MODULE, &#data_attr_ident
> + );
> + &#data_tp_ident
> + }
> + };
> +
> + quote! {
> + {
> + #attr_list
> + #(#attrs)*
> + #data_type
> + }
> + }
> +}
> diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
> index d18fbf4daa0a5..fdab8804e1ba9 100644
> --- a/rust/macros/helpers.rs
> +++ b/rust/macros/helpers.rs
> @@ -4,6 +4,7 @@
> use quote::ToTokens;
> use syn::{
> parse::{
> + discouraged::Speculative,
> Parse,
> ParseStream, //
> },
> @@ -54,6 +55,20 @@ pub(crate) fn file() -> String {
> }
> }
>
> +pub(crate) fn try_parse<T>(
> + input: ParseStream<'_>,
> + parser: impl FnOnce(ParseStream<'_>) -> Result<T>,
> +) -> Result<T> {
> + let fork = input.fork();
> + match parser(&fork) {
> + Ok(value) => {
> + input.advance_to(&fork);
> + Ok(value)
> + }
> + Err(e) => Err(e),
> + }
> +}
> +
> /// Obtain all `#[cfg]` attributes.
> pub(crate) fn gather_cfg_attrs(attr: &[Attribute]) -> impl Iterator<Item = &Attribute> + '_ {
> attr.iter().filter(|a| a.path().is_ident("cfg"))
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 2cfd59e0f9e7c..745ba83c828b9 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -15,6 +15,8 @@
> #![cfg_attr(not(CONFIG_RUSTC_HAS_SPAN_FILE), feature(proc_macro_span))]
>
> mod concat_idents;
> +#[cfg(CONFIG_CONFIGFS_FS)]
> +mod configfs_attrs;
> mod export;
> mod fmt;
> mod helpers;
> @@ -489,3 +491,86 @@ pub fn kunit_tests(attr: TokenStream, input: TokenStream) -> TokenStream {
> .unwrap_or_else(|e| e.into_compile_error())
> .into()
> }
> +
> +/// Define a list of configfs attributes statically.
> +///
> +/// # Examples
> +///
> +/// ```ignore
> +/// let item_type = configfs_attrs! {
> +/// container: configfs::Subsystem<Configuration>,
> +/// data: Configuration,
> +/// child: Child,
> +/// attributes: [
> +/// message: 0,
> +/// bar: 1,
> +/// ],
> +/// };
> +///```
> +///
> +/// Expands the following output:
> +/// let item_type = {
> +/// static CONFIGURATION_ATTR_LIST: kernel::configfs::AttributeList<
> +/// 3usize,
> +/// Configuration,
> +/// > = unsafe { kernel::configfs::AttributeList::new() };
> +/// static MESSAGE_ATTR: kernel::configfs::Attribute<
> +/// 0u64,
> +/// Configuration,
> +/// Configuration,
> +/// > = unsafe {
> +/// kernel::configfs::Attribute::new({
> +/// const S: &str = "message\u{0}";
> +/// const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
> +/// S.as_bytes(),
> +/// ) {
> +/// Ok(v) => v,
> +/// Err(_) => {
> +/// ::core::panicking::panic_fmt(
> +/// format_args!("string contains interior NUL"),
> +/// );
> +/// }
> +/// };
> +/// C
> +/// })
> +/// };
> +/// unsafe { CONFIGURATION_ATTR_LIST.add::<0usize, 0u64, _>(&MESSAGE_ATTR) }
> +/// static BAR_ATTR: kernel::configfs::Attribute<
> +/// 1u64,
> +/// Configuration,
> +/// Configuration,
> +/// > = unsafe {
> +/// kernel::configfs::Attribute::new({
> +/// const S: &str = "bar\u{0}";
> +/// const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
> +/// S.as_bytes(),
> +/// ) {
> +/// Ok(v) => v,
> +/// Err(_) => {
> +/// ::core::panicking::panic_fmt(
> +/// format_args!("string contains interior NUL"),
> +/// );
> +/// }
> +/// };
> +/// C
> +/// })
> +/// };
> +/// unsafe { CONFIGURATION_ATTR_LIST.add::<1usize, 1u64, _>(&BAR_ATTR) }
> +/// {
> +/// static CONFIGURATION_TPE: kernel::configfs::ItemType<
> +/// Subsystem<Configuration>,
> +/// Configuration,
> +/// > = kernel::configfs::ItemType::<
> +/// Subsystem<Configuration>,
> +/// Configuration,
> +/// >::new_with_child_ctor::<3usize, Child>(&THIS_MODULE, &CONFIGURATION_ATTR_LIST);
> +/// &CONFIGURATION_TPE
> +/// }
> +/// };
> +///
> +#[cfg(CONFIG_CONFIGFS_FS)]
> +#[proc_macro]
> +pub fn configfs_attrs(input: TokenStream) -> TokenStream {
> + configfs_attrs::configfs_attrs(parse_macro_input!(input as configfs_attrs::ConfigfsAttrs))
> + .into()
> +}
> diff --git a/samples/rust/rust_configfs.rs b/samples/rust/rust_configfs.rs
> index a1bd9db6010da..876462f7789d1 100644
> --- a/samples/rust/rust_configfs.rs
> +++ b/samples/rust/rust_configfs.rs
> @@ -4,7 +4,7 @@
>
> use kernel::alloc::flags;
> use kernel::configfs;
> -use kernel::configfs::configfs_attrs;
> +use kernel::macros::configfs_attrs;
> use kernel::new_mutex;
> use kernel::page::PAGE_SIZE;
> use kernel::prelude::*;
>
> ---
> base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
> change-id: 20260417-configfs-syn-191e07130027
>
> Best regards,
^ permalink raw reply
* Re: [PATCH 0/5] blk-cgroup: fix blkg list and policy data races
From: Jens Axboe @ 2026-06-03 14:35 UTC (permalink / raw)
To: Yu Kuai
Cc: Tejun Heo, Josef Bacik, Ming Lei, Nilay Shroff, Bart Van Assche,
linux-block, cgroups, linux-kernel
In-Reply-To: <cover.1780492756.git.yukuai@fygo.io>
On 6/3/26 7:27 AM, Yu Kuai wrote:
> This small series fixes races between blkg destruction, q->blkg_list
> iteration, and blkcg policy activation.
In spam:
Authentication-Results: mx.google.com; spf=pass (google.com: domain of srs0=eh6w=d7=fygo.io=yukuai@kernel.org designates 2600:3c04:e001:324:0:1991:8:25 as permitted sender) smtp.mailfrom="SRS0=EH6w=D7=fygo.io=yukuai@kernel.org"; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=QUARANTINE) header.from=fygo.io
if you don't get this sorted out, your messages will be missed as they
end up in spam for most people.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH 0/5] blk-cgroup: fix blkg list and policy data races
From: Jens Axboe @ 2026-06-03 15:08 UTC (permalink / raw)
To: yukuai, Yu Kuai
Cc: Tejun Heo, Josef Bacik, Ming Lei, Nilay Shroff, Bart Van Assche,
linux-block, cgroups, linux-kernel
In-Reply-To: <189b8601-87a6-423a-b267-9d550c31c086@fnnas.com>
On 6/3/26 9:06 AM, yukuai wrote:
> Hi,
>
> ? 2026/6/3 22:35, Jens Axboe ??:
>> On 6/3/26 7:27 AM, Yu Kuai wrote:
>>> This small series fixes races between blkg destruction, q->blkg_list
>>> iteration, and blkcg policy activation.
>> In spam: Authentication-Results: mx.google.com <mx.google.com>; spf=pass (google.com: <google.com:> domain of srs0=eh6w=d7=fygo.io=yukuai@kernel.org designates 2600:3c04:e001:324:0:1991:8:25 as permitted sender) smtp.mailfrom="SRS0=EH6w=D7=fygo.io=yukuai@kernel.org"; dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=QUARANTINE) header.from=fygo.io <header.from=fygo.io> if you don't get this sorted out, your messages will be missed as they end up in spam for most people.
>
> Thanks again for the notice. I think I finally figured out why dmarc failed.
This email looks better!
--
Jens Axboe
^ permalink raw reply
* [PATCH blktests] throtl/008: Add a test for the iocost cgroup controller
From: Bart Van Assche @ 2026-06-03 20:50 UTC (permalink / raw)
To: Shin'ichiro Kawasaki; +Cc: Damien Le Moal, linux-block, Bart Van Assche
Add a test for read and write IOPS throttling. The test implementation
has been generated by Antigravity and Gemini with a few manual edits.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
tests/throtl/008 | 152 +++++++++++++++++++++++++++++++++++++++++++
tests/throtl/008.out | 2 +
2 files changed, 154 insertions(+)
create mode 100755 tests/throtl/008
create mode 100644 tests/throtl/008.out
diff --git a/tests/throtl/008 b/tests/throtl/008
new file mode 100755
index 000000000000..0570fc0188e0
--- /dev/null
+++ b/tests/throtl/008
@@ -0,0 +1,152 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2026 Google LLC
+#
+# Test cgroup iocost IOPS limiting.
+
+. tests/block/rc
+. common/null_blk
+. common/cgroup
+. common/fio
+
+DESCRIPTION="test cgroup iocost controller limits"
+
+requires() {
+ _have_cgroup2_controller io
+ _have_null_blk
+ _have_fio
+ _have_program bc
+ if [[ ! -e "$(_cgroup2_base_dir)/io.cost.qos" ]]; then
+ SKIP_REASONS+=("iocost controller not supported (CONFIG_BLK_CGROUP_IOCOST)")
+ return 1
+ fi
+}
+
+test() {
+ echo "Running ${TEST_NAME}"
+
+ # Create a null_blk instance
+ local null_blk_params=(
+ blocksize=4096
+ completion_nsec=0
+ memory_backed=0
+ size=1024 # MB
+ submit_queues=1
+ power=1
+ )
+ _init_null_blk nr_devices=0 queue_mode=2 &&
+ if ! _configure_null_blk nullb0 "${null_blk_params[@]}"; then
+ echo "Configuring null_blk failed"
+ return 1
+ fi
+
+ local dev_t
+ dev_t=$(</sys/block/nullb0/dev)
+ if [[ -z $dev_t ]]; then
+ echo "Failed to get major:minor for nullb0"
+ _exit_null_blk
+ return 1
+ fi
+
+ # Initialize cgroups
+ if ! _init_cgroup2; then
+ echo "Initializing cgroup2 failed"
+ _exit_null_blk
+ return 1
+ fi
+
+ # Enable io controller in the subtree
+ local deactivate_io_ctrlr=false
+ if ! grep -wq io "$(_cgroup2_base_dir)/cgroup.subtree_control"; then
+ if ! echo "+io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"; then
+ echo "Failed to enable io controller on cgroup root"
+ _exit_cgroup2
+ _exit_null_blk
+ return 1
+ fi
+ deactivate_io_ctrlr=true
+ fi
+
+ if ! echo "+io" > "$CGROUP2_DIR/cgroup.subtree_control"; then
+ echo "Failed to enable io controller on $CGROUP2_DIR"
+ if [[ $deactivate_io_ctrlr == true ]]; then
+ echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
+ fi
+ _exit_cgroup2
+ _exit_null_blk
+ return 1
+ fi
+
+ # Configure iocost controller globally for the device
+ # min=100.00 max=100.00 forces vrate to be fixed at 100%
+ if ! echo "$dev_t enable=1 min=100.00 max=100.00" > "$(_cgroup2_base_dir)/io.cost.qos"; then
+ echo "Failed to configure io.cost.qos"
+ if [[ $deactivate_io_ctrlr == true ]]; then
+ echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
+ fi
+ _exit_cgroup2
+ _exit_null_blk
+ return 1
+ fi
+
+ # Limit read iops to 100 and write iops to 10.
+ # Setting rbps/wbps to 0 means they are ignored (costs calculated purely based on IOPS)
+ if ! echo "$dev_t ctrl=user model=linear rbps=0 rseqiops=100 rrandiops=100 wbps=0 wseqiops=10 wrandiops=10" > "$(_cgroup2_base_dir)/io.cost.model"; then
+ echo "Failed to configure io.cost.model"
+ echo "$dev_t enable=0" > "$(_cgroup2_base_dir)/io.cost.qos"
+ if [[ $deactivate_io_ctrlr == true ]]; then
+ echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
+ fi
+ _exit_cgroup2
+ _exit_null_blk
+ return 1
+ fi
+
+ # Create a child cgroup for test
+ local cg_name="testgrp"
+ local cg_path="$CGROUP2_DIR/$cg_name"
+ mkdir "$cg_path"
+
+ # 1. Run FIO read test
+ local -a FIO_PERF_FIELDS
+ FIO_PERF_FIELDS=("read iops")
+ if ! _fio_perf --bs=4k --rw=randread --name=read-test --filename=/dev/nullb0 \
+ --ioengine=libaio --direct=1 --iodepth=64 --time_based --runtime=10 \
+ --cgroup="blktests/$cg_name" > /dev/null 2>&1; then
+ echo "FIO read test failed"
+ else
+ local read_iops=${TEST_RUN["read iops"]}
+ if [[ -z $read_iops ]]; then
+ echo "Read IOPS is empty"
+ elif (( "$(echo "$read_iops < 90 || $read_iops > 110" | bc -l)" )); then
+ echo "Read IOPS $read_iops not in expected range [90, 110]"
+ fi
+ fi
+
+ # 2. Run FIO write test
+ FIO_PERF_FIELDS=("write iops")
+ if ! _fio_perf --bs=4k --rw=randwrite --name=write-test --filename=/dev/nullb0 \
+ --ioengine=libaio --direct=1 --iodepth=64 --time_based --runtime=10 \
+ --cgroup="blktests/$cg_name" > /dev/null 2>&1; then
+ echo "FIO write test failed"
+ else
+ local write_iops=${TEST_RUN["write iops"]}
+ if [[ -z $write_iops ]]; then
+ echo "Write IOPS is empty"
+ elif (( "$(echo "$write_iops < 8 || $write_iops > 12" | bc -l)" )); then
+ echo "Write IOPS $write_iops not in expected range [8, 12]"
+ fi
+ fi
+
+ # Clean up cgroups
+ rmdir "$cg_path"
+ if [[ $deactivate_io_ctrlr == true ]]; then
+ { echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"; } &> "${FULL}"
+ fi
+ _exit_cgroup2
+
+ # Clean up null_blk
+ _exit_null_blk
+
+ echo "Test complete"
+}
diff --git a/tests/throtl/008.out b/tests/throtl/008.out
new file mode 100644
index 000000000000..890ff7f226d1
--- /dev/null
+++ b/tests/throtl/008.out
@@ -0,0 +1,2 @@
+Running throtl/008
+Test complete
^ permalink raw reply related
* [syzbot] Monthly block report (Jun 2026)
From: syzbot @ 2026-06-04 4:32 UTC (permalink / raw)
To: linux-block, linux-kernel, syzkaller-bugs
Hello block maintainers/developers,
This is a 31-day syzbot report for the block subsystem.
All related reports/information can be found at:
https://syzkaller.appspot.com/upstream/s/block
During the period, 0 new issues were detected and 0 were fixed.
In total, 17 issues are still open and 112 have already been fixed.
There are also 26 low-priority issues.
Some of the still happening issues:
Ref Crashes Repro Title
<1> 9088 Yes KMSAN: kernel-infoleak in filemap_read
https://syzkaller.appspot.com/bug?extid=905d785c4923bea2c1db
<2> 1729 Yes INFO: task hung in blkdev_fallocate
https://syzkaller.appspot.com/bug?extid=39b75c02b8be0a061bfc
<3> 215 Yes general protection fault in rtlock_slowlock_locked
https://syzkaller.appspot.com/bug?extid=08df3e4c9b304b37cb04
<4> 60724 Yes possible deadlock in __submit_bio
https://syzkaller.appspot.com/bug?extid=949ae54e95a2fab4cbb4
<5> 69 Yes INFO: task hung in blk_trace_remove (2)
https://syzkaller.appspot.com/bug?extid=2373f6be3e6de4f92562
<6> 18 No KASAN: slab-out-of-bounds Read in blk_mq_free_rqs
https://syzkaller.appspot.com/bug?extid=e90526cab23b9efcd03c
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
To disable reminders for individual bugs, reply with the following command:
#syz set <Ref> no-reminders
To change bug's subsystems, reply with:
#syz set <Ref> subsystems: new-subsystem
You may send multiple commands in a single email message.
^ permalink raw reply
* Re: [PATCH blktests] throtl/008: Add a test for the iocost cgroup controller
From: Shin'ichiro Kawasaki @ 2026-06-04 6:03 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Damien Le Moal, linux-block
In-Reply-To: <20260603205007.2654971-1-bvanassche@acm.org>
[-- Attachment #1: Type: text/plain, Size: 3597 bytes --]
On Jun 03, 2026 / 13:50, Bart Van Assche wrote:
> Add a test for read and write IOPS throttling. The test implementation
> has been generated by Antigravity and Gemini with a few manual edits.
Hi Bart, thanks for the patch. I think it's nice to add this test case to
cover the iocost controller code paths. It might be the better to note
'the iocost controller' in the commit message also. It will be consistent
with the DESCRIPTION in the test case.
>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> tests/throtl/008 | 152 +++++++++++++++++++++++++++++++++++++++++++
> tests/throtl/008.out | 2 +
> 2 files changed, 154 insertions(+)
> create mode 100755 tests/throtl/008
> create mode 100644 tests/throtl/008.out
>
> diff --git a/tests/throtl/008 b/tests/throtl/008
> new file mode 100755
> index 000000000000..0570fc0188e0
> --- /dev/null
> +++ b/tests/throtl/008
> @@ -0,0 +1,152 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2026 Google LLC
> +#
> +# Test cgroup iocost IOPS limiting.
> +
> +. tests/block/rc
I guess this should be tests/throtl/rc. With this, the two lines below will not
be required.
> +. common/null_blk
> +. common/cgroup
> +. common/fio
> +
> +DESCRIPTION="test cgroup iocost controller limits"
> +
> +requires() {
> + _have_cgroup2_controller io
> + _have_null_blk
> + _have_fio
> + _have_program bc
group_require() in tsts/throtl/rc covers the 3 lines out of the 4 lines above.
> + if [[ ! -e "$(_cgroup2_base_dir)/io.cost.qos" ]]; then
> + SKIP_REASONS+=("iocost controller not supported (CONFIG_BLK_CGROUP_IOCOST)")
> + return 1
> + fi
> +}
> +
> +test() {
> + echo "Running ${TEST_NAME}"
> +
> + # Create a null_blk instance
> + local null_blk_params=(
> + blocksize=4096
> + completion_nsec=0
> + memory_backed=0
> + size=1024 # MB
> + submit_queues=1
> + power=1
> + )
> + _init_null_blk nr_devices=0 queue_mode=2 &&
> + if ! _configure_null_blk nullb0 "${null_blk_params[@]}"; then
> + echo "Configuring null_blk failed"
> + return 1
> + fi
> +
> + local dev_t
> + dev_t=$(</sys/block/nullb0/dev)
> + if [[ -z $dev_t ]]; then
> + echo "Failed to get major:minor for nullb0"
> + _exit_null_blk
> + return 1
> + fi
> +
> + # Initialize cgroups
> + if ! _init_cgroup2; then
> + echo "Initializing cgroup2 failed"
> + _exit_null_blk
> + return 1
> + fi
> +
> + # Enable io controller in the subtree
> + local deactivate_io_ctrlr=false
> + if ! grep -wq io "$(_cgroup2_base_dir)/cgroup.subtree_control"; then
> + if ! echo "+io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"; then
> + echo "Failed to enable io controller on cgroup root"
> + _exit_cgroup2
> + _exit_null_blk
> + return 1
> + fi
> + deactivate_io_ctrlr=true
> + fi
> +
> + if ! echo "+io" > "$CGROUP2_DIR/cgroup.subtree_control"; then
> + echo "Failed to enable io controller on $CGROUP2_DIR"
> + if [[ $deactivate_io_ctrlr == true ]]; then
> + echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
> + fi
> + _exit_cgroup2
> + _exit_null_blk
> + return 1
> + fi
The initialization part above has duplications with _set_up_throtl() in
tests/throtl/rc. Is it considered to use _set_up_throtl()? I guess it will
simplify this test case, and make it consisten with other test cases in the
throtl group. It will also allow to run the test case for scsi_debug.
I created a trial patch that applis on top of this patch, which uses
_set_up_throtl(). It is attached to this mail for your reference. It drops
some null_blk options. As far as I did trial runs, it does not look affecting
the run results.
[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 4582 bytes --]
diff --git a/tests/throtl/008 b/tests/throtl/008
index 0570fc0..b1bcf84 100755
--- a/tests/throtl/008
+++ b/tests/throtl/008
@@ -4,76 +4,36 @@
#
# Test cgroup iocost IOPS limiting.
-. tests/block/rc
-. common/null_blk
-. common/cgroup
+. tests/throtl/rc
. common/fio
DESCRIPTION="test cgroup iocost controller limits"
requires() {
- _have_cgroup2_controller io
- _have_null_blk
_have_fio
- _have_program bc
if [[ ! -e "$(_cgroup2_base_dir)/io.cost.qos" ]]; then
SKIP_REASONS+=("iocost controller not supported (CONFIG_BLK_CGROUP_IOCOST)")
return 1
fi
}
+set_conditions() {
+ _set_throtl_blkdev_type "$@"
+}
+
test() {
echo "Running ${TEST_NAME}"
- # Create a null_blk instance
- local null_blk_params=(
- blocksize=4096
- completion_nsec=0
- memory_backed=0
- size=1024 # MB
- submit_queues=1
- power=1
- )
- _init_null_blk nr_devices=0 queue_mode=2 &&
- if ! _configure_null_blk nullb0 "${null_blk_params[@]}"; then
- echo "Configuring null_blk failed"
+ # Set up throtl device and cgroup
+ if ! _set_up_throtl; then
return 1
fi
local dev_t
- dev_t=$(</sys/block/nullb0/dev)
+ dev_t=$(</sys/block/"$THROTL_DEV"/dev)
if [[ -z $dev_t ]]; then
- echo "Failed to get major:minor for nullb0"
- _exit_null_blk
- return 1
- fi
-
- # Initialize cgroups
- if ! _init_cgroup2; then
- echo "Initializing cgroup2 failed"
- _exit_null_blk
- return 1
- fi
-
- # Enable io controller in the subtree
- local deactivate_io_ctrlr=false
- if ! grep -wq io "$(_cgroup2_base_dir)/cgroup.subtree_control"; then
- if ! echo "+io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"; then
- echo "Failed to enable io controller on cgroup root"
- _exit_cgroup2
- _exit_null_blk
- return 1
- fi
- deactivate_io_ctrlr=true
- fi
-
- if ! echo "+io" > "$CGROUP2_DIR/cgroup.subtree_control"; then
- echo "Failed to enable io controller on $CGROUP2_DIR"
- if [[ $deactivate_io_ctrlr == true ]]; then
- echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
- fi
- _exit_cgroup2
- _exit_null_blk
+ echo "Failed to get major:minor for $THROTL_DEV"
+ _clean_up_throtl
return 1
fi
@@ -81,11 +41,7 @@ test() {
# min=100.00 max=100.00 forces vrate to be fixed at 100%
if ! echo "$dev_t enable=1 min=100.00 max=100.00" > "$(_cgroup2_base_dir)/io.cost.qos"; then
echo "Failed to configure io.cost.qos"
- if [[ $deactivate_io_ctrlr == true ]]; then
- echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
- fi
- _exit_cgroup2
- _exit_null_blk
+ _clean_up_throtl
return 1
fi
@@ -94,25 +50,21 @@ test() {
if ! echo "$dev_t ctrl=user model=linear rbps=0 rseqiops=100 rrandiops=100 wbps=0 wseqiops=10 wrandiops=10" > "$(_cgroup2_base_dir)/io.cost.model"; then
echo "Failed to configure io.cost.model"
echo "$dev_t enable=0" > "$(_cgroup2_base_dir)/io.cost.qos"
- if [[ $deactivate_io_ctrlr == true ]]; then
- echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
- fi
- _exit_cgroup2
- _exit_null_blk
+ _clean_up_throtl
return 1
fi
# Create a child cgroup for test
local cg_name="testgrp"
- local cg_path="$CGROUP2_DIR/$cg_name"
+ local cg_path="$CGROUP2_DIR/$THROTL_DIR/$cg_name"
mkdir "$cg_path"
# 1. Run FIO read test
local -a FIO_PERF_FIELDS
FIO_PERF_FIELDS=("read iops")
- if ! _fio_perf --bs=4k --rw=randread --name=read-test --filename=/dev/nullb0 \
+ if ! _fio_perf --bs=4k --rw=randread --name=read-test --filename=/dev/"$THROTL_DEV" \
--ioengine=libaio --direct=1 --iodepth=64 --time_based --runtime=10 \
- --cgroup="blktests/$cg_name" > /dev/null 2>&1; then
+ --cgroup="blktests/$THROTL_DIR/$cg_name" > /dev/null 2>&1; then
echo "FIO read test failed"
else
local read_iops=${TEST_RUN["read iops"]}
@@ -125,9 +77,9 @@ test() {
# 2. Run FIO write test
FIO_PERF_FIELDS=("write iops")
- if ! _fio_perf --bs=4k --rw=randwrite --name=write-test --filename=/dev/nullb0 \
+ if ! _fio_perf --bs=4k --rw=randwrite --name=write-test --filename=/dev/"$THROTL_DEV" \
--ioengine=libaio --direct=1 --iodepth=64 --time_based --runtime=10 \
- --cgroup="blktests/$cg_name" > /dev/null 2>&1; then
+ --cgroup="blktests/$THROTL_DIR/$cg_name" > /dev/null 2>&1; then
echo "FIO write test failed"
else
local write_iops=${TEST_RUN["write iops"]}
@@ -140,13 +92,7 @@ test() {
# Clean up cgroups
rmdir "$cg_path"
- if [[ $deactivate_io_ctrlr == true ]]; then
- { echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"; } &> "${FULL}"
- fi
- _exit_cgroup2
-
- # Clean up null_blk
- _exit_null_blk
+ _clean_up_throtl
echo "Test complete"
}
^ permalink raw reply related
* [PATCH] nvmet-rdma: reject inline data with a nonzero offset
From: Bryam Vargas @ 2026-06-04 8:46 UTC (permalink / raw)
To: Christoph Hellwig, Sagi Grimberg, Keith Busch, Chaitanya Kulkarni
Cc: linux-nvme, linux-rdma, linux-block
In-Reply-To: <ahm6Ksr3rfGdnOsN@kbusch-mbp>
nvmet_rdma_map_sgl_inline() takes a host-controlled offset and length
from the inline SGL descriptor and bounds-checks them against the
per-port inline_data_size:
u64 off = le64_to_cpu(sgl->addr);
u32 len = le32_to_cpu(sgl->length);
...
if (off + len > rsp->queue->dev->inline_data_size)
return NVME_SC_SGL_INVALID_OFFSET | NVME_STATUS_DNR;
This is unsound whenever the offset is nonzero:
- "off + len" is evaluated in u64 and wraps modulo 2^64. A descriptor
with addr = 0xfffffffffffffe00 and length = 0x1000 wraps the sum to
0xe00 and passes the check. nvmet_rdma_use_inline_sg() then stores
the offset into scatterlist::offset (unsigned int) and the block
layer reads out of bounds of the inline page; a large len also makes
num_pages(len) exceed NVMET_RDMA_MAX_INLINE_SGE and overruns the
fixed-size inline_sg[] array.
- Even computed without wrapping, inline_data_size is configurable up
to max(SZ_16K, PAGE_SIZE). An offset in (PAGE_SIZE, inline_data_size]
passes the bound and then "PAGE_SIZE - off" in
nvmet_rdma_use_inline_sg() underflows, leaving scatterlist::length at
~4 GiB and the offset pointing past the first inline page.
A nonzero inline offset is never legitimate here. nvmet advertises
icdoff = 0, nvme_rdma_setup_ctrl() refuses to use a controller that
reports a nonzero icdoff ("icdoff is not supported!"), and
nvme_rdma_map_sg_inline() sets the inline descriptor addr to icdoff, so
a compliant initiator always sends offset 0. nvmet_rdma_use_inline_sg()
likewise assumes the inline data begins at the start of the first inline
page (the RNIC DMAs it to page offset 0); any nonzero offset also
mis-describes the scatterlist even when it is in bounds.
Reject a nonzero offset directly. This closes the u64 overflow, the
inline_sg[] overrun and the PAGE_SIZE - off underflow together, and is
simpler than bounding the offset.
Fixes: 0d5ee2b2ab4f ("nvmet-rdma: support max(16KB, PAGE_SIZE) inline data")
Cc: stable@vger.kernel.org
Reported-by: Bryam Vargas <hexlabsecurity@proton.me>
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
Keith, thanks for the suggested form
if (off > rsp->queue->dev->inline_data_size ||
len > rsp->queue->dev->inline_data_size - off)
It does stop the u64 overflow, but while testing it I found it is still
incomplete when a port is configured with inline_data_size > PAGE_SIZE
(it is settable up to max(SZ_16K, PAGE_SIZE)): an offset in
(PAGE_SIZE, inline_data_size] passes that bound and then "PAGE_SIZE - off"
in nvmet_rdma_use_inline_sg() underflows, leaving scatterlist::length at
~4 GiB pointing past the first inline page. The block backend then
executes the out-of-bounds read (KASAN trace below). Since a compliant
initiator never sends a nonzero inline offset (nvmet advertises
icdoff = 0 and nvme_rdma_setup_ctrl() refuses a nonzero icdoff),
rejecting off != 0 closes that case too and is even simpler, so this
formal patch uses that instead of bounding the offset.
Verified on a KASAN build (inline_data_size = 16384) over an rdma_rxe
soft-RoCE loopback nvmet-rdma target with a block backend:
- offset 0, 4 KiB inline write: succeeds, clean (control).
- offset 8192, len 4096: without this patch the bounds check passes
and the block backend executes the out-of-bounds read
BUG: KASAN: slab-out-of-bounds in copy_folio_from_iter_atomic
Read of size 4096 ...
with this patch it is rejected ("invalid inline data offset!").
- offset 4095 (< PAGE_SIZE): without this patch it is in bounds but
mis-describes the SGL (NVME_SC_SGL_INVALID_DATA, no OOB); with this
patch it is rejected up front.
- offset 0 keeps working (no regression for compliant initiators).
drivers/nvme/target/rdma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -854,7 +854,7 @@ static u16 nvmet_rdma_map_sgl_inline(struct nvmet_rdma_rsp *rsp)
return NVME_SC_INVALID_FIELD | NVME_STATUS_DNR;
}
- if (off + len > rsp->queue->dev->inline_data_size) {
+ if (off || len > rsp->queue->dev->inline_data_size) {
pr_err("invalid inline data offset!\n");
return NVME_SC_SGL_INVALID_OFFSET | NVME_STATUS_DNR;
}
--
2.43.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox