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

[Daniel Almeida <daniel.almeida@collabora.com>]

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

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