Linux block layer
 help / color / mirror / Atom feed
* [RFC PATCH 1/2] rust: block: mq: safely expose TagSet flags
From: Wenzhao Liao @ 2026-04-10  9:08 UTC (permalink / raw)
  To: axboe, a.hindborg, ojeda, linux-block, rust-for-linux
  Cc: bjorn3_gh, aliceryhl, lossin, boqun, dakr, gary, sunke, tmgross,
	linux-kernel
In-Reply-To: <20260410090829.1409430-1-wenzhaoliao@ruc.edu.cn>

TagSet::new() currently hardcodes blk_mq_tag_set.flags to 0.

That prevents Rust block drivers from declaring blk-mq queue flags.

Introduce TagSetFlags as a typed wrapper for BLK_MQ_F_* bits.

Add TagSet::new_with_flags() so drivers can pass flags explicitly.

Keep TagSet::new() as a compatibility wrapper using empty flags.

Re-export TagSetFlags from kernel::block::mq for driver imports.

Build-tested with LLVM=-15 in an out-of-tree rust-next build.

Validation includes vmlinux and drivers/block/rnull/rnull_mod.ko.

Signed-off-by: Wenzhao Liao <wenzhaoliao@ruc.edu.cn>
---
 rust/kernel/block/mq.rs         |  2 +-
 rust/kernel/block/mq/tag_set.rs | 86 ++++++++++++++++++++++++++++++++-
 2 files changed, 86 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
index 1fd0d54dd549..799afdf36539 100644
--- a/rust/kernel/block/mq.rs
+++ b/rust/kernel/block/mq.rs
@@ -100,4 +100,4 @@
 
 pub use operations::Operations;
 pub use request::Request;
-pub use tag_set::TagSet;
+pub use tag_set::{TagSet, TagSetFlags};
diff --git a/rust/kernel/block/mq/tag_set.rs b/rust/kernel/block/mq/tag_set.rs
index dae9df408a86..72d9bce5b11f 100644
--- a/rust/kernel/block/mq/tag_set.rs
+++ b/rust/kernel/block/mq/tag_set.rs
@@ -16,6 +16,80 @@
 use core::{convert::TryInto, marker::PhantomData};
 use pin_init::{pin_data, pinned_drop, PinInit};
 
+/// Flags that control blk-mq tag set behavior.
+///
+/// They can be combined with the operators `|`, `&`, and `!`.
+#[derive(Clone, Copy, PartialEq, Eq)]
+pub struct TagSetFlags(u32);
+
+impl TagSetFlags {
+    /// Returns an empty instance where no flags are set.
+    pub const fn empty() -> Self {
+        Self(0)
+    }
+
+    /// Register as a blocking blk-mq driver device.
+    pub const BLOCKING: Self = Self::new(bindings::BLK_MQ_F_BLOCKING as u32);
+
+    /// Use an underlying blk-mq device for completing I/O.
+    pub const STACKING: Self = Self::new(bindings::BLK_MQ_F_STACKING as u32);
+
+    /// Share hardware contexts between tags.
+    pub const TAG_HCTX_SHARED: Self = Self::new(bindings::BLK_MQ_F_TAG_HCTX_SHARED as u32);
+
+    /// Allocate tags on a round-robin basis.
+    pub const TAG_RR: Self = Self::new(bindings::BLK_MQ_F_TAG_RR as u32);
+
+    /// Disable the I/O scheduler by default.
+    pub const NO_SCHED_BY_DEFAULT: Self =
+        Self::new(bindings::BLK_MQ_F_NO_SCHED_BY_DEFAULT as u32);
+
+    /// Check whether `flags` is contained in `self`.
+    pub fn contains(self, flags: Self) -> bool {
+        (self & flags) == flags
+    }
+
+    pub(crate) const fn as_raw(self) -> u32 {
+        self.0
+    }
+
+    const fn all_bits() -> u32 {
+        Self::BLOCKING.0
+            | Self::STACKING.0
+            | Self::TAG_HCTX_SHARED.0
+            | Self::TAG_RR.0
+            | Self::NO_SCHED_BY_DEFAULT.0
+    }
+
+    const fn new(value: u32) -> Self {
+        Self(value)
+    }
+}
+
+impl core::ops::BitOr for TagSetFlags {
+    type Output = Self;
+
+    fn bitor(self, rhs: Self) -> Self::Output {
+        Self(self.0 | rhs.0)
+    }
+}
+
+impl core::ops::BitAnd for TagSetFlags {
+    type Output = Self;
+
+    fn bitand(self, rhs: Self) -> Self::Output {
+        Self(self.0 & rhs.0)
+    }
+}
+
+impl core::ops::Not for TagSetFlags {
+    type Output = Self;
+
+    fn not(self) -> Self::Output {
+        Self(!self.0 & Self::all_bits())
+    }
+}
+
 /// A wrapper for the C `struct blk_mq_tag_set`.
 ///
 /// `struct blk_mq_tag_set` contains a `struct list_head` and so must be pinned.
@@ -37,6 +111,16 @@ pub fn new(
         nr_hw_queues: u32,
         num_tags: u32,
         num_maps: u32,
+    ) -> impl PinInit<Self, error::Error> {
+        Self::new_with_flags(nr_hw_queues, num_tags, num_maps, TagSetFlags::empty())
+    }
+
+    /// Try to create a new tag set with the given blk-mq flags.
+    pub fn new_with_flags(
+        nr_hw_queues: u32,
+        num_tags: u32,
+        num_maps: u32,
+        flags: TagSetFlags,
     ) -> impl PinInit<Self, error::Error> {
         let tag_set: bindings::blk_mq_tag_set = pin_init::zeroed();
         let tag_set: Result<_> = core::mem::size_of::<RequestDataWrapper>()
@@ -49,7 +133,7 @@ pub fn new(
                     numa_node: bindings::NUMA_NO_NODE,
                     queue_depth: num_tags,
                     cmd_size,
-                    flags: 0,
+                    flags: flags.as_raw(),
                     driver_data: core::ptr::null_mut::<crate::ffi::c_void>(),
                     nr_maps: num_maps,
                     ..tag_set
-- 
2.34.1


^ permalink raw reply related

* [RFC PATCH 2/2] block: rnull: support BLK_MQ_F_BLOCKING via configfs
From: Wenzhao Liao @ 2026-04-10  9:08 UTC (permalink / raw)
  To: axboe, a.hindborg, ojeda, linux-block, rust-for-linux
  Cc: bjorn3_gh, aliceryhl, lossin, boqun, dakr, gary, sunke, tmgross,
	linux-kernel
In-Reply-To: <20260410090829.1409430-1-wenzhaoliao@ruc.edu.cn>

Add a new configfs boolean attribute named blocking.

Advertise blocking in the rnull features list.

On power-on, map blocking=1 to TagSetFlags::BLOCKING.

Create the tag set with TagSet::new_with_flags().

Keep default blocking=0 to preserve existing behavior.

Like other parameters, blocking writes return -EBUSY while powered on.

Validated with LLVM=-15 out-of-tree builds and QEMU runtime tests.

Signed-off-by: Wenzhao Liao <wenzhaoliao@ruc.edu.cn>
---
 drivers/block/rnull/configfs.rs | 32 +++++++++++++++++++++++++++++++-
 drivers/block/rnull/rnull.rs    | 11 +++++++++--
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
index 7c2eb5c0b722..a9d46a511340 100644
--- a/drivers/block/rnull/configfs.rs
+++ b/drivers/block/rnull/configfs.rs
@@ -35,7 +35,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,irqmode\n")?;
+        writer.write_str("blocksize,size,rotational,irqmode,blocking\n")?;
         Ok(writer.bytes_written())
     }
 }
@@ -58,6 +58,7 @@ fn make_group(
                 rotational: 2,
                 size: 3,
                 irqmode: 4,
+                blocking: 5,
             ],
         };
 
@@ -73,6 +74,7 @@ fn make_group(
                     disk: None,
                     capacity_mib: 4096,
                     irq_mode: IRQMode::None,
+                    blocking: false,
                     name: name.try_into()?,
                 }),
             }),
@@ -122,6 +124,7 @@ struct DeviceConfigInner {
     rotational: bool,
     capacity_mib: u64,
     irq_mode: IRQMode,
+    blocking: bool,
     disk: Option<GenDisk<NullBlkDevice>>,
 }
 
@@ -152,6 +155,7 @@ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
                 guard.rotational,
                 guard.capacity_mib,
                 guard.irq_mode,
+                guard.blocking,
             )?);
             guard.powered = true;
         } else if guard.powered && !power_op {
@@ -259,3 +263,29 @@ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
         Ok(())
     }
 }
+
+#[vtable]
+impl configfs::AttributeOperations<5> 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().blocking {
+            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().blocking = kstrtobool_bytes(page)?;
+        Ok(())
+    }
+}
diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index 0ca8715febe8..d7ebd504d8df 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -11,7 +11,7 @@
         mq::{
             self,
             gen_disk::{self, GenDisk},
-            Operations, TagSet,
+            Operations, TagSet, TagSetFlags,
         },
     },
     prelude::*,
@@ -51,8 +51,15 @@ fn new(
         rotational: bool,
         capacity_mib: u64,
         irq_mode: IRQMode,
+        blocking: bool,
     ) -> Result<GenDisk<Self>> {
-        let tagset = Arc::pin_init(TagSet::new(1, 256, 1), GFP_KERNEL)?;
+        let flags = if blocking {
+            TagSetFlags::BLOCKING
+        } else {
+            TagSetFlags::empty()
+        };
+
+        let tagset = Arc::pin_init(TagSet::new_with_flags(1, 256, 1, flags), GFP_KERNEL)?;
 
         let queue_data = Box::new(QueueData { irq_mode }, GFP_KERNEL)?;
 
-- 
2.34.1


^ permalink raw reply related

* [RFC PATCH 0/2] rust block-mq TagSet flags plumbing and rnull blocking wiring
From: Wenzhao Liao @ 2026-04-10  9:08 UTC (permalink / raw)
  To: axboe, a.hindborg, ojeda, linux-block, rust-for-linux
  Cc: bjorn3_gh, aliceryhl, lossin, boqun, dakr, gary, sunke, tmgross,
	linux-kernel

This RFC series fills a practical gap in the Rust block-mq abstraction by
exposing blk_mq_tag_set.flags safely, then wires one in-tree consumer
(`rnull`) via configfs as a reference.

Patch 1 introduces `TagSetFlags` and `TagSet::new_with_flags(...)` while
keeping `TagSet::new(...)` for compatibility.

Patch 2 adds a `blocking` configfs attribute to `rnull` and maps it to
`TagSetFlags::BLOCKING` when powering on a device.

Validation summary:
- Out-of-tree build on rust-next (LLVM=-15): defconfig, rustavailable.
- Enabled CONFIG_RUST=y, CONFIG_CONFIGFS_FS=y, CONFIG_BLK_DEV_RUST_NULL=m.
- Built vmlinux and drivers/block/rnull/rnull_mod.ko successfully.
- QEMU runtime test (initramfs automation) verifies:
  - `/sys/kernel/config/rnull/features` contains `blocking`;
  - writing `blocking` while powered on fails with busy semantics;
  - writing `blocking` after power off succeeds again.

Comments on API naming and any preferred follow-up scope are welcome.

Wenzhao Liao (2):
  rust: block: mq: safely expose TagSet flags
  block: rnull: support BLK_MQ_F_BLOCKING via configfs

 drivers/block/rnull/configfs.rs | 32 +++++++++++-
 drivers/block/rnull/rnull.rs    | 11 ++++-
 rust/kernel/block/mq.rs         |  2 +-
 rust/kernel/block/mq/tag_set.rs | 86 ++++++++++++++++++++++++++++++++-
 4 files changed, 126 insertions(+), 5 deletions(-)


base-commit: 3418d862679ac6da0b6bd681b18b3189c4fad20d
-- 
2.34.1

^ permalink raw reply

* Re: [PATCH 07/13] libmultipath: Add delayed removal support
From: John Garry @ 2026-04-10  8:55 UTC (permalink / raw)
  To: Nilay Shroff, Hannes Reinecke, hch, kbusch, sagi, axboe,
	martin.petersen, james.bottomley, hare
  Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
	bmarzins, dm-devel, linux-block, linux-kernel
In-Reply-To: <da2bfbb0-70ef-4c3a-a235-1343b4a02489@linux.ibm.com>

On 10/04/2026 08:06, Nilay Shroff wrote:
>>    # Part b: Ensure writes work for intermittent disconnect
>>      _nvme_connect_subsys
>>
>>      nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
>>      ns=$(_find_nvme_ns "${def_subsys_uuid}")
>>      echo 10 > "/sys/block/"$ns"/delayed_removal_secs"
>>      bytes_written=$(run_xfs_io_pwritev2 /dev/"$ns" 4096)
>>      if [ "$bytes_written" != 4096 ]; then
>>          echo "could not write successfully initially"
>>      fi
>>      sleep 1
>>      _nvme_disconnect_ctrl "${nvmedev}"
>>      sleep 1
>>      ns=$(_find_nvme_ns "${def_subsys_uuid}")
>>      if [[ "${ns}" = "" ]]; then
>>          echo "could not find ns after disconnect"
>>      fi
>>      _delayed_nvme_reconnect_ctrl &
>>      sleep 1
>>      bytes_written=$(run_xfs_io_pwritev2 /dev/"$ns" 4096)
>>      if [ "$bytes_written" != 4096 ]; then
>>          echo "could not write successfully with reconnect"
>>      fi
> 
> It seems there may be a race here if we attempt to write to $ns before
> the reconnect has completed in _delayed_nvme_reconnect_ctrl.
> 
> If the intention is simply to verify that the controller reconnect occurs
> within the delayed removal window and test pwrite,

Not exactly. I want to verify that if I write between the disconnect and 
the reconnect, then we write succeeds.

> then it may be 
> sufficient
> to:
> - perform the reconnect, and
> - then validate the write (pwrite) afterwards.

I think that this is something subtly different.

For your revised test, if we reconnect, we always expect the subsequent 
write to succeed even without the delayed removal, so I am not sure what 
we achieve.

> 
> In that case, we could either:
> - run _delayed_nvme_reconnect_ctrl in the foreground, or
> - open-code the reconnect directly in the script before issuing the write.
> 

How would that open-code reconnect look? I was just using the subsystem 
connect, which I think is not optimal.

Thanks,
John

^ permalink raw reply

* Re: [PATCH 07/13] libmultipath: Add delayed removal support
From: Nilay Shroff @ 2026-04-10  7:06 UTC (permalink / raw)
  To: John Garry, Hannes Reinecke, hch, kbusch, sagi, axboe,
	martin.petersen, james.bottomley, hare
  Cc: jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
	bmarzins, dm-devel, linux-block, linux-kernel
In-Reply-To: <ccfc867c-e744-42a2-9b22-47245a6c06d7@oracle.com>

On 4/9/26 6:30 PM, John Garry wrote:
> On 09/04/2026 07:37, Nilay Shroff wrote:
>>>
>>> You mean a common blktests testcase, right?
>>>
>>> For NVMe, that test would:
>>> a. try to remove NVMe ko when we have the delayed removal active
>>> b. ensure that we can queue for no path
>>>
>>> I suppose that a common testcase could be possible (with dm mpath), but doesn't dm have its own testsuite?
>>>
>> Yes, I'd add a blktest for 'queue_if_no_path' feature. But as we know we have
>> separate test suite for dm under blktests, I'd first target nvme testcase and
>> then later add another testcase for dm-multipath.
> 
> Testing a. is a challenge to be effective, as we would typically not be able to remove the nvme modules anyway due to many other references.
> 
> For b, how about something like the following:
> 
> set_conditions() {
>      _set_nvme_trtype "$@"
> }
> 
> _delayed_nvme_reconnect_ctrl() {
>      sleep 2
>      _nvme_connect_subsys
> }
> 
> test() {
>      echo "Running ${TEST_NAME}"
> 
>      _setup_nvmet
> 
>      local nvmedev
>      local ns
>      local bytes_written
> 
>      _nvmet_target_setup
>      _nvme_connect_subsys
> 
>      # Part a: Ensure writes fail when no path returns
>      nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
>      ns=$(_find_nvme_ns "${def_subsys_uuid}")
>      echo 10 > "/sys/block/"$ns"/delayed_removal_secs"
>      bytes_written=$(run_xfs_io_pwritev2 /dev/"$ns" 4096)
>      if [ "$bytes_written" != 4096 ]; then
>          echo "could not write successfully initially"
>      fi
>      sleep 1
>      _nvme_disconnect_ctrl "${nvmedev}"
>      sleep 1
>      ns=$(_find_nvme_ns "${def_subsys_uuid}")
>      if [[ "${ns}" = "" ]]; then
>          echo "could not find ns after disconnect"
>      fi
>      bytes_written=$(run_xfs_io_pwritev2 /dev/"$ns" 4096)
>      if [ "$bytes_written" == 4096 ]; then
>          echo "wrote successfully after disconnect"
>      fi
>      sleep 10
>      ns=$(_find_nvme_ns "${def_subsys_uuid}")
>      if [[ !"${ns}" = "" ]]; then
>          echo "found ns after delayed removal"
>      fi
> 
>      #echo "now part 2"
>      # Part b: Ensure writes work for intermittent disconnect
>      _nvme_connect_subsys
> 
>      nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
>      ns=$(_find_nvme_ns "${def_subsys_uuid}")
>      echo 10 > "/sys/block/"$ns"/delayed_removal_secs"
>      bytes_written=$(run_xfs_io_pwritev2 /dev/"$ns" 4096)
>      if [ "$bytes_written" != 4096 ]; then
>          echo "could not write successfully initially"
>      fi
>      sleep 1
>      _nvme_disconnect_ctrl "${nvmedev}"
>      sleep 1
>      ns=$(_find_nvme_ns "${def_subsys_uuid}")
>      if [[ "${ns}" = "" ]]; then
>          echo "could not find ns after disconnect"
>      fi
>      _delayed_nvme_reconnect_ctrl &
>      sleep 1
>      bytes_written=$(run_xfs_io_pwritev2 /dev/"$ns" 4096)
>      if [ "$bytes_written" != 4096 ]; then
>          echo "could not write successfully with reconnect"
>      fi

It seems there may be a race here if we attempt to write to $ns before
the reconnect has completed in _delayed_nvme_reconnect_ctrl.

If the intention is simply to verify that the controller reconnect occurs
within the delayed removal window and test pwrite, then it may be sufficient
to:
- perform the reconnect, and
- then validate the write (pwrite) afterwards.

In that case, we could either:
- run _delayed_nvme_reconnect_ctrl in the foreground, or
- open-code the reconnect directly in the script before issuing the write.

>      sleep 10
>      ns=$(_find_nvme_ns "${def_subsys_uuid}")
>      if [[ "${ns}" = "" ]]; then
>          echo "could not find ns after delayed reconnect"
>      fi
> 
>      # Final tidy-up
>      echo 0 > /sys/block/"$ns"/delayed_removal_secs
>      nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
>      _nvme_disconnect_ctrl "${nvmedev}"
>      _nvmet_target_cleanup
> 
>      echo "Test complete"
> }

Otherwise overall this looks good to me.

Thanks,
--Nilay

^ permalink raw reply

* Re: [PATCH 8/8] RFC: use a TASK_FIFO kthread for read completion support
From: Christoph Hellwig @ 2026-04-10  6:19 UTC (permalink / raw)
  To: Tal Zussman
  Cc: Christoph Hellwig, Jens Axboe, Matthew Wilcox (Oracle),
	Christian Brauner, Darrick J. Wong, Carlos Maiolino, Al Viro,
	Jan Kara, Dave Chinner, Bart Van Assche, Gao Xiang, linux-block,
	linux-kernel, linux-xfs, linux-fsdevel, linux-mm
In-Reply-To: <2cdaa767-c071-4e84-b9d7-1c944407f5bb@columbia.edu>

On Thu, Apr 09, 2026 at 03:06:47PM -0400, Tal Zussman wrote:
> > -#include <linux/llist.h>
> > +#include <linux/freezer.h>
> 
> Why freezer.h and not kthread.h?

I needed freezer.h to try to make the thread freezable, but that didn't
work out.  kthread.h seems to be pulled in implicitly.

> >   struct bio_complete_batch {
> > -	struct llist_head list;
> 
> If we go with this approach, we should remove the newly-added bi_llist from
> struct bio too.

Yes.


^ permalink raw reply

* Re: [PATCH 4/8] FOLD: block: change the defer in task context interface to be procedural
From: Christoph Hellwig @ 2026-04-10  6:17 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Christoph Hellwig, Tal Zussman, Jens Axboe, Christian Brauner,
	Darrick J. Wong, Carlos Maiolino, Al Viro, Jan Kara, Dave Chinner,
	Bart Van Assche, Gao Xiang, linux-block, linux-kernel, linux-xfs,
	linux-fsdevel, linux-mm
In-Reply-To: <adgJqiA0vivaW7NA@casper.infradead.org>

On Thu, Apr 09, 2026 at 09:18:50PM +0100, Matthew Wilcox wrote:
> On Thu, Apr 09, 2026 at 06:02:17PM +0200, Christoph Hellwig wrote:
> > @@ -1836,9 +1837,7 @@ void bio_endio(struct bio *bio)
> >  	}
> >  #endif
> >  
> > -	if (!in_task() && bio_flagged(bio, BIO_COMPLETE_IN_TASK))
> > -		bio_queue_completion(bio);
> > -	else if (bio->bi_end_io)
> > +	if (bio->bi_end_io)
> >  		bio->bi_end_io(bio);
> 
> What I liked about this before is that we had one central place that
> needed to be changed.  This change means that every bi_end_io now needs
> to check whether the BIO can be completed in its context.

Yes.  On the other hand we can actually use it when we don't know if
we need to offload beforehand, which enabls the two later conversions
and probably more.

> 
> > +++ b/fs/buffer.c
> > @@ -2673,6 +2673,9 @@ static void end_bio_bh_io_sync(struct bio *bio)
> >  {
> >  	struct buffer_head *bh = bio->bi_private;
> >  
> > +	if (buffer_dropbehind(bh) && bio_complete_in_task(bio))
> > +		return;
> 
> I really don't like this.  It assumes there's only one reason to
> complete in task context -- whether the buffer belongs to a dropbehind
> folio.  I want there to be other reasons.  Why would you introduce the
> new BH_dropbehind flag instead of checking BIO_COMPLETE_IN_TASK?

Because there's no BIO_COMPLETE_IN_TASK? left.

> >  	struct iomap_ioend *ioend = iomap_ioend_from_bio(bio);
> >  
> > +	/* Page cache invalidation cannot be done in irq context. */
> > +	if (ioend->io_flags & IOMAP_IOEND_DONTCACHE) {
> > +		if (bio_complete_in_task(bio))
> > +			return;
> > +	}
> 
> I thought we agreed to kill off IOMAP_IOEND_DONTCACHE?

Only IFF we don't need it.  With this version there is no way to just
remove it.

^ permalink raw reply

* [PATCH blktests 1/1] common/scsi_debug: use _patient_rmmod() to unload scsi_debug
From: Shin'ichiro Kawasaki @ 2026-04-10  5:30 UTC (permalink / raw)
  To: linux-block; +Cc: Yi Zhang, Shin'ichiro Kawasaki

The helper _have_scsi_debug_group_number_stats() loads and unloads
scsi_debug to check parameters of scsi_debug. However, the unload is too
soon after the load, then it often fails. To avoid such unload failures,
use _patient_rmmod() to unload scsi_debug.

Reported-by: Yi Zhang <yi.zhang@redhat.com>
Closes: https://github.com/linux-blktests/blktests/issues/241
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 common/scsi_debug | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/scsi_debug b/common/scsi_debug
index 6aa7420..86ea3f4 100644
--- a/common/scsi_debug
+++ b/common/scsi_debug
@@ -57,7 +57,7 @@ _have_scsi_debug_group_number_stats() {
 		SKIP_REASONS+=("scsi_debug does not support group number statistics")
 		ret=1
 	fi
-	modprobe -qr scsi_debug >&/dev/null
+	_patient_rmmod scsi_debug >&/dev/null
 	return ${ret}
 }
 
-- 
2.49.0


^ permalink raw reply related

* Re: [PATCH 2/2] block: allow different-pgmap pages as separate bvecs in bio_add_page
From: Naman Jain @ 2026-04-10  3:38 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Chaitanya Kulkarni, John Hubbard, Logan Gunthorpe,
	linux-kernel, linux-block, Saurabh Sengar, Long Li,
	Michael Kelley
In-Reply-To: <20260408060825.GA24532@lst.de>



On 4/8/2026 11:38 AM, Christoph Hellwig wrote:
> Hi Naman,
> 
> On Tue, Apr 07, 2026 at 12:38:30PM +0530, Naman Jain wrote:
>>> So the zone_device_pages_have_same_pgmap check should go into
>>> zone_device_pages_compatible and we need to stop building the bio
>>> as well in that case.
>>
>> Ok, so rest all things same, from my last email, but my previous compatible
>> function would look like this:
>>
>> static inline bool zone_device_pages_compatible(const struct page *a,
>>                          const struct page *b)
>> {
>>      if (is_pci_p2pdma_page(a) || is_pci_p2pdma_page(b))
>>          return zone_device_pages_have_same_pgmap(a, b);
>>      return true;
>> }
>>
>> This would prevent two P2PDMA pages from different pgmaps (different PCI
>> devices) passing the compatible check and both get added to the bio.
>> Please correct me if that is not what you meant. I'll wait for a couple
>> more days and send the next version with this, and we can review this
>> again.
> 
> This looks good modulo the tabs vs spaces in the indentation.

Hi,
Thanks.

Regards,
Naman


^ permalink raw reply

* Re: [PATCH v10 13/13] docs: add io_queue flag to isolcpus
From: Ming Lei @ 2026-04-10  2:44 UTC (permalink / raw)
  To: Aaron Tomlin
  Cc: Ming Lei, axboe, kbusch, hch, sagi, mst, aacraid, James.Bottomley,
	martin.petersen, liyihang9, kashyap.desai, sumit.saxena,
	shivasharan.srikanteshwara, chandrakanth.patil, sathya.prakash,
	sreekanth.reddy, suganath-prabu.subramani, ranjan.kumar,
	jinpu.wang, tglx, mingo, peterz, juri.lelli, vincent.guittot,
	akpm, maz, ruanjinjie, bigeasy, yphbchou0911, wagi, frederic,
	longman, chenridong, hare, kch, steve, sean, chjohnst, neelx,
	mproche, linux-block, linux-kernel, virtualization, linux-nvme,
	linux-scsi, megaraidlinux.pdl, mpi3mr-linuxdrv.pdl,
	MPT-FusionLinux.pdl
In-Reply-To: <a566smu6morqeefqal23eek4ibezfuiwhs774xtxhyyclpbtsx@uzzwgbzmwdjd>

On Thu, Apr 09, 2026 at 09:45:04PM -0400, Aaron Tomlin wrote:
> On Thu, Apr 09, 2026 at 11:00:09PM +0800, Ming Lei wrote:
> > How can the isolated core be scheduled for running polling task?
> > 
> > Who triggered it?
> > 
> > > loop waiting for the hardware completion. This would completely monopolise
> > > the core and destroy any real time isolation guarantees without the user
> > > space application ever having requested it.
> > 
> > No.
> > 
> > IOPOLL queue doesn't have interrupt, and the ->poll() is only run from
> > the submission context.  So if you don't submitted polled IO on isolated
> > CPU cores, everything is just fine.  This is simpler than irq IO actually.
> 
> Yes, you are entirely correct. The ->iopoll() is indeed executed strictly
> within the submission context. In the example below, the file operations
> iopoll callback is iocb_bio_iopoll():
> 
>       // file->f_op->iopoll(&rw->kiocb, iob, poll_flags)
>       iocb_bio_iopoll(&rw->kiocb, iob, poll_flags)
>       {
>         struct bio *bio
> 
>         bio = READ_ONCE(kiocb->private)
>         if (bio)
>           bio_poll(bio, iob, flags)
>             if (queue_is_mq(q))
>               blk_mq_poll(q, cookie, iob, flags)
>               {
>                 if (!blk_mq_can_poll(q))
>                   return 0
> 
>                 blk_hctx_poll(q, q->queue_hw_ctx[cookie], iob, flags)
>                 {
>                     int ret
> 
>                     do {
>                         ret = q->mq_ops->poll(hctx, iob)
>                         if (ret > 0)
>                             return ret
>                         if (task_sigpending(current))
>                             return 1
>                         if (ret < 0 || (flags & BLK_POLL_ONESHOT))
>                             break
>                         cpu_relax()
>                     } while (!need_resched())
> 
>                     return 0
>                 }
>               }
> 
> If an application on an isolated CPU does not explicitly submit a polled
> I/O request, it will not poll. Thank you for correcting me on this.

Great, you finally get the point.

> 
> > Can you share one example in which managed irq can't address?
> 
> Without io_queue, the block layer maps isolated CPUs to these queues, and
> the device will fire unmanaged interrupts that can freely land on isolated

For unmanaged interrupts, user can set irq affinity on housekeeping cpus
from /proc or kernel command line.

Why is unmanaged interrupts involved with this patchset?

> CPUs, thereby breaking isolation. By applying the constraint via io_queue
> at the block layer, we restrict the hardware queue count and map the
> isolated CPUs to the housekeeping queues, ensuring isolation is maintained
> regardless of whether the driver uses managed interrupts.
> 
> Does the above help?

As I mentioned, managed irq already covers it:

- typically application submits IO from housekeeping CPUs, which is mapped
  to one hardware, which effective interrupt affinity excludes isolated
  CPUs if possible.

I'd suggest to share some real problems you found instead of something
imaginary.

> 
> > > >
> > > > IMO, only two differences from this viewpoint:
> > > >
> > > > 1) `io_queue` may reduce nr_hw_queues
> > > >
> > > > 2) when application submits IO from isolated CPUs, `io_queue` can complete
> > > > IO from housekeeping CPUs.
> > >
> > > Acknowledged.
> > 
> > Are there other major differences besides the two mentioned above?
> 
> I believe the above is sufficient. Please let me know your thoughts.

Both two are small improvement, not bug fixes. However the user has to pay
the cost of potential failing of offlining CPU. Not mention the little 
complicated change: `19 files changed, 378 insertions(+), 48 deletions(-)`

But I won't object if you can update the commit log/kernel command line
doc and fix the issue found in review.

Thanks,
Ming

^ permalink raw reply

* Re: [PATCH v2 01/10] ublk: add UBLK_U_CMD_REG_BUF/UNREG_BUF control commands
From: Ming Lei @ 2026-04-10  2:28 UTC (permalink / raw)
  To: Caleb Sander Mateos; +Cc: Ming Lei, Jens Axboe, linux-block
In-Reply-To: <CADUfDZqnq1Ls9NdcBXgbAJymPNvcUOa883zSTNMNcaXODGEEEg@mail.gmail.com>

On Thu, Apr 09, 2026 at 02:22:24PM -0700, Caleb Sander Mateos wrote:
> On Thu, Apr 9, 2026 at 5:18 AM Ming Lei <tom.leiming@gmail.com> wrote:
> >
> > On Wed, Apr 08, 2026 at 08:20:12AM -0700, Caleb Sander Mateos wrote:
> > > On Tue, Apr 7, 2026 at 7:23 PM Ming Lei <ming.lei@redhat.com> wrote:
> > > >
> > > > On Tue, Apr 07, 2026 at 12:35:49PM -0700, Caleb Sander Mateos wrote:
> > > > > On Tue, Mar 31, 2026 at 8:32 AM Ming Lei <ming.lei@redhat.com> wrote:
> > > > > >
> > > > > > Add control commands for registering and unregistering shared memory
> > > > > > buffers for zero-copy I/O:
> > > > > >
> > > > > > - UBLK_U_CMD_REG_BUF (0x18): pins pages from userspace, inserts PFN
> > > > > >   ranges into a per-device maple tree for O(log n) lookup during I/O.
> > > > > >   Buffer pointers are tracked in a per-device xarray. Returns the
> > > > > >   assigned buffer index.
> > > > > >
> > > > > > - UBLK_U_CMD_UNREG_BUF (0x19): removes PFN entries and unpins pages.
> > > > > >
> > > > > > Queue freeze/unfreeze is handled internally so userspace need not
> > > > > > quiesce the device during registration.
> > > > > >
> > > > > > Also adds:
> > > > > > - UBLK_IO_F_SHMEM_ZC flag and addr encoding helpers in UAPI header
> > > > > >   (16-bit buffer index supporting up to 65536 buffers)
> > > > > > - Data structures (ublk_buf, ublk_buf_range) and xarray/maple tree
> > > > > > - __ublk_ctrl_reg_buf() helper for PFN insertion with error unwinding
> > > > > > - __ublk_ctrl_unreg_buf() helper for cleanup reuse
> > > > > > - ublk_support_shmem_zc() / ublk_dev_support_shmem_zc() stubs
> > > > > >   (returning false — feature not enabled yet)
> > > > > >
> > > > > > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > > > > > ---
> > > > > >  drivers/block/ublk_drv.c      | 300 ++++++++++++++++++++++++++++++++++
> > > > > >  include/uapi/linux/ublk_cmd.h |  72 ++++++++
> > > > > >  2 files changed, 372 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > > > > > index 71c7c56b38ca..ac6ccc174d44 100644
> > > > > > --- a/drivers/block/ublk_drv.c
> > > > > > +++ b/drivers/block/ublk_drv.c
> > > > > > @@ -46,6 +46,8 @@
> > > > > >  #include <linux/kref.h>
> > > > > >  #include <linux/kfifo.h>
> > > > > >  #include <linux/blk-integrity.h>
> > > > > > +#include <linux/maple_tree.h>
> > > > > > +#include <linux/xarray.h>
> > > > > >  #include <uapi/linux/fs.h>
> > > > > >  #include <uapi/linux/ublk_cmd.h>
> > > > > >
> > > > > > @@ -58,6 +60,8 @@
> > > > > >  #define UBLK_CMD_UPDATE_SIZE   _IOC_NR(UBLK_U_CMD_UPDATE_SIZE)
> > > > > >  #define UBLK_CMD_QUIESCE_DEV   _IOC_NR(UBLK_U_CMD_QUIESCE_DEV)
> > > > > >  #define UBLK_CMD_TRY_STOP_DEV  _IOC_NR(UBLK_U_CMD_TRY_STOP_DEV)
> > > > > > +#define UBLK_CMD_REG_BUF       _IOC_NR(UBLK_U_CMD_REG_BUF)
> > > > > > +#define UBLK_CMD_UNREG_BUF     _IOC_NR(UBLK_U_CMD_UNREG_BUF)
> > > > > >
> > > > > >  #define UBLK_IO_REGISTER_IO_BUF                _IOC_NR(UBLK_U_IO_REGISTER_IO_BUF)
> > > > > >  #define UBLK_IO_UNREGISTER_IO_BUF      _IOC_NR(UBLK_U_IO_UNREGISTER_IO_BUF)
> > > > > > @@ -289,6 +293,20 @@ struct ublk_queue {
> > > > > >         struct ublk_io ios[] __counted_by(q_depth);
> > > > > >  };
> > > > > >
> > > > > > +/* Per-registered shared memory buffer */
> > > > > > +struct ublk_buf {
> > > > > > +       struct page **pages;
> > > > > > +       unsigned int nr_pages;
> > > > > > +};
> > > > > > +
> > > > > > +/* Maple tree value: maps a PFN range to buffer location */
> > > > > > +struct ublk_buf_range {
> > > > > > +       unsigned long base_pfn;
> > > > > > +       unsigned short buf_index;
> > > > > > +       unsigned short flags;
> > > > > > +       unsigned int base_offset;       /* byte offset within buffer */
> > > > > > +};
> > > > > > +
> > > > > >  struct ublk_device {
> > > > > >         struct gendisk          *ub_disk;
> > > > > >
> > > > > > @@ -323,6 +341,10 @@ struct ublk_device {
> > > > > >
> > > > > >         bool                    block_open; /* protected by open_mutex */
> > > > > >
> > > > > > +       /* shared memory zero copy */
> > > > > > +       struct maple_tree       buf_tree;
> > > > > > +       struct xarray           bufs_xa;
> > > > > > +
> > > > > >         struct ublk_queue       *queues[];
> > > > > >  };
> > > > > >
> > > > > > @@ -334,6 +356,7 @@ struct ublk_params_header {
> > > > > >
> > > > > >  static void ublk_io_release(void *priv);
> > > > > >  static void ublk_stop_dev_unlocked(struct ublk_device *ub);
> > > > > > +static void ublk_buf_cleanup(struct ublk_device *ub);
> > > > > >  static void ublk_abort_queue(struct ublk_device *ub, struct ublk_queue *ubq);
> > > > > >  static inline struct request *__ublk_check_and_get_req(struct ublk_device *ub,
> > > > > >                 u16 q_id, u16 tag, struct ublk_io *io);
> > > > > > @@ -398,6 +421,16 @@ static inline bool ublk_dev_support_zero_copy(const struct ublk_device *ub)
> > > > > >         return ub->dev_info.flags & UBLK_F_SUPPORT_ZERO_COPY;
> > > > > >  }
> > > > > >
> > > > > > +static inline bool ublk_support_shmem_zc(const struct ublk_queue *ubq)
> > > > > > +{
> > > > > > +       return false;
> > > > > > +}
> > > > > > +
> > > > > > +static inline bool ublk_dev_support_shmem_zc(const struct ublk_device *ub)
> > > > > > +{
> > > > > > +       return false;
> > > > > > +}
> > > > > > +
> > > > > >  static inline bool ublk_support_auto_buf_reg(const struct ublk_queue *ubq)
> > > > > >  {
> > > > > >         return ubq->flags & UBLK_F_AUTO_BUF_REG;
> > > > > > @@ -1460,6 +1493,7 @@ static blk_status_t ublk_setup_iod(struct ublk_queue *ubq, struct request *req)
> > > > > >         iod->op_flags = ublk_op | ublk_req_build_flags(req);
> > > > > >         iod->nr_sectors = blk_rq_sectors(req);
> > > > > >         iod->start_sector = blk_rq_pos(req);
> > > > > > +
> > > > >
> > > > > nit: unrelated whitespace change?
> > > > >
> > > > > >         iod->addr = io->buf.addr;
> > > > > >
> > > > > >         return BLK_STS_OK;
> > > > > > @@ -1665,6 +1699,7 @@ static bool ublk_start_io(const struct ublk_queue *ubq, struct request *req,
> > > > > >  {
> > > > > >         unsigned mapped_bytes = ublk_map_io(ubq, req, io);
> > > > > >
> > > > > > +
> > > > >
> > > > > nit: unrelated whitespace change?
> > > > >
> > > > > >         /* partially mapped, update io descriptor */
> > > > > >         if (unlikely(mapped_bytes != blk_rq_bytes(req))) {
> > > > > >                 /*
> > > > > > @@ -4206,6 +4241,7 @@ static void ublk_cdev_rel(struct device *dev)
> > > > > >  {
> > > > > >         struct ublk_device *ub = container_of(dev, struct ublk_device, cdev_dev);
> > > > > >
> > > > > > +       ublk_buf_cleanup(ub);
> > > > > >         blk_mq_free_tag_set(&ub->tag_set);
> > > > > >         ublk_deinit_queues(ub);
> > > > > >         ublk_free_dev_number(ub);
> > > > > > @@ -4625,6 +4661,8 @@ static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_cmd *header)
> > > > > >         mutex_init(&ub->mutex);
> > > > > >         spin_lock_init(&ub->lock);
> > > > > >         mutex_init(&ub->cancel_mutex);
> > > > > > +       mt_init(&ub->buf_tree);
> > > > > > +       xa_init_flags(&ub->bufs_xa, XA_FLAGS_ALLOC);
> > > > > >         INIT_WORK(&ub->partition_scan_work, ublk_partition_scan_work);
> > > > > >
> > > > > >         ret = ublk_alloc_dev_number(ub, header->dev_id);
> > > > > > @@ -5168,6 +5206,260 @@ static int ublk_char_dev_permission(struct ublk_device *ub,
> > > > > >         return err;
> > > > > >  }
> > > > > >
> > > > > > +/*
> > > > > > + * Drain inflight I/O and quiesce the queue. Freeze drains all inflight
> > > > > > + * requests, quiesce_nowait marks the queue so no new requests dispatch,
> > > > > > + * then unfreeze allows new submissions (which won't dispatch due to
> > > > > > + * quiesce). This keeps freeze and ub->mutex non-nested.
> > > > > > + */
> > > > > > +static void ublk_quiesce_and_release(struct gendisk *disk)
> > > > > > +{
> > > > > > +       unsigned int memflags;
> > > > > > +
> > > > > > +       memflags = blk_mq_freeze_queue(disk->queue);
> > > > > > +       blk_mq_quiesce_queue_nowait(disk->queue);
> > > > > > +       blk_mq_unfreeze_queue(disk->queue, memflags);
> > > > > > +}
> > > > > > +
> > > > > > +static void ublk_unquiesce_and_resume(struct gendisk *disk)
> > > > > > +{
> > > > > > +       blk_mq_unquiesce_queue(disk->queue);
> > > > > > +}
> > > > > > +
> > > > > > +/*
> > > > > > + * Insert PFN ranges of a registered buffer into the maple tree,
> > > > > > + * coalescing consecutive PFNs into single range entries.
> > > > > > + * Returns 0 on success, negative error with partial insertions unwound.
> > > > > > + */
> > > > > > +/* Erase coalesced PFN ranges from the maple tree for pages [0, nr_pages) */
> > > > > > +static void ublk_buf_erase_ranges(struct ublk_device *ub,
> > > > > > +                                 struct ublk_buf *ubuf,
> > > > > > +                                 unsigned long nr_pages)
> > > > > > +{
> > > > > > +       unsigned long i;
> > > > > > +
> > > > > > +       for (i = 0; i < nr_pages; ) {
> > > > > > +               unsigned long pfn = page_to_pfn(ubuf->pages[i]);
> > > > > > +               unsigned long start = i;
> > > > > > +
> > > > > > +               while (i + 1 < nr_pages &&
> > > > > > +                      page_to_pfn(ubuf->pages[i + 1]) == pfn + (i - start) + 1)
> > > > > > +                       i++;
> > > > > > +               i++;
> > > > > > +               kfree(mtree_erase(&ub->buf_tree, pfn));
> > > > > > +       }
> > > > > > +}
> > > > > > +
> > > > > > +static int __ublk_ctrl_reg_buf(struct ublk_device *ub,
> > > > > > +                              struct ublk_buf *ubuf, int index,
> > > > > > +                              unsigned short flags)
> > > > > > +{
> > > > > > +       unsigned long nr_pages = ubuf->nr_pages;
> > > > > > +       unsigned long i;
> > > > > > +       int ret;
> > > > > > +
> > > > > > +       for (i = 0; i < nr_pages; ) {
> > > > > > +               unsigned long pfn = page_to_pfn(ubuf->pages[i]);
> > > > > > +               unsigned long start = i;
> > > > > > +               struct ublk_buf_range *range;
> > > > > > +
> > > > > > +               /* Find run of consecutive PFNs */
> > > > > > +               while (i + 1 < nr_pages &&
> > > > > > +                      page_to_pfn(ubuf->pages[i + 1]) == pfn + (i - start) + 1)
> > > > > > +                       i++;
> > > > > > +               i++;    /* past the last page in this run */
> > > > >
> > > > > Move this increment to the for loop so you don't need the "- 1" in the
> > > > > mtree_insert_range() call?
> > > >
> > > > Good catch!
> > > >
> > > > >
> > > > > > +
> > > > > > +               range = kzalloc(sizeof(*range), GFP_KERNEL);
> > > > >
> > > > > Not sure kzalloc() is necessary; all the fields are initialized below
> > > >
> > > > Yeah, kmalloc() is fine, and we shouldn't add more fields to `range` in
> > > > future.
> > > >
> > > > >
> > > > > > +               if (!range) {
> > > > > > +                       ret = -ENOMEM;
> > > > > > +                       goto unwind;
> > > > > > +               }
> > > > > > +               range->buf_index = index;
> > > > > > +               range->flags = flags;
> > > > > > +               range->base_pfn = pfn;
> > > > > > +               range->base_offset = start << PAGE_SHIFT;
> > > > > > +
> > > > > > +               ret = mtree_insert_range(&ub->buf_tree, pfn,
> > > > > > +                                        pfn + (i - start) - 1,
> > > > > > +                                        range, GFP_KERNEL);
> > > > > > +               if (ret) {
> > > > > > +                       kfree(range);
> > > > > > +                       goto unwind;
> > > > > > +               }
> > > > > > +       }
> > > > > > +       return 0;
> > > > > > +
> > > > > > +unwind:
> > > > > > +       ublk_buf_erase_ranges(ub, ubuf, i);
> > > > > > +       return ret;
> > > > > > +}
> > > > > > +
> > > > > > +/*
> > > > > > + * Register a shared memory buffer for zero-copy I/O.
> > > > > > + * Pins pages, builds PFN maple tree, freezes/unfreezes the queue
> > > > > > + * internally. Returns buffer index (>= 0) on success.
> > > > > > + */
> > > > > > +static int ublk_ctrl_reg_buf(struct ublk_device *ub,
> > > > > > +                            struct ublksrv_ctrl_cmd *header)
> > > > > > +{
> > > > > > +       void __user *argp = (void __user *)(unsigned long)header->addr;
> > > > > > +       struct ublk_shmem_buf_reg buf_reg;
> > > > > > +       unsigned long addr, size, nr_pages;
> > > > >
> > > > > size and nr_pages could be u32
> > > >
> > > > Yeah, it was caused by internal change on `ublk_shmem_buf_reg`.
> > > >
> > > > >
> > > > > > +       unsigned int gup_flags;
> > > > > > +       struct gendisk *disk;
> > > > > > +       struct ublk_buf *ubuf;
> > > > > > +       long pinned;
> > > > >
> > > > > pinned could be int to match the return type of pin_user_pages_fast()
> > > >
> > > > OK.
> > > >
> > > > >
> > > > > > +       u32 index;
> > > > > > +       int ret;
> > > > > > +
> > > > > > +       if (!ublk_dev_support_shmem_zc(ub))
> > > > > > +               return -EOPNOTSUPP;
> > > > > > +
> > > > > > +       memset(&buf_reg, 0, sizeof(buf_reg));
> > > > > > +       if (copy_from_user(&buf_reg, argp,
> > > > > > +                          min_t(size_t, header->len, sizeof(buf_reg))))
> > > > > > +               return -EFAULT;
> > > > > > +
> > > > > > +       if (buf_reg.flags & ~UBLK_SHMEM_BUF_READ_ONLY)
> > > > > > +               return -EINVAL;
> > > > > > +
> > > > > > +       addr = buf_reg.addr;
> > > > > > +       size = buf_reg.len;
> > > > >
> > > > > nit: don't see much value in these additional variables that are just
> > > > > copies of buf_reg fields
> > > > >
> > > > > > +       nr_pages = size >> PAGE_SHIFT;
> > > > > > +
> > > > > > +       if (!size || !PAGE_ALIGNED(size) || !PAGE_ALIGNED(addr))
> > > > > > +               return -EINVAL;
> > > > > > +
> > > > > > +       disk = ublk_get_disk(ub);
> > > > > > +       if (!disk)
> > > > > > +               return -ENODEV;
> > > > >
> > > > > So buffers can't be registered before the ublk device is started? Is
> > > > > there a reason why that's not possible? Could we just make the
> > > > > ublk_quiesce_and_release() and ublk_unquiesce_and_resume() conditional
> > > > > on disk being non-NULL? I guess we'd have to hold the ublk_device
> > > > > mutex before calling ublk_get_disk() to prevent it from being assigned
> > > > > concurrently.
> > > >
> > > > Here `disk` is used for freeze & quiesce queue.
> > > >
> > > > But the implementation can be a bit more complicated given the dependency
> > > > between ub->mutex and freeze queue should be avoided.
> > > >
> > > > Anyway, it is one nice requirement, I will try to relax the constraint.
> > > >
> > > > >
> > > > > > +
> > > > > > +       /* Pin pages before quiescing (may sleep) */
> > > > > > +       ubuf = kzalloc(sizeof(*ubuf), GFP_KERNEL);
> > > > > > +       if (!ubuf) {
> > > > > > +               ret = -ENOMEM;
> > > > > > +               goto put_disk;
> > > > > > +       }
> > > > > > +
> > > > > > +       ubuf->pages = kvmalloc_array(nr_pages, sizeof(*ubuf->pages),
> > > > > > +                                    GFP_KERNEL);
> > > > > > +       if (!ubuf->pages) {
> > > > > > +               ret = -ENOMEM;
> > > > > > +               goto err_free;
> > > > > > +       }
> > > > > > +
> > > > > > +       gup_flags = FOLL_LONGTERM;
> > > > > > +       if (!(buf_reg.flags & UBLK_SHMEM_BUF_READ_ONLY))
> > > > > > +               gup_flags |= FOLL_WRITE;
> > > > > > +
> > > > > > +       pinned = pin_user_pages_fast(addr, nr_pages, gup_flags, ubuf->pages);
> > > > > > +       if (pinned < 0) {
> > > > > > +               ret = pinned;
> > > > > > +               goto err_free_pages;
> > > > > > +       }
> > > > > > +       if (pinned != nr_pages) {
> > > > > > +               ret = -EFAULT;
> > > > > > +               goto err_unpin;
> > > > > > +       }
> > > > > > +       ubuf->nr_pages = nr_pages;
> > > > > > +
> > > > > > +       /*
> > > > > > +        * Drain inflight I/O and quiesce the queue so no new requests
> > > > > > +        * are dispatched while we modify the maple tree. Keep freeze
> > > > > > +        * and mutex non-nested to avoid lock dependency.
> > > > > > +        */
> > > > > > +       ublk_quiesce_and_release(disk);
> > > > > > +
> > > > > > +       mutex_lock(&ub->mutex);
> > > > >
> > > > > Looks like the xarray and maple tree do their own spinlocking, is this needed?
> > > >
> > > > Right, looks it isn't needed now, and it was added from beginning with plain
> > > > array.
> > > >
> > > > >
> > > > > > +
> > > > > > +       ret = xa_alloc(&ub->bufs_xa, &index, ubuf, xa_limit_16b, GFP_KERNEL);
> > > > > > +       if (ret)
> > > > > > +               goto err_unlock;
> > > > > > +
> > > > > > +       ret = __ublk_ctrl_reg_buf(ub, ubuf, index, buf_reg.flags);
> > > > > > +       if (ret) {
> > > > > > +               xa_erase(&ub->bufs_xa, index);
> > > > > > +               goto err_unlock;
> > > > > > +       }
> > > > > > +
> > > > > > +       mutex_unlock(&ub->mutex);
> > > > > > +
> > > > > > +       ublk_unquiesce_and_resume(disk);
> > > > > > +       ublk_put_disk(disk);
> > > > > > +       return index;
> > > > > > +
> > > > > > +err_unlock:
> > > > > > +       mutex_unlock(&ub->mutex);
> > > > > > +       ublk_unquiesce_and_resume(disk);
> > > > > > +err_unpin:
> > > > > > +       unpin_user_pages(ubuf->pages, pinned);
> > > > > > +err_free_pages:
> > > > > > +       kvfree(ubuf->pages);
> > > > > > +err_free:
> > > > > > +       kfree(ubuf);
> > > > > > +put_disk:
> > > > > > +       ublk_put_disk(disk);
> > > > > > +       return ret;
> > > > > > +}
> > > > > > +
> > > > > > +static void __ublk_ctrl_unreg_buf(struct ublk_device *ub,
> > > > > > +                                 struct ublk_buf *ubuf)
> > > > > > +{
> > > > > > +       ublk_buf_erase_ranges(ub, ubuf, ubuf->nr_pages);
> > > > > > +       unpin_user_pages(ubuf->pages, ubuf->nr_pages);
> > > > > > +       kvfree(ubuf->pages);
> > > > > > +       kfree(ubuf);
> > > > > > +}
> > > > > > +
> > > > > > +static int ublk_ctrl_unreg_buf(struct ublk_device *ub,
> > > > > > +                              struct ublksrv_ctrl_cmd *header)
> > > > > > +{
> > > > > > +       int index = (int)header->data[0];
> > > > > > +       struct gendisk *disk;
> > > > > > +       struct ublk_buf *ubuf;
> > > > > > +
> > > > > > +       if (!ublk_dev_support_shmem_zc(ub))
> > > > > > +               return -EOPNOTSUPP;
> > > > > > +
> > > > > > +       disk = ublk_get_disk(ub);
> > > > > > +       if (!disk)
> > > > > > +               return -ENODEV;
> > > > > > +
> > > > > > +       /* Drain inflight I/O before modifying the maple tree */
> > > > > > +       ublk_quiesce_and_release(disk);
> > > > > > +
> > > > > > +       mutex_lock(&ub->mutex);
> > > > > > +
> > > > > > +       ubuf = xa_erase(&ub->bufs_xa, index);
> > > > > > +       if (!ubuf) {
> > > > > > +               mutex_unlock(&ub->mutex);
> > > > > > +               ublk_unquiesce_and_resume(disk);
> > > > > > +               ublk_put_disk(disk);
> > > > > > +               return -ENOENT;
> > > > > > +       }
> > > > > > +
> > > > > > +       __ublk_ctrl_unreg_buf(ub, ubuf);
> > > > > > +
> > > > > > +       mutex_unlock(&ub->mutex);
> > > > > > +
> > > > > > +       ublk_unquiesce_and_resume(disk);
> > > > > > +       ublk_put_disk(disk);
> > > > > > +       return 0;
> > > > > > +}
> > > > > > +
> > > > > > +static void ublk_buf_cleanup(struct ublk_device *ub)
> > > > > > +{
> > > > > > +       struct ublk_buf *ubuf;
> > > > > > +       unsigned long index;
> > > > > > +
> > > > > > +       xa_for_each(&ub->bufs_xa, index, ubuf)
> > > > > > +               __ublk_ctrl_unreg_buf(ub, ubuf);
> > > > >
> > > > > This looks quadratic in the number of registered buffers. Can we do a
> > > > > single pass over the xarray  and the maple tree?
> > > >
> > > > It can be done two passes: first pass walks maple tree for unpinning
> > > > pages, the 2nd pass is for freeing buffers in xarray, which looks more
> > > > clean.
> > > >
> > > > But the current way can reuse __ublk_ctrl_unreg_buf(), also
> > > > ublk_buf_cleanup() is only called one-shot in device release handler, so we can
> > > > leave it for future optimization.
> > > >
> > > > >
> > > > > > +       xa_destroy(&ub->bufs_xa);
> > > > > > +       mtree_destroy(&ub->buf_tree);
> > > > > > +}
> > > > > > +
> > > > > > +
> > > > > > +
> > > > > >  static int ublk_ctrl_uring_cmd_permission(struct ublk_device *ub,
> > > > > >                 u32 cmd_op, struct ublksrv_ctrl_cmd *header)
> > > > > >  {
> > > > > > @@ -5225,6 +5517,8 @@ static int ublk_ctrl_uring_cmd_permission(struct ublk_device *ub,
> > > > > >         case UBLK_CMD_UPDATE_SIZE:
> > > > > >         case UBLK_CMD_QUIESCE_DEV:
> > > > > >         case UBLK_CMD_TRY_STOP_DEV:
> > > > > > +       case UBLK_CMD_REG_BUF:
> > > > > > +       case UBLK_CMD_UNREG_BUF:
> > > > > >                 mask = MAY_READ | MAY_WRITE;
> > > > > >                 break;
> > > > > >         default:
> > > > > > @@ -5350,6 +5644,12 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd,
> > > > > >         case UBLK_CMD_TRY_STOP_DEV:
> > > > > >                 ret = ublk_ctrl_try_stop_dev(ub);
> > > > > >                 break;
> > > > > > +       case UBLK_CMD_REG_BUF:
> > > > > > +               ret = ublk_ctrl_reg_buf(ub, &header);
> > > > > > +               break;
> > > > > > +       case UBLK_CMD_UNREG_BUF:
> > > > > > +               ret = ublk_ctrl_unreg_buf(ub, &header);
> > > > > > +               break;
> > > > > >         default:
> > > > > >                 ret = -EOPNOTSUPP;
> > > > > >                 break;
> > > > > > diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> > > > > > index a88876756805..52bb9b843d73 100644
> > > > > > --- a/include/uapi/linux/ublk_cmd.h
> > > > > > +++ b/include/uapi/linux/ublk_cmd.h
> > > > > > @@ -57,6 +57,44 @@
> > > > > >         _IOWR('u', 0x16, struct ublksrv_ctrl_cmd)
> > > > > >  #define UBLK_U_CMD_TRY_STOP_DEV                \
> > > > > >         _IOWR('u', 0x17, struct ublksrv_ctrl_cmd)
> > > > > > +/*
> > > > > > + * Register a shared memory buffer for zero-copy I/O.
> > > > > > + * Input:  ctrl_cmd.addr points to struct ublk_buf_reg (buffer VA + size)
> > > > > > + *         ctrl_cmd.len  = sizeof(struct ublk_buf_reg)
> > > > > > + * Result: >= 0 is the assigned buffer index, < 0 is error
> > > > > > + *
> > > > > > + * The kernel pins pages from the calling process's address space
> > > > > > + * and inserts PFN ranges into a per-device maple tree. When a block
> > > > > > + * request's pages match registered pages, the driver sets
> > > > > > + * UBLK_IO_F_SHMEM_ZC and encodes the buffer index + offset in addr,
> > > > > > + * allowing the server to access the data via its own mapping of the
> > > > > > + * same shared memory — true zero copy.
> > > > > > + *
> > > > > > + * The memory can be backed by memfd, hugetlbfs, or any GUP-compatible
> > > > > > + * shared mapping. Queue freeze is handled internally.
> > > > > > + *
> > > > > > + * The buffer VA and size are passed via a user buffer (not inline in
> > > > > > + * ctrl_cmd) so that unprivileged devices can prepend the device path
> > > > > > + * to ctrl_cmd.addr without corrupting the VA.
> > > > > > + */
> > > > > > +#define UBLK_U_CMD_REG_BUF             \
> > > > > > +       _IOWR('u', 0x18, struct ublksrv_ctrl_cmd)
> > > > > > +/*
> > > > > > + * Unregister a shared memory buffer.
> > > > > > + * Input:  ctrl_cmd.data[0] = buffer index
> > > > > > + */
> > > > > > +#define UBLK_U_CMD_UNREG_BUF           \
> > > > > > +       _IOWR('u', 0x19, struct ublksrv_ctrl_cmd)
> > > > > > +
> > > > > > +/* Parameter buffer for UBLK_U_CMD_REG_BUF, pointed to by ctrl_cmd.addr */
> > > > > > +struct ublk_shmem_buf_reg {
> > > > > > +       __u64   addr;   /* userspace virtual address of shared memory */
> > > > > > +       __u32   len;    /* buffer size in bytes (page-aligned, max 4GB) */
> > > > > > +       __u32   flags;
> > > > > > +};
> > > > > > +
> > > > > > +/* Pin pages without FOLL_WRITE; usable with write-sealed memfd */
> > > > > > +#define UBLK_SHMEM_BUF_READ_ONLY       (1U << 0)
> > > > > >  /*
> > > > > >   * 64bits are enough now, and it should be easy to extend in case of
> > > > > >   * running out of feature flags
> > > > > > @@ -370,6 +408,7 @@
> > > > > >  /* Disable automatic partition scanning when device is started */
> > > > > >  #define UBLK_F_NO_AUTO_PART_SCAN (1ULL << 18)
> > > > > >
> > > > > > +
> > > > > >  /* device state */
> > > > > >  #define UBLK_S_DEV_DEAD        0
> > > > > >  #define UBLK_S_DEV_LIVE        1
> > > > > > @@ -469,6 +508,12 @@ struct ublksrv_ctrl_dev_info {
> > > > > >  #define                UBLK_IO_F_NEED_REG_BUF          (1U << 17)
> > > > > >  /* Request has an integrity data buffer */
> > > > > >  #define                UBLK_IO_F_INTEGRITY             (1UL << 18)
> > > > > > +/*
> > > > > > + * I/O buffer is in a registered shared memory buffer. When set, the addr
> > > > > > + * field in ublksrv_io_desc encodes buffer index and byte offset instead
> > > > > > + * of a userspace virtual address.
> > > > > > + */
> > > > > > +#define                UBLK_IO_F_SHMEM_ZC              (1U << 19)
> > > > > >
> > > > > >  /*
> > > > > >   * io cmd is described by this structure, and stored in share memory, indexed
> > > > > > @@ -743,4 +788,31 @@ struct ublk_params {
> > > > > >         struct ublk_param_integrity     integrity;
> > > > > >  };
> > > > > >
> > > > > > +/*
> > > > > > + * Shared memory zero-copy addr encoding for UBLK_IO_F_SHMEM_ZC.
> > > > > > + *
> > > > > > + * When UBLK_IO_F_SHMEM_ZC is set, ublksrv_io_desc.addr is encoded as:
> > > > > > + *   bits [0:31]  = byte offset within the buffer (up to 4GB)
> > > > > > + *   bits [32:47] = buffer index (up to 65536)
> > > > > > + *   bits [48:63] = reserved (must be zero)
> > > > >
> > > > > I wonder whether the "buffer index" is necessary. Can iod->addr and
> > > > > UBLK_U_CMD_UNREG_BUF refer to the buffer by the virtual address used
> > > > > with UBLK_U_CMD_REG_BUF? Then struct ublksrv_io_desc's addr field
> > > > > would retain its meaning. We would also avoid needing to compare the
> > > > > range buf_index values in ublk_try_buf_match(). And the xarray
> > > > > wouldn't be necessary to allocate buffer indices.
> > > >
> > > > There are several reasons for choosing "buffer index":
> > > >
> > > > - avoid to add extra storage in `struct ublk_buf_range`, extra
> > > >   `vaddr` field is required for returning virtual address; otherwise one extra
> > > >   lookup from buffer index is needed in fast path of ublk_try_buf_match()
> > >
> > > Yes, struct ublk_buf_range would have to track the virtual address,
> > > but that would replace both buf_index and base_offset. And you could
> > > store flags in the lower bits of the virtual address (since it has to
> > > be page-aligned) if you want to keep it as 16 bytes. I'm not sure what
> > > you mean about "one extra lookup from buffer index"; I was thinking it
> > > would be possible to get rid of buffer indices entirely.
> > >
> > > >
> > > > - I want ublk server to know the difference between shmem_zc buffer and the
> > > >   plain IO buffer clearly, both two shouldn't be mixed, otherwise it is easy to
> > > >   cause data corruption. For example, client is using buf A, but the
> > > >   ublk server fallback code path may be using it at the same time.
> > >
> > > Yes, certainly the ublk server should only use a shared-memory buffer
> > > when an incoming UBLK_IO_F_SHMEM_ZC request specifies it. I don't see
> > > the connection to referring to buffers by virtual address instead of
> > > buffer index, though. The kernel can't prevent the ublk server from
> > > writing to a registered shared memory region it has mapped into its
> > > address space.
> > >
> > > It seems like a simpler interface if iod->addr indicates the virtual
> > > address of the data buffer regardless of the UBLK_IO_F_SHMEM_ZC flag.
> > > Then the ublk server doesn't have to care whether the data is in
> > > shared memory or was copied automatically by the
> > > ublk_map_io()/ublk_unmap_io() fallback path; it just accesses the
> > > memory at iod->addr and lets the kernel take care of the copying (if
> > > necessary).
> > >
> > > >
> > > > - total registered buffers can be limited naturally by `u16` buffer_index
> > >
> > > I don't really see a reason to limit the number of SHMEM_ZC buffers if
> > > they don't consume any per-buffer resources. I think it would make
> > > more sense to limit it based on the _total size_ of registered
> > > buffers, but the page pinning should already provide that.
> > >
> > > >
> > > > - it is proved that buffer index is one nice pattern wrt. buffer
> > > >   registration, such as io_uring fixed buffer; and it helps userspace
> > > >   to manage multiple buffers, given each one has unique ID.
> > >
> > > If you really prefer the (buf_index, buf_offset) interface, I won't
> > > stand in the way. It just seems to me like the only thing the ublk
> > > server cares about a shared memory buffer is its virtual address, so
> > > that's a more natural way to refer to it.
> >
> > It looks a little simpler from UAPI viewpoint, but storing vaddr in maple
> > tree introduces some implementation issues:
> >
> > - vaddr is bound with task, so ublk device has to track task context, such
> >   as, when we allow to register buffer before adding device, the buffer has
> >   to be guaranteed to belong to ublk daemon(tracked in future)
> 
> Don't all tasks within a process share the same virtual address space?
> Then the threads in the ublk server process should all have the same
> virtual address for the registered buffer. I guess it's theoretically
> possible that a different process would register a shared memory
> region on behalf of the ublk server, and then they could have
> different virtual addresses for it, but that seems like a bizarre use
> case.

I meant processes:

- control command can be issued from any task/processes for
  registering/unregistering shared memory, before opening ublk char
  device, when it is hard to track ublk daemon

- difference processes are involved in whole ublk device lifetime in
case of recover

> 
> >
> > - not like the plain page copy code path, in which the buffer vaddr is
> >   always passed in daemon context; but buffer register can be done in any
> >   task context.
> >
> > - not get ID for buffer, it could become a bit complicated to handle
> >   recover if we store vaddr in mapple tree; vaddr can become invalid in
> >   device lifetime, but buf index is device-wide, which can't become obsolete.
> 
> Fair point, though I'm not even sure how the new ublk server process
> would be aware of the shared memory regions that belonged to the
> previous process. If they were mapped from an anonymous memfd, the new
> process wouldn't have any way to access them. I think it probably
> makes more sense to unregister the shared memory regions when the ublk
> server process exits.

So far, let's ublk server deal with it.

But we can improve this situation in future.

> 
> >
> > Anyway, I don't object to switch to vaddr based UAPI, and we have enough
> > time to think it though and take it before 7.1 release.
> >
> > I will send patchset to cover other review comments first.
> 
> Sure, I think you have good reasons for the buf_index representation
> and it doesn't really matter either way. I'm fine keeping this
> interface.

OK.


Thanks,
Ming

^ permalink raw reply

* Re: [PATCH v10 13/13] docs: add io_queue flag to isolcpus
From: Aaron Tomlin @ 2026-04-10  1:45 UTC (permalink / raw)
  To: Ming Lei
  Cc: axboe, kbusch, hch, sagi, mst, aacraid, James.Bottomley,
	martin.petersen, liyihang9, kashyap.desai, sumit.saxena,
	shivasharan.srikanteshwara, chandrakanth.patil, sathya.prakash,
	sreekanth.reddy, suganath-prabu.subramani, ranjan.kumar,
	jinpu.wang, tglx, mingo, peterz, juri.lelli, vincent.guittot,
	akpm, maz, ruanjinjie, bigeasy, yphbchou0911, wagi, frederic,
	longman, chenridong, hare, kch, steve, sean, chjohnst, neelx,
	mproche, linux-block, linux-kernel, virtualization, linux-nvme,
	linux-scsi, megaraidlinux.pdl, mpi3mr-linuxdrv.pdl,
	MPT-FusionLinux.pdl, Lei, Ming
In-Reply-To: <CAFj5m9JE5e4DRGbzQFxDdZWU76ZPQ3G+C9JpLu0mhTB6aesZ9g@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2958 bytes --]

On Thu, Apr 09, 2026 at 11:00:09PM +0800, Ming Lei wrote:
> How can the isolated core be scheduled for running polling task?
> 
> Who triggered it?
> 
> > loop waiting for the hardware completion. This would completely monopolise
> > the core and destroy any real time isolation guarantees without the user
> > space application ever having requested it.
> 
> No.
> 
> IOPOLL queue doesn't have interrupt, and the ->poll() is only run from
> the submission context.  So if you don't submitted polled IO on isolated
> CPU cores, everything is just fine.  This is simpler than irq IO actually.

Yes, you are entirely correct. The ->iopoll() is indeed executed strictly
within the submission context. In the example below, the file operations
iopoll callback is iocb_bio_iopoll():

      // file->f_op->iopoll(&rw->kiocb, iob, poll_flags)
      iocb_bio_iopoll(&rw->kiocb, iob, poll_flags)
      {
        struct bio *bio

        bio = READ_ONCE(kiocb->private)
        if (bio)
          bio_poll(bio, iob, flags)
            if (queue_is_mq(q))
              blk_mq_poll(q, cookie, iob, flags)
              {
                if (!blk_mq_can_poll(q))
                  return 0

                blk_hctx_poll(q, q->queue_hw_ctx[cookie], iob, flags)
                {
                    int ret

                    do {
                        ret = q->mq_ops->poll(hctx, iob)
                        if (ret > 0)
                            return ret
                        if (task_sigpending(current))
                            return 1
                        if (ret < 0 || (flags & BLK_POLL_ONESHOT))
                            break
                        cpu_relax()
                    } while (!need_resched())

                    return 0
                }
              }

If an application on an isolated CPU does not explicitly submit a polled
I/O request, it will not poll. Thank you for correcting me on this.

> Can you share one example in which managed irq can't address?

Without io_queue, the block layer maps isolated CPUs to these queues, and
the device will fire unmanaged interrupts that can freely land on isolated
CPUs, thereby breaking isolation. By applying the constraint via io_queue
at the block layer, we restrict the hardware queue count and map the
isolated CPUs to the housekeeping queues, ensuring isolation is maintained
regardless of whether the driver uses managed interrupts.

Does the above help?

> > >
> > > IMO, only two differences from this viewpoint:
> > >
> > > 1) `io_queue` may reduce nr_hw_queues
> > >
> > > 2) when application submits IO from isolated CPUs, `io_queue` can complete
> > > IO from housekeeping CPUs.
> >
> > Acknowledged.
> 
> Are there other major differences besides the two mentioned above?

I believe the above is sufficient. Please let me know your thoughts.


Kind regards,
-- 
Aaron Tomlin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH] block: refactor blkdev_zone_mgmt_ioctl
From: Jens Axboe @ 2026-04-10  1:15 UTC (permalink / raw)
  To: dlemoal, Christoph Hellwig; +Cc: linux-block
In-Reply-To: <20260327090032.3722065-1-hch@lst.de>


On Fri, 27 Mar 2026 10:00:32 +0100, Christoph Hellwig wrote:
> Split the zone reset case into a separate helper so that the conditional
> locking goes away.

Applied, thanks!

[1/1] block: refactor blkdev_zone_mgmt_ioctl
      commit: 539fb773a3f7c07cf7fd00617f33ed4e33058d72

Best regards,
-- 
Jens Axboe




^ permalink raw reply

* Re: [PATCH 00/20] DRBD 9 rework
From: Jens Axboe @ 2026-04-10  1:14 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Christoph Böhmwalder, drbd-dev, linux-kernel, Lars Ellenberg,
	Philipp Reisner, linux-block
In-Reply-To: <addJ6uTc8Zo4GKpW@infradead.org>

On 4/9/26 12:40 AM, Christoph Hellwig wrote:
> First an apology, I thought it was in your tree, but it looks like
> the drbd branch just has minor fixes.  So a lot less urgency.

No worries, figured you hadn't seen the previous discussion.

> On Wed, Apr 08, 2026 at 06:58:58AM -0600, Jens Axboe wrote:
>> See the previous discussion,
> 
> Do you have a pointer to that discussion?  I can't remember one.

I'm on pretty poor connectivity right now, but find some of my complaining
on linux-block in response to a previous Christian email.

>> the goal is to sync the two drbd code
>> bases. It's followed the "usual" pattern of the in-kernel driver being
>> neglected and development and users pushed to the out-of-tree one,
>> which is highly annoying.
> 
> I don't think that's a a usual pattern.  Also the new version looks
> like a complete rewrite and not something incremental:
> 
>  45 files changed, 45891 insertions(+), 16264 deletions(-)
> 
> For a code base that is "29482 total".
> 
> I think reviewing it would be easier by just adding an new drbd9 driver
> and then steering people toward it carefully, as that is actually
> reviewable compared to non-bisectable patches changing large chunks
> of code in a non-atomic way.

That is another approach we could take, but I don't think that would
make it any easier to review, to be honest. Nobody reviews a full
driver, their eyes just kind of gloss over.

Heads up - OOO for 1 week, will prep merge window stuff to the best
of my abilities, but won't be super responsive outside of that. As
this particular driver isn't going anywhere right now, there's no
urgency on that side of things.

-- 
Jens Axboe


^ permalink raw reply

* Re: [PATCH 0/7] ublk: followup fixes for SHMEM_ZC
From: Jens Axboe @ 2026-04-10  1:12 UTC (permalink / raw)
  To: Ming Lei, linux-block; +Cc: Caleb Sander Mateos
In-Reply-To: <20260409133020.3780098-1-tom.leiming@gmail.com>

On 4/9/26 7:30 AM, Ming Lei wrote:
> Hello Jens,
> 
> Followup fixes for the SHMEM_ZC (shared memory zero copy) patch series,
> addressing review feedback from Caleb Sander Mateos.
> 
> - Widen ublk_shmem_buf_reg.len to __u64 so 4GB buffers can be registered
>   (the __u32 field overflowed to 0 for exactly 4GB)
> - Verify all pages in multi-page bvecs fall within the registered maple
>   tree range, removing base_pfn from ublk_buf_range since mas.index
>   provides the range start PFN
> - Simplify the PFN range coalescing loop in __ublk_ctrl_reg_buf
> - Replace xarray with IDA for buffer index allocation, removing the
>   unnecessary struct ublk_buf
> - Allow buffer registration before device is started by taking ub->mutex
>   before freezing the queue (same ordering as ublk_stop_dev_unlocked)
> - Address documentation review comments
> - Update MAINTAINERS email

Applied, but I'm unsure what base you used, I'm guessing your old base
rather than my for-7.1/block which already had fixups for the issues I
mentioned? In any case, just ensure that future updates are against the
actual tree, not on top of your previous tree.

-- 
Jens Axboe

^ permalink raw reply

* Re: [PATCH 0/7] ublk: followup fixes for SHMEM_ZC
From: Jens Axboe @ 2026-04-10  1:11 UTC (permalink / raw)
  To: linux-block, Ming Lei; +Cc: Caleb Sander Mateos
In-Reply-To: <20260409133020.3780098-1-tom.leiming@gmail.com>


On Thu, 09 Apr 2026 21:30:12 +0800, Ming Lei wrote:
> Followup fixes for the SHMEM_ZC (shared memory zero copy) patch series,
> addressing review feedback from Caleb Sander Mateos.
> 
> - Widen ublk_shmem_buf_reg.len to __u64 so 4GB buffers can be registered
>   (the __u32 field overflowed to 0 for exactly 4GB)
> - Verify all pages in multi-page bvecs fall within the registered maple
>   tree range, removing base_pfn from ublk_buf_range since mas.index
>   provides the range start PFN
> - Simplify the PFN range coalescing loop in __ublk_ctrl_reg_buf
> - Replace xarray with IDA for buffer index allocation, removing the
>   unnecessary struct ublk_buf
> - Allow buffer registration before device is started by taking ub->mutex
>   before freezing the queue (same ordering as ublk_stop_dev_unlocked)
> - Address documentation review comments
> - Update MAINTAINERS email
> 
> [...]

Applied, thanks!

[1/7] ublk: widen ublk_shmem_buf_reg.len to __u64 for 4GB buffer support
      commit: 23b3b6f0b584b70a427d5bb826d320151890d7da
[2/7] ublk: verify all pages in multi-page bvec fall within registered range
      commit: 211ff1602b67e26125977f8b2f369d7c2847628c
[3/7] ublk: simplify PFN range loop in __ublk_ctrl_reg_buf
      commit: 8ea8566a9aeef746699d8c84bed3ac44edbfaa0e
[4/7] ublk: replace xarray with IDA for shmem buffer index allocation
      commit: 5e864438e2853ef5112d7905fadcc3877e2be70a
[5/7] ublk: allow buffer registration before device is started
      commit: 365ea7cc62447caac508706b429cdf031cc15a9f
[6/7] Documentation: ublk: address review comments for SHMEM_ZC docs
      commit: 289653bb76c46149f88939c3cfef55cdb236ace2
[7/7] MAINTAINERS: update ublk driver maintainer email
      commit: b774765fb804045ee774476ded8e52482ae5ecb7

Best regards,
-- 
Jens Axboe




^ permalink raw reply

* Re: [PATCH RFC v4 1/3] block: add BIO_COMPLETE_IN_TASK for task-context completion
From: Jens Axboe @ 2026-04-10  0:46 UTC (permalink / raw)
  To: Tal Zussman
  Cc: Matthew Wilcox, Christian Brauner, Darrick J. Wong,
	Carlos Maiolino, Alexander Viro, Jan Kara, Christoph Hellwig,
	linux-block, linux-kernel, linux-xfs, linux-fsdevel, linux-mm
In-Reply-To: <7e468bd2-e52b-4165-95c6-3f04e1dca21e@columbia.edu>

On 4/9/26 12:54 PM, Tal Zussman wrote:
> 
> 
> On 4/8/26 7:36 PM, Jens Axboe wrote:
>> On Apr 8, 2026, at 4:51?PM, Tal Zussman <tz2294@columbia.edu> wrote:
>>>> On 4/8/26 3:51 PM, Jens Axboe wrote:
>>>>> On 4/8/26 12:?48 PM, Tal Zussman wrote: > On 3/25/26 4:?14 PM, Jens Axboe wrote:
>>>>>
>>>>> Thanks! I'm going to give Dave's llist suggestion a shot on top of
>>>>> this as it seems like it'll simplify this nicely. Looks like that'll
>>>>> involve turning bio::bi_next into a union with a struct llist_node.
>>>>
>>>> Since these lists can get long, I'd keep an eye on llist reversal
>>>> overhead there...
>>>>
>>>
>>> Going to send v5 shortly -- tested with and without the llist reversal and
>>> it didn't seem to make much of a difference. This was on a single-disk VM
>>> though, so any stress testing you could do would be very helpful.
>>>
>>
>> With all due respect, a single test like that isn?t going to be that useful. I?d be wary of making that change willy nilly and just thinking ?it?s fine, worked fine on the one case I tested?.
> 
> Understood -- unfortunately that's what I have access to at the moment. I
> can requisition a machine with 2, maybe 3 disks, and test more thoroughly on
> that before sending the next version, but that'll take a few days. You had
> previously offered to test on your big box, so was hoping that was still on
> the table :)

I can do that, but I'm OOO for the next week, so won't be until I'm
back.

> (Although Christoph seems to have proposed moving away from llist again)

I think that's a good idea, not a fan of using llist for this, in case
that wasn't clear. These are per-cpu lists anyway, and having a constant
overhead is better than needing to reverse an llist. IMHO.

-- 
Jens Axboe

^ permalink raw reply

* Re: [PATCH v2 01/10] ublk: add UBLK_U_CMD_REG_BUF/UNREG_BUF control commands
From: Caleb Sander Mateos @ 2026-04-09 21:22 UTC (permalink / raw)
  To: Ming Lei; +Cc: Ming Lei, Jens Axboe, linux-block
In-Reply-To: <adeZKBrNEX7vx7O3@fedora>

On Thu, Apr 9, 2026 at 5:18 AM Ming Lei <tom.leiming@gmail.com> wrote:
>
> On Wed, Apr 08, 2026 at 08:20:12AM -0700, Caleb Sander Mateos wrote:
> > On Tue, Apr 7, 2026 at 7:23 PM Ming Lei <ming.lei@redhat.com> wrote:
> > >
> > > On Tue, Apr 07, 2026 at 12:35:49PM -0700, Caleb Sander Mateos wrote:
> > > > On Tue, Mar 31, 2026 at 8:32 AM Ming Lei <ming.lei@redhat.com> wrote:
> > > > >
> > > > > Add control commands for registering and unregistering shared memory
> > > > > buffers for zero-copy I/O:
> > > > >
> > > > > - UBLK_U_CMD_REG_BUF (0x18): pins pages from userspace, inserts PFN
> > > > >   ranges into a per-device maple tree for O(log n) lookup during I/O.
> > > > >   Buffer pointers are tracked in a per-device xarray. Returns the
> > > > >   assigned buffer index.
> > > > >
> > > > > - UBLK_U_CMD_UNREG_BUF (0x19): removes PFN entries and unpins pages.
> > > > >
> > > > > Queue freeze/unfreeze is handled internally so userspace need not
> > > > > quiesce the device during registration.
> > > > >
> > > > > Also adds:
> > > > > - UBLK_IO_F_SHMEM_ZC flag and addr encoding helpers in UAPI header
> > > > >   (16-bit buffer index supporting up to 65536 buffers)
> > > > > - Data structures (ublk_buf, ublk_buf_range) and xarray/maple tree
> > > > > - __ublk_ctrl_reg_buf() helper for PFN insertion with error unwinding
> > > > > - __ublk_ctrl_unreg_buf() helper for cleanup reuse
> > > > > - ublk_support_shmem_zc() / ublk_dev_support_shmem_zc() stubs
> > > > >   (returning false — feature not enabled yet)
> > > > >
> > > > > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > > > > ---
> > > > >  drivers/block/ublk_drv.c      | 300 ++++++++++++++++++++++++++++++++++
> > > > >  include/uapi/linux/ublk_cmd.h |  72 ++++++++
> > > > >  2 files changed, 372 insertions(+)
> > > > >
> > > > > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > > > > index 71c7c56b38ca..ac6ccc174d44 100644
> > > > > --- a/drivers/block/ublk_drv.c
> > > > > +++ b/drivers/block/ublk_drv.c
> > > > > @@ -46,6 +46,8 @@
> > > > >  #include <linux/kref.h>
> > > > >  #include <linux/kfifo.h>
> > > > >  #include <linux/blk-integrity.h>
> > > > > +#include <linux/maple_tree.h>
> > > > > +#include <linux/xarray.h>
> > > > >  #include <uapi/linux/fs.h>
> > > > >  #include <uapi/linux/ublk_cmd.h>
> > > > >
> > > > > @@ -58,6 +60,8 @@
> > > > >  #define UBLK_CMD_UPDATE_SIZE   _IOC_NR(UBLK_U_CMD_UPDATE_SIZE)
> > > > >  #define UBLK_CMD_QUIESCE_DEV   _IOC_NR(UBLK_U_CMD_QUIESCE_DEV)
> > > > >  #define UBLK_CMD_TRY_STOP_DEV  _IOC_NR(UBLK_U_CMD_TRY_STOP_DEV)
> > > > > +#define UBLK_CMD_REG_BUF       _IOC_NR(UBLK_U_CMD_REG_BUF)
> > > > > +#define UBLK_CMD_UNREG_BUF     _IOC_NR(UBLK_U_CMD_UNREG_BUF)
> > > > >
> > > > >  #define UBLK_IO_REGISTER_IO_BUF                _IOC_NR(UBLK_U_IO_REGISTER_IO_BUF)
> > > > >  #define UBLK_IO_UNREGISTER_IO_BUF      _IOC_NR(UBLK_U_IO_UNREGISTER_IO_BUF)
> > > > > @@ -289,6 +293,20 @@ struct ublk_queue {
> > > > >         struct ublk_io ios[] __counted_by(q_depth);
> > > > >  };
> > > > >
> > > > > +/* Per-registered shared memory buffer */
> > > > > +struct ublk_buf {
> > > > > +       struct page **pages;
> > > > > +       unsigned int nr_pages;
> > > > > +};
> > > > > +
> > > > > +/* Maple tree value: maps a PFN range to buffer location */
> > > > > +struct ublk_buf_range {
> > > > > +       unsigned long base_pfn;
> > > > > +       unsigned short buf_index;
> > > > > +       unsigned short flags;
> > > > > +       unsigned int base_offset;       /* byte offset within buffer */
> > > > > +};
> > > > > +
> > > > >  struct ublk_device {
> > > > >         struct gendisk          *ub_disk;
> > > > >
> > > > > @@ -323,6 +341,10 @@ struct ublk_device {
> > > > >
> > > > >         bool                    block_open; /* protected by open_mutex */
> > > > >
> > > > > +       /* shared memory zero copy */
> > > > > +       struct maple_tree       buf_tree;
> > > > > +       struct xarray           bufs_xa;
> > > > > +
> > > > >         struct ublk_queue       *queues[];
> > > > >  };
> > > > >
> > > > > @@ -334,6 +356,7 @@ struct ublk_params_header {
> > > > >
> > > > >  static void ublk_io_release(void *priv);
> > > > >  static void ublk_stop_dev_unlocked(struct ublk_device *ub);
> > > > > +static void ublk_buf_cleanup(struct ublk_device *ub);
> > > > >  static void ublk_abort_queue(struct ublk_device *ub, struct ublk_queue *ubq);
> > > > >  static inline struct request *__ublk_check_and_get_req(struct ublk_device *ub,
> > > > >                 u16 q_id, u16 tag, struct ublk_io *io);
> > > > > @@ -398,6 +421,16 @@ static inline bool ublk_dev_support_zero_copy(const struct ublk_device *ub)
> > > > >         return ub->dev_info.flags & UBLK_F_SUPPORT_ZERO_COPY;
> > > > >  }
> > > > >
> > > > > +static inline bool ublk_support_shmem_zc(const struct ublk_queue *ubq)
> > > > > +{
> > > > > +       return false;
> > > > > +}
> > > > > +
> > > > > +static inline bool ublk_dev_support_shmem_zc(const struct ublk_device *ub)
> > > > > +{
> > > > > +       return false;
> > > > > +}
> > > > > +
> > > > >  static inline bool ublk_support_auto_buf_reg(const struct ublk_queue *ubq)
> > > > >  {
> > > > >         return ubq->flags & UBLK_F_AUTO_BUF_REG;
> > > > > @@ -1460,6 +1493,7 @@ static blk_status_t ublk_setup_iod(struct ublk_queue *ubq, struct request *req)
> > > > >         iod->op_flags = ublk_op | ublk_req_build_flags(req);
> > > > >         iod->nr_sectors = blk_rq_sectors(req);
> > > > >         iod->start_sector = blk_rq_pos(req);
> > > > > +
> > > >
> > > > nit: unrelated whitespace change?
> > > >
> > > > >         iod->addr = io->buf.addr;
> > > > >
> > > > >         return BLK_STS_OK;
> > > > > @@ -1665,6 +1699,7 @@ static bool ublk_start_io(const struct ublk_queue *ubq, struct request *req,
> > > > >  {
> > > > >         unsigned mapped_bytes = ublk_map_io(ubq, req, io);
> > > > >
> > > > > +
> > > >
> > > > nit: unrelated whitespace change?
> > > >
> > > > >         /* partially mapped, update io descriptor */
> > > > >         if (unlikely(mapped_bytes != blk_rq_bytes(req))) {
> > > > >                 /*
> > > > > @@ -4206,6 +4241,7 @@ static void ublk_cdev_rel(struct device *dev)
> > > > >  {
> > > > >         struct ublk_device *ub = container_of(dev, struct ublk_device, cdev_dev);
> > > > >
> > > > > +       ublk_buf_cleanup(ub);
> > > > >         blk_mq_free_tag_set(&ub->tag_set);
> > > > >         ublk_deinit_queues(ub);
> > > > >         ublk_free_dev_number(ub);
> > > > > @@ -4625,6 +4661,8 @@ static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_cmd *header)
> > > > >         mutex_init(&ub->mutex);
> > > > >         spin_lock_init(&ub->lock);
> > > > >         mutex_init(&ub->cancel_mutex);
> > > > > +       mt_init(&ub->buf_tree);
> > > > > +       xa_init_flags(&ub->bufs_xa, XA_FLAGS_ALLOC);
> > > > >         INIT_WORK(&ub->partition_scan_work, ublk_partition_scan_work);
> > > > >
> > > > >         ret = ublk_alloc_dev_number(ub, header->dev_id);
> > > > > @@ -5168,6 +5206,260 @@ static int ublk_char_dev_permission(struct ublk_device *ub,
> > > > >         return err;
> > > > >  }
> > > > >
> > > > > +/*
> > > > > + * Drain inflight I/O and quiesce the queue. Freeze drains all inflight
> > > > > + * requests, quiesce_nowait marks the queue so no new requests dispatch,
> > > > > + * then unfreeze allows new submissions (which won't dispatch due to
> > > > > + * quiesce). This keeps freeze and ub->mutex non-nested.
> > > > > + */
> > > > > +static void ublk_quiesce_and_release(struct gendisk *disk)
> > > > > +{
> > > > > +       unsigned int memflags;
> > > > > +
> > > > > +       memflags = blk_mq_freeze_queue(disk->queue);
> > > > > +       blk_mq_quiesce_queue_nowait(disk->queue);
> > > > > +       blk_mq_unfreeze_queue(disk->queue, memflags);
> > > > > +}
> > > > > +
> > > > > +static void ublk_unquiesce_and_resume(struct gendisk *disk)
> > > > > +{
> > > > > +       blk_mq_unquiesce_queue(disk->queue);
> > > > > +}
> > > > > +
> > > > > +/*
> > > > > + * Insert PFN ranges of a registered buffer into the maple tree,
> > > > > + * coalescing consecutive PFNs into single range entries.
> > > > > + * Returns 0 on success, negative error with partial insertions unwound.
> > > > > + */
> > > > > +/* Erase coalesced PFN ranges from the maple tree for pages [0, nr_pages) */
> > > > > +static void ublk_buf_erase_ranges(struct ublk_device *ub,
> > > > > +                                 struct ublk_buf *ubuf,
> > > > > +                                 unsigned long nr_pages)
> > > > > +{
> > > > > +       unsigned long i;
> > > > > +
> > > > > +       for (i = 0; i < nr_pages; ) {
> > > > > +               unsigned long pfn = page_to_pfn(ubuf->pages[i]);
> > > > > +               unsigned long start = i;
> > > > > +
> > > > > +               while (i + 1 < nr_pages &&
> > > > > +                      page_to_pfn(ubuf->pages[i + 1]) == pfn + (i - start) + 1)
> > > > > +                       i++;
> > > > > +               i++;
> > > > > +               kfree(mtree_erase(&ub->buf_tree, pfn));
> > > > > +       }
> > > > > +}
> > > > > +
> > > > > +static int __ublk_ctrl_reg_buf(struct ublk_device *ub,
> > > > > +                              struct ublk_buf *ubuf, int index,
> > > > > +                              unsigned short flags)
> > > > > +{
> > > > > +       unsigned long nr_pages = ubuf->nr_pages;
> > > > > +       unsigned long i;
> > > > > +       int ret;
> > > > > +
> > > > > +       for (i = 0; i < nr_pages; ) {
> > > > > +               unsigned long pfn = page_to_pfn(ubuf->pages[i]);
> > > > > +               unsigned long start = i;
> > > > > +               struct ublk_buf_range *range;
> > > > > +
> > > > > +               /* Find run of consecutive PFNs */
> > > > > +               while (i + 1 < nr_pages &&
> > > > > +                      page_to_pfn(ubuf->pages[i + 1]) == pfn + (i - start) + 1)
> > > > > +                       i++;
> > > > > +               i++;    /* past the last page in this run */
> > > >
> > > > Move this increment to the for loop so you don't need the "- 1" in the
> > > > mtree_insert_range() call?
> > >
> > > Good catch!
> > >
> > > >
> > > > > +
> > > > > +               range = kzalloc(sizeof(*range), GFP_KERNEL);
> > > >
> > > > Not sure kzalloc() is necessary; all the fields are initialized below
> > >
> > > Yeah, kmalloc() is fine, and we shouldn't add more fields to `range` in
> > > future.
> > >
> > > >
> > > > > +               if (!range) {
> > > > > +                       ret = -ENOMEM;
> > > > > +                       goto unwind;
> > > > > +               }
> > > > > +               range->buf_index = index;
> > > > > +               range->flags = flags;
> > > > > +               range->base_pfn = pfn;
> > > > > +               range->base_offset = start << PAGE_SHIFT;
> > > > > +
> > > > > +               ret = mtree_insert_range(&ub->buf_tree, pfn,
> > > > > +                                        pfn + (i - start) - 1,
> > > > > +                                        range, GFP_KERNEL);
> > > > > +               if (ret) {
> > > > > +                       kfree(range);
> > > > > +                       goto unwind;
> > > > > +               }
> > > > > +       }
> > > > > +       return 0;
> > > > > +
> > > > > +unwind:
> > > > > +       ublk_buf_erase_ranges(ub, ubuf, i);
> > > > > +       return ret;
> > > > > +}
> > > > > +
> > > > > +/*
> > > > > + * Register a shared memory buffer for zero-copy I/O.
> > > > > + * Pins pages, builds PFN maple tree, freezes/unfreezes the queue
> > > > > + * internally. Returns buffer index (>= 0) on success.
> > > > > + */
> > > > > +static int ublk_ctrl_reg_buf(struct ublk_device *ub,
> > > > > +                            struct ublksrv_ctrl_cmd *header)
> > > > > +{
> > > > > +       void __user *argp = (void __user *)(unsigned long)header->addr;
> > > > > +       struct ublk_shmem_buf_reg buf_reg;
> > > > > +       unsigned long addr, size, nr_pages;
> > > >
> > > > size and nr_pages could be u32
> > >
> > > Yeah, it was caused by internal change on `ublk_shmem_buf_reg`.
> > >
> > > >
> > > > > +       unsigned int gup_flags;
> > > > > +       struct gendisk *disk;
> > > > > +       struct ublk_buf *ubuf;
> > > > > +       long pinned;
> > > >
> > > > pinned could be int to match the return type of pin_user_pages_fast()
> > >
> > > OK.
> > >
> > > >
> > > > > +       u32 index;
> > > > > +       int ret;
> > > > > +
> > > > > +       if (!ublk_dev_support_shmem_zc(ub))
> > > > > +               return -EOPNOTSUPP;
> > > > > +
> > > > > +       memset(&buf_reg, 0, sizeof(buf_reg));
> > > > > +       if (copy_from_user(&buf_reg, argp,
> > > > > +                          min_t(size_t, header->len, sizeof(buf_reg))))
> > > > > +               return -EFAULT;
> > > > > +
> > > > > +       if (buf_reg.flags & ~UBLK_SHMEM_BUF_READ_ONLY)
> > > > > +               return -EINVAL;
> > > > > +
> > > > > +       addr = buf_reg.addr;
> > > > > +       size = buf_reg.len;
> > > >
> > > > nit: don't see much value in these additional variables that are just
> > > > copies of buf_reg fields
> > > >
> > > > > +       nr_pages = size >> PAGE_SHIFT;
> > > > > +
> > > > > +       if (!size || !PAGE_ALIGNED(size) || !PAGE_ALIGNED(addr))
> > > > > +               return -EINVAL;
> > > > > +
> > > > > +       disk = ublk_get_disk(ub);
> > > > > +       if (!disk)
> > > > > +               return -ENODEV;
> > > >
> > > > So buffers can't be registered before the ublk device is started? Is
> > > > there a reason why that's not possible? Could we just make the
> > > > ublk_quiesce_and_release() and ublk_unquiesce_and_resume() conditional
> > > > on disk being non-NULL? I guess we'd have to hold the ublk_device
> > > > mutex before calling ublk_get_disk() to prevent it from being assigned
> > > > concurrently.
> > >
> > > Here `disk` is used for freeze & quiesce queue.
> > >
> > > But the implementation can be a bit more complicated given the dependency
> > > between ub->mutex and freeze queue should be avoided.
> > >
> > > Anyway, it is one nice requirement, I will try to relax the constraint.
> > >
> > > >
> > > > > +
> > > > > +       /* Pin pages before quiescing (may sleep) */
> > > > > +       ubuf = kzalloc(sizeof(*ubuf), GFP_KERNEL);
> > > > > +       if (!ubuf) {
> > > > > +               ret = -ENOMEM;
> > > > > +               goto put_disk;
> > > > > +       }
> > > > > +
> > > > > +       ubuf->pages = kvmalloc_array(nr_pages, sizeof(*ubuf->pages),
> > > > > +                                    GFP_KERNEL);
> > > > > +       if (!ubuf->pages) {
> > > > > +               ret = -ENOMEM;
> > > > > +               goto err_free;
> > > > > +       }
> > > > > +
> > > > > +       gup_flags = FOLL_LONGTERM;
> > > > > +       if (!(buf_reg.flags & UBLK_SHMEM_BUF_READ_ONLY))
> > > > > +               gup_flags |= FOLL_WRITE;
> > > > > +
> > > > > +       pinned = pin_user_pages_fast(addr, nr_pages, gup_flags, ubuf->pages);
> > > > > +       if (pinned < 0) {
> > > > > +               ret = pinned;
> > > > > +               goto err_free_pages;
> > > > > +       }
> > > > > +       if (pinned != nr_pages) {
> > > > > +               ret = -EFAULT;
> > > > > +               goto err_unpin;
> > > > > +       }
> > > > > +       ubuf->nr_pages = nr_pages;
> > > > > +
> > > > > +       /*
> > > > > +        * Drain inflight I/O and quiesce the queue so no new requests
> > > > > +        * are dispatched while we modify the maple tree. Keep freeze
> > > > > +        * and mutex non-nested to avoid lock dependency.
> > > > > +        */
> > > > > +       ublk_quiesce_and_release(disk);
> > > > > +
> > > > > +       mutex_lock(&ub->mutex);
> > > >
> > > > Looks like the xarray and maple tree do their own spinlocking, is this needed?
> > >
> > > Right, looks it isn't needed now, and it was added from beginning with plain
> > > array.
> > >
> > > >
> > > > > +
> > > > > +       ret = xa_alloc(&ub->bufs_xa, &index, ubuf, xa_limit_16b, GFP_KERNEL);
> > > > > +       if (ret)
> > > > > +               goto err_unlock;
> > > > > +
> > > > > +       ret = __ublk_ctrl_reg_buf(ub, ubuf, index, buf_reg.flags);
> > > > > +       if (ret) {
> > > > > +               xa_erase(&ub->bufs_xa, index);
> > > > > +               goto err_unlock;
> > > > > +       }
> > > > > +
> > > > > +       mutex_unlock(&ub->mutex);
> > > > > +
> > > > > +       ublk_unquiesce_and_resume(disk);
> > > > > +       ublk_put_disk(disk);
> > > > > +       return index;
> > > > > +
> > > > > +err_unlock:
> > > > > +       mutex_unlock(&ub->mutex);
> > > > > +       ublk_unquiesce_and_resume(disk);
> > > > > +err_unpin:
> > > > > +       unpin_user_pages(ubuf->pages, pinned);
> > > > > +err_free_pages:
> > > > > +       kvfree(ubuf->pages);
> > > > > +err_free:
> > > > > +       kfree(ubuf);
> > > > > +put_disk:
> > > > > +       ublk_put_disk(disk);
> > > > > +       return ret;
> > > > > +}
> > > > > +
> > > > > +static void __ublk_ctrl_unreg_buf(struct ublk_device *ub,
> > > > > +                                 struct ublk_buf *ubuf)
> > > > > +{
> > > > > +       ublk_buf_erase_ranges(ub, ubuf, ubuf->nr_pages);
> > > > > +       unpin_user_pages(ubuf->pages, ubuf->nr_pages);
> > > > > +       kvfree(ubuf->pages);
> > > > > +       kfree(ubuf);
> > > > > +}
> > > > > +
> > > > > +static int ublk_ctrl_unreg_buf(struct ublk_device *ub,
> > > > > +                              struct ublksrv_ctrl_cmd *header)
> > > > > +{
> > > > > +       int index = (int)header->data[0];
> > > > > +       struct gendisk *disk;
> > > > > +       struct ublk_buf *ubuf;
> > > > > +
> > > > > +       if (!ublk_dev_support_shmem_zc(ub))
> > > > > +               return -EOPNOTSUPP;
> > > > > +
> > > > > +       disk = ublk_get_disk(ub);
> > > > > +       if (!disk)
> > > > > +               return -ENODEV;
> > > > > +
> > > > > +       /* Drain inflight I/O before modifying the maple tree */
> > > > > +       ublk_quiesce_and_release(disk);
> > > > > +
> > > > > +       mutex_lock(&ub->mutex);
> > > > > +
> > > > > +       ubuf = xa_erase(&ub->bufs_xa, index);
> > > > > +       if (!ubuf) {
> > > > > +               mutex_unlock(&ub->mutex);
> > > > > +               ublk_unquiesce_and_resume(disk);
> > > > > +               ublk_put_disk(disk);
> > > > > +               return -ENOENT;
> > > > > +       }
> > > > > +
> > > > > +       __ublk_ctrl_unreg_buf(ub, ubuf);
> > > > > +
> > > > > +       mutex_unlock(&ub->mutex);
> > > > > +
> > > > > +       ublk_unquiesce_and_resume(disk);
> > > > > +       ublk_put_disk(disk);
> > > > > +       return 0;
> > > > > +}
> > > > > +
> > > > > +static void ublk_buf_cleanup(struct ublk_device *ub)
> > > > > +{
> > > > > +       struct ublk_buf *ubuf;
> > > > > +       unsigned long index;
> > > > > +
> > > > > +       xa_for_each(&ub->bufs_xa, index, ubuf)
> > > > > +               __ublk_ctrl_unreg_buf(ub, ubuf);
> > > >
> > > > This looks quadratic in the number of registered buffers. Can we do a
> > > > single pass over the xarray  and the maple tree?
> > >
> > > It can be done two passes: first pass walks maple tree for unpinning
> > > pages, the 2nd pass is for freeing buffers in xarray, which looks more
> > > clean.
> > >
> > > But the current way can reuse __ublk_ctrl_unreg_buf(), also
> > > ublk_buf_cleanup() is only called one-shot in device release handler, so we can
> > > leave it for future optimization.
> > >
> > > >
> > > > > +       xa_destroy(&ub->bufs_xa);
> > > > > +       mtree_destroy(&ub->buf_tree);
> > > > > +}
> > > > > +
> > > > > +
> > > > > +
> > > > >  static int ublk_ctrl_uring_cmd_permission(struct ublk_device *ub,
> > > > >                 u32 cmd_op, struct ublksrv_ctrl_cmd *header)
> > > > >  {
> > > > > @@ -5225,6 +5517,8 @@ static int ublk_ctrl_uring_cmd_permission(struct ublk_device *ub,
> > > > >         case UBLK_CMD_UPDATE_SIZE:
> > > > >         case UBLK_CMD_QUIESCE_DEV:
> > > > >         case UBLK_CMD_TRY_STOP_DEV:
> > > > > +       case UBLK_CMD_REG_BUF:
> > > > > +       case UBLK_CMD_UNREG_BUF:
> > > > >                 mask = MAY_READ | MAY_WRITE;
> > > > >                 break;
> > > > >         default:
> > > > > @@ -5350,6 +5644,12 @@ static int ublk_ctrl_uring_cmd(struct io_uring_cmd *cmd,
> > > > >         case UBLK_CMD_TRY_STOP_DEV:
> > > > >                 ret = ublk_ctrl_try_stop_dev(ub);
> > > > >                 break;
> > > > > +       case UBLK_CMD_REG_BUF:
> > > > > +               ret = ublk_ctrl_reg_buf(ub, &header);
> > > > > +               break;
> > > > > +       case UBLK_CMD_UNREG_BUF:
> > > > > +               ret = ublk_ctrl_unreg_buf(ub, &header);
> > > > > +               break;
> > > > >         default:
> > > > >                 ret = -EOPNOTSUPP;
> > > > >                 break;
> > > > > diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> > > > > index a88876756805..52bb9b843d73 100644
> > > > > --- a/include/uapi/linux/ublk_cmd.h
> > > > > +++ b/include/uapi/linux/ublk_cmd.h
> > > > > @@ -57,6 +57,44 @@
> > > > >         _IOWR('u', 0x16, struct ublksrv_ctrl_cmd)
> > > > >  #define UBLK_U_CMD_TRY_STOP_DEV                \
> > > > >         _IOWR('u', 0x17, struct ublksrv_ctrl_cmd)
> > > > > +/*
> > > > > + * Register a shared memory buffer for zero-copy I/O.
> > > > > + * Input:  ctrl_cmd.addr points to struct ublk_buf_reg (buffer VA + size)
> > > > > + *         ctrl_cmd.len  = sizeof(struct ublk_buf_reg)
> > > > > + * Result: >= 0 is the assigned buffer index, < 0 is error
> > > > > + *
> > > > > + * The kernel pins pages from the calling process's address space
> > > > > + * and inserts PFN ranges into a per-device maple tree. When a block
> > > > > + * request's pages match registered pages, the driver sets
> > > > > + * UBLK_IO_F_SHMEM_ZC and encodes the buffer index + offset in addr,
> > > > > + * allowing the server to access the data via its own mapping of the
> > > > > + * same shared memory — true zero copy.
> > > > > + *
> > > > > + * The memory can be backed by memfd, hugetlbfs, or any GUP-compatible
> > > > > + * shared mapping. Queue freeze is handled internally.
> > > > > + *
> > > > > + * The buffer VA and size are passed via a user buffer (not inline in
> > > > > + * ctrl_cmd) so that unprivileged devices can prepend the device path
> > > > > + * to ctrl_cmd.addr without corrupting the VA.
> > > > > + */
> > > > > +#define UBLK_U_CMD_REG_BUF             \
> > > > > +       _IOWR('u', 0x18, struct ublksrv_ctrl_cmd)
> > > > > +/*
> > > > > + * Unregister a shared memory buffer.
> > > > > + * Input:  ctrl_cmd.data[0] = buffer index
> > > > > + */
> > > > > +#define UBLK_U_CMD_UNREG_BUF           \
> > > > > +       _IOWR('u', 0x19, struct ublksrv_ctrl_cmd)
> > > > > +
> > > > > +/* Parameter buffer for UBLK_U_CMD_REG_BUF, pointed to by ctrl_cmd.addr */
> > > > > +struct ublk_shmem_buf_reg {
> > > > > +       __u64   addr;   /* userspace virtual address of shared memory */
> > > > > +       __u32   len;    /* buffer size in bytes (page-aligned, max 4GB) */
> > > > > +       __u32   flags;
> > > > > +};
> > > > > +
> > > > > +/* Pin pages without FOLL_WRITE; usable with write-sealed memfd */
> > > > > +#define UBLK_SHMEM_BUF_READ_ONLY       (1U << 0)
> > > > >  /*
> > > > >   * 64bits are enough now, and it should be easy to extend in case of
> > > > >   * running out of feature flags
> > > > > @@ -370,6 +408,7 @@
> > > > >  /* Disable automatic partition scanning when device is started */
> > > > >  #define UBLK_F_NO_AUTO_PART_SCAN (1ULL << 18)
> > > > >
> > > > > +
> > > > >  /* device state */
> > > > >  #define UBLK_S_DEV_DEAD        0
> > > > >  #define UBLK_S_DEV_LIVE        1
> > > > > @@ -469,6 +508,12 @@ struct ublksrv_ctrl_dev_info {
> > > > >  #define                UBLK_IO_F_NEED_REG_BUF          (1U << 17)
> > > > >  /* Request has an integrity data buffer */
> > > > >  #define                UBLK_IO_F_INTEGRITY             (1UL << 18)
> > > > > +/*
> > > > > + * I/O buffer is in a registered shared memory buffer. When set, the addr
> > > > > + * field in ublksrv_io_desc encodes buffer index and byte offset instead
> > > > > + * of a userspace virtual address.
> > > > > + */
> > > > > +#define                UBLK_IO_F_SHMEM_ZC              (1U << 19)
> > > > >
> > > > >  /*
> > > > >   * io cmd is described by this structure, and stored in share memory, indexed
> > > > > @@ -743,4 +788,31 @@ struct ublk_params {
> > > > >         struct ublk_param_integrity     integrity;
> > > > >  };
> > > > >
> > > > > +/*
> > > > > + * Shared memory zero-copy addr encoding for UBLK_IO_F_SHMEM_ZC.
> > > > > + *
> > > > > + * When UBLK_IO_F_SHMEM_ZC is set, ublksrv_io_desc.addr is encoded as:
> > > > > + *   bits [0:31]  = byte offset within the buffer (up to 4GB)
> > > > > + *   bits [32:47] = buffer index (up to 65536)
> > > > > + *   bits [48:63] = reserved (must be zero)
> > > >
> > > > I wonder whether the "buffer index" is necessary. Can iod->addr and
> > > > UBLK_U_CMD_UNREG_BUF refer to the buffer by the virtual address used
> > > > with UBLK_U_CMD_REG_BUF? Then struct ublksrv_io_desc's addr field
> > > > would retain its meaning. We would also avoid needing to compare the
> > > > range buf_index values in ublk_try_buf_match(). And the xarray
> > > > wouldn't be necessary to allocate buffer indices.
> > >
> > > There are several reasons for choosing "buffer index":
> > >
> > > - avoid to add extra storage in `struct ublk_buf_range`, extra
> > >   `vaddr` field is required for returning virtual address; otherwise one extra
> > >   lookup from buffer index is needed in fast path of ublk_try_buf_match()
> >
> > Yes, struct ublk_buf_range would have to track the virtual address,
> > but that would replace both buf_index and base_offset. And you could
> > store flags in the lower bits of the virtual address (since it has to
> > be page-aligned) if you want to keep it as 16 bytes. I'm not sure what
> > you mean about "one extra lookup from buffer index"; I was thinking it
> > would be possible to get rid of buffer indices entirely.
> >
> > >
> > > - I want ublk server to know the difference between shmem_zc buffer and the
> > >   plain IO buffer clearly, both two shouldn't be mixed, otherwise it is easy to
> > >   cause data corruption. For example, client is using buf A, but the
> > >   ublk server fallback code path may be using it at the same time.
> >
> > Yes, certainly the ublk server should only use a shared-memory buffer
> > when an incoming UBLK_IO_F_SHMEM_ZC request specifies it. I don't see
> > the connection to referring to buffers by virtual address instead of
> > buffer index, though. The kernel can't prevent the ublk server from
> > writing to a registered shared memory region it has mapped into its
> > address space.
> >
> > It seems like a simpler interface if iod->addr indicates the virtual
> > address of the data buffer regardless of the UBLK_IO_F_SHMEM_ZC flag.
> > Then the ublk server doesn't have to care whether the data is in
> > shared memory or was copied automatically by the
> > ublk_map_io()/ublk_unmap_io() fallback path; it just accesses the
> > memory at iod->addr and lets the kernel take care of the copying (if
> > necessary).
> >
> > >
> > > - total registered buffers can be limited naturally by `u16` buffer_index
> >
> > I don't really see a reason to limit the number of SHMEM_ZC buffers if
> > they don't consume any per-buffer resources. I think it would make
> > more sense to limit it based on the _total size_ of registered
> > buffers, but the page pinning should already provide that.
> >
> > >
> > > - it is proved that buffer index is one nice pattern wrt. buffer
> > >   registration, such as io_uring fixed buffer; and it helps userspace
> > >   to manage multiple buffers, given each one has unique ID.
> >
> > If you really prefer the (buf_index, buf_offset) interface, I won't
> > stand in the way. It just seems to me like the only thing the ublk
> > server cares about a shared memory buffer is its virtual address, so
> > that's a more natural way to refer to it.
>
> It looks a little simpler from UAPI viewpoint, but storing vaddr in maple
> tree introduces some implementation issues:
>
> - vaddr is bound with task, so ublk device has to track task context, such
>   as, when we allow to register buffer before adding device, the buffer has
>   to be guaranteed to belong to ublk daemon(tracked in future)

Don't all tasks within a process share the same virtual address space?
Then the threads in the ublk server process should all have the same
virtual address for the registered buffer. I guess it's theoretically
possible that a different process would register a shared memory
region on behalf of the ublk server, and then they could have
different virtual addresses for it, but that seems like a bizarre use
case.

>
> - not like the plain page copy code path, in which the buffer vaddr is
>   always passed in daemon context; but buffer register can be done in any
>   task context.
>
> - not get ID for buffer, it could become a bit complicated to handle
>   recover if we store vaddr in mapple tree; vaddr can become invalid in
>   device lifetime, but buf index is device-wide, which can't become obsolete.

Fair point, though I'm not even sure how the new ublk server process
would be aware of the shared memory regions that belonged to the
previous process. If they were mapped from an anonymous memfd, the new
process wouldn't have any way to access them. I think it probably
makes more sense to unregister the shared memory regions when the ublk
server process exits.

>
> Anyway, I don't object to switch to vaddr based UAPI, and we have enough
> time to think it though and take it before 7.1 release.
>
> I will send patchset to cover other review comments first.

Sure, I think you have good reasons for the buf_index representation
and it doesn't really matter either way. I'm fine keeping this
interface.

Best,
Caleb

^ permalink raw reply

* Re: [PATCH 4/8] FOLD: block: change the defer in task context interface to be procedural
From: Matthew Wilcox @ 2026-04-09 20:18 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Tal Zussman, Jens Axboe, Christian Brauner, Darrick J. Wong,
	Carlos Maiolino, Al Viro, Jan Kara, Dave Chinner, Bart Van Assche,
	Gao Xiang, linux-block, linux-kernel, linux-xfs, linux-fsdevel,
	linux-mm
In-Reply-To: <20260409160243.1008358-5-hch@lst.de>

On Thu, Apr 09, 2026 at 06:02:17PM +0200, Christoph Hellwig wrote:
> @@ -1836,9 +1837,7 @@ void bio_endio(struct bio *bio)
>  	}
>  #endif
>  
> -	if (!in_task() && bio_flagged(bio, BIO_COMPLETE_IN_TASK))
> -		bio_queue_completion(bio);
> -	else if (bio->bi_end_io)
> +	if (bio->bi_end_io)
>  		bio->bi_end_io(bio);

What I liked about this before is that we had one central place that
needed to be changed.  This change means that every bi_end_io now needs
to check whether the BIO can be completed in its context.

> +++ b/fs/buffer.c
> @@ -2673,6 +2673,9 @@ static void end_bio_bh_io_sync(struct bio *bio)
>  {
>  	struct buffer_head *bh = bio->bi_private;
>  
> +	if (buffer_dropbehind(bh) && bio_complete_in_task(bio))
> +		return;

I really don't like this.  It assumes there's only one reason to
complete in task context -- whether the buffer belongs to a dropbehind
folio.  I want there to be other reasons.  Why would you introduce the
new BH_dropbehind flag instead of checking BIO_COMPLETE_IN_TASK?

>  	struct iomap_ioend *ioend = iomap_ioend_from_bio(bio);
>  
> +	/* Page cache invalidation cannot be done in irq context. */
> +	if (ioend->io_flags & IOMAP_IOEND_DONTCACHE) {
> +		if (bio_complete_in_task(bio))
> +			return;
> +	}

I thought we agreed to kill off IOMAP_IOEND_DONTCACHE?


^ permalink raw reply

* Re: [PATCH 8/8] RFC: use a TASK_FIFO kthread for read completion support
From: Tal Zussman @ 2026-04-09 19:06 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Matthew Wilcox (Oracle),
	Christian Brauner, Darrick J. Wong, Carlos Maiolino, Al Viro,
	Jan Kara
  Cc: Dave Chinner, Bart Van Assche, Gao Xiang, linux-block,
	linux-kernel, linux-xfs, linux-fsdevel, linux-mm
In-Reply-To: <20260409160243.1008358-9-hch@lst.de>



On 4/9/26 12:02 PM, Christoph Hellwig wrote:
> Commit 3fffb589b9a6 ("erofs: add per-cpu threads for decompression as an
> option") explains why workqueue aren't great for low-latency completion
> handling.  Switch to a per-cpu kthread to handle it instead.  This code
> is based on the erofs code in the above commit, but further simplified
> by directly using a kthread instead of a kthread_work.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   block/bio.c | 117 +++++++++++++++++++++++++++++-----------------------
>   1 file changed, 65 insertions(+), 52 deletions(-)
> 
> diff --git a/block/bio.c b/block/bio.c
> index 88d191455762..6a993fb129a0 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -19,7 +19,7 @@
>   #include <linux/blk-crypto.h>
>   #include <linux/xarray.h>
>   #include <linux/kmemleak.h>
> -#include <linux/llist.h>
> +#include <linux/freezer.h>

Why freezer.h and not kthread.h?

>   #include <trace/events/block.h>
>   #include "blk.h"
> @@ -1718,51 +1718,83 @@ void bio_check_pages_dirty(struct bio *bio)
>   EXPORT_SYMBOL_GPL(bio_check_pages_dirty);
>   
>   struct bio_complete_batch {
> -	struct llist_head list;

If we go with this approach, we should remove the newly-added bi_llist from
struct bio too.

> -	struct delayed_work work;
> -	int cpu;
> +	spinlock_t lock;
> +	struct bio_list bios;
> +	struct task_struct *worker;
>   };
>   
>   static DEFINE_PER_CPU(struct bio_complete_batch, bio_complete_batch);
> -static struct workqueue_struct *bio_complete_wq;
>   
> -static void bio_complete_work_fn(struct work_struct *w)
> +static bool bio_try_complete_batch(struct bio_complete_batch *batch)
>   {
> -	struct delayed_work *dw = to_delayed_work(w);
> -	struct bio_complete_batch *batch =
> -		container_of(dw, struct bio_complete_batch, work);
> -	struct llist_node *node;
> -	struct bio *bio, *next;
> +	struct bio_list bios;
> +	unsigned long flags;
> +	struct bio *bio;
>   
> -	do {
> -		node = llist_del_all(&batch->list);
> -		if (!node)
> -			break;
> +	spin_lock_irqsave(&batch->lock, flags);
> +	bios = batch->bios;
> +	bio_list_init(&batch->bios);
> +	spin_unlock_irqrestore(&batch->lock, flags);
>   
> -		node = llist_reverse_order(node);
> -		llist_for_each_entry_safe(bio, next, node, bi_llist)
> -			bio->bi_end_io(bio);
> +	if (bio_list_empty(&bios))
> +		return false;
>   
> -		if (need_resched()) {
> -			if (!llist_empty(&batch->list))
> -				mod_delayed_work_on(batch->cpu,
> -						    bio_complete_wq,
> -						    &batch->work, 0);
> -			break;
> -		}
> -	} while (1);
> +	__set_current_state(TASK_RUNNING);
> +	while ((bio = bio_list_pop(&bios)))
> +		bio->bi_end_io(bio);
> +	return true;
> +}
> +
> +static int bio_complete_thread(void *private)
> +{
> +	struct bio_complete_batch *batch = private;
> +
> +	for (;;) {
> +		set_current_state(TASK_INTERRUPTIBLE);
> +		if (!bio_try_complete_batch(batch))
> +			schedule();
> +	}
> +
> +	return 0;
>   }
>   
>   void __bio_complete_in_task(struct bio *bio)
>   {
> -	struct bio_complete_batch *batch = this_cpu_ptr(&bio_complete_batch);
> +	struct bio_complete_batch *batch;
> +	unsigned long flags;
> +	bool wake;
> +
> +	get_cpu();
> +	batch = this_cpu_ptr(&bio_complete_batch);
> +	spin_lock_irqsave(&batch->lock, flags);
> +	wake = bio_list_empty(&batch->bios);
> +	bio_list_add(&batch->bios, bio);
> +	spin_unlock_irqrestore(&batch->lock, flags);
> +	put_cpu();
>   
> -	if (llist_add(&bio->bi_llist, &batch->list))
> -		mod_delayed_work_on(batch->cpu, bio_complete_wq,
> -				    &batch->work, 1);
> +	if (wake)
> +		wake_up_process(batch->worker);
>   }
>   EXPORT_SYMBOL_GPL(__bio_complete_in_task);
>   
> +static void __init bio_complete_batch_init(int cpu)
> +{
> +	struct bio_complete_batch *batch =
> +		per_cpu_ptr(&bio_complete_batch, cpu);
> +	struct task_struct *worker;
> +
> +	worker = kthread_create_on_cpu(bio_complete_thread,
> +			per_cpu_ptr(&bio_complete_batch, cpu),
> +			cpu, "bio_worker/%u");
> +	if (IS_ERR(worker))
> +		panic("bio: can't create kthread_work");
> +	sched_set_fifo_low(worker);
> +
> +	spin_lock_init(&batch->lock);
> +	bio_list_init(&batch->bios);
> +	batch->worker = worker;
> +}
> +
>   static inline bool bio_remaining_done(struct bio *bio)
>   {
>   	/*
> @@ -2028,16 +2060,7 @@ EXPORT_SYMBOL(bioset_init);
>    */
>   static int bio_complete_batch_cpu_dead(unsigned int cpu)
>   {
> -	struct bio_complete_batch *batch =
> -		per_cpu_ptr(&bio_complete_batch, cpu);
> -	struct llist_node *node;
> -	struct bio *bio, *next;
> -
> -	node = llist_del_all(&batch->list);
> -	node = llist_reverse_order(node);
> -	llist_for_each_entry_safe(bio, next, node, bi_llist)
> -		bio->bi_end_io(bio);
> -
> +	bio_try_complete_batch(per_cpu_ptr(&bio_complete_batch, cpu));
>   	return 0;
>   }
>   
> @@ -2055,18 +2078,8 @@ static int __init init_bio(void)
>   				SLAB_HWCACHE_ALIGN | SLAB_PANIC, NULL);
>   	}
>   
> -	for_each_possible_cpu(i) {
> -		struct bio_complete_batch *batch =
> -			per_cpu_ptr(&bio_complete_batch, i);
> -
> -		init_llist_head(&batch->list);
> -		INIT_DELAYED_WORK(&batch->work, bio_complete_work_fn);
> -		batch->cpu = i;
> -	}
> -
> -	bio_complete_wq = alloc_workqueue("bio_complete", WQ_MEM_RECLAIM, 0);
> -	if (!bio_complete_wq)
> -		panic("bio: can't allocate bio_complete workqueue\n");
> +	for_each_possible_cpu(i)
> +		bio_complete_batch_init(i);
>   
>   	cpuhp_setup_state(CPUHP_BP_PREPARE_DYN, "block/bio:complete:dead",
>   				NULL, bio_complete_batch_cpu_dead);
> -- 
> 2.47.3
> 


^ permalink raw reply

* Re: [PATCH RFC v5 3/3] block: enable RWF_DONTCACHE for block devices
From: Tal Zussman @ 2026-04-09 18:59 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Matthew Wilcox (Oracle), Christian Brauner,
	Darrick J. Wong, Carlos Maiolino, Alexander Viro, Jan Kara,
	Dave Chinner, Bart Van Assche, linux-block, linux-kernel,
	linux-xfs, linux-fsdevel, linux-mm
In-Reply-To: <adfPA0_U312XhCYF@infradead.org>

On 4/9/26 12:08 PM, Christoph Hellwig wrote:
> For the next round please split this into the buffer.c parts for
> the infrastructure and the use in the block device file operations.

Will do.

^ permalink raw reply

* Re: [PATCH RFC v5 0/3] block: enable RWF_DONTCACHE for block devices
From: Tal Zussman @ 2026-04-09 18:56 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jens Axboe, Matthew Wilcox (Oracle), Christian Brauner,
	Darrick J. Wong, Carlos Maiolino, Alexander Viro, Jan Kara,
	Dave Chinner, Bart Van Assche, linux-block, linux-kernel,
	linux-xfs, linux-fsdevel, linux-mm
In-Reply-To: <addM6bkKKtoMvDIG@infradead.org>

On 4/9/26 2:53 AM, Christoph Hellwig wrote:
> What tree is this against?  I tried Jens' for-next and for-7.1/block
> trees, linux-next and current mainline and it does not apply to any
> of them.

Sorry about that -- I had previously based this on an earlier rc and forgot
to rebase. For the next iteration, is there a preferred tree to rebase
onto? I see you rebased this on linux-next.


^ permalink raw reply

* Re: [PATCH RFC v4 1/3] block: add BIO_COMPLETE_IN_TASK for task-context completion
From: Tal Zussman @ 2026-04-09 18:54 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Matthew Wilcox, Christian Brauner, Darrick J. Wong,
	Carlos Maiolino, Alexander Viro, Jan Kara, Christoph Hellwig,
	linux-block, linux-kernel, linux-xfs, linux-fsdevel, linux-mm
In-Reply-To: <95F28FA1-5CEF-4E80-BBB7-A429B4437D12@kernel.dk>



On 4/8/26 7:36 PM, Jens Axboe wrote:
> On Apr 8, 2026, at 4:51 PM, Tal Zussman <tz2294@columbia.edu> wrote:
>>> On 4/8/26 3:51 PM, Jens Axboe wrote:
>>>> On 4/8/26 12: 48 PM, Tal Zussman wrote: > On 3/25/26 4: 14 PM, Jens Axboe wrote:
>>>> 
>>>> Thanks! I'm going to give Dave's llist suggestion a shot on top of
>>>> this as it seems like it'll simplify this nicely. Looks like that'll
>>>> involve turning bio::bi_next into a union with a struct llist_node.
>>> 
>>> Since these lists can get long, I'd keep an eye on llist reversal
>>> overhead there...
>>> 
>> 
>> Going to send v5 shortly -- tested with and without the llist reversal and
>> it didn't seem to make much of a difference. This was on a single-disk VM
>> though, so any stress testing you could do would be very helpful.
>> 
> 
> With all due respect, a single test like that isn’t going to be that useful. I’d be wary of making that change willy nilly and just thinking “it’s fine, worked fine on the one case I tested”.

Understood -- unfortunately that's what I have access to at the moment. I
can requisition a machine with 2, maybe 3 disks, and test more thoroughly on
that before sending the next version, but that'll take a few days. You had
previously offered to test on your big box, so was hoping that was still on
the table :)
(Although Christoph seems to have proposed moving away from llist again)

> —
> Jens Axboe
> 


^ permalink raw reply

* Re: [PATCH 00/61] treewide: Use IS_ERR_OR_NULL over manual NULL check - refactor
From: Al Viro @ 2026-04-09 18:16 UTC (permalink / raw)
  To: Philipp Hahn
  Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
	gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
	linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
	linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
	linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
	linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
	linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
	linux-sctp, linux-security-module, linux-sh, linux-sound,
	linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
	netdev, ntfs3, samba-technical, sched-ext, target-devel,
	tipc-discussion, v9fs, Julia Lawall, Nicolas Palix, Chris Mason,
	David Sterba, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
	Theodore Ts'o, Andreas Dilger, Steve French, Paulo Alcantara,
	Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Bharath SM,
	Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
	Christian Schoenebeck, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
	Sandeep Dhavale, Hongbo Li, Chunhai Guo, Miklos Szeredi,
	Konstantin Komarov, Andreas Gruenbacher, Kees Cook, Tony Luck,
	Guilherme G. Piccoli, Jan Kara, Phillip Lougher,
	Christian Brauner, Jan Kara, Steven Rostedt, Masami Hiramatsu,
	Mathieu Desnoyers, Tejun Heo, David Vernet, Andrea Righi,
	Changwoo Min, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Vincent Guittot, Dietmar Eggemann, Ben Segall, Mel Gorman,
	Valentin Schneider, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Aaron Tomlin, Sylwester Nawrocki, Liam Girdwood,
	Mark Brown, Jaroslav Kysela, Takashi Iwai, Max Filippov,
	Paolo Bonzini, John Johansen, Paul Moore, James Morris,
	Serge E. Hallyn, Andrew Morton, Alasdair Kergon, Mike Snitzer,
	Mikulas Patocka, Benjamin Marzinski, David S. Miller, David Ahern,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
	Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
	John Fastabend, Stanislav Fomichev, Jamal Hadi Salim, Jiri Pirko,
	Marcelo Ricardo Leitner, Xin Long, Trond Myklebust,
	Anna Schumaker, Chuck Lever, Jeff Layton, NeilBrown,
	Olga Kornievskaia, Dai Ngo, Jon Maloy, Johannes Berg,
	Catalin Marinas, Russell King, John Crispin, Thomas Bogendoerfer,
	Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
	Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Zhenyu Wang,
	Zhi Wang, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
	Tvrtko Ursulin, Alex Deucher, Christian König, Sandy Huang,
	Heiko Stübner, Andy Yan, Igor Russkikh, Andrew Lunn,
	Pavan Chebbi, Michael Chan, Potnuri Bharat Teja, Tony Nguyen,
	Przemek Kitszel, Taras Chornyi, Maxime Coquelin, Alexandre Torgue,
	Iyappan Subramanian, Keyur Chudgar, Quan Nguyen, Heiner Kallweit,
	Marc Zyngier, Thomas Gleixner, Andrew Lunn, Gregory Clement,
	Sebastian Hesselbarth, Vinod Koul, Linus Walleij, Ulf Hansson,
	Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, Martin K. Petersen,
	Eduardo Valentin, Keerthy, Rafael J. Wysocki, Daniel Lezcano,
	Zhang Rui, Lukasz Luba, Alex Williamson, Mark Greer,
	Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
	Shuah Khan, Kieran Bingham, Mauro Carvalho Chehab, Joerg Roedel,
	Will Deacon, Robin Murphy, Lee Jones, Pavel Machek, Dave Penkler,
	K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
	Justin Sanders, Jens Axboe, Georgi Djakov, Michael Turquette,
	Stephen Boyd, Philipp Zabel, Borislav Petkov, Dave Hansen, x86,
	H. Peter Anvin, Pali Rohár, Dmitry Torokhov
In-Reply-To: <20260310-b4-is_err_or_null-v1-0-bd63b656022d@avm.de>

On Tue, Mar 10, 2026 at 12:48:26PM +0100, Philipp Hahn wrote:
> While doing some static code analysis I stumbled over a common pattern,
> where IS_ERR() is combined with a NULL check. For that there is
> IS_ERR_OR_NULL().

... and valid uses of IS_ERR_OR_NULL are rare as hen teeth.
Most of those are "I'm not sure how this function returns an
error, let's use that just in case".

Please, do not introduce more of that crap.

^ permalink raw reply

* Re: [PATCH RFC v5 3/3] block: enable RWF_DONTCACHE for block devices
From: Christoph Hellwig @ 2026-04-09 16:08 UTC (permalink / raw)
  To: Tal Zussman
  Cc: Jens Axboe, Matthew Wilcox (Oracle), Christian Brauner,
	Darrick J. Wong, Carlos Maiolino, Alexander Viro, Jan Kara,
	Christoph Hellwig, Dave Chinner, Bart Van Assche, linux-block,
	linux-kernel, linux-xfs, linux-fsdevel, linux-mm
In-Reply-To: <20260408-blk-dontcache-v5-3-0f080c20a96f@columbia.edu>

For the next round please split this into the buffer.c parts for
the infrastructure and the use in the block device file operations.


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox