* 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
* [PATCH 8/8] RFC: use a TASK_FIFO kthread for read completion support
From: Christoph Hellwig @ 2026-04-09 16:02 UTC (permalink / raw)
To: Tal Zussman, 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-1-hch@lst.de>
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>
#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;
- 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 related
* [PATCH 7/8] iomap: use bio_complete_in_task for buffered write completions
From: Christoph Hellwig @ 2026-04-09 16:02 UTC (permalink / raw)
To: Tal Zussman, 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-1-hch@lst.de>
Replace out own hand-crafted complete in task context scheme with the
generic block code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/iomap/ioend.c | 53 +++++-------------------------------------------
1 file changed, 5 insertions(+), 48 deletions(-)
diff --git a/fs/iomap/ioend.c b/fs/iomap/ioend.c
index a32ece8a3ee3..160224007486 100644
--- a/fs/iomap/ioend.c
+++ b/fs/iomap/ioend.c
@@ -72,63 +72,20 @@ static u32 iomap_finish_ioend_buffered_write(struct iomap_ioend *ioend)
return folio_count;
}
-static DEFINE_SPINLOCK(failed_ioend_lock);
-static LIST_HEAD(failed_ioend_list);
-
-static void
-iomap_fail_ioends(
- struct work_struct *work)
-{
- struct iomap_ioend *ioend;
- struct list_head tmp;
- unsigned long flags;
-
- spin_lock_irqsave(&failed_ioend_lock, flags);
- list_replace_init(&failed_ioend_list, &tmp);
- spin_unlock_irqrestore(&failed_ioend_lock, flags);
-
- while ((ioend = list_first_entry_or_null(&tmp, struct iomap_ioend,
- io_list))) {
- list_del_init(&ioend->io_list);
- iomap_finish_ioend_buffered_write(ioend);
- cond_resched();
- }
-}
-
-static DECLARE_WORK(failed_ioend_work, iomap_fail_ioends);
-
-static void iomap_fail_ioend_buffered(struct iomap_ioend *ioend)
-{
- unsigned long flags;
-
- /*
- * Bounce I/O errors to a workqueue to avoid nested i_lock acquisitions
- * in the fserror code. The caller no longer owns the ioend reference
- * after the spinlock drops.
- */
- spin_lock_irqsave(&failed_ioend_lock, flags);
- if (list_empty(&failed_ioend_list))
- WARN_ON_ONCE(!schedule_work(&failed_ioend_work));
- list_add_tail(&ioend->io_list, &failed_ioend_list);
- spin_unlock_irqrestore(&failed_ioend_lock, flags);
-}
-
static void ioend_writeback_end_bio(struct bio *bio)
{
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) {
+ /*
+ * Page cache invalidation and error reporting cannot be done in irq
+ * context.
+ */
+ if ((ioend->io_flags & IOMAP_IOEND_DONTCACHE) || bio->bi_status) {
if (bio_complete_in_task(bio))
return;
}
ioend->io_error = blk_status_to_errno(bio->bi_status);
- if (ioend->io_error) {
- iomap_fail_ioend_buffered(ioend);
- return;
- }
-
iomap_finish_ioend_buffered_write(ioend);
}
--
2.47.3
^ permalink raw reply related
* [PATCH 6/8] iomap: use bio_complete_in_task for buffered read errors
From: Christoph Hellwig @ 2026-04-09 16:02 UTC (permalink / raw)
To: Tal Zussman, 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-1-hch@lst.de>
Replace out own hand-crafted complete in task context scheme with the
generic block code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/iomap/bio.c | 44 +-------------------------------------------
1 file changed, 1 insertion(+), 43 deletions(-)
diff --git a/fs/iomap/bio.c b/fs/iomap/bio.c
index 4504f4633f17..5b9b91198ec8 100644
--- a/fs/iomap/bio.c
+++ b/fs/iomap/bio.c
@@ -9,9 +9,6 @@
#include "internal.h"
#include "trace.h"
-static DEFINE_SPINLOCK(failed_read_lock);
-static struct bio_list failed_read_list = BIO_EMPTY_LIST;
-
static u32 __iomap_read_end_io(struct bio *bio, int error)
{
struct folio_iter fi;
@@ -27,49 +24,10 @@ static u32 __iomap_read_end_io(struct bio *bio, int error)
return folio_count;
}
-static void
-iomap_fail_reads(
- struct work_struct *work)
-{
- struct bio *bio;
- struct bio_list tmp = BIO_EMPTY_LIST;
- unsigned long flags;
-
- spin_lock_irqsave(&failed_read_lock, flags);
- bio_list_merge_init(&tmp, &failed_read_list);
- spin_unlock_irqrestore(&failed_read_lock, flags);
-
- while ((bio = bio_list_pop(&tmp)) != NULL) {
- __iomap_read_end_io(bio, blk_status_to_errno(bio->bi_status));
- cond_resched();
- }
-}
-
-static DECLARE_WORK(failed_read_work, iomap_fail_reads);
-
-static void iomap_fail_buffered_read(struct bio *bio)
-{
- unsigned long flags;
-
- /*
- * Bounce I/O errors to a workqueue to avoid nested i_lock acquisitions
- * in the fserror code. The caller no longer owns the bio reference
- * after the spinlock drops.
- */
- spin_lock_irqsave(&failed_read_lock, flags);
- if (bio_list_empty(&failed_read_list))
- WARN_ON_ONCE(!schedule_work(&failed_read_work));
- bio_list_add(&failed_read_list, bio);
- spin_unlock_irqrestore(&failed_read_lock, flags);
-}
-
static void iomap_read_end_io(struct bio *bio)
{
- if (bio->bi_status) {
- iomap_fail_buffered_read(bio);
+ if (bio->bi_status && bio_complete_in_task(bio))
return;
- }
-
__iomap_read_end_io(bio, 0);
}
--
2.47.3
^ permalink raw reply related
* [PATCH 5/8] FOLD: don't use in_task() to decide for offloading
From: Christoph Hellwig @ 2026-04-09 16:02 UTC (permalink / raw)
To: Tal Zussman, 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-1-hch@lst.de>
As described in commit c99fab6e80b76, some block drivers might call
into ->bi_end_io from non-preemptible context. Copy and past the
logic from that commit, although having a core helper for it would
be nicer.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/bio.h | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 45c311e5ff71..72664807c757 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -375,6 +375,16 @@ static inline struct bio *bio_alloc(struct block_device *bdev,
void submit_bio(struct bio *bio);
+/* Offload from atomic contexts to minimize scheduling overhead */
+static inline bool bio_in_atomic(void)
+{
+ if (IS_ENABLED(CONFIG_PREEMPTION) && rcu_preempt_depth())
+ return true;
+ if (!IS_ENABLED(CONFIG_PREEMPT_COUNT))
+ return true;
+ return !preemptible();
+}
+
void __bio_complete_in_task(struct bio *bio);
/**
@@ -386,10 +396,11 @@ void __bio_complete_in_task(struct bio *bio);
*/
static inline bool bio_complete_in_task(struct bio *bio)
{
- if (in_task())
- return false;
- __bio_complete_in_task(bio);
- return true;
+ if (bio_in_atomic()) {
+ __bio_complete_in_task(bio);
+ return true;
+ }
+ return false;
}
extern void bio_endio(struct bio *);
--
2.47.3
^ permalink raw reply related
* [PATCH 4/8] FOLD: block: change the defer in task context interface to be procedural
From: Christoph Hellwig @ 2026-04-09 16:02 UTC (permalink / raw)
To: Tal Zussman, 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-1-hch@lst.de>
Replace the bio-flag based interface with an explicit
bio_complete_in_task() API. The advantage is that this can also be
called from inside the ->bi_end_io callback and thus dynamically.
This will be important to use it for fserror reporting.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/bio.c | 7 +++----
fs/buffer.c | 5 ++++-
fs/iomap/ioend.c | 11 ++++++++---
include/linux/bio.h | 17 +++++++++++++++++
include/linux/blk_types.h | 1 -
include/linux/buffer_head.h | 2 ++
include/linux/iomap.h | 6 +++++-
7 files changed, 39 insertions(+), 10 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 550eb770bfa6..88d191455762 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1753,7 +1753,7 @@ static void bio_complete_work_fn(struct work_struct *w)
} while (1);
}
-static void bio_queue_completion(struct bio *bio)
+void __bio_complete_in_task(struct bio *bio)
{
struct bio_complete_batch *batch = this_cpu_ptr(&bio_complete_batch);
@@ -1761,6 +1761,7 @@ static void bio_queue_completion(struct bio *bio)
mod_delayed_work_on(batch->cpu, bio_complete_wq,
&batch->work, 1);
}
+EXPORT_SYMBOL_GPL(__bio_complete_in_task);
static inline bool bio_remaining_done(struct bio *bio)
{
@@ -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);
}
EXPORT_SYMBOL(bio_endio);
diff --git a/fs/buffer.c b/fs/buffer.c
index 289ab33fe3fd..b5de776c8491 100644
--- a/fs/buffer.c
+++ 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;
+
if (unlikely(bio_flagged(bio, BIO_QUIET)))
set_bit(BH_Quiet, &bh->b_state);
@@ -2725,7 +2728,7 @@ static void submit_bh_wbc(blk_opf_t opf, struct buffer_head *bh,
buffer_set_crypto_ctx(bio, bh, GFP_NOIO);
if (folio_test_dropbehind(bh->b_folio))
- bio_set_flag(bio, BIO_COMPLETE_IN_TASK);
+ set_buffer_dropbehind(bh);
bio->bi_iter.bi_sector = bh->b_blocknr * (bh->b_size >> 9);
bio->bi_write_hint = write_hint;
diff --git a/fs/iomap/ioend.c b/fs/iomap/ioend.c
index 892dbfc77ae9..a32ece8a3ee3 100644
--- a/fs/iomap/ioend.c
+++ b/fs/iomap/ioend.c
@@ -117,6 +117,12 @@ static void ioend_writeback_end_bio(struct bio *bio)
{
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;
+ }
+
ioend->io_error = blk_status_to_errno(bio->bi_status);
if (ioend->io_error) {
iomap_fail_ioend_buffered(ioend);
@@ -237,6 +243,8 @@ ssize_t iomap_add_to_ioend(struct iomap_writepage_ctx *wpc, struct folio *folio,
if (wpc->iomap.flags & IOMAP_F_SHARED)
ioend_flags |= IOMAP_IOEND_SHARED;
+ if (folio_test_dropbehind(folio))
+ ioend_flags |= IOMAP_IOEND_DONTCACHE;
if (pos == wpc->iomap.offset && (wpc->iomap.flags & IOMAP_F_BOUNDARY))
ioend_flags |= IOMAP_IOEND_BOUNDARY;
@@ -253,9 +261,6 @@ ssize_t iomap_add_to_ioend(struct iomap_writepage_ctx *wpc, struct folio *folio,
if (!bio_add_folio(&ioend->io_bio, folio, map_len, poff))
goto new_ioend;
- if (folio_test_dropbehind(folio))
- bio_set_flag(&ioend->io_bio, BIO_COMPLETE_IN_TASK);
-
/*
* Clamp io_offset and io_size to the incore EOF so that ondisk
* file size updates in the ioend completion are byte-accurate.
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 0b6744557b42..45c311e5ff71 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -375,6 +375,23 @@ static inline struct bio *bio_alloc(struct block_device *bdev,
void submit_bio(struct bio *bio);
+void __bio_complete_in_task(struct bio *bio);
+
+/**
+ * bio_complete_in_task - ensure a bio is complete in preemptible task context
+ * @bio: bio to complete
+ *
+ * If called from non-task context, offload the bio completion to worker thread
+ * and return %true. Else return %false and do nothing.
+ */
+static inline bool bio_complete_in_task(struct bio *bio)
+{
+ if (in_task())
+ return false;
+ __bio_complete_in_task(bio);
+ return true;
+}
+
extern void bio_endio(struct bio *);
static inline void bio_io_error(struct bio *bio)
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 0b55159d110d..8419f42de14f 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -326,7 +326,6 @@ enum {
BIO_REMAPPED,
BIO_ZONE_WRITE_PLUGGING, /* bio handled through zone write plugging */
BIO_EMULATES_ZONE_APPEND, /* bio emulates a zone append operation */
- BIO_COMPLETE_IN_TASK, /* complete bi_end_io() in task context */
BIO_FLAG_LAST
};
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index 4ce50882d621..bd7df5883cc8 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -35,6 +35,7 @@ enum bh_state_bits {
BH_Prio, /* Buffer should be submitted with REQ_PRIO */
BH_Defer_Completion, /* Defer AIO completion to workqueue */
BH_Migrate, /* Buffer is being migrated (norefs) */
+ BH_Dropbehind, /* drop pages on IO completion */
BH_PrivateStart,/* not a state bit, but the first bit available
* for private allocation by other entities
@@ -136,6 +137,7 @@ BUFFER_FNS(Unwritten, unwritten)
BUFFER_FNS(Meta, meta)
BUFFER_FNS(Prio, prio)
BUFFER_FNS(Defer_Completion, defer_completion)
+BUFFER_FNS(Dropbehind, dropbehind)
static __always_inline void set_buffer_uptodate(struct buffer_head *bh)
{
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index bf49ba71dd42..2c5685adf3a9 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -399,12 +399,16 @@ sector_t iomap_bmap(struct address_space *mapping, sector_t bno,
#define IOMAP_IOEND_BOUNDARY (1U << 2)
/* is direct I/O */
#define IOMAP_IOEND_DIRECT (1U << 3)
+/* is DONTCACHE I/O */
+#define IOMAP_IOEND_DONTCACHE (1U << 4)
+
/*
* Flags that if set on either ioend prevent the merge of two ioends.
* (IOMAP_IOEND_BOUNDARY also prevents merges, but only one-way)
*/
#define IOMAP_IOEND_NOMERGE_FLAGS \
- (IOMAP_IOEND_SHARED | IOMAP_IOEND_UNWRITTEN | IOMAP_IOEND_DIRECT)
+ (IOMAP_IOEND_SHARED | IOMAP_IOEND_UNWRITTEN | IOMAP_IOEND_DIRECT | \
+ IOMAP_IOEND_DONTCACHE)
/*
* Structure for writeback I/O completions.
--
2.47.3
^ permalink raw reply related
* [PATCH 3/8] block: enable RWF_DONTCACHE for block devices
From: Christoph Hellwig @ 2026-04-09 16:02 UTC (permalink / raw)
To: Tal Zussman, 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-1-hch@lst.de>
From: Tal Zussman <tz2294@columbia.edu>
Block device buffered reads and writes already pass through
filemap_read() and iomap_file_buffered_write() respectively, both of
which handle IOCB_DONTCACHE. Enable RWF_DONTCACHE for block device files
by setting FOP_DONTCACHE in def_blk_fops.
For CONFIG_BUFFER_HEAD=y paths, add block_write_begin_iocb() which
threads the kiocb through so that buffer_head-based I/O can use
DONTCACHE behavior. The existing block_write_begin() is preserved as a
wrapper that passes a NULL iocb. Set BIO_COMPLETE_IN_TASK in
submit_bh_wbc() when the folio has dropbehind so that buffer_head
writeback completions get deferred to task context.
CONFIG_BUFFER_HEAD=n paths are handled by the previously added iomap
BIO_COMPLETE_IN_TASK support.
This support is useful for databases that operate on raw block devices,
among other userspace applications.
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
block/fops.c | 5 +++--
fs/buffer.c | 22 +++++++++++++++++++---
include/linux/buffer_head.h | 3 +++
3 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/block/fops.c b/block/fops.c
index bb6642b45937..31b073181d87 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -504,7 +504,8 @@ static int blkdev_write_begin(const struct kiocb *iocb,
unsigned len, struct folio **foliop,
void **fsdata)
{
- return block_write_begin(mapping, pos, len, foliop, blkdev_get_block);
+ return block_write_begin_iocb(iocb, mapping, pos, len, foliop,
+ blkdev_get_block);
}
static int blkdev_write_end(const struct kiocb *iocb,
@@ -966,7 +967,7 @@ const struct file_operations def_blk_fops = {
.splice_write = iter_file_splice_write,
.fallocate = blkdev_fallocate,
.uring_cmd = blkdev_uring_cmd,
- .fop_flags = FOP_BUFFER_RASYNC,
+ .fop_flags = FOP_BUFFER_RASYNC | FOP_DONTCACHE,
};
static __init int blkdev_init(void)
diff --git a/fs/buffer.c b/fs/buffer.c
index d6e062c42a8d..289ab33fe3fd 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -2131,14 +2131,19 @@ EXPORT_SYMBOL(block_commit_write);
*
* The filesystem needs to handle block truncation upon failure.
*/
-int block_write_begin(struct address_space *mapping, loff_t pos, unsigned len,
+int block_write_begin_iocb(const struct kiocb *iocb,
+ struct address_space *mapping, loff_t pos, unsigned len,
struct folio **foliop, get_block_t *get_block)
{
pgoff_t index = pos >> PAGE_SHIFT;
+ fgf_t fgp_flags = FGP_WRITEBEGIN;
struct folio *folio;
int status;
- folio = __filemap_get_folio(mapping, index, FGP_WRITEBEGIN,
+ if (iocb && iocb->ki_flags & IOCB_DONTCACHE)
+ fgp_flags |= FGP_DONTCACHE;
+
+ folio = __filemap_get_folio(mapping, index, fgp_flags,
mapping_gfp_mask(mapping));
if (IS_ERR(folio))
return PTR_ERR(folio);
@@ -2153,6 +2158,13 @@ int block_write_begin(struct address_space *mapping, loff_t pos, unsigned len,
*foliop = folio;
return status;
}
+
+int block_write_begin(struct address_space *mapping, loff_t pos, unsigned len,
+ struct folio **foliop, get_block_t *get_block)
+{
+ return block_write_begin_iocb(NULL, mapping, pos, len, foliop,
+ get_block);
+}
EXPORT_SYMBOL(block_write_begin);
int block_write_end(loff_t pos, unsigned len, unsigned copied,
@@ -2481,7 +2493,8 @@ int cont_write_begin(const struct kiocb *iocb, struct address_space *mapping,
(*bytes)++;
}
- return block_write_begin(mapping, pos, len, foliop, get_block);
+ return block_write_begin_iocb(iocb, mapping, pos, len, foliop,
+ get_block);
}
EXPORT_SYMBOL(cont_write_begin);
@@ -2711,6 +2724,9 @@ static void submit_bh_wbc(blk_opf_t opf, struct buffer_head *bh,
if (IS_ENABLED(CONFIG_FS_ENCRYPTION))
buffer_set_crypto_ctx(bio, bh, GFP_NOIO);
+ if (folio_test_dropbehind(bh->b_folio))
+ bio_set_flag(bio, BIO_COMPLETE_IN_TASK);
+
bio->bi_iter.bi_sector = bh->b_blocknr * (bh->b_size >> 9);
bio->bi_write_hint = write_hint;
diff --git a/include/linux/buffer_head.h b/include/linux/buffer_head.h
index e4939e33b4b5..4ce50882d621 100644
--- a/include/linux/buffer_head.h
+++ b/include/linux/buffer_head.h
@@ -260,6 +260,9 @@ int block_read_full_folio(struct folio *, get_block_t *);
bool block_is_partially_uptodate(struct folio *, size_t from, size_t count);
int block_write_begin(struct address_space *mapping, loff_t pos, unsigned len,
struct folio **foliop, get_block_t *get_block);
+int block_write_begin_iocb(const struct kiocb *iocb,
+ struct address_space *mapping, loff_t pos, unsigned len,
+ struct folio **foliop, get_block_t *get_block);
int __block_write_begin(struct folio *folio, loff_t pos, unsigned len,
get_block_t *get_block);
int block_write_end(loff_t pos, unsigned len, unsigned copied, struct folio *);
--
2.47.3
^ permalink raw reply related
* [PATCH 2/8] iomap: use BIO_COMPLETE_IN_TASK for dropbehind writeback
From: Christoph Hellwig @ 2026-04-09 16:02 UTC (permalink / raw)
To: Tal Zussman, 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-1-hch@lst.de>
From: Tal Zussman <tz2294@columbia.edu>
Set BIO_COMPLETE_IN_TASK on iomap writeback bios when a dropbehind folio
is added. This ensures that bi_end_io runs in task context, where
folio_end_dropbehind() can safely invalidate folios.
With the bio layer now handling task-context deferral generically,
IOMAP_IOEND_DONTCACHE is no longer needed, as XFS no longer needs to
route DONTCACHE ioends through its completion workqueue. Remove the flag
and its NOMERGE entry.
Without the NOMERGE, regular I/Os that get merged with a dropbehind
folio will also have their completion deferred to task context.
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
fs/iomap/ioend.c | 5 +++--
fs/xfs/xfs_aops.c | 4 ----
include/linux/iomap.h | 6 +-----
3 files changed, 4 insertions(+), 11 deletions(-)
diff --git a/fs/iomap/ioend.c b/fs/iomap/ioend.c
index acf3cf98b23a..892dbfc77ae9 100644
--- a/fs/iomap/ioend.c
+++ b/fs/iomap/ioend.c
@@ -237,8 +237,6 @@ ssize_t iomap_add_to_ioend(struct iomap_writepage_ctx *wpc, struct folio *folio,
if (wpc->iomap.flags & IOMAP_F_SHARED)
ioend_flags |= IOMAP_IOEND_SHARED;
- if (folio_test_dropbehind(folio))
- ioend_flags |= IOMAP_IOEND_DONTCACHE;
if (pos == wpc->iomap.offset && (wpc->iomap.flags & IOMAP_F_BOUNDARY))
ioend_flags |= IOMAP_IOEND_BOUNDARY;
@@ -255,6 +253,9 @@ ssize_t iomap_add_to_ioend(struct iomap_writepage_ctx *wpc, struct folio *folio,
if (!bio_add_folio(&ioend->io_bio, folio, map_len, poff))
goto new_ioend;
+ if (folio_test_dropbehind(folio))
+ bio_set_flag(&ioend->io_bio, BIO_COMPLETE_IN_TASK);
+
/*
* Clamp io_offset and io_size to the incore EOF so that ondisk
* file size updates in the ioend completion are byte-accurate.
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index f279055fcea0..0dcf78beae8a 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -511,10 +511,6 @@ xfs_ioend_needs_wq_completion(
if (ioend->io_flags & (IOMAP_IOEND_UNWRITTEN | IOMAP_IOEND_SHARED))
return true;
- /* Page cache invalidation cannot be done in irq context. */
- if (ioend->io_flags & IOMAP_IOEND_DONTCACHE)
- return true;
-
return false;
}
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 2c5685adf3a9..bf49ba71dd42 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -399,16 +399,12 @@ sector_t iomap_bmap(struct address_space *mapping, sector_t bno,
#define IOMAP_IOEND_BOUNDARY (1U << 2)
/* is direct I/O */
#define IOMAP_IOEND_DIRECT (1U << 3)
-/* is DONTCACHE I/O */
-#define IOMAP_IOEND_DONTCACHE (1U << 4)
-
/*
* Flags that if set on either ioend prevent the merge of two ioends.
* (IOMAP_IOEND_BOUNDARY also prevents merges, but only one-way)
*/
#define IOMAP_IOEND_NOMERGE_FLAGS \
- (IOMAP_IOEND_SHARED | IOMAP_IOEND_UNWRITTEN | IOMAP_IOEND_DIRECT | \
- IOMAP_IOEND_DONTCACHE)
+ (IOMAP_IOEND_SHARED | IOMAP_IOEND_UNWRITTEN | IOMAP_IOEND_DIRECT)
/*
* Structure for writeback I/O completions.
--
2.47.3
^ permalink raw reply related
* [PATCH 1/8] block: add BIO_COMPLETE_IN_TASK for task-context completion
From: Christoph Hellwig @ 2026-04-09 16:02 UTC (permalink / raw)
To: Tal Zussman, 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-1-hch@lst.de>
From: Tal Zussman <tz2294@columbia.edu>
Some bio completion handlers need to run in task context but bio_endio()
can be called from IRQ context (e.g. buffer_head writeback). Add a
BIO_COMPLETE_IN_TASK flag that bio submitters can set to request
task-context completion of their bi_end_io callback.
When bio_endio() sees this flag and is running in non-task context, it
queues the bio to a per-cpu lockless list and schedules a delayed work
item to call bi_end_io() from task context. The delayed work uses a
1-jiffie delay to allow batches of completions to accumulate before
processing. A CPU hotplug dead callback drains any remaining bios from
the departing CPU's batch.
This will be used to enable RWF_DONTCACHE for block devices, and could
be used for other subsystems like fscrypt that need task-context bio
completion.
Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
block/bio.c | 83 ++++++++++++++++++++++++++++++++++++++-
include/linux/blk_types.h | 7 +++-
2 files changed, 88 insertions(+), 2 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index 641ef0928d73..550eb770bfa6 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -19,6 +19,7 @@
#include <linux/blk-crypto.h>
#include <linux/xarray.h>
#include <linux/kmemleak.h>
+#include <linux/llist.h>
#include <trace/events/block.h>
#include "blk.h"
@@ -1716,6 +1717,51 @@ void bio_check_pages_dirty(struct bio *bio)
}
EXPORT_SYMBOL_GPL(bio_check_pages_dirty);
+struct bio_complete_batch {
+ struct llist_head list;
+ struct delayed_work work;
+ int cpu;
+};
+
+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)
+{
+ 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;
+
+ do {
+ node = llist_del_all(&batch->list);
+ if (!node)
+ break;
+
+ node = llist_reverse_order(node);
+ llist_for_each_entry_safe(bio, next, node, bi_llist)
+ bio->bi_end_io(bio);
+
+ if (need_resched()) {
+ if (!llist_empty(&batch->list))
+ mod_delayed_work_on(batch->cpu,
+ bio_complete_wq,
+ &batch->work, 0);
+ break;
+ }
+ } while (1);
+}
+
+static void bio_queue_completion(struct bio *bio)
+{
+ struct bio_complete_batch *batch = this_cpu_ptr(&bio_complete_batch);
+
+ if (llist_add(&bio->bi_llist, &batch->list))
+ mod_delayed_work_on(batch->cpu, bio_complete_wq,
+ &batch->work, 1);
+}
+
static inline bool bio_remaining_done(struct bio *bio)
{
/*
@@ -1790,7 +1836,9 @@ void bio_endio(struct bio *bio)
}
#endif
- if (bio->bi_end_io)
+ if (!in_task() && bio_flagged(bio, BIO_COMPLETE_IN_TASK))
+ bio_queue_completion(bio);
+ else if (bio->bi_end_io)
bio->bi_end_io(bio);
}
EXPORT_SYMBOL(bio_endio);
@@ -1976,6 +2024,24 @@ int bioset_init(struct bio_set *bs,
}
EXPORT_SYMBOL(bioset_init);
+/*
+ * Drain a dead CPU's deferred bio completions.
+ */
+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);
+
+ return 0;
+}
+
static int __init init_bio(void)
{
int i;
@@ -1990,6 +2056,21 @@ 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");
+
+ cpuhp_setup_state(CPUHP_BP_PREPARE_DYN, "block/bio:complete:dead",
+ NULL, bio_complete_batch_cpu_dead);
cpuhp_setup_state_multi(CPUHP_BIO_DEAD, "block/bio:dead", NULL,
bio_cpu_dead);
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 8808ee76e73c..0b55159d110d 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -11,6 +11,7 @@
#include <linux/device.h>
#include <linux/ktime.h>
#include <linux/rw_hint.h>
+#include <linux/llist.h>
struct bio_set;
struct bio;
@@ -208,7 +209,10 @@ typedef unsigned int blk_qc_t;
* stacking drivers)
*/
struct bio {
- struct bio *bi_next; /* request queue link */
+ union {
+ struct bio *bi_next; /* request queue link */
+ struct llist_node bi_llist; /* deferred completion */
+ };
struct block_device *bi_bdev;
blk_opf_t bi_opf; /* bottom bits REQ_OP, top bits
* req_flags.
@@ -322,6 +326,7 @@ enum {
BIO_REMAPPED,
BIO_ZONE_WRITE_PLUGGING, /* bio handled through zone write plugging */
BIO_EMULATES_ZONE_APPEND, /* bio emulates a zone append operation */
+ BIO_COMPLETE_IN_TASK, /* complete bi_end_io() in task context */
BIO_FLAG_LAST
};
--
2.47.3
^ permalink raw reply related
* bio completion in task enhancements / experiments
From: Christoph Hellwig @ 2026-04-09 16:02 UTC (permalink / raw)
To: Tal Zussman, 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
Hi all,
this series builds on top of:
Subject: [PATCH RFC v5 0/3] block: enable RWF_DONTCACHE for block devices
which I fixed up to apply to linux-next. If you want to seriously review
or test this, you're best off using the git branch here:
https://git.infradead.org/?p=users/hch/misc.git;a=shortlog;h=refs/heads/bio-task-completion
it first makes the complete in task interface more flexible so that it
can also be used from inside the ->bi_end_io handlers, which we'll need
for a few uses cases. The second patch fixes the offload condition, the
next two then convert to uses in iomap added in the current merge window
over to the interface.
The last patch plays with the implementation and reuses concepts from
erofs to reduce the completion latency at the expense of more always
alive threads.
There's a few other places that could benefit from this, like erofs
decompression, PI verification in the block and file systems paths, or
fscrypt decryption.
Diffstat:
block/bio.c | 93 ++++++++++++++++++++++++++++++++++++++++++++
block/fops.c | 5 +-
fs/buffer.c | 25 ++++++++++-
fs/iomap/bio.c | 44 --------------------
fs/iomap/ioend.c | 53 +++----------------------
fs/xfs/xfs_aops.c | 4 -
include/linux/bio.h | 28 +++++++++++++
include/linux/blk_types.h | 6 ++
include/linux/buffer_head.h | 5 ++
9 files changed, 165 insertions(+), 98 deletions(-)
^ permalink raw reply
* Re: [PATCH 11/13] libmultipath: Add support for block device IOCTL
From: John Garry @ 2026-04-09 15:20 UTC (permalink / raw)
To: Benjamin Marzinski
Cc: hch, kbusch, sagi, axboe, martin.petersen, james.bottomley, hare,
jmeneghi, linux-nvme, linux-scsi, michael.christie, snitzer,
dm-devel, linux-block, linux-kernel
In-Reply-To: <aaH18HKCMdjuUhUh@redhat.com>
On 27/02/2026 19:52, Benjamin Marzinski wrote:
>> diff --git a/include/linux/multipath.h b/include/linux/multipath.h
>> index 3846ea8cfd319..40dda6a914c5f 100644
>> --- a/include/linux/multipath.h
>> +++ b/include/linux/multipath.h
>> @@ -72,6 +72,9 @@ struct mpath_head_template {
>> bool (*is_disabled)(struct mpath_device *);
>> bool (*is_optimized)(struct mpath_device *);
>> enum mpath_access_state (*get_access_state)(struct mpath_device *);
>> + int (*bdev_ioctl)(struct block_device *bdev, struct mpath_device *,
>> + blk_mode_t mode, unsigned int cmd, unsigned long arg,
>> + int srcu_idx);
> I don't know that this API is going to work out. SCSI persistent
> reservations need access to all the mpath_devices, not just one, and
> they are commonly handled via SG_IO ioctls. Unless you want to disallow
> SCSI persistent reservations via SG_IO, you need to be able to detect
> them, and handle them using the persistent reservation code with the
> mpath_head.
I'm just coming back to this ... so I am thinking of not supporting PR
for scsi initially - like you mentioned, scsi pr has lots of nuances.
I am thinking of something like this:
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ded9cb8c57ea..c82adfc6871c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1295,6 +1295,12 @@ static blk_status_t scsi_setup_scsi_cmnd(struct
scsi_device *sdev,
{
struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req);
+ if (sdev->scsi_mpath_dev) {
+ blk_status_t ret = scsi_mpath_setup_scsi_cmnd(cmd);
+ if (ret)
+ return ret;
+ }
+
/*
* Passthrough requests may transfer data, in which case they must
* a bio attached to them. Or they might contain a SCSI command
diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
index 1489c7e97916..1daa62361dac 100644
--- a/drivers/scsi/scsi_multipath.c
+++ b/drivers/scsi/scsi_multipath.c
@@ -280,6 +280,18 @@ static int scsi_multipath_sdev_init(struct
scsi_device *sdev)
return 0;
}
+
+blk_status_t scsi_mpath_setup_scsi_cmnd(struct scsi_cmnd *scmd)
+{
+ switch (scmd->cmnd[0]) {
+ /* Special handling required which is not yet supported */
+ case PERSISTENT_RESERVE_IN:
+ case PERSISTENT_RESERVE_OUT:
+ return BLK_STS_NOTSUPP;
+ }
+ return BLK_STS_OK;
+}
+
Which should catch SG_IO PR-related commands.
^ permalink raw reply related
* Re: [PATCH v10 13/13] docs: add io_queue flag to isolcpus
From: Ming Lei @ 2026-04-09 15:00 UTC (permalink / raw)
To: Aaron Tomlin
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: <zawhqvn53mcp4wf7axsmuq4cg73upxs5h2zgrfta5dpat3sfy4@zctfbz2ttz5m>
On Wed, Apr 8, 2026 at 11:58 PM Aaron Tomlin <atomlin@atomlin.com> wrote:
>
> On Mon, Apr 06, 2026 at 11:29:38AM +0800, Ming Lei wrote:
> > I don't think there is such breaking isolation thing. For iopoll, if
> > applications won't submit polled IO on isolated CPUs, everything is just
> > fine. If they do it, IO may be reaped from isolated CPUs, that is just their
> > choice, anything is wrong?
>
> Hi Ming,
>
> Thank you for your follow up. You make a fair point regarding polling
> queues and application choice; if an application explicitly binds to an
> isolated CPU and submits polled operations, it is indeed actively electing
> to utilise that core and accept the resulting behaviour.
>
> However, the architectural challenge arises from how the kernel handles
> these queues structurally when the application does not explicitly make
> that choice. Because poll queues never utilise interrupts, they are
> completely invisible to the managed interrupt subsystem.
>
> If we were to rely exclusively on the managed irq flag, the block layer
> would blindly map these non interrupt driven polling queues to isolated
> CPUs. If a general background storage operation were then routed to
> that queue, the isolated core would be forced to spin actively in a tight
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.
>
> This illustrates precisely why the io queue flag is a mechanical necessity.
> Its primary objective is to act as a comprehensive block layer isolation
> boundary. It structurally restricts both hardware queue placement and
> managed interrupt affinity strictly to housekeeping CPUs, ensuring that no
> storage queue operations of any kind are mapped to an isolated CPU.
>
> To achieve this reliably, this series expands the struct irq affinity
> structure to incorporate a new CPU mask [1]. This mask is explicitly set to
> the result of blk mq online queue affinity. By passing this housekeeping
> mask directly through the interrupt affinity parameters, we ensure that the
> native affinity calculation is strictly bounded to non isolated CPUs from
> the moment the device probes.
>
> This structural enhancement allows device drivers to seamlessly inherit the
> isolation constraints without requiring bespoke, driver specific logic. A
> clear example of this application can be seen in the modifications to the
> Broadcom MPI3 Storage Controller [2]. By leveraging the expanded struct irq
> affinity, the driver guarantees that its queues and corresponding managed
> interrupts are perfectly aligned with the system housekeeping
> configuration, completely avoiding the isolated CPUs during allocation.
>
> [1]: https://lore.kernel.org/lkml/20260401222312.772334-5-atomlin@atomlin.com/
> [2]: https://lore.kernel.org/lkml/20260401222312.772334-8-atomlin@atomlin.com/
>
> I hope this better illustrates the mechanical necessity of the io_queue
> flag and the corresponding changes to the interrupt affinity structures.
Can you share one example in which managed irq can't address?
>
> > > Every logical CPU, including the isolated ones, must logically map to a
> > > hardware context in order to submit input and output requests, saying they
> > > are completely restricted is indeed stale and technically inaccurate. The
> > > isolation mechanism actually ensures that the hardware contexts themselves
> > > are serviced by the housekeeping CPUs, while the isolated CPUs are simply
> > > mapped onto these housekeeping queues for submission purposes. I will
> > > rewrite this paragraph to accurately reflect this topology, ensuring it
> > > aligns perfectly with the behaviour introduced in patch 10.
> >
> > I am not sure if the above words is helpful from administrator viewpoint about
> > the two kernel parameters.
> >
> > 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?
Thanks,
Ming
^ permalink raw reply
* [PATCH 7/7] MAINTAINERS: update ublk driver maintainer email
From: Ming Lei @ 2026-04-09 13:30 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Caleb Sander Mateos, Ming Lei
In-Reply-To: <20260409133020.3780098-1-tom.leiming@gmail.com>
Update the ublk userspace block driver maintainer email address
from ming.lei@redhat.com to tom.leiming@gmail.com as the original
email will become invalid.
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 77fdfcb55f06..4abb3345bc4e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26992,7 +26992,7 @@ F: Documentation/filesystems/ubifs.rst
F: fs/ubifs/
UBLK USERSPACE BLOCK DRIVER
-M: Ming Lei <ming.lei@redhat.com>
+M: Ming Lei <tom.leiming@gmail.com>
L: linux-block@vger.kernel.org
S: Maintained
F: Documentation/block/ublk.rst
--
2.53.0
^ permalink raw reply related
* [PATCH 6/7] Documentation: ublk: address review comments for SHMEM_ZC docs
From: Ming Lei @ 2026-04-09 13:30 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Caleb Sander Mateos, Ming Lei
In-Reply-To: <20260409133020.3780098-1-tom.leiming@gmail.com>
- Use "physical pages" instead of "page frame numbers (PFNs)" for
clarity
- Remove "without any per-I/O overhead" claim from zero-copy
description
- Add scatter/gather limitation: each I/O's data must be contiguous
within a single registered buffer
Suggested-by: Caleb Sander Mateos <csander@purestorage.com>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
Documentation/block/ublk.rst | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/Documentation/block/ublk.rst b/Documentation/block/ublk.rst
index a818e09a4b66..c39d111af2d2 100644
--- a/Documentation/block/ublk.rst
+++ b/Documentation/block/ublk.rst
@@ -492,8 +492,8 @@ The ``UBLK_F_SHMEM_ZC`` feature provides an alternative zero-copy path
that works by sharing physical memory pages between the client application
and the ublk server. Unlike the io_uring fixed buffer approach above,
shared memory zero copy does not require io_uring buffer registration
-per I/O — instead, it relies on the kernel matching page frame numbers
-(PFNs) at I/O time. This allows the ublk server to access the shared
+per I/O — instead, it relies on the kernel matching physical pages
+at I/O time. This allows the ublk server to access the shared
buffer directly, which is unlikely for the io_uring fixed buffer
approach.
@@ -507,8 +507,7 @@ tells the server where the data already lives.
``UBLK_F_SHMEM_ZC`` can be thought of as a supplement for optimized client
applications — when the client is willing to allocate I/O buffers from
-shared memory, the entire data path becomes zero-copy without any per-I/O
-overhead.
+shared memory, the entire data path becomes zero-copy.
Use Cases
~~~~~~~~~
@@ -584,6 +583,9 @@ Limitations
the page cache, which allocates its own pages. These kernel-allocated
pages will never match the registered shared buffer. Only ``O_DIRECT``
puts the client's buffer pages directly into the block I/O.
+- **Contiguous data only**: each I/O request's data must be contiguous
+ within a single registered buffer. Scatter/gather I/O that spans
+ multiple non-adjacent registered buffers cannot use the zero-copy path.
Control Commands
~~~~~~~~~~~~~~~~
--
2.53.0
^ permalink raw reply related
* [PATCH 5/7] ublk: allow buffer registration before device is started
From: Ming Lei @ 2026-04-09 13:30 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Caleb Sander Mateos, Ming Lei
In-Reply-To: <20260409133020.3780098-1-tom.leiming@gmail.com>
Before START_DEV, there is no disk, no queue, no I/O dispatch, so
the maple tree can be safely modified under ub->mutex alone without
freezing the queue.
Add ublk_lock_buf_tree()/ublk_unlock_buf_tree() helpers that take
ub->mutex first, then freeze the queue if device is started. This
ordering (mutex -> freeze) is safe because ublk_stop_dev_unlocked()
already holds ub->mutex when calling del_gendisk() which freezes
the queue.
Suggested-by: Caleb Sander Mateos <csander@purestorage.com>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
drivers/block/ublk_drv.c | 82 +++++++++++++---------------------------
1 file changed, 27 insertions(+), 55 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 8b686e70cf28..79178f13f198 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -5233,30 +5233,31 @@ static int ublk_char_dev_permission(struct ublk_device *ub,
}
/*
- * 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.
+ * Lock for maple tree modification: acquire ub->mutex, then freeze queue
+ * if device is started. If device is not yet started, only mutex is
+ * needed since no I/O path can access the tree.
+ *
+ * This ordering (mutex -> freeze) is safe because ublk_stop_dev_unlocked()
+ * already holds ub->mutex when calling del_gendisk() which freezes the queue.
*/
-static void ublk_quiesce_and_release(struct gendisk *disk)
+static unsigned int ublk_lock_buf_tree(struct ublk_device *ub)
{
- unsigned int memflags;
+ unsigned int memflags = 0;
- memflags = blk_mq_freeze_queue(disk->queue);
- blk_mq_quiesce_queue_nowait(disk->queue);
- blk_mq_unfreeze_queue(disk->queue, memflags);
+ mutex_lock(&ub->mutex);
+ if (ub->ub_disk)
+ memflags = blk_mq_freeze_queue(ub->ub_disk->queue);
+
+ return memflags;
}
-static void ublk_unquiesce_and_resume(struct gendisk *disk)
+static void ublk_unlock_buf_tree(struct ublk_device *ub, unsigned int memflags)
{
- blk_mq_unquiesce_queue(disk->queue);
+ if (ub->ub_disk)
+ blk_mq_unfreeze_queue(ub->ub_disk->queue, memflags);
+ mutex_unlock(&ub->mutex);
}
-/*
- * 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 matching buf_index */
static void ublk_buf_erase_ranges(struct ublk_device *ub, int buf_index)
{
@@ -5327,7 +5328,7 @@ static int ublk_ctrl_reg_buf(struct ublk_device *ub,
unsigned long addr, size, nr_pages;
struct page **pages = NULL;
unsigned int gup_flags;
- struct gendisk *disk;
+ unsigned int memflags;
long pinned;
int index;
int ret;
@@ -5354,16 +5355,10 @@ static int ublk_ctrl_reg_buf(struct ublk_device *ub,
!PAGE_ALIGNED(size) || !PAGE_ALIGNED(addr))
return -EINVAL;
- disk = ublk_get_disk(ub);
- if (!disk)
- return -ENODEV;
-
- /* Pin pages before quiescing (may sleep) */
+ /* Pin pages before any locks (may sleep) */
pages = kvmalloc_array(nr_pages, sizeof(*pages), GFP_KERNEL);
- if (!pages) {
- ret = -ENOMEM;
- goto put_disk;
- }
+ if (!pages)
+ return -ENOMEM;
gup_flags = FOLL_LONGTERM;
if (!(buf_reg.flags & UBLK_SHMEM_BUF_READ_ONLY))
@@ -5379,14 +5374,7 @@ static int ublk_ctrl_reg_buf(struct ublk_device *ub,
goto err_unpin;
}
- /*
- * 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);
+ memflags = ublk_lock_buf_tree(ub);
index = ida_alloc_max(&ub->buf_ida, USHRT_MAX, GFP_KERNEL);
if (index < 0) {
@@ -5400,22 +5388,16 @@ static int ublk_ctrl_reg_buf(struct ublk_device *ub,
goto err_unlock;
}
- mutex_unlock(&ub->mutex);
-
+ ublk_unlock_buf_tree(ub, memflags);
kvfree(pages);
- ublk_unquiesce_and_resume(disk);
- ublk_put_disk(disk);
return index;
err_unlock:
- mutex_unlock(&ub->mutex);
- ublk_unquiesce_and_resume(disk);
+ ublk_unlock_buf_tree(ub, memflags);
err_unpin:
unpin_user_pages(pages, pinned);
err_free_pages:
kvfree(pages);
-put_disk:
- ublk_put_disk(disk);
return ret;
}
@@ -5459,7 +5441,7 @@ static int ublk_ctrl_unreg_buf(struct ublk_device *ub,
struct ublksrv_ctrl_cmd *header)
{
int index = (int)header->data[0];
- struct gendisk *disk;
+ unsigned int memflags;
int ret;
if (!ublk_dev_support_shmem_zc(ub))
@@ -5468,23 +5450,13 @@ static int ublk_ctrl_unreg_buf(struct ublk_device *ub,
if (index < 0 || index > USHRT_MAX)
return -EINVAL;
- 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);
+ memflags = ublk_lock_buf_tree(ub);
ret = __ublk_ctrl_unreg_buf(ub, index);
if (!ret)
ida_free(&ub->buf_ida, index);
- mutex_unlock(&ub->mutex);
-
- ublk_unquiesce_and_resume(disk);
- ublk_put_disk(disk);
+ ublk_unlock_buf_tree(ub, memflags);
return ret;
}
--
2.53.0
^ permalink raw reply related
* [PATCH 4/7] ublk: replace xarray with IDA for shmem buffer index allocation
From: Ming Lei @ 2026-04-09 13:30 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Caleb Sander Mateos, Ming Lei
In-Reply-To: <20260409133020.3780098-1-tom.leiming@gmail.com>
Remove struct ublk_buf which only contained nr_pages that was never
read after registration. Use IDA for pure index allocation instead
of xarray. Make __ublk_ctrl_unreg_buf() return int so the caller
can detect invalid index without a separate lookup.
Simplify ublk_buf_cleanup() to walk the maple tree directly and
unpin all pages in one pass, instead of iterating the xarray by
buffer index.
Suggested-by: Caleb Sander Mateos <csander@purestorage.com>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
drivers/block/ublk_drv.c | 92 ++++++++++++++++++++--------------------
1 file changed, 46 insertions(+), 46 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index efbb22fe481c..8b686e70cf28 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -297,11 +297,6 @@ struct ublk_queue {
struct ublk_io ios[] __counted_by(q_depth);
};
-/* Per-registered shared memory buffer */
-struct ublk_buf {
- unsigned int nr_pages;
-};
-
/* Maple tree value: maps a PFN range to buffer location */
struct ublk_buf_range {
unsigned short buf_index;
@@ -345,7 +340,7 @@ struct ublk_device {
/* shared memory zero copy */
struct maple_tree buf_tree;
- struct xarray bufs_xa;
+ struct ida buf_ida;
struct ublk_queue *queues[];
};
@@ -4693,7 +4688,7 @@ static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_cmd *header)
spin_lock_init(&ub->lock);
mutex_init(&ub->cancel_mutex);
mt_init(&ub->buf_tree);
- xa_init_flags(&ub->bufs_xa, XA_FLAGS_ALLOC);
+ ida_init(&ub->buf_ida);
INIT_WORK(&ub->partition_scan_work, ublk_partition_scan_work);
ret = ublk_alloc_dev_number(ub, header->dev_id);
@@ -5279,11 +5274,9 @@ static void ublk_buf_erase_ranges(struct ublk_device *ub, int buf_index)
}
static int __ublk_ctrl_reg_buf(struct ublk_device *ub,
- struct ublk_buf *ubuf,
- struct page **pages, int index,
- unsigned short flags)
+ struct page **pages, unsigned long nr_pages,
+ int index, unsigned short flags)
{
- unsigned long nr_pages = ubuf->nr_pages;
unsigned long i;
int ret;
@@ -5335,9 +5328,8 @@ static int ublk_ctrl_reg_buf(struct ublk_device *ub,
struct page **pages = NULL;
unsigned int gup_flags;
struct gendisk *disk;
- struct ublk_buf *ubuf;
long pinned;
- u32 index;
+ int index;
int ret;
if (!ublk_dev_support_shmem_zc(ub))
@@ -5367,16 +5359,10 @@ static int ublk_ctrl_reg_buf(struct ublk_device *ub,
return -ENODEV;
/* Pin pages before quiescing (may sleep) */
- ubuf = kzalloc(sizeof(*ubuf), GFP_KERNEL);
- if (!ubuf) {
- ret = -ENOMEM;
- goto put_disk;
- }
-
pages = kvmalloc_array(nr_pages, sizeof(*pages), GFP_KERNEL);
if (!pages) {
ret = -ENOMEM;
- goto err_free;
+ goto put_disk;
}
gup_flags = FOLL_LONGTERM;
@@ -5392,7 +5378,6 @@ static int ublk_ctrl_reg_buf(struct ublk_device *ub,
ret = -EFAULT;
goto err_unpin;
}
- ubuf->nr_pages = nr_pages;
/*
* Drain inflight I/O and quiesce the queue so no new requests
@@ -5403,13 +5388,15 @@ static int ublk_ctrl_reg_buf(struct ublk_device *ub,
mutex_lock(&ub->mutex);
- ret = xa_alloc(&ub->bufs_xa, &index, ubuf, xa_limit_16b, GFP_KERNEL);
- if (ret)
+ index = ida_alloc_max(&ub->buf_ida, USHRT_MAX, GFP_KERNEL);
+ if (index < 0) {
+ ret = index;
goto err_unlock;
+ }
- ret = __ublk_ctrl_reg_buf(ub, ubuf, pages, index, buf_reg.flags);
+ ret = __ublk_ctrl_reg_buf(ub, pages, nr_pages, index, buf_reg.flags);
if (ret) {
- xa_erase(&ub->bufs_xa, index);
+ ida_free(&ub->buf_ida, index);
goto err_unlock;
}
@@ -5427,19 +5414,17 @@ static int ublk_ctrl_reg_buf(struct ublk_device *ub,
unpin_user_pages(pages, pinned);
err_free_pages:
kvfree(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, int buf_index)
+static int __ublk_ctrl_unreg_buf(struct ublk_device *ub, int buf_index)
{
MA_STATE(mas, &ub->buf_tree, 0, ULONG_MAX);
struct ublk_buf_range *range;
struct page *pages[32];
+ int ret = -ENOENT;
mas_lock(&mas);
mas_for_each(&mas, range, ULONG_MAX) {
@@ -5448,6 +5433,7 @@ static void __ublk_ctrl_unreg_buf(struct ublk_device *ub,
if (range->buf_index != buf_index)
continue;
+ ret = 0;
base = mas.index;
nr = mas.last - base + 1;
mas_erase(&mas);
@@ -5465,7 +5451,8 @@ static void __ublk_ctrl_unreg_buf(struct ublk_device *ub,
kfree(range);
}
mas_unlock(&mas);
- kfree(ubuf);
+
+ return ret;
}
static int ublk_ctrl_unreg_buf(struct ublk_device *ub,
@@ -5473,11 +5460,14 @@ static int ublk_ctrl_unreg_buf(struct ublk_device *ub,
{
int index = (int)header->data[0];
struct gendisk *disk;
- struct ublk_buf *ubuf;
+ int ret;
if (!ublk_dev_support_shmem_zc(ub))
return -EOPNOTSUPP;
+ if (index < 0 || index > USHRT_MAX)
+ return -EINVAL;
+
disk = ublk_get_disk(ub);
if (!disk)
return -ENODEV;
@@ -5487,32 +5477,42 @@ static int ublk_ctrl_unreg_buf(struct ublk_device *ub,
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, index);
+ ret = __ublk_ctrl_unreg_buf(ub, index);
+ if (!ret)
+ ida_free(&ub->buf_ida, index);
mutex_unlock(&ub->mutex);
ublk_unquiesce_and_resume(disk);
ublk_put_disk(disk);
- return 0;
+ return ret;
}
static void ublk_buf_cleanup(struct ublk_device *ub)
{
- struct ublk_buf *ubuf;
- unsigned long index;
+ MA_STATE(mas, &ub->buf_tree, 0, ULONG_MAX);
+ struct ublk_buf_range *range;
+ struct page *pages[32];
+
+ mas_for_each(&mas, range, ULONG_MAX) {
+ unsigned long base = mas.index;
+ unsigned long nr = mas.last - base + 1;
+ unsigned long off;
- xa_for_each(&ub->bufs_xa, index, ubuf)
- __ublk_ctrl_unreg_buf(ub, ubuf, index);
- xa_destroy(&ub->bufs_xa);
+ for (off = 0; off < nr; ) {
+ unsigned int batch = min_t(unsigned long,
+ nr - off, 32);
+ unsigned int j;
+
+ for (j = 0; j < batch; j++)
+ pages[j] = pfn_to_page(base + off + j);
+ unpin_user_pages(pages, batch);
+ off += batch;
+ }
+ kfree(range);
+ }
mtree_destroy(&ub->buf_tree);
+ ida_destroy(&ub->buf_ida);
}
/* Check if request pages match a registered shared memory buffer */
--
2.53.0
^ permalink raw reply related
* [PATCH 3/7] ublk: simplify PFN range loop in __ublk_ctrl_reg_buf
From: Ming Lei @ 2026-04-09 13:30 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Caleb Sander Mateos, Ming Lei
In-Reply-To: <20260409133020.3780098-1-tom.leiming@gmail.com>
Use the for-loop increment instead of a manual `i++` past the last
page, and fix the mtree_insert_range end key accordingly.
Suggested-by: Caleb Sander Mateos <csander@purestorage.com>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
drivers/block/ublk_drv.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index f990c10e963a..efbb22fe481c 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -5287,7 +5287,7 @@ static int __ublk_ctrl_reg_buf(struct ublk_device *ub,
unsigned long i;
int ret;
- for (i = 0; i < nr_pages; ) {
+ for (i = 0; i < nr_pages; i++) {
unsigned long pfn = page_to_pfn(pages[i]);
unsigned long start = i;
struct ublk_buf_range *range;
@@ -5296,7 +5296,6 @@ static int __ublk_ctrl_reg_buf(struct ublk_device *ub,
while (i + 1 < nr_pages &&
page_to_pfn(pages[i + 1]) == pfn + (i - start) + 1)
i++;
- i++; /* past the last page in this run */
range = kzalloc(sizeof(*range), GFP_KERNEL);
if (!range) {
@@ -5308,7 +5307,7 @@ static int __ublk_ctrl_reg_buf(struct ublk_device *ub,
range->base_offset = start << PAGE_SHIFT;
ret = mtree_insert_range(&ub->buf_tree, pfn,
- pfn + (i - start) - 1,
+ pfn + (i - start),
range, GFP_KERNEL);
if (ret) {
kfree(range);
--
2.53.0
^ permalink raw reply related
* [PATCH 2/7] ublk: verify all pages in multi-page bvec fall within registered range
From: Ming Lei @ 2026-04-09 13:30 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Caleb Sander Mateos, Ming Lei
In-Reply-To: <20260409133020.3780098-1-tom.leiming@gmail.com>
rq_for_each_bvec() yields multi-page bvecs where bv_page is only the
first page. ublk_try_buf_match() only validated the start PFN against
the maple tree, but a bvec can span multiple pages past the end of a
registered range.
Use mas_walk() instead of mtree_load() to obtain the range boundaries
stored in the maple tree, and check that the bvec's end PFN does not
exceed the range. Also remove base_pfn from struct ublk_buf_range
since mas.index already provides the range start PFN.
Reported-by: Caleb Sander Mateos <csander@purestorage.com>
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
drivers/block/ublk_drv.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index ada9a2e32ea9..f990c10e963a 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -304,7 +304,6 @@ struct ublk_buf {
/* 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 */
@@ -5306,7 +5305,6 @@ static int __ublk_ctrl_reg_buf(struct ublk_device *ub,
}
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,
@@ -5451,8 +5449,8 @@ static void __ublk_ctrl_unreg_buf(struct ublk_device *ub,
if (range->buf_index != buf_index)
continue;
- base = range->base_pfn;
- nr = mas.last - mas.index + 1;
+ base = mas.index;
+ nr = mas.last - base + 1;
mas_erase(&mas);
for (off = 0; off < nr; ) {
@@ -5531,15 +5529,22 @@ static bool ublk_try_buf_match(struct ublk_device *ub,
rq_for_each_bvec(bv, rq, iter) {
unsigned long pfn = page_to_pfn(bv.bv_page);
+ unsigned long end_pfn = pfn +
+ ((bv.bv_offset + bv.bv_len - 1) >> PAGE_SHIFT);
struct ublk_buf_range *range;
unsigned long off;
+ MA_STATE(mas, &ub->buf_tree, pfn, pfn);
- range = mtree_load(&ub->buf_tree, pfn);
+ range = mas_walk(&mas);
if (!range)
return false;
+ /* verify all pages in this bvec fall within the range */
+ if (end_pfn > mas.last)
+ return false;
+
off = range->base_offset +
- (pfn - range->base_pfn) * PAGE_SIZE + bv.bv_offset;
+ (pfn - mas.index) * PAGE_SIZE + bv.bv_offset;
if (first) {
/* Read-only buffer can't serve READ (kernel writes) */
--
2.53.0
^ permalink raw reply related
* [PATCH 1/7] ublk: widen ublk_shmem_buf_reg.len to __u64 for 4GB buffer support
From: Ming Lei @ 2026-04-09 13:30 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Caleb Sander Mateos, Ming Lei
In-Reply-To: <20260409133020.3780098-1-tom.leiming@gmail.com>
The __u32 len field cannot represent a 4GB buffer (0x100000000
overflows to 0). Change it to __u64 so buffers up to 4GB can be
registered. Add a reserved field for alignment and validate it
is zero.
The kernel enforces a default max of 4GB (UBLK_SHMEM_BUF_SIZE_MAX)
which may be increased in future.
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
drivers/block/ublk_drv.c | 9 ++++++++-
include/uapi/linux/ublk_cmd.h | 3 ++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 2e475bdc54dd..ada9a2e32ea9 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -63,6 +63,9 @@
#define UBLK_CMD_REG_BUF _IOC_NR(UBLK_U_CMD_REG_BUF)
#define UBLK_CMD_UNREG_BUF _IOC_NR(UBLK_U_CMD_UNREG_BUF)
+/* Default max shmem buffer size: 4GB (may be increased in future) */
+#define UBLK_SHMEM_BUF_SIZE_MAX (1ULL << 32)
+
#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)
@@ -5351,11 +5354,15 @@ static int ublk_ctrl_reg_buf(struct ublk_device *ub,
if (buf_reg.flags & ~UBLK_SHMEM_BUF_READ_ONLY)
return -EINVAL;
+ if (buf_reg.reserved)
+ return -EINVAL;
+
addr = buf_reg.addr;
size = buf_reg.len;
nr_pages = size >> PAGE_SHIFT;
- if (!size || !PAGE_ALIGNED(size) || !PAGE_ALIGNED(addr))
+ if (!size || size > UBLK_SHMEM_BUF_SIZE_MAX ||
+ !PAGE_ALIGNED(size) || !PAGE_ALIGNED(addr))
return -EINVAL;
disk = ublk_get_disk(ub);
diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
index ecd258847d3d..66d93efccd51 100644
--- a/include/uapi/linux/ublk_cmd.h
+++ b/include/uapi/linux/ublk_cmd.h
@@ -89,8 +89,9 @@
/* 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) */
+ __u64 len; /* buffer size in bytes, page-aligned, default max 4GB */
__u32 flags;
+ __u32 reserved;
};
/* Pin pages without FOLL_WRITE; usable with write-sealed memfd */
--
2.53.0
^ permalink raw reply related
* [PATCH 0/7] ublk: followup fixes for SHMEM_ZC
From: Ming Lei @ 2026-04-09 13:30 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Caleb Sander Mateos, Ming Lei
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
Thanks,
Ming Lei (7):
ublk: widen ublk_shmem_buf_reg.len to __u64 for 4GB buffer support
ublk: verify all pages in multi-page bvec fall within registered range
ublk: simplify PFN range loop in __ublk_ctrl_reg_buf
ublk: replace xarray with IDA for shmem buffer index allocation
ublk: allow buffer registration before device is started
Documentation: ublk: address review comments for SHMEM_ZC docs
MAINTAINERS: update ublk driver maintainer email
Documentation/block/ublk.rst | 10 +-
MAINTAINERS | 2 +-
drivers/block/ublk_drv.c | 201 ++++++++++++++++------------------
include/uapi/linux/ublk_cmd.h | 3 +-
4 files changed, 101 insertions(+), 115 deletions(-)
--
2.53.0
^ permalink raw reply
* Re: [PATCH v3 02/12] block/bdev: Annotate the blk_holder_ops callback invocations
From: Marco Elver @ 2026-04-09 13:27 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Bart Van Assche, Jens Axboe, linux-block, Damien Le Moal,
Nathan Chancellor
In-Reply-To: <20260409064221.GA8378@lst.de>
On Thu, 9 Apr 2026 at 08:42, Christoph Hellwig <hch@lst.de> wrote:
> On Thu, Apr 02, 2026 at 11:39:34AM -0700, Bart Van Assche wrote:
> > The four callback functions in blk_holder_ops all release the
> > bd_holder_lock. Add __release() annotations where appropriate to prepare
> > for enabling thread-safety analysis. Explicit __release() annotations have
> > been added since Clang does not support adding a __releases() annotation
> > to a function pointer.
>
> I have to say I still hate this with passion. If we can't propagate
> the lock context over function pointers it isn't ready for prime time
> unfortunately, as much as I'm looking forward to it.
Let's see if we can land this: https://github.com/llvm/llvm-project/pull/191187
I think we'll be bumping min Clang version to 23 for context analysis,
so hopefully it'll be included in Clang 23 and should solve the
function pointer false positives.
^ permalink raw reply
* Re: [PATCH 07/13] libmultipath: Add delayed removal support
From: John Garry @ 2026-04-09 13:00 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: <79725a83-3dc1-4398-ac86-c3e317e0e107@linux.ibm.com>
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
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"
}
#
#
^ permalink raw reply
* Re: [PATCH v2 01/10] ublk: add UBLK_U_CMD_REG_BUF/UNREG_BUF control commands
From: Ming Lei @ 2026-04-09 12:18 UTC (permalink / raw)
To: Caleb Sander Mateos; +Cc: Ming Lei, Jens Axboe, linux-block
In-Reply-To: <CADUfDZrs=6srDdL15KG-uL2PX9p5_piJVU0GZZTYjE0VfDQJKw@mail.gmail.com>
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)
- 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.
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.
Thanks,
Ming
^ permalink raw reply
* Re: [PATCH RFC v5 0/3] block: enable RWF_DONTCACHE for block devices
From: Christoph Hellwig @ 2026-04-09 6:53 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-0-0f080c20a96f@columbia.edu>
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.
^ permalink raw reply
* Re: [PATCH v3 02/12] block/bdev: Annotate the blk_holder_ops callback invocations
From: Christoph Hellwig @ 2026-04-09 6:42 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Damien Le Moal,
Marco Elver, Nathan Chancellor
In-Reply-To: <20260402183950.3626956-3-bvanassche@acm.org>
On Thu, Apr 02, 2026 at 11:39:34AM -0700, Bart Van Assche wrote:
> The four callback functions in blk_holder_ops all release the
> bd_holder_lock. Add __release() annotations where appropriate to prepare
> for enabling thread-safety analysis. Explicit __release() annotations have
> been added since Clang does not support adding a __releases() annotation
> to a function pointer.
I have to say I still hate this with passion. If we can't propagate
the lock context over function pointers it isn't ready for prime time
unfortunately, as much as I'm looking forward to it.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox