Re: [PATCH 24/79] block: rust: add `BadBlocks` for bad block tracking

[Alice Ryhl <aliceryhl@google.com>]

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.

> +/// # Examples
> +///
> +/// Basic usage:
> +///
> +/// ```rust
> +/// # use kernel::block::badblocks::{BadBlocks, BlockStatus};
> +/// # use kernel::prelude::*;
> +///

Unhide the imports or remove this empty line.

> +/// // 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.

Also, formatting here is off (but ignored by rustfmt due to macro.)

> +    /// 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.

> +    }
> +
> +    /// 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.

> +    /// # 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.

> +    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.

Alice