[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aHYlPkFKTafRypNu@google.com>
Date: Tue, 15 Jul 2025 09:54:06 +0000
From: Alice Ryhl <aliceryhl@...gle.com>
To: Andreas Hindborg <a.hindborg@...nel.org>
Cc: Boqun Feng <boqun.feng@...il.com>, Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>, Gary Guo <gary@...yguo.net>,
"Björn Roy Baron" <bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>,
Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, Jens Axboe <axboe@...nel.dk>,
linux-block@...r.kernel.org, rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 16/16] rnull: add soft-irq completion support
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@...nel.org>
Reviewed-by: Alice Ryhl <aliceryhl@...gle.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
>
>
Powered by blists - more mailing lists