* [PATCH v9] blk-mq: add tracepoint block_rq_tag_wait
From: Aaron Tomlin @ 2026-05-25 0:51 UTC (permalink / raw)
To: axboe, rostedt, mhiramat, mathieu.desnoyers
Cc: bvanassche, johannes.thumshirn, kch, dlemoal, ritesh.list,
john.g.garry, loberman, neelx, sean, mproche, chjohnst,
linux-block, linux-kernel, linux-trace-kernel
In high-performance storage environments, particularly when utilising
RAID controllers with shared tag sets (BLK_MQ_F_TAG_HCTX_SHARED), severe
latency spikes can occur when fast devices (SSDs) are starved of hardware
tags when sharing the same blk_mq_tag_set.
Currently, diagnosing this specific hardware queue contention is
difficult. When a CPU thread exhausts the tag pool, blk_mq_get_tag()
forces the current thread to block uninterruptible via io_schedule().
While this can be inferred via sched:sched_switch or dynamically
traced by attaching a kprobe to blk_mq_mark_tag_wait(), there is no
dedicated, out-of-the-box observability for this event.
This patch introduces the block_rq_tag_wait tracepoint in the tag
allocation slow-path. It triggers immediately before the task state
is altered to TASK_UNINTERRUPTIBLE (ensuring safety for PREEMPT_RT
locks). It exposes the exact hardware context (hctx) that is starved,
the specific pool experiencing starvation (driver, software scheduler,
or reserved), and the exact pool depth.
This provides storage engineers with a zero-configuration, low-overhead
mechanism to definitively identify shared-tag bottlenecks. For example,
userspace can trivially replicate tag starvation counters using bpftrace:
# bpftrace -e 'tracepoint:block:block_rq_tag_wait { @tag_waits[cpu] = count(); }'
Attaching 1 probe...
^C
@tag_waits[4]: 12
@tag_waits[12]: 87
Signed-off-by: Aaron Tomlin <atomlin@atomlin.com>
---
Hi Johannes, Damien, Chaitanya, Laurence,
I have dropped the earlier "Reviewed-by:" and "Tested-by:" tags because
of functional logic changes in the tracepoint assignment block. A fresh
review would be highly appreciated. Thank you.
Changes since v8 [1]:
- Fixed the standard pool depth calculation in TP_fast_assign to
accurately report the unreserved capacity by mathematically
subtracting nr_reserved_tags from nr_tags
- Removed "Reviewed-by:" and "Tested-by:" tags due to the functional
logic updates in the tracepoint assignment block
Changes since v7 [2]:
- Added an is_reserved boolean to the trace record to explicitly expose
reserved pool starvation to userspace
- Fixed TP_fast_assign to report the correct nr_reserved_tags depth
when I/O schedulers utilise the reserved pool
Changes since v6 [3]:
- Dropped Patch 2. Observability is now driven entirely by the tracepoint,
with the commit message updated to demonstrate how userspace (e.g.,
bpftrace) can safely replicate counting out-of-band (Jens Axboe)
- Moved tracepoint call above sbitmap_prepare_to_wait(). This prevents
inadvertently resetting the task state under PREEMPT_RT locks
- Updated the tracepoint signature and TP_fast_assign block to evaluate
the allocation flags. If the submitting context is starved of a reserved
tag (BLK_MQ_REQ_RESERVED), the tracepoint now accurately reports the
severely constrained nr_reserved_tags depth instead of the total nr_tags
depth.
Changes since v5 [4]:
- Replaced this_cpu_inc() with raw_cpu_inc() within
blk_mq_debugfs_inc_wait_tags(). This resolves a preemption warning
triggered under CONFIG_DEBUG_PREEMPT=y, as the routine is invoked from a
preemptible context immediately prior to io_schedule(). This adjustment
deliberately prioritises the reduction of execution overhead over
absolute statistical precision for this diagnostic interface.
Changes since v4 [5]:
- Prevented a NULL pointer dereference in the tracepoint fast-assign for
disk-less request queues by safely checking q->disk before resolving the
dev_t
- Fixed a Use-After-Free (UAF) and permanent memory leak by decoupling
the per-CPU counter allocation from the volatile debugfs lifecycle and
tying it directly to the core hctx lifecycle (i.e., blk_mq_init_hctx()
and blk_mq_exit_hctx())
- Fixed a potential compiler double-fetch bug by wrapping the per-CPU
pointer evaluations with READ_ONCE() in blk_mq_debugfs_inc_wait_tags()
- Passed the appropriate gfp_t flags down to the allocation routines to
maintain the strict GFP_NOIO context
- Updated kernel-doc descriptions to clarify that the NULL pointer
checks guard against memory allocation failures under pressure, rather
than initialisation race conditions
Changes since v3 [6]:
- Transitioned tracking architecture from shared atomic_t variables to
dynamically allocated per-CPU counters to resolve cache line bouncing
(Bart Van Assche)
Changes since v2 [7]:
- Added "Reviewed-by:" and "Tested-by:" tags for patch 1
- Evaluate is_sched_tag directly within TP_fast_assign (Steven Rostedt)
- Introduced atomic counters via debugfs
Changes since v1 [8]:
- Improved the description of the trace point (Damien Le Moal)
- Removed the redundant "active requests" (Laurence Oberman)
- Introduced pool-specific starvation tracking
[1]: https://lore.kernel.org/lkml/20260524014204.622699-1-atomlin@atomlin.com/
[2]: https://lore.kernel.org/lkml/20260523200942.587199-1-atomlin@atomlin.com/
[3]: https://lore.kernel.org/lkml/20260517213614.350367-1-atomlin@atomlin.com/
[4]: https://lore.kernel.org/lkml/20260427020142.358912-1-atomlin@atomlin.com/
[5]: https://lore.kernel.org/lkml/20260419023036.1419514-1-atomlin@atomlin.com/
[6]: https://lore.kernel.org/lkml/20260319221956.332770-1-atomlin@atomlin.com/
[7]: https://lore.kernel.org/lkml/20260319015300.287653-1-atomlin@atomlin.com/
[8]: https://lore.kernel.org/lkml/20260317182835.258183-1-atomlin@atomlin.com/
---
block/blk-mq-tag.c | 6 ++++
include/trace/events/block.h | 59 ++++++++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+)
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 33946cdb5716..35deee5bbc73 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -13,6 +13,7 @@
#include <linux/kmemleak.h>
#include <linux/delay.h>
+#include <trace/events/block.h>
#include "blk.h"
#include "blk-mq.h"
#include "blk-mq-sched.h"
@@ -181,6 +182,11 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
if (tag != BLK_MQ_NO_TAG)
break;
+ /* Log the starvation event before altering task state */
+ trace_block_rq_tag_wait(data->q, data->hctx,
+ data->rq_flags & RQF_SCHED_TAGS,
+ data->flags);
+
sbitmap_prepare_to_wait(bt, ws, &wait, TASK_UNINTERRUPTIBLE);
tag = __blk_mq_get_tag(data, bt);
diff --git a/include/trace/events/block.h b/include/trace/events/block.h
index 6aa79e2d799c..9c97a16850b9 100644
--- a/include/trace/events/block.h
+++ b/include/trace/events/block.h
@@ -226,6 +226,65 @@ DECLARE_EVENT_CLASS(block_rq,
IOPRIO_PRIO_LEVEL(__entry->ioprio), __entry->comm)
);
+/**
+ * block_rq_tag_wait - triggered when a request is starved of a tag
+ * @q: request queue of the target device
+ * @hctx: hardware context of the request experiencing starvation
+ * @is_sched_tag: indicates whether the starved pool is the software scheduler
+ * @alloc_flags: allocation flags dictating the specific tag pool
+ *
+ * Called immediately before the submitting context is forced to block due
+ * to the exhaustion of available tags (i.e., physical hardware driver
+ * tags, software scheduler tags, or reserved tags). This trace point
+ * indicates that the context will be placed into an uninterruptible state
+ * via sbitmap_prepare_to_wait(). If a tag is not acquired in the final
+ * lockless retry, the context will yield the CPU via io_schedule() until
+ * an active request completes and relinquishes its assigned tag.
+ */
+TRACE_EVENT(block_rq_tag_wait,
+
+ TP_PROTO(struct request_queue *q, struct blk_mq_hw_ctx *hctx,
+ bool is_sched_tag, unsigned int alloc_flags),
+
+ TP_ARGS(q, hctx, is_sched_tag, alloc_flags),
+
+ TP_STRUCT__entry(
+ __field( dev_t, dev )
+ __field( u32, hctx_id )
+ __field( u32, nr_tags )
+ __field( bool, is_sched_tag )
+ __field( bool, is_reserved )
+ ),
+
+ TP_fast_assign(
+ __entry->dev = q->disk ? disk_devt(q->disk) : 0;
+ __entry->hctx_id = hctx->queue_num;
+ __entry->is_sched_tag = is_sched_tag;
+ __entry->is_reserved = alloc_flags & BLK_MQ_REQ_RESERVED;
+
+ if (__entry->is_reserved) {
+ __entry->nr_tags = is_sched_tag ?
+ hctx->sched_tags->nr_reserved_tags :
+ hctx->tags->nr_reserved_tags;
+ } else {
+ if (is_sched_tag)
+ __entry->nr_tags = hctx->sched_tags->nr_tags -
+ hctx->sched_tags->nr_reserved_tags;
+ else
+ __entry->nr_tags = hctx->tags->nr_tags -
+ hctx->tags->nr_reserved_tags;
+ }
+
+ ),
+
+ TP_printk("%d,%d hctx=%u starved on %s%s tags (depth=%u)",
+ MAJOR(__entry->dev), MINOR(__entry->dev),
+ __entry->hctx_id,
+ __entry->is_sched_tag ? "scheduler" : "hardware",
+ __entry->is_reserved ? " reserved" : "",
+ __entry->nr_tags)
+);
+
/**
* block_rq_insert - insert block operation request into queue
* @rq: block IO operation request
base-commit: 6779b50faa562e6cca1aa6a4649a4d764c6c7e28
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v5] block: propagate in_flight to whole disk on partition I/O
From: Yizhou Tang @ 2026-05-25 2:07 UTC (permalink / raw)
To: Keith Busch
Cc: Tang Yizhou, axboe, hch, yukuai, linux-block, linux-kernel,
Leon Hwang
In-Reply-To: <ahBnKR-IunwxVDzg@kbusch-mbp>
On Fri, May 22, 2026 at 10:53 PM Keith Busch <kbusch@kernel.org> wrote:
>
> On Fri, May 22, 2026 at 10:16:38PM +0800, Tang Yizhou wrote:
> > @@ -1073,7 +1073,7 @@ void bdev_end_io_acct(struct block_device *bdev, enum req_op op,
> > part_stat_inc(bdev, ios[sgrp]);
> > part_stat_add(bdev, sectors[sgrp], sectors);
> > part_stat_add(bdev, nsecs[sgrp], jiffies_to_nsecs(duration));
> > - part_stat_local_dec(bdev, in_flight[op_is_write(op)]);
> > + bdev_inc_in_flight(bdev, op);
>
> This one should be bdev_dec_in_flight().
Thanks for pointing that out.
Best regards,
Yi
>
^ permalink raw reply
* [PATCH v6] block: propagate in_flight to whole disk on partition I/O
From: Tang Yizhou @ 2026-05-25 2:19 UTC (permalink / raw)
To: axboe, hch, kbusch
Cc: yukuai, linux-block, linux-kernel, Tang Yizhou, Leon Hwang
From: Tang Yizhou <yizhou.tang@shopee.com>
Now when I/O is submitted to a partition, the per-CPU in_flight[]
counter is incremented only on the partition's block_device, not on the
underlying whole disk. This leads to a problem which can be shown by a
fio test:
lsblk
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINTS
mydev 252:1 0 20G 0 disk
└─mydev1 259:0 0 10G 0 part
iostat -xp 1
Device r/s rkB/s ... aqu-sz %util
mydev 128153.00 512612.00 ... 13.22 72.20
mydev1 128154.00 512616.00 ... 13.22 100.00
%util is different between mydev and mydev1, which is unexpected.
This is the cumulative effect of a series of patches. The root cause is
commit e016b78201a2 ("block: return just one value from part_in_flight"),
which deleted the branch in part_in_flight() that aggregated the whole-disk
in_flight count on top of the partition's. Then the second commit is
commit 10ec5e86f9b8 ("block: merge part_{inc,dev}_in_flight into their
only callers"), which folded the whole-disk in_flight accounting into
generic_start_io_acct() and generic_end_io_acct(). Those two helpers
were then removed by commit e722fff238bb ("block: remove
generic_{start,end}_io_acct"), and from that point on the whole disk's
in_flight is no longer accounted at all.
In update_io_ticks(), if calling bdev_count_inflight() finds that the
inflight value of the whole device is 0, the accumulation of io_ticks will
be skipped, causing the reported util% value to be underestimated.
Fix it by restoring the whole-disk in_flight accounting.
Fixes: e016b78201a2 ("block: return just one value from part_in_flight")
Suggested-by: Leon Hwang <leon.huangfu@shopee.com>
Signed-off-by: Tang Yizhou <yizhou.tang@shopee.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
v2: Update commit message.
v3: Take Christoph's advice and factor the common code into two helpers.
v4: Remove my redundant new line in blk.h. Add Christoph's Reviewed-by
tag.
v5: Remove the changelog from the commit message.
v6: Accept Keith's suggestion and fix the bug in bdev_end_io_acct().
block/blk-core.c | 4 ++--
block/blk-mq.c | 5 ++---
block/blk.h | 21 +++++++++++++++++++++
3 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 17450058ea6d..cee4e4a37503 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1042,7 +1042,7 @@ unsigned long bdev_start_io_acct(struct block_device *bdev, enum req_op op,
{
part_stat_lock();
update_io_ticks(bdev, start_time, false);
- part_stat_local_inc(bdev, in_flight[op_is_write(op)]);
+ bdev_inc_in_flight(bdev, op);
part_stat_unlock();
return start_time;
@@ -1073,7 +1073,7 @@ void bdev_end_io_acct(struct block_device *bdev, enum req_op op,
part_stat_inc(bdev, ios[sgrp]);
part_stat_add(bdev, sectors[sgrp], sectors);
part_stat_add(bdev, nsecs[sgrp], jiffies_to_nsecs(duration));
- part_stat_local_dec(bdev, in_flight[op_is_write(op)]);
+ bdev_dec_in_flight(bdev, op);
part_stat_unlock();
}
EXPORT_SYMBOL(bdev_end_io_acct);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index d0c37daf568f..6bdfe642bd93 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1082,8 +1082,7 @@ static inline void blk_account_io_done(struct request *req, u64 now)
update_io_ticks(req->part, jiffies, true);
part_stat_inc(req->part, ios[sgrp]);
part_stat_add(req->part, nsecs[sgrp], now - req->start_time_ns);
- part_stat_local_dec(req->part,
- in_flight[op_is_write(req_op(req))]);
+ bdev_dec_in_flight(req->part, req_op(req));
part_stat_unlock();
}
}
@@ -1143,7 +1142,7 @@ static inline void blk_account_io_start(struct request *req)
part_stat_lock();
update_io_ticks(req->part, jiffies, false);
- part_stat_local_inc(req->part, in_flight[op_is_write(req_op(req))]);
+ bdev_inc_in_flight(req->part, req_op(req));
part_stat_unlock();
}
diff --git a/block/blk.h b/block/blk.h
index b998a7761faf..11245a494c43 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -4,6 +4,7 @@
#include <linux/bio-integrity.h>
#include <linux/blk-crypto.h>
+#include <linux/part_stat.h>
#include <linux/lockdep.h>
#include <linux/memblock.h> /* for max_pfn/max_low_pfn */
#include <linux/sched/sysctl.h>
@@ -485,6 +486,26 @@ static inline void req_set_nomerge(struct request_queue *q, struct request *req)
q->last_merge = NULL;
}
+static inline void bdev_inc_in_flight(struct block_device *bdev,
+ enum req_op op)
+{
+ bool rw = op_is_write(op);
+
+ part_stat_local_inc(bdev, in_flight[rw]);
+ if (bdev_is_partition(bdev))
+ part_stat_local_inc(bdev_whole(bdev), in_flight[rw]);
+}
+
+static inline void bdev_dec_in_flight(struct block_device *bdev,
+ enum req_op op)
+{
+ bool rw = op_is_write(op);
+
+ part_stat_local_dec(bdev, in_flight[rw]);
+ if (bdev_is_partition(bdev))
+ part_stat_local_dec(bdev_whole(bdev), in_flight[rw]);
+}
+
/*
* Internal io_context interface
*/
--
2.43.0
^ permalink raw reply related
* [PATCH v3] loop: Fix NULL pointer dereference in lo_rw_aio()
From: Tetsuo Handa @ 2026-05-25 3:40 UTC (permalink / raw)
To: Ming Lei, Jens Axboe
Cc: Bart Van Assche, Christoph Hellwig, Damien Le Moal, linux-block,
LKML, Andrew Morton
In-Reply-To: <ag1223nAa0wZ8ALC@fedora>
Some commit which was merged in the merge window for 7.1 broke the loop
driver; a race window where lo_release() clears the backing file via
__loop_clr_fd() despite some I/O requests are pending was introduced [1][2].
The exact commit which changed the behavior is not known due to lack of
reproducer and timing dependent behavior, but it seems that we need to
solve this problem in the loop driver despite there was no change for the
loop driver during this merge window.
To close this race, try to flush pending I/O requests. However, calling
drain_workqueue() from __loop_clr_fd() with disk->open_mutex held causes
lockdep warnings [3][4]. We need to flush pending I/O requests without
disk->open_mutex held.
In the past, commit 322c4293ecc5 ("loop: make autoclear operation
asynchronous") has tried to defer __loop_clr_fd() to WQ context. But it was
reverted by commit bf23747ee053 ("loop: revert "make autoclear operation
asynchronous"") because userspace might be expecting that fput() on the
backing file is processed before lo_release() from close() returns to user
mode.
Therefore, this patch tries to defer __loop_clr_fd() to task work context.
__loop_clr_fd() is split into three steps:
Step 1: Flush pending I/O requests without holding disk->open_mutex.
Step 2: Do what __loop_clr_fd() from lo_release() was doing with
disk->open_mutex held.
Step 3: Drop refcounts without holding disk->open_mutex.
A potential side effect of this approach is that a userspace program who
issued open() request before __loop_clr_fd() completes might be confused
by observing -ENXIO because lo_open() can be called before __loop_clr_fd()
completes.
Except for the side effect above, I expect this patch to work by the
following reasons.
- The existing Lo_rundown state safely guarantees that any subsequent
lo_open() attempts will immediately fail with -ENXIO, preventing races
even after disk->open_mutex is temporarily released.
- Since returning from lo_release() normally allows the block layer to
immediately drop module and device references, this patch explicitly
increments the refcounts (__module_get() and get_device()) before
deferring the work, and safely releases them at the end of Step 3
inside __loop_clr_fd().
- It prefers task_work so that userspace processes expecting immediate
completion (such as fput() side-effects) receive a deterministic
behavior before returning from close(). It falls back to schedule_work()
if the current context is a kernel thread (PF_KTHREAD) or if
task_work_add() fails.
Link: https://syzkaller.appspot.com/bug?extid=cd8a9a308e879a4e2c28 [1]
Link: https://syzkaller.appspot.com/bug?extid=bc273027d5643e48e5b3 [2]
Link: https://syzkaller.appspot.com/bug?extid=2f62807dc3239b8f584e [3]
Link: https://syzkaller.appspot.com/bug?extid=c4e9d077bcc86bee08dc [4]
Analyzed-by: AI Mode in Google Search (no mail address)
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
drivers/block/loop.c | 86 ++++++++++++++++++++++++++++++++++++--------
kernel/task_work.c | 1 +
2 files changed, 73 insertions(+), 14 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 0000913f7efc..d97aa2c209e3 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -36,6 +36,7 @@
#include <linux/blk-mq.h>
#include <linux/spinlock.h>
#include <uapi/linux/loop.h>
+#include <linux/task_work.h>
/* Possible states of device */
enum {
@@ -74,6 +75,10 @@ struct loop_device {
struct gendisk *lo_disk;
struct mutex lo_mutex;
bool idr_visible;
+ union {
+ struct callback_head lo_clr_task_work;
+ struct work_struct lo_clr_work;
+ };
};
struct loop_cmd {
@@ -1112,12 +1117,34 @@ static int loop_configure(struct loop_device *lo, blk_mode_t mode,
return error;
}
-static void __loop_clr_fd(struct loop_device *lo)
+static void __loop_clr_fd(struct callback_head *callback)
{
+ struct loop_device *lo = container_of(callback, struct loop_device, lo_clr_task_work);
struct queue_limits lim;
struct file *filp;
gfp_t gfp = lo->old_gfp_mask;
+ /* Step 1: Flush all outstanding I/O, without open_mutex held. */
+
+ /*
+ * Now that loop_queue_rq() sees lo->lo_state != Lo_bound,
+ * wait for already started loop_queue_rq() to complete.
+ */
+ synchronize_rcu();
+ /*
+ * Now that no more works are scheduled by loop_queue_rq(),
+ * wait for already scheduled works to complete.
+ */
+ drain_workqueue(lo->workqueue);
+ /*
+ * Now that no more AIO requests are scheduled by lo_rw_aio(),
+ * wait for already started AIO to complete.
+ */
+ blk_mq_unfreeze_queue(lo->lo_queue, blk_mq_freeze_queue(lo->lo_queue));
+
+ /* Step 2: Perform remaining cleanup, with open_mutex held. */
+ mutex_lock(&lo->lo_disk->open_mutex);
+
spin_lock_irq(&lo->lo_lock);
filp = lo->lo_backing_file;
lo->lo_backing_file = NULL;
@@ -1128,12 +1155,7 @@ static void __loop_clr_fd(struct loop_device *lo)
lo->lo_sizelimit = 0;
memset(lo->lo_file_name, 0, LO_NAME_SIZE);
- /*
- * Reset the block size to the default.
- *
- * No queue freezing needed because this is called from the final
- * ->release call only, so there can't be any outstanding I/O.
- */
+ /* Reset the block size to the default. */
lim = queue_limits_start_update(lo->lo_queue);
lim.logical_block_size = SECTOR_SIZE;
lim.physical_block_size = SECTOR_SIZE;
@@ -1145,8 +1167,6 @@ static void __loop_clr_fd(struct loop_device *lo)
/* let user-space know about this change */
kobject_uevent(&disk_to_dev(lo->lo_disk)->kobj, KOBJ_CHANGE);
mapping_set_gfp_mask(filp->f_mapping, gfp);
- /* This is safe: open() is still holding a reference. */
- module_put(THIS_MODULE);
disk_force_media_change(lo->lo_disk);
@@ -1154,9 +1174,6 @@ static void __loop_clr_fd(struct loop_device *lo)
int err;
/*
- * open_mutex has been held already in release path, so don't
- * acquire it if this function is called in such case.
- *
* If the reread partition isn't from release path, lo_refcnt
* must be at least one and it can only become zero when the
* current holder is released.
@@ -1181,12 +1198,31 @@ static void __loop_clr_fd(struct loop_device *lo)
WRITE_ONCE(lo->lo_state, Lo_unbound);
mutex_unlock(&lo->lo_mutex);
+ /* Step 3: Drop refcounts, without open_mutex held. */
+ mutex_unlock(&lo->lo_disk->open_mutex);
+
/*
* Need not hold lo_mutex to fput backing file. Calling fput holding
* lo_mutex triggers a circular lock dependency possibility warning as
* fput can take open_mutex which is usually taken before lo_mutex.
*/
fput(filp);
+
+ /*
+ * Drop all references that would have been dropped as soon as
+ * returning from lo_release() and releasing disk->open_mutex.
+ */
+ module_put(lo->lo_disk->fops->owner);
+ put_device(disk_to_dev(lo->lo_disk));
+
+ module_put(THIS_MODULE);
+}
+
+static void loop_clr_work(struct work_struct *work)
+{
+ struct loop_device *lo = container_of(work, struct loop_device, lo_clr_work);
+
+ __loop_clr_fd(&lo->lo_clr_task_work);
}
static int loop_clr_fd(struct loop_device *lo)
@@ -1747,8 +1783,30 @@ static void lo_release(struct gendisk *disk)
need_clear = (lo->lo_state == Lo_rundown);
mutex_unlock(&lo->lo_mutex);
- if (need_clear)
- __loop_clr_fd(lo);
+ /*
+ * In order to flush pending I/O requests before clearing the backing device,
+ * defer __loop_clr_fd() to task work context or normal workqueue context.
+ * The Lo_rundown state guarantees that lo_open() will fail with -ENXIO.
+ */
+ if (need_clear) {
+ /*
+ * Grab all references that will be dropped as soon as returning from
+ * lo_release() and releasing disk->open_mutex.
+ */
+ get_device(disk_to_dev(disk));
+ __module_get(disk->fops->owner);
+ /*
+ * Prefer task work, for userspace might be expecting that fput()
+ * on the backing file is processed before lo_release() from close()
+ * returns to user mode.
+ */
+ init_task_work(&lo->lo_clr_task_work, __loop_clr_fd);
+ if ((current->flags & PF_KTHREAD) ||
+ task_work_add(current, &lo->lo_clr_task_work, TWA_RESUME)) {
+ INIT_WORK(&lo->lo_clr_work, loop_clr_work);
+ schedule_work(&lo->lo_clr_work);
+ }
+ }
}
static void lo_free_disk(struct gendisk *disk)
diff --git a/kernel/task_work.c b/kernel/task_work.c
index 0f7519f8e7c9..45fd146b85df 100644
--- a/kernel/task_work.c
+++ b/kernel/task_work.c
@@ -102,6 +102,7 @@ int task_work_add(struct task_struct *task, struct callback_head *work,
return 0;
}
+EXPORT_SYMBOL_GPL(task_work_add);
/**
* task_work_cancel_match - cancel a pending work added by task_work_add()
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v6 1/4] block: add task-context bio completion infrastructure
From: Christoph Hellwig @ 2026-05-25 5:17 UTC (permalink / raw)
To: Tal Zussman
Cc: Christoph Hellwig, Jens Axboe, Matthew Wilcox (Oracle),
Christian Brauner, Darrick J. Wong, Carlos Maiolino,
Alexander Viro, Jan Kara, Dave Chinner, Bart Van Assche,
linux-block, linux-kernel, linux-xfs, linux-fsdevel, linux-mm,
Gao Xiang
In-Reply-To: <ea1fa305-3ba2-4cfd-b7cb-86875032a300@columbia.edu>
On Fri, May 22, 2026 at 06:47:43PM -0400, Tal Zussman wrote:
> > But this 1-jiffie delay also means we unconditionally increase
> > completion latency, which feels like a bad idea. Do you have any
> > measurements that show where it does benefit? Note that queing work
> > already often has very measurable latency on it's own. This also
> > directly contradics the erofs experience that even went to a RT
> > thread to reduce the latency.
>
> I added this per Dave's feedback on v4, where he noted that XFS inodegc
> uses a delayed work item to avoid context switch storms. There's only a
> delay for the first bio in a batch to complete, as we only delay when the
> list is empty. I'll run some experiments and measure context switches,
> completion latency, etc. to see if this is necessary.
The difference is that XFS inodegc is not latency bound. Most of the
time no one cares if it is delayed a bit, in the cases where someone
cares we explicitly flush the queues. I/O completion on the other hand
is something where users very much care about latency.
^ permalink raw reply
* Re: [PATCH v6 1/4] block: add task-context bio completion infrastructure
From: Christoph Hellwig @ 2026-05-25 5:24 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, Gao Xiang,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
linux-rt-devel
In-Reply-To: <ea6fc01f-5cb7-4a04-9f92-bbd2791fea51@columbia.edu>
[adding the PREEMPT-RT maintainers and list for one and a half questions
for them a bit below]
On Fri, May 22, 2026 at 07:09:59PM -0400, Tal Zussman wrote:
> > + while ((bio = bio_list_pop(&list)))
> > + bio->bi_end_io(bio);
>
> Q: "Does this code handle bios that do not have a completion callback?
> If a bio is submitted with the BIO_COMPLETE_IN_TASK flag but has a NULL
> bi_end_io pointer (like some flush or discard bios), executing
> bio->bi_end_io(bio) directly will cause a NULL pointer dereference.
> The synchronous completion path in bio_endio() checks if bio->bi_end_io is
> non-NULL before calling it. Should a similar check be added here?"
>
> A: Every bio that reaches this point should have bi_end_io set. There's no
> point in deferring a bio without a completion handler.
Yes. The only bios without bi_end_io that can end up in bio_endio
are those attached to passthrough requests, and deferring them of
course doesn't make sense.
> Q: "Could this loop also monopolize the CPU and cause latency spikes?
> Because the work uses a 1-jiffie delay, tens of thousands of bios might
> accumulate under heavy I/O load. Processing the entire batch sequentially
> without yielding or calling cond_resched() inside the inner loop could
> lead to scheduler latency or watchdog warnings on non-preemptible kernels."
>
> A: With PREEMPT_LAZY as the default, the worker is preemptible, as per
> Hillf and Christoph, so this is unnecessary.
Yes. Sashiko tends to spew this a lot, which is odd as there were clear
recommendations to only do this absolutely if needed even before
PREEMPT_LAZY.
> > +
> > + local_lock_irqsave(&bio_complete_batch.lock, flags);
>
> Q: "Is it safe to use local_lock_irqsave() here when called from an atomic
> context?
> On CONFIG_PREEMPT_RT kernels, local_lock_t maps to a per-CPU spinlock_t,
> which is a sleepable rt_mutex. Since __bio_complete_in_task() is specifically
> called when bio_in_atomic() is true (which includes hardware interrupts or
> execution under a raw_spinlock_t), attempting to acquire a sleepable lock
> here would trigger an "Invalid wait context" lockdep warning.
> Would a lockless list (llist) be more appropriate here to avoid sleeping
> in atomic contexts?"
>
> A: This seems legit, but I'm not super familiar with PREEMPT_RT. I don't want
> to switch to raw_spinlock_t, as it seems like that would add unnecessary
> overhead on non-PREEMPT_RT kernels. I think switching to use local_irq_save()
> (as is done for the per-CPU bio allocation cache) should work.
Adding the PREEMPT_RT maintainers for this as it is above my pay grade.
> Q: "Does creating this workqueue with WQ_MEM_RECLAIM break the local_lock_irq()
> protection in bio_complete_work_fn()?
> When a workqueue has WQ_MEM_RECLAIM, it spawns a global rescuer thread that
> can execute per-CPU work items during worker pool congestion. This rescuer
> thread executes unbound, meaning it could run on CPU B while processing
> CPU A's work item.
> Since local_lock operates strictly on the currently executing CPU, the
> rescuer thread on CPU B would acquire CPU B's lock, while popping elements
> from CPU A's list (derived via container_of()).
> If an interrupt on CPU A concurrently calls __bio_complete_in_task(),
> it will acquire CPU A's lock and modify the same list without mutual
> exclusion, potentially causing list corruption."
>
> A: The rescuer should run on the same CPU, not unbound, so this is not an
> issue.
This is another area where the PREEMPT_RT/scheduler folks might be able
to help.
> static inline bool bio_complete_in_task(struct bio *bio)
> {
> if (bio_flagged(bio, BIO_COMPLETE_IN_TASK))
> return false;
> if (!bio_in_atomic())
> return false;
> bio_set_flag(bio, BIO_COMPLETE_IN_TASK);
> __bio_complete_in_task(bio);
> return true;
> }
>
> We can use the BIO_COMPLETE_IN_TASK flag to indicate that it's already
> been deferred to the workqueue as is safe to run.
Would be nice to avoid this, but yes.
^ permalink raw reply
* Re: [PATCH v6 3/4] buffer: add dropbehind writeback support
From: Christoph Hellwig @ 2026-05-25 5:25 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, Gao Xiang
In-Reply-To: <d98a35c1-882b-4752-a2c7-378872fc97a3@columbia.edu>
On Fri, May 22, 2026 at 07:14:46PM -0400, Tal Zussman wrote:
> > if (IS_ERR(folio))
> > return PTR_ERR(folio);
> > @@ -2160,6 +2165,13 @@ int block_write_begin(struct address_space *mapping, loff_t pos, unsigned len,
> > *foliop = folio;
> > return status;
> > }
>
> Q: "Is an EXPORT_SYMBOL() or EXPORT_SYMBOL_GPL() needed for
> block_write_begin_iocb()?
> Since the legacy wrapper block_write_begin() remains exported, it appears
> filesystems compiled as loadable modules (like ext4 or fat) will encounter
> linker errors if they attempt to transition to the new API."
>
> A: Can be added once filesystems want to use it.
Yeah, Sashiko is really stupid and counterproductive here.
> > @@ -2715,6 +2727,9 @@ static void submit_bh_wbc(blk_opf_t opf, struct buffer_head *bh,
> >
> > bio = bio_alloc(bh->b_bdev, 1, opf, GFP_NOIO);
> >
> > + if (folio_test_dropbehind(bh->b_folio))
> > + bio_set_flag(bio, BIO_COMPLETE_IN_TASK);
> > +
>
> Q: "Does this add unnecessary workqueue overhead for read operations?
> While task-context deferral is required for asynchronous writeback completions
> to allow folio_end_writeback() to sleep and invalidate the folio, read
> dropbehind invalidation is handled synchronously in the reader's task context
> via filemap_end_dropbehind_read().
> Could this be gated by an op == REQ_OP_WRITE or op_is_write(opf) check so
> that read bio completions aren't forced into a workqueue?"
>
> A: Seems worth adding a op_is_write(opf) check to avoid deferring read
> dropbehind completions.
Yes.
^ permalink raw reply
* Re: Observing higher CPU utilization during random IO fio testing
From: Yu Kuai @ 2026-05-25 5:28 UTC (permalink / raw)
To: Jens Axboe, Wen Xiong, linux-block
Cc: tom.leiming, jmoyer, Gjoyce, wenxiong, yukuai
In-Reply-To: <1cb74987-34e2-422f-93cf-9174fe913538@kernel.dk>
Hi,
在 2026/5/22 5:52, Jens Axboe 写道:
> On 5/21/26 1:44 PM, Wen Xiong wrote:
>> Hi All,
>>
>> Our performance team observed the higher CPU utilization in RHEL10 compared to RHEL9.8, observed the similar issue in upstream kernel(v7.1-rc4) as well when running FIO random IO tests.
>>
>> System configuration:
>> 47 dedicate cores
>> 120 GB memory
>> PCIe4 2-Port 64Gb FC Adapter
>> FlashSystem: FS9500, 12 LUNs/FC port, 100G each LUN.
>>
>> Random IO tests are more CPU intensive than sequential IO tests due to several factors: more context switching, Interrupt Handling, cache Inefficiency etc. We found out the following patch which caused the higher CPU utilization in rhel10 and newer linux kernel:
>>
>> commit 060406c61c7cb4bbd82a02d179decca9c9bb3443 (HEAD)
>> Author: Yu Kuai <yukuai3@huawei.com>
>> Date: Thu May 9 20:38:25 2024 +0800
>>
>> block: add plug while submitting IO
>>
>> So that if caller didn't use plug, for example, __blkdev_direct_IO_simple()
>> and __blkdev_direct_IO_async(), block layer can still benefit from caching
>> nsec time in the plug.
>>
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>> Link: https://lore.kernel.org/r/20240509123825.3225207-1-yukuai1@huaweicloud.com
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>
>> We reverted above patch in rhel10 kernel and upstream 7.1-rc4, saw lower CPU utilization when doing the same FIO test.
>>
>> The patch adds plugging in __submit_bio() in block layer, maybe cause performance degradation:
>> - Random IO tests have less merging, flush overhead.
>> - More IO scheduler interaction, forces requests through scheduler instead of direct dispatch(direct dispatch to hardware queue)
I don't understand this point. Can you explain more? I think plug should not matter if request go through scheduler or not.
>> - Poor cache locality during plug operation
>>
>> Below are some performance data that our performance team collected:
>>
>> RHEL9.8 comparison RHEL10.0
>> Iotype qd nj rmix mpstat busy delta lparstat delta
>> Randrw 1 20 100 135% 109%
>> Randrw 1 40 100 72% 81%
>> Randrw 1 20 70 278% 174%
>> Randrw 1 40 70 272% 191%
>> Randrw 1 20 0 93% 30%
>> Randrw 1 40 0 104% 36%
>>
>> RHEL 9.8 comparison RHEL10 with reverting above plugging patch in block layer.h
>> Iotype qd nj rmix mpstat busy delta lparstat deltab
>> Randrw 1 20 100 -12% 20%
>> Randrw 1 40 100 -42% -4%
>> Randrw 1 20 70 70% 71%
>> Randrw 1 40 70 %51 60%
>> Randrw 1 20 0 -14% -43%
>> Randrw 1 40 0 -33% -51%
>>
>> Can a block layer expert help us resolve this high CPU utilization performance issue?
And I assume you're testing raw disk, because filesystems should always enable plug.
>> Let us know if you need more performance data or other perf data.
Yes, perf data will be helpful. And please show your test in details and I'll
check if I can reproduce it.
> Let's CC Yu Kuai who wrote that commit, that might help.
>
--
Thansk,
Kuai
^ permalink raw reply
* Re: [PATCH v6 4/4] block: enable RWF_DONTCACHE for block devices
From: Christoph Hellwig @ 2026-05-25 5:30 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, Gao Xiang
In-Reply-To: <f8d509c7-c0c0-4196-9a82-a6f8ae24ffc2@columbia.edu>
On Fri, May 22, 2026 at 07:17:15PM -0400, Tal Zussman wrote:
> A: So this actually seems legit... doesn't look like anything actually calls
> blkdev_write_begin() or blkdev_write_end(), unless I'm missing something.
> block_write_begin_iocb() usage seems necessary for bh-based filesystems, but
> block devices seem to use iomap for writes unconditionally.
Yes. Maybe send a separate patch to remove these now unused methods?
Or I could do that since I forgot to remove them when I should have.
^ permalink raw reply
* Re: [PATCH] block: skip sync_blockdev() on surprise removal in bdev_mark_dead()
From: Christoph Hellwig @ 2026-05-25 5:58 UTC (permalink / raw)
To: Chao Shi
Cc: Jens Axboe, Christoph Hellwig, Christian Brauner, Josef Bacik,
linux-block, linux-kernel, Sungwoo Kim, Dave Tian, Weidong Zhu
In-Reply-To: <20260522220025.1770388-1-coshi036@gmail.com>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH v1] block: switch numa_node to int in blk_mq_hw_ctx and init_request
From: Christoph Hellwig @ 2026-05-25 6:03 UTC (permalink / raw)
To: Mateusz Nowicki
Cc: Jens Axboe, Caleb Sander Mateos, Sung-woo Kim, Josef Bacik,
Alasdair Kergon, Mike Snitzer, Mikulas Patocka,
Benjamin Marzinski, Ulf Hansson, Richard Weinberger, Zhihao Cheng,
Miquel Raynal, Vignesh Raghavendra, Sven Peter, Janne Grunau,
Neal Gompa, Keith Busch, Christoph Hellwig, Sagi Grimberg,
Justin Tee, Naresh Gottumukkala, Paul Ely, Chaitanya Kulkarni,
James E.J. Bottomley, Martin K. Petersen, Thomas Fourier, Al Viro,
Luke Wang, Kees Cook, linux-block, linux-kernel, nbd, dm-devel,
linux-mmc, linux-mtd, asahi, linux-arm-kernel, linux-nvme,
linux-scsi
In-Reply-To: <20260523125210.272274-1-mateusz.nowicki@posteo.net>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH] block: Add bvec_folio()
From: Christoph Hellwig @ 2026-05-25 6:06 UTC (permalink / raw)
To: Matthew Wilcox (Oracle)
Cc: Jens Axboe, linux-block, linux-kernel, io-uring, linux-mm,
Leon Romanovsky
In-Reply-To: <20260522182122.2489391-1-willy@infradead.org>
> +/**
> + * bvec_folio - Return the first folio referenced by this bvec
> + * @bv: bvec to access
> + *
> + * bvecs can span multiple folios. Unless you know that this
> + * bvec does not, you may be better off using something like
> + * bio_for_each_folio_all() which iterates over all folios.
> + */
> +static inline struct folio *bvec_folio(const struct bio_vec *bv)
> +{
> + return page_folio(bv->bv_page);
> +}
The comment here is confusing. bio_for_each_folio_all is a helper that
only works in the submitter side, and not for anything using the
bvec_iter required for drivers or anything else sitting below a
potential bio clone/split or using bvecs from an upper layer (like
ITER_BVEC direct I/O). Additionally bv_page can be a different
page than the fist page due to large bv_offset on split bios.
So I'm not against the function per se, but the documentation must
explain the minefields it is stepping into a bit better.
^ permalink raw reply
* Re: [PATCH] block: Avoid mounting the bdev pseudo-filesystem in userspace
From: Christoph Hellwig @ 2026-05-25 6:07 UTC (permalink / raw)
To: Denis Arefev; +Cc: Jens Axboe, linux-block, linux-kernel, lvc-project, stable
In-Reply-To: <20260521072857.5078-1-arefev@swemel.ru>
On Thu, May 21, 2026 at 10:28:56AM +0300, Denis Arefev wrote:
> The bdev pseudo-filesystem is an internal kernel filesystem with which
> userspace should not interfere. Unregister it so that userspace cannot
> even attempt to mount it.
>
> This fixes a bug [1] that occurs when attempting to access files,
> because the system call move_mount() uses pointers declared in the
> inode_operations structure, which for the bdev pseudo-filesystem
> are always equal to 0. `inode->i_op = &empty_iops;`
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH] block: partitions: replace __get_free_page() with kmalloc()
From: Christoph Hellwig @ 2026-05-25 6:08 UTC (permalink / raw)
To: Mike Rapoport (Microsoft); +Cc: Jens Axboe, linux-block, linux-kernel, linux-mm
In-Reply-To: <20260520-block-v1-1-6463dc2cf042@kernel.org>
On Wed, May 20, 2026 at 11:15:52AM +0300, Mike Rapoport (Microsoft) wrote:
> check_partition() allocates a buffer to use as backing buffer for
> seq_buf.
>
> This buffer can be allocated with kmalloc() as there's nothing special
> about it to go directly to the page allocator.
>
> Replace use of __get_free_page() with kmalloc() and free_page() with
> kfree().
So I heard various vague references that we should replace
__get_free_page with kmalloc, but nothing definitive. Can you please
point to a good resource for that?
^ permalink raw reply
* Re: [PATCH v2 05/21] Add a function to kmap one page of a multipage bio_vec
From: Christoph Hellwig @ 2026-05-25 6:13 UTC (permalink / raw)
To: David Howells
Cc: Christian Brauner, Matthew Wilcox, Christoph Hellwig,
Paulo Alcantara, Jens Axboe, Leon Romanovsky, Steve French,
ChenXiaoSong, Marc Dionne, Eric Van Hensbergen,
Dominique Martinet, Ilya Dryomov, Trond Myklebust, netfs,
linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs, linux-erofs,
linux-fsdevel, linux-kernel, linux-block
In-Reply-To: <20260518222959.488126-6-dhowells@redhat.com>
On Mon, May 18, 2026 at 11:29:37PM +0100, David Howells wrote:
> Add a function to kmap one page of a multipage bio_vec by offset (which is
> added to the offset in the bio_vec internally). The caller is responsible
> for calculating how much of the page is then available.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
> cc: Matthew Wilcox <willy@infradead.org>
> cc: Christoph Hellwig <hch@infradead.org>
> cc: Jens Axboe <axboe@kernel.dk>
> cc: linux-block@vger.kernel.org
> cc: netfs@lists.linux.dev
> cc: linux-fsdevel@vger.kernel.org
> ---
> include/linux/bvec.h | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/include/linux/bvec.h b/include/linux/bvec.h
> index d36dd476feda..9df4a56fef61 100644
> --- a/include/linux/bvec.h
> +++ b/include/linux/bvec.h
> @@ -299,4 +299,21 @@ static inline phys_addr_t bvec_phys(const struct bio_vec *bvec)
> return page_to_phys(bvec->bv_page) + bvec->bv_offset;
> }
>
> +/**
> + * kmap_local_bvec - Map part of a bvec into the kernel virtual address space
> + * @bvec: bvec to map
> + * @offset: Offset into bvec
> + *
> + * Map the page containing the byte at @offset into the kernel virtual address
> + * space. The caller is responsible for making sure this doesn't overrun.
> + *
> + * Call kunmap_local on the returned address to unmap.
> + */
> +static inline void *kmap_local_bvec(struct bio_vec *bvec, size_t offset)
The name is rather confusing for something that does not map the entire
bvec, and is an anagram of the existing bvec_kmap_local. So please
rename it to bvec_kmap_partial or something.
> +{
> + offset += bvec->bv_offset;
> +
> + return kmap_local_page(bvec->bv_page + offset / PAGE_SIZE) + offset % PAGE_SIZE;
Overly long line. Also this can use shits and byte masking to be a tad
more efficient and matching the rest of the bvec code.
Users of this would be interesting and why you're not simply using a
bvec_iter at page granularity, which is what other block kmap code
does.
^ permalink raw reply
* Re: [PATCH v2 06/21] iov_iter: Make iov_iter_get_pages*() wrap iov_iter_extract_pages()
From: Christoph Hellwig @ 2026-05-25 6:13 UTC (permalink / raw)
To: David Howells
Cc: Christian Brauner, Matthew Wilcox, Christoph Hellwig,
Paulo Alcantara, Jens Axboe, Leon Romanovsky, Steve French,
ChenXiaoSong, Marc Dionne, Eric Van Hensbergen,
Dominique Martinet, Ilya Dryomov, Trond Myklebust, netfs,
linux-afs, linux-cifs, linux-nfs, ceph-devel, v9fs, linux-erofs,
linux-fsdevel, linux-kernel, linux-block
In-Reply-To: <20260518222959.488126-7-dhowells@redhat.com>
On Mon, May 18, 2026 at 11:29:38PM +0100, David Howells wrote:
> Make iov_iter_get_pages*() wrap iov_iter_extract_pages() for kernel
> iterator types (e.g. ITER_BVEC, ITER_FOLIOQ, ITER_XARRAY). The pages
> obtained have their refcounts incremented afterwards if they're not slab
> pages. ITER_KVEC is left returning -EFAULT.
Just kill off iov_iter_get_pages*, please. Or at least stop using it
where these types matter.
^ permalink raw reply
* Re: [PATCH] block: partitions: replace __get_free_page() with kmalloc()
From: Mike Rapoport @ 2026-05-25 6:52 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, linux-kernel, linux-mm
In-Reply-To: <ahPnX-CwVgK-9chp@infradead.org>
On Sun, May 24, 2026 at 11:08:31PM -0700, Christoph Hellwig wrote:
> On Wed, May 20, 2026 at 11:15:52AM +0300, Mike Rapoport (Microsoft) wrote:
> > check_partition() allocates a buffer to use as backing buffer for
> > seq_buf.
> >
> > This buffer can be allocated with kmalloc() as there's nothing special
> > about it to go directly to the page allocator.
> >
> > Replace use of __get_free_page() with kmalloc() and free_page() with
> > kfree().
>
> So I heard various vague references that we should replace
> __get_free_page with kmalloc, but nothing definitive. Can you please
> point to a good resource for that?
There was quite recent discussion when I posted patches that change
__get_free_page to return void *:
https://lore.kernel.org/all/20251018093002.3660549-1-rppt@kernel.org/
And an old thread when Al posted similar patches:
https://lore.kernel.org/all/CA+55aFwp4iy4rtX2gE2WjBGFL=NxMVnoFeHqYa2j1dYOMMGqxg@mail.gmail.com/T/#u<S-Del>
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH] block: partitions: replace __get_free_page() with kmalloc()
From: Christoph Hellwig @ 2026-05-25 7:16 UTC (permalink / raw)
To: Mike Rapoport
Cc: Christoph Hellwig, Jens Axboe, linux-block, linux-kernel,
linux-mm
In-Reply-To: <ahPxmSqT4Y4jcAbQ@kernel.org>
On Mon, May 25, 2026 at 09:52:09AM +0300, Mike Rapoport wrote:
> On Sun, May 24, 2026 at 11:08:31PM -0700, Christoph Hellwig wrote:
> > On Wed, May 20, 2026 at 11:15:52AM +0300, Mike Rapoport (Microsoft) wrote:
> > > check_partition() allocates a buffer to use as backing buffer for
> > > seq_buf.
> > >
> > > This buffer can be allocated with kmalloc() as there's nothing special
> > > about it to go directly to the page allocator.
> > >
> > > Replace use of __get_free_page() with kmalloc() and free_page() with
> > > kfree().
> >
> > So I heard various vague references that we should replace
> > __get_free_page with kmalloc, but nothing definitive. Can you please
> > point to a good resource for that?
>
> There was quite recent discussion when I posted patches that change
> __get_free_page to return void *:
>
> https://lore.kernel.org/all/20251018093002.3660549-1-rppt@kernel.org/
This doesn't tell much more.
> And an old thread when Al posted similar patches:
>
> https://lore.kernel.org/all/CA+55aFwp4iy4rtX2gE2WjBGFL=NxMVnoFeHqYa2j1dYOMMGqxg@mail.gmail.com/T/#u<S-Del>
This does, but it still fails to explain why kmalloc performs just as
well as __get_free_page(s) these days.
And such an explanation or link to it should go into every patch doing
this switch.
^ permalink raw reply
* Re: [PATCH v3 04/10] block: introduce dma map backed bio type
From: Pavel Begunkov @ 2026-05-25 7:29 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, Keith Busch, Sagi Grimberg, Alexander Viro,
Christian Brauner, Andrew Morton, Sumit Semwal,
Christian König, linux-block, linux-kernel, linux-nvme,
linux-fsdevel, io-uring, linux-media, dri-devel, linaro-mm-sig,
Nitesh Shetty, Kanchan Joshi, Anuj Gupta, Tushar Gohad,
William Power, Phil Cayton, Jason Gunthorpe
In-Reply-To: <20260520083043.GA18893@lst.de>
On 5/20/26 09:30, Christoph Hellwig wrote:
> On Mon, May 18, 2026 at 11:29:54AM +0100, Pavel Begunkov wrote:
>>>> BIO_ZONE_WRITE_PLUGGING, /* bio handled through zone write plugging */
>>>> BIO_EMULATES_ZONE_APPEND, /* bio emulates a zone append operation */
>>>> + BIO_DMABUF_MAP, /* Using premmaped dma buffers */
>>>
>>> Shouldn't this be a REQ_ flag as we should never mix and match bios with
>>> and without this flag in a single request?
>>
>> Do you mean adding both and propagating it from bio to req? submit_bio()
>> takes a bio, so we still need to set it there before it reaches blk-mq.
>> And there might be bio-based drivers using it in the future.
>
> I think I forgot to reply to this, so let's do this now.
>
> REQ_ is actually used by both bios and requests, so if you set it in
> bio->bi_opf it will automatically get propagated to the request, but
> it can also always be tested on the bio, including by bio-based
> drivers.
Ah yes, good point, thanks
--
Pavel Begunkov
^ permalink raw reply
* Re: [PATCH] block, nvme: export and use passthrough stats
From: Christoph Hellwig @ 2026-05-25 7:32 UTC (permalink / raw)
To: Keith Busch; +Cc: linux-block, linux-nvme, axboe, hch, nilay, Keith Busch
In-Reply-To: <20260522151537.1509784-1-kbusch@meta.com>
On Fri, May 22, 2026 at 08:15:37AM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> So stacking drivers can also report passthrough workloads through
> iostat.
Except that this patch also wires it up in nvme. So please split
it and write proper commit messages for both parts.
> +static inline bool blk_rq_passthrough_stats(struct request *req)
> +{
And maybe add a kerneldoc comment now that this is public.
^ permalink raw reply
* Re: [PATCH] block, nvme: export and use passthrough stats
From: Christoph Hellwig @ 2026-05-25 7:35 UTC (permalink / raw)
To: Keith Busch; +Cc: linux-block, linux-nvme, axboe, hch, nilay, Keith Busch
In-Reply-To: <20260525073219.GA5305@lst.de>
On Mon, May 25, 2026 at 09:32:19AM +0200, Christoph Hellwig wrote:
> On Fri, May 22, 2026 at 08:15:37AM -0700, Keith Busch wrote:
> > From: Keith Busch <kbusch@kernel.org>
> >
> > So stacking drivers can also report passthrough workloads through
> > iostat.
>
> Except that this patch also wires it up in nvme. So please split
> it and write proper commit messages for both parts.
.. and while we're at it - it actually is called from the lower nvme
drivers, just conditional on being a mpath request. So this is
in many ways a bit weird, and the comments / commit logs should
probably explain it a bit.
^ permalink raw reply
* Re: [PATCH] block: fix dio leak on integrity metadata mapping failure
From: Christoph Hellwig @ 2026-05-25 7:45 UTC (permalink / raw)
To: Aaron Esau
Cc: linux-block, Jens Axboe, Anuj Gupta, Kanchan Joshi,
Christoph Hellwig, Keith Busch, linux-kernel, stable
In-Reply-To: <20260518074258.1600307-1-aaron1esau@gmail.com>
> + if (unlikely(ret)) {
> + bio->bi_status = BLK_STS_IOERR;
> + bio_endio(bio);
> + break;
> + }
AFAICS the same issue also exists for the other goto fail case,
so we should convert the code at that label to a bio_endio().
I think both this and the existing -EAGAIN case and even the
-EIOCBQUEUED case leak the reference on the original bio. Or am
I missing something?
It might makes sense to stop playing games with that bio refcount
and just have a status field in struct blkdev_dio.
^ permalink raw reply
* Re: [PATCH] block: partitions: replace __get_free_page() with kmalloc()
From: Mike Rapoport @ 2026-05-25 9:35 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Jens Axboe, linux-block, linux-kernel, linux-mm
In-Reply-To: <ahP3R2nH8-bHMVf0@infradead.org>
On Mon, May 25, 2026 at 12:16:23AM -0700, Christoph Hellwig wrote:
> On Mon, May 25, 2026 at 09:52:09AM +0300, Mike Rapoport wrote:
> > On Sun, May 24, 2026 at 11:08:31PM -0700, Christoph Hellwig wrote:
> > > On Wed, May 20, 2026 at 11:15:52AM +0300, Mike Rapoport (Microsoft) wrote:
> > > > check_partition() allocates a buffer to use as backing buffer for
> > > > seq_buf.
> > > >
> > > > This buffer can be allocated with kmalloc() as there's nothing special
> > > > about it to go directly to the page allocator.
> > > >
> > > > Replace use of __get_free_page() with kmalloc() and free_page() with
> > > > kfree().
> > >
> > > So I heard various vague references that we should replace
> > > __get_free_page with kmalloc, but nothing definitive. Can you please
> > > point to a good resource for that?
> >
> > There was quite recent discussion when I posted patches that change
> > __get_free_page to return void *:
> >
> > https://lore.kernel.org/all/20251018093002.3660549-1-rppt@kernel.org/
>
> This doesn't tell much more.
>
> > And an old thread when Al posted similar patches:
> >
> > https://lore.kernel.org/all/CA+55aFwp4iy4rtX2gE2WjBGFL=NxMVnoFeHqYa2j1dYOMMGqxg@mail.gmail.com/T/#u<S-Del>
>
> This does, but it still fails to explain why kmalloc performs just as
> well as __get_free_page(s) these days.
I don't think that in this case - a single allocation on the cold path -
the performance difference is even measurable.
Nevertheless allocations from slab caches are way faster than
__get_free_page() (i.e. alloc_pages()) as it's essentially lockless
cmpxchg. Allocations that need to refill the cache do alloc_pages() with a
little of slab bookkeeping overhead.
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH 4/4] dm crypt: batch all sectors of a bio per crypto request
From: Mikulas Patocka @ 2026-05-25 12:02 UTC (permalink / raw)
To: Leonid Ravich
Cc: Herbert Xu, David S . Miller, Mike Snitzer, Alasdair Kergon,
Ard Biesheuvel, Eric Biggers, Jens Axboe, Horia Geanta,
Gilad Ben-Yossef, linux-crypto, dm-devel, linux-block
In-Reply-To: <20260519120002.27267-5-lravich@amazon.com>
[-- Attachment #1: Type: text/plain, Size: 15091 bytes --]
Hi
On Tue, 19 May 2026, Leonid Ravich wrote:
> When the underlying skcipher driver advertises support for multiple
> data units in a single request (CRYPTO_ALG_SKCIPHER_MULTI_DATA_UNIT),
> configure the cipher with cc->sector_size as data_unit_size and
> submit one request per bio instead of one request per sector. This
> removes per-sector overhead in the crypto API hot path: request
> allocation, callback dispatch, completion handling, and SG setup.
>
> The optimisation is enabled automatically at table load when all
> of the following hold:
>
> - the cipher is non-aead (i.e. skcipher);
> - tfms_count is 1 (interleaved per-sector keys would break batching);
> - the IV mode is plain or plain64 (the only modes whose generator
> produces a sequential 64-bit little-endian counter that the cipher
> can extend by adding the data-unit index, matching the convention
> documented in crypto_skcipher_set_data_unit_size());
> - the iv_gen_ops->post() hook is unset (lmk and tcw use it; both are
> already excluded by the IV-mode test, but the explicit check makes
> the assumption durable against future IV modes);
> - dm-integrity is not stacked (no integrity tag or integrity IV);
> - the cipher driver advertises multi-data-unit support.
>
> A new CRYPT_MULTI_DATA_UNIT cipher_flag, set once at construction
> time, gates the multi-data-unit path. The existing per-sector path
> in crypt_convert_block_skcipher() is unchanged; the new
> crypt_convert_block_skcipher_multi() is reached from a small dispatch
> in crypt_convert() and shares the same backlog/-EBUSY/-EINPROGRESS
> flow control with the per-sector path.
>
> Heap-allocated scatterlists are stashed in dm_crypt_request and freed
> in crypt_free_req_skcipher() to avoid races between the synchronous-
> success free path and async-completion reuse from the request pool.
>
> On -ENOMEM during scatterlist allocation, the bio is requeued via
> BLK_STS_DEV_RESOURCE rather than failed, matching the behaviour of
> the existing -ENOMEM path for crypto request allocation.
You should make sure that you do not attempt to use the multi-data-unit
mode when you retry the bio, otherwise it could loop indefinitely. Note
that there are people who swap to dm-crypt - and so, it must work even if
the memory is totally exhausted.
You should also use GFP_NOIO | __GFP_NORETRY instead of GFP_NOIO, so that
the code doesn't loop in the allocator forever.
Perhaps __bio_for_each_bvec would be better than __bio_for_each_segment,
so that it works faster with folios.
Mikulas
> Verified end-to-end with a byte-equivalence test: encrypted output of
> plain64 dm-crypt with the multi-data-unit path matches output of the
> single-data-unit path bit-for-bit over a 256 MB device.
>
> Signed-off-by: Leonid Ravich <lravich@amazon.com>
> ---
> drivers/md/dm-crypt.c | 248 ++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 241 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 5ef43231fe77..b35831d43f0e 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -98,6 +98,14 @@ struct dm_crypt_request {
> struct scatterlist sg_in[4];
> struct scatterlist sg_out[4];
> u64 iv_sector;
> + /*
> + * Heap-allocated scatterlists used by the multi-data-unit path
> + * when one bio is processed in a single skcipher request. NULL
> + * when the inline sg_in[]/sg_out[] arrays above are sufficient
> + * (single-data-unit path). Freed in crypt_free_req_skcipher().
> + */
> + struct scatterlist *sg_in_ext;
> + struct scatterlist *sg_out_ext;
> };
>
> struct crypt_config;
> @@ -149,6 +157,7 @@ enum cipher_flags {
> CRYPT_IV_LARGE_SECTORS, /* Calculate IV from sector_size, not 512B sectors */
> CRYPT_ENCRYPT_PREPROCESS, /* Must preprocess data for encryption (elephant) */
> CRYPT_KEY_MAC_SIZE_SET, /* The integrity_key_size option was used */
> + CRYPT_MULTI_DATA_UNIT, /* Batch all sectors of a bio per crypto request */
> };
>
> /*
> @@ -1501,12 +1510,139 @@ static int crypt_convert_block_skcipher(struct crypt_config *cc,
> return r;
> }
>
> +/*
> + * Multi-data-unit variant of crypt_convert_block_skcipher. Submits all
> + * remaining sectors of the current bio in one skcipher request whose
> + * data_unit_size is cc->sector_size. The cipher walks the IV between
> + * data units (see crypto_skcipher_set_data_unit_size()).
> + *
> + * Returns the same set of values as crypt_convert_block_skcipher:
> + * 0 on synchronous success (full chunk processed),
> + * -EINPROGRESS / -EBUSY on asynchronous dispatch,
> + * -ENOMEM if scatterlist allocation fails (caller maps to
> + * BLK_STS_DEV_RESOURCE so the bio is requeued, not failed),
> + * negative errno otherwise.
> + *
> + * On success the bio iterators have been advanced by the chunk size.
> + */
> +static int crypt_convert_block_skcipher_multi(struct crypt_config *cc,
> + struct convert_context *ctx,
> + struct skcipher_request *req,
> + unsigned int *out_processed)
> +{
> + const unsigned int sector_size = cc->sector_size;
> + unsigned int total_in = ctx->iter_in.bi_size;
> + unsigned int total_out = ctx->iter_out.bi_size;
> + unsigned int total = min(total_in, total_out);
> + unsigned int n_sectors;
> + unsigned int n_sg_in = 0, n_sg_out = 0;
> + struct dm_crypt_request *dmreq = dmreq_of_req(cc, req);
> + struct scatterlist *sg_in = NULL, *sg_out = NULL;
> + struct bvec_iter iter_in, iter_out;
> + struct bio_vec bv;
> + u8 *iv, *org_iv;
> + int r;
> +
> + if (unlikely(total < sector_size))
> + return -EIO;
> + n_sectors = total / sector_size;
> + total = n_sectors * sector_size;
> +
> + /*
> + * Walk the bio_vec iterators to count how many SG entries we need
> + * for exactly @total bytes. bi_size of the iterators is at least
> + * @total by construction above.
> + */
> + iter_in = ctx->iter_in;
> + iter_in.bi_size = total;
> + __bio_for_each_segment(bv, ctx->bio_in, iter_in, iter_in)
> + n_sg_in++;
> +
> + iter_out = ctx->iter_out;
> + iter_out.bi_size = total;
> + __bio_for_each_segment(bv, ctx->bio_out, iter_out, iter_out)
> + n_sg_out++;
> +
> + sg_in = kmalloc_array(n_sg_in, sizeof(*sg_in), GFP_NOIO);
> + sg_out = (ctx->bio_in == ctx->bio_out) ? sg_in :
> + kmalloc_array(n_sg_out, sizeof(*sg_out), GFP_NOIO);
> + if (!sg_in || !sg_out) {
> + kfree(sg_in);
> + if (sg_out != sg_in)
> + kfree(sg_out);
> + return -ENOMEM;
> + }
> +
> + sg_init_table(sg_in, n_sg_in);
> + {
> + unsigned int i = 0;
> +
> + iter_in = ctx->iter_in;
> + iter_in.bi_size = total;
> + __bio_for_each_segment(bv, ctx->bio_in, iter_in, iter_in)
> + sg_set_page(&sg_in[i++], bv.bv_page, bv.bv_len,
> + bv.bv_offset);
> + }
> +
> + if (sg_out != sg_in) {
> + unsigned int i = 0;
> +
> + sg_init_table(sg_out, n_sg_out);
> + iter_out = ctx->iter_out;
> + iter_out.bi_size = total;
> + __bio_for_each_segment(bv, ctx->bio_out, iter_out, iter_out)
> + sg_set_page(&sg_out[i++], bv.bv_page, bv.bv_len,
> + bv.bv_offset);
> + }
> +
> + /*
> + * Compute the IV for the first data unit. The cipher will derive
> + * IVs for subsequent data units by treating this one as a 128-bit
> + * little-endian counter and adding the data-unit index, which
> + * matches the layout produced by plain and plain64.
> + */
> + dmreq->iv_sector = ctx->cc_sector;
> + if (test_bit(CRYPT_IV_LARGE_SECTORS, &cc->cipher_flags))
> + dmreq->iv_sector >>= cc->sector_shift;
> + dmreq->ctx = ctx;
> +
> + iv = iv_of_dmreq(cc, dmreq);
> + org_iv = org_iv_of_dmreq(cc, dmreq);
> + r = cc->iv_gen_ops->generator(cc, org_iv, dmreq);
> + if (r < 0)
> + goto out_free_sg;
> + memcpy(iv, org_iv, cc->iv_size);
> +
> + /* Stash the SG arrays for cleanup on completion / free. */
> + dmreq->sg_in_ext = sg_in;
> + dmreq->sg_out_ext = (sg_out == sg_in) ? NULL : sg_out;
> +
> + skcipher_request_set_crypt(req, sg_in, sg_out, total, iv);
> +
> + if (bio_data_dir(ctx->bio_in) == WRITE)
> + r = crypto_skcipher_encrypt(req);
> + else
> + r = crypto_skcipher_decrypt(req);
> +
> + *out_processed = total;
> + return r;
> +
> +out_free_sg:
> + kfree(sg_in);
> + if (sg_out != sg_in)
> + kfree(sg_out);
> + dmreq->sg_in_ext = NULL;
> + dmreq->sg_out_ext = NULL;
> + return r;
> +}
> +
> static void kcryptd_async_done(void *async_req, int error);
>
> static int crypt_alloc_req_skcipher(struct crypt_config *cc,
> struct convert_context *ctx)
> {
> unsigned int key_index = ctx->cc_sector & (cc->tfms_count - 1);
> + struct dm_crypt_request *dmreq;
>
> if (!ctx->r.req) {
> ctx->r.req = mempool_alloc(&cc->req_pool, in_interrupt() ? GFP_ATOMIC : GFP_NOIO);
> @@ -1516,6 +1652,18 @@ static int crypt_alloc_req_skcipher(struct crypt_config *cc,
>
> skcipher_request_set_tfm(ctx->r.req, cc->cipher_tfm.tfms[key_index]);
>
> + /*
> + * Initialise the heap-allocated scatterlist pointers so that
> + * crypt_free_req_skcipher() does not read uninitialised memory
> + * for paths that don't take the multi-data-unit branch. The
> + * dmreq trailer lives in the per-bio data area which is not
> + * zeroed by the dm core, and the request is reused from the
> + * mempool across many bios.
> + */
> + dmreq = dmreq_of_req(cc, ctx->r.req);
> + dmreq->sg_in_ext = NULL;
> + dmreq->sg_out_ext = NULL;
> +
> /*
> * Use REQ_MAY_BACKLOG so a cipher driver internally backlogs
> * requests if driver request queue is full.
> @@ -1562,6 +1710,12 @@ static void crypt_free_req_skcipher(struct crypt_config *cc,
> struct skcipher_request *req, struct bio *base_bio)
> {
> struct dm_crypt_io *io = dm_per_bio_data(base_bio, cc->per_bio_data_size);
> + struct dm_crypt_request *dmreq = dmreq_of_req(cc, req);
> +
> + kfree(dmreq->sg_in_ext);
> + dmreq->sg_in_ext = NULL;
> + kfree(dmreq->sg_out_ext);
> + dmreq->sg_out_ext = NULL;
>
> if ((struct skcipher_request *)(io + 1) != req)
> mempool_free(req, &cc->req_pool);
> @@ -1590,7 +1744,9 @@ static void crypt_free_req(struct crypt_config *cc, void *req, struct bio *base_
> static blk_status_t crypt_convert(struct crypt_config *cc,
> struct convert_context *ctx, bool atomic, bool reset_pending)
> {
> - unsigned int sector_step = cc->sector_size >> SECTOR_SHIFT;
> + const unsigned int sector_step = cc->sector_size >> SECTOR_SHIFT;
> + const bool multi_du = test_bit(CRYPT_MULTI_DATA_UNIT, &cc->cipher_flags);
> + unsigned int processed;
> int r;
>
> /*
> @@ -1611,8 +1767,13 @@ static blk_status_t crypt_convert(struct crypt_config *cc,
>
> atomic_inc(&ctx->cc_pending);
>
> + processed = cc->sector_size;
> if (crypt_integrity_aead(cc))
> r = crypt_convert_block_aead(cc, ctx, ctx->r.req_aead, ctx->tag_offset);
> + else if (multi_du)
> + r = crypt_convert_block_skcipher_multi(cc, ctx,
> + ctx->r.req,
> + &processed);
> else
> r = crypt_convert_block_skcipher(cc, ctx, ctx->r.req, ctx->tag_offset);
>
> @@ -1634,8 +1795,19 @@ static blk_status_t crypt_convert(struct crypt_config *cc,
> * exit and continue processing in a workqueue
> */
> ctx->r.req = NULL;
> - ctx->tag_offset++;
> - ctx->cc_sector += sector_step;
> + if (!multi_du) {
> + ctx->tag_offset++;
> + ctx->cc_sector += sector_step;
> + } else {
> + bio_advance_iter(ctx->bio_in,
> + &ctx->iter_in,
> + processed);
> + bio_advance_iter(ctx->bio_out,
> + &ctx->iter_out,
> + processed);
> + ctx->cc_sector +=
> + processed >> SECTOR_SHIFT;
> + }
> return BLK_STS_DEV_RESOURCE;
> }
> } else {
> @@ -1649,19 +1821,42 @@ static blk_status_t crypt_convert(struct crypt_config *cc,
> */
> case -EINPROGRESS:
> ctx->r.req = NULL;
> - ctx->tag_offset++;
> - ctx->cc_sector += sector_step;
> + if (!multi_du) {
> + ctx->tag_offset++;
> + ctx->cc_sector += sector_step;
> + } else {
> + bio_advance_iter(ctx->bio_in, &ctx->iter_in,
> + processed);
> + bio_advance_iter(ctx->bio_out, &ctx->iter_out,
> + processed);
> + ctx->cc_sector += processed >> SECTOR_SHIFT;
> + }
> continue;
> /*
> * The request was already processed (synchronously).
> */
> case 0:
> atomic_dec(&ctx->cc_pending);
> - ctx->cc_sector += sector_step;
> - ctx->tag_offset++;
> + if (!multi_du) {
> + ctx->cc_sector += sector_step;
> + ctx->tag_offset++;
> + } else {
> + bio_advance_iter(ctx->bio_in, &ctx->iter_in,
> + processed);
> + bio_advance_iter(ctx->bio_out, &ctx->iter_out,
> + processed);
> + ctx->cc_sector += processed >> SECTOR_SHIFT;
> + }
> if (!atomic)
> cond_resched();
> continue;
> + /*
> + * Out of memory for the multi-DU SG arrays — bounce back
> + * to the caller for requeue rather than failing the bio.
> + */
> + case -ENOMEM:
> + atomic_dec(&ctx->cc_pending);
> + return BLK_STS_DEV_RESOURCE;
> /*
> * There was a data integrity error.
> */
> @@ -3142,6 +3337,45 @@ static int crypt_ctr_cipher(struct dm_target *ti, char *cipher_in, char *key)
> }
> }
>
> + /*
> + * Enable multi-data-unit batching when the cipher supports it and
> + * the IV layout is one we can derive per-DU from a single starting
> + * IV: plain or plain64 produce a sequential 64-bit little-endian
> + * counter, which matches the convention of
> + * crypto_skcipher_set_data_unit_size(). Restrict to the simple
> + * case (single tfm, no integrity, no per-sector post() callback)
> + * to keep the consumer path small; modes like essiv, lmk, tcw,
> + * eboiv, plain64be, random, null, benbi, and elephant are
> + * deliberately excluded because their generators or post-IV hooks
> + * cannot be re-derived by the cipher between data units.
> + */
> + if (!crypt_integrity_aead(cc) && cc->tfms_count == 1 &&
> + cc->iv_gen_ops &&
> + (cc->iv_gen_ops == &crypt_iv_plain_ops ||
> + cc->iv_gen_ops == &crypt_iv_plain64_ops) &&
> + !cc->iv_gen_ops->post &&
> + !cc->integrity_tag_size && !cc->integrity_iv_size &&
> + crypto_skcipher_supports_multi_data_unit(cc->cipher_tfm.tfms[0])) {
> + ret = crypto_skcipher_set_data_unit_size(cc->cipher_tfm.tfms[0],
> + cc->sector_size);
> + if (!ret) {
> + set_bit(CRYPT_MULTI_DATA_UNIT, &cc->cipher_flags);
> + DMINFO("Using multi-data-unit crypto offload (du=%u)",
> + cc->sector_size);
> + } else {
> + /*
> + * The driver advertised the capability via cra_flags
> + * but rejected the requested data unit size. This is
> + * a driver bug worth seeing in dmesg; fall back to
> + * the per-sector path so the device still activates.
> + */
> + DMWARN_LIMIT("multi-DU offload disabled: %s rejected du=%u (%d)",
> + crypto_skcipher_driver_name(cc->cipher_tfm.tfms[0]),
> + cc->sector_size, ret);
> + ret = 0;
> + }
> + }
> +
> /* wipe the kernel key payload copy */
> if (cc->key_string)
> memset(cc->key, 0, cc->key_size * sizeof(u8));
> --
> 2.47.3
>
^ permalink raw reply
* Re: blktests failures with v7.1-rc1 kernel
From: Nilay Shroff @ 2026-05-25 12:44 UTC (permalink / raw)
To: Shin'ichiro Kawasaki, linux-block@vger.kernel.org,
linux-nvme@lists.infradead.org, linux-scsi@vger.kernel.org, nbd,
linux-rdma
In-Reply-To: <afB5syZbUrppgsDQ@shinmob>
hi Shinichiro,
On 4/28/26 2:43 PM, Shin'ichiro Kawasaki wrote:
> Hi all,
>
> I ran the latest blktests (git hash: ea5472c1adc8) with the v7.1-rc1 kernel. I
> observed 8 failures listed below. Comparing with the previous report for the
> v7.0 kernel [1], 2 failures are new (nvme/045, scsi/002). Your actions for fix
> will be welcomed as always.
>
> [1]https://lore.kernel.org/linux-block/aeCDXI5hY_ivSWm4@shinmob/
>
>
> List of failures
> ================
> #1: nvme/005,063 (tcp transport)
> #2: nvme/045 (new)(kmemleak)
> #3: nvme/058 (fc transport)(hang)(kmemleak)
> #4: nvme/060
> #5: nvme/061 (rdma transport, siw driver)(kmemleak)
> #6: nvme/061 (fc transport)
> #7: nbd/002
> #8: scsi/002 (new)
>
>
> Failure description
> ===================
>
> #1: nvme/005,063 (tcp transport)
>
> The test cases nvme/005 and 063 fail for tcp transport due to the lockdep
> WARN related to the three locks q->q_usage_counter, q->elevator_lock and
> set->srcu. The failure was reported first time for nvme/063 and v6.16-rc1
> kernel [2].
>
> Chaitanya provided a fix patch (thanks!), and it is queued for v7.1-rcX tags
> [3]. However, nvme/005 and 063 still fail even when I apply the fix patch to
> v7.1-rc1 kernel. The call traces of the lockdep WARN are different between
> "v7.1-rc1" kernel [4] and "v7.1-rc1+the fix patch" kernel [5]. I guess that
> there exist two lockdep problems with similar symptoms and patch [3] fixed
> one of them. I guess that still one problem is left.
>
> [2]https://lore.kernel.org/linux-block/4fdm37so3o4xricdgfosgmohn63aa7wj3ua4e5vpihoamwg3ui@fq42f5q5t5ic/
> [3]https://lore.kernel.org/all/20260413171628.6204-1-kch@nvidia.com/
I looked into this lockdep warning, and it seems that Chaitanya's patch indeed fixes the
original issue reported in [4]. However, the new warning reported in [5] appears to be a
separate lockdep splat and, from what I can tell, likely a false positive. There are two
reasons why I think so:
1. The lockdep report suggests that thread #1 is sending data over a TCP socket while
another thread #2 is still in the process of establishing that same socket connection.
In practice, this should not be possible because request dispatch over the socket can
only happen after the connection setup has completed successfully.
2. The warning also suggests that while thread #0 is deleting the gendisk and unregistering
the corresponding request queue, another thread #5 is concurrently attempting to change
the queue elevator. However, once gendisk deletion starts, elevator switching is already
inhibited for that queue (see disable_elv_switch()), so the reported locking scenario
should not be reachable in practice.
Based on the above, I suspect this is a lockdep false positive caused by dependency tracking
across different queue/socket lifecycle phases. We may need to suppress lock dependency tracking
in some of these paths to avoid the false warning.
Thanks,
--Nilay
^ 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