Re: [PATCH v3 16/16] rnull: add soft-irq completion support

[Alice Ryhl <aliceryhl@google.com>]

On Fri, Jul 11, 2025 at 01:43:17PM +0200, Andreas Hindborg wrote:
> rnull currently only supports direct completion. Add option for completing
> requests across CPU nodes via soft IRQ or IPI.
> 
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

>  drivers/block/rnull/configfs.rs | 61 +++++++++++++++++++++++++++++++++++++++--
>  drivers/block/rnull/rnull.rs    | 32 +++++++++++++--------
>  2 files changed, 80 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
> index 6c0e3bbb36ec..3ae84dfc8d62 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(kernel::error::code::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>>,
>  }
>  
> @@ -126,6 +157,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 {
> @@ -218,3 +250,28 @@ 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(|_| kernel::error::code::EINVAL)?;

EINVAL is in the prelude.

> +
> +        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 371786be7f47..85b1509a3106 100644
> --- a/drivers/block/rnull/rnull.rs
> +++ b/drivers/block/rnull/rnull.rs
> @@ -4,6 +4,7 @@
>  
>  mod configfs;
>  
> +use configfs::IRQMode;
>  use kernel::{
>      alloc::flags,
>      block::{
> @@ -54,35 +55,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), flags::GFP_KERNEL)?;
>  
> +        let queue_data = Box::new(QueueData { irq_mode }, flags::GFP_KERNEL)?;
> +
>          gen_disk::GenDiskBuilder::new()
>              .capacity_sectors(capacity_mib << (20 - block::SECTOR_SHIFT))
>              .logical_block_size(block_size)?
>              .physical_block_size(block_size)?
>              .rotational(rotational)
> -            .build(fmt!("{}", name.to_str()?), tagset, ())
> +            .build(fmt!("{}", name.to_str()?), tagset, queue_data)
>      }
>  }
>  
> +struct QueueData {
> +    irq_mode: IRQMode,
> +}
> +
>  #[vtable]
>  impl Operations for NullBlkDevice {
> -    type QueueData = ();
> +    type QueueData = KBox<QueueData>;
>  
>      #[inline(always)]
> -    fn queue_rq(_queue_data: (), rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
> -        mq::Request::end_ok(rq)
> -            .map_err(|_e| kernel::error::code::EIO)
> -            // We take no refcounts on the request, so we expect to be able to
> -            // end the request. The request reference must be unique at this
> -            // point, and so `end_ok` cannot fail.
> -            .expect("Fatal error - expected to be able to end request");
> -
> +    fn queue_rq(queue_data: &QueueData, rq: ARef<mq::Request<Self>>, _is_last: bool) -> Result {
> +        match queue_data.irq_mode {
> +            IRQMode::None => mq::Request::end_ok(rq)
> +                .map_err(|_e| kernel::error::code::EIO)
> +                // We take no refcounts on the request, so we expect to be able to
> +                // end the request. The request reference must be unique at this
> +                // point, and so `end_ok` cannot fail.
> +                .expect("Fatal error - expected to be able to end request"),
> +            IRQMode::Soft => mq::Request::complete(rq),
> +        }
>          Ok(())
>      }
>  
> -    fn commit_rqs(_queue_data: ()) {}
> +    fn commit_rqs(_queue_data: &QueueData) {}
>  
>      fn complete(rq: ARef<mq::Request<Self>>) {
>          mq::Request::end_ok(rq)
> 
> -- 
> 2.47.2
> 
>