Re: [PATCH v6 18/18] rnull: add soft-irq completion support

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

> On 22 Aug 2025, at 09:14, Andreas Hindborg <a.hindborg@kernel.org> wrote:
> 
> rnull currently only supports direct completion. Add option for completing
> requests across CPU nodes via soft IRQ or IPI.
> 
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
> drivers/block/rnull/configfs.rs | 59 +++++++++++++++++++++++++++++++++++++++--
> drivers/block/rnull/rnull.rs    | 32 ++++++++++++++--------
> 2 files changed, 78 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
> index 46710a1e1af4..8498e9bae6fd 100644
> --- a/drivers/block/rnull/configfs.rs
> +++ b/drivers/block/rnull/configfs.rs
> @@ -1,7 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> 
> use super::{NullBlkDevice, THIS_MODULE};
> -use core::fmt::Write;
> +use core::fmt::{Display, Write};
> use kernel::{
>     block::mq::gen_disk::{GenDisk, GenDiskBuilder},
>     c_str,
> @@ -36,7 +36,7 @@ impl AttributeOperations<0> for Config {
> 
>     fn show(_this: &Config, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
>         let mut writer = kernel::str::Formatter::new(page);
> -        writer.write_str("blocksize,size,rotational\n")?;
> +        writer.write_str("blocksize,size,rotational,irqmode\n")?;
>         Ok(writer.bytes_written())
>     }
> }
> @@ -58,6 +58,7 @@ fn make_group(
>                 blocksize: 1,
>                 rotational: 2,
>                 size: 3,
> +                irqmode: 4,
>             ],
>         };
> 
> @@ -72,6 +73,7 @@ fn make_group(
>                     rotational: false,
>                     disk: None,
>                     capacity_mib: 4096,
> +                    irq_mode: IRQMode::None,
>                     name: name.try_into()?,
>                 }),
>             }),
> @@ -79,6 +81,34 @@ fn make_group(
>     }
> }
> 
> +#[derive(Debug, Clone, Copy)]
> +pub(crate) enum IRQMode {
> +    None,
> +    Soft,
> +}
> +
> +impl TryFrom<u8> for IRQMode {
> +    type Error = kernel::error::Error;
> +
> +    fn try_from(value: u8) -> Result<Self> {
> +        match value {
> +            0 => Ok(Self::None),
> +            1 => Ok(Self::Soft),
> +            _ => Err(EINVAL),
> +        }
> +    }
> +}
> +
> +impl Display for IRQMode {
> +    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
> +        match self {
> +            Self::None => f.write_str("0")?,
> +            Self::Soft => f.write_str("1")?,
> +        }
> +        Ok(())
> +    }
> +}
> +
> #[pin_data]
> pub(crate) struct DeviceConfig {
>     #[pin]
> @@ -92,6 +122,7 @@ struct DeviceConfigInner {
>     block_size: u32,
>     rotational: bool,
>     capacity_mib: u64,
> +    irq_mode: IRQMode,
>     disk: Option<GenDisk<NullBlkDevice>>,
> }
> 
> @@ -121,6 +152,7 @@ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
>                 guard.block_size,
>                 guard.rotational,
>                 guard.capacity_mib,
> +                guard.irq_mode,
>             )?);
>             guard.powered = true;
>         } else if guard.powered && !power_op {
> @@ -205,3 +237,26 @@ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
>         Ok(())
>     }
> }
> +
> +#[vtable]
> +impl configfs::AttributeOperations<4> for DeviceConfig {
> +    type Data = DeviceConfig;
> +
> +    fn show(this: &DeviceConfig, page: &mut [u8; PAGE_SIZE]) -> Result<usize> {
> +        let mut writer = kernel::str::Formatter::new(page);
> +        writer.write_fmt(fmt!("{}\n", this.data.lock().irq_mode))?;
> +        Ok(writer.bytes_written())
> +    }
> +
> +    fn store(this: &DeviceConfig, page: &[u8]) -> Result {
> +        if this.data.lock().powered {
> +            return Err(EBUSY);
> +        }
> +
> +        let text = core::str::from_utf8(page)?.trim();
> +        let value = text.parse::<u8>().map_err(|_| EINVAL)?;
> +
> +        this.data.lock().irq_mode = IRQMode::try_from(value)?;
> +        Ok(())
> +    }
> +}
> diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
> index a19c55717c4f..1ec694d7f1a6 100644
> --- a/drivers/block/rnull/rnull.rs
> +++ b/drivers/block/rnull/rnull.rs
> @@ -4,6 +4,7 @@
> 
> mod configfs;
> 
> +use configfs::IRQMode;
> use kernel::{
>     block::{
>         self,
> @@ -53,35 +54,44 @@ fn new(
>         block_size: u32,
>         rotational: bool,
>         capacity_mib: u64,
> +        irq_mode: IRQMode,
>     ) -> Result<GenDisk<Self>> {
>         let tagset = Arc::pin_init(TagSet::new(1, 256, 1), GFP_KERNEL)?;
> 
> +        let queue_data = Box::new(QueueData { irq_mode }, GFP_KERNEL)?;
> +
>         gen_disk::GenDiskBuilder::new()
>             .capacity_sectors(capacity_mib << (20 - block::SECTOR_SHIFT))
>             .logical_block_size(block_size)?
>             .physical_block_size(block_size)?
>             .rotational(rotational)
> -            .build(fmt!("{}", name.to_str()?), tagset, ())
> +            .build(fmt!("{}", name.to_str()?), tagset, queue_data)
>     }
> }
> 
> +struct QueueData {
> +    irq_mode: IRQMode,
> +}
> +
> #[vtable]
> impl Operations for NullBlkDevice {
> -    type QueueData = ();
> +    type QueueData = KBox<QueueData>;
> 
>     #[inline(always)]
> -    fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
> -        mq::Request::end_ok(rq)
> -            .map_err(|_e| kernel::error::code::EIO)
> -            // We take no refcounts on the request, so we expect to be able to
> -            // end the request. The request reference must be unique at this
> -            // point, and so `end_ok` cannot fail.
> -            .expect("Fatal error - expected to be able to end request");

Same comment here as on the last patch. Otherwise LGTM.

> -
> +    fn queue_rq(queue_data: &QueueData, rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
> +        match queue_data.irq_mode {
> +            IRQMode::None => mq::Request::end_ok(rq)
> +                .map_err(|_e| kernel::error::code::EIO)
> +                // We take no refcounts on the request, so we expect to be able to
> +                // end the request. The request reference must be unique at this
> +                // point, and so `end_ok` cannot fail.
> +                .expect("Fatal error - expected to be able to end request"),
> +            IRQMode::Soft => mq::Request::complete(rq),
> +        }
>         Ok(())
>     }
> 
> -    fn commit_rqs(_queue_data: ()) {}
> +    fn commit_rqs(_queue_data: &QueueData) {}
> 
>     fn complete(rq: ARef<mq::Request<Self>>) {
>         mq::Request::end_ok(rq)
> 
> -- 
> 2.47.2
> 
> 
>