* Re: [PATCH] block: Fix blk_sync_queue() to properly stop timeout timer
From: Casey Chen @ 2026-06-09 23:11 UTC (permalink / raw)
To: Mohamed Khalfella
Cc: Keith Busch, James Smart, Jens Axboe, Christoph Hellwig,
Sagi Grimberg, linux-nvme, linux-kernel, linux-block,
Yuanyuan Zhong, Michael Liang, Randy Jennings
In-Reply-To: <20250620183419.GC4836-mkhalfella@purestorage.com>
On 2025-06-20 11:34:19 -0700, Mohamed Khalfella wrote:
> Following up on this patch. I think the issue with blk_sync_queue()
> needs to be addressed. If adding a queue flag is not the preferred way
> to do it, please let me know what do you suggest and I will make the
> code changes.
I'd like to add a second data point in support of this patch. We hit
the same blk_sync_queue() race in nvme-rdma during torture testing of
an nvme-multipath/dm-mpath stacked configuration, with a call pattern
distinct from the nvme-fc case Mohamed reported. The symptom on our
side is request double-completion leading to dm-rq clone double-free,
not the self-deadlock Mohamed observed; but the root cause is the
same q->timeout rearm window inside blk_sync_queue().
Importantly, nvme-rdma's blk_sync_queue() call happens from a
separate workqueue (nvme_rdma_error_recovery_work), NOT from inside
blk_mq_timeout_work. So Keith's earlier suggestion that the consumer
"just not sync the queues within the timeout workqueue context" --
which addresses nvme-fc's self-deadlock -- does not apply to us:
we already are outside that context, and the race still manifests
because blk_sync_queue() itself doesn't fence q->timeout_work
against post-return rearm. The fix has to be in blk_sync_queue().
Setup:
- Linux 6.6.77, ~64 CPUs, ~50 nvme-rdma controllers, nvme-mpath
paths each carrying a dm-mpath clone in flight.
- "Stall an SPM" torture: target-side faults inducing per-controller
nvme_rdma_error_recovery_work in parallel across many ctrls.
Call pattern:
blk_mq_timeout_work (per-queue, kblockd)
... nvme_rdma_timeout(rq) -- ctrl state != NVME_CTRL_LIVE
path A -> nvme_rdma_complete_timed_out(rq)
blk_mq_complete_request(rq) <- forced completion A
nvme_rdma_error_recovery_work (per-ctrl, separate workqueue)
nvme_rdma_teardown_io_queues
nvme_quiesce_io_queues
nvme_sync_io_queues <- calls blk_sync_queue per ns->queue
nvme_rdma_stop_io_queues
nvme_rdma_cancel_tagset
blk_mq_tagset_busy_iter(nvme_cancel_request)
nvme_cancel_request(rq)
blk_mq_complete_request(rq) <- forced completion B
We instrumented the kernel to capture per-request event histories
(stack + cpu + timestamp) at each forced-completion entry point, at
blk_mq_complete_request() entry on the orig, and at
multipath_release_clone(). In an unpatched 6.6 we captured the
following sequence on the same nvme-rdma clone within ~5 ms, which
walks step-by-step from the two racing producers through the
double-completion of the orig to the double-free of the clone.
Records below are verbatim from the kernel log:
rq=ffff889589967d00 tag=10 qid=16 ctrl=nvme33 op=DRV_IN
(1) Timeout (T) and cancel (C) reach forced-completion entry within
~1 us, on different CPUs (note 'age=4751us' vs 'age=4750us'):
by-source[2] source=T age=4751us cpu=8 op=DRV_IN nr_bytes=32768 sts=0 ctx=ffff889589967d00 stack(10):
[0] nvme_rdma_timeout+0x58/0x110 [nvme_rdma]
[1] blk_mq_handle_expired+0x5d/0x90
[2] bt_iter+0x7e/0x90
[3] blk_mq_queue_tag_busy_iter+0x2b3/0x590
[4] blk_mq_timeout_work+0x15f/0x1b0
[5] process_one_work+0x133/0x380
[6] worker_thread+0x30d/0x420
[7] kthread+0xe5/0x110
[8] ret_from_fork+0x2d/0x50
[9] ret_from_fork_asm+0x11/0x20
by-source[1] source=C age=4750us cpu=14 op=DRV_IN nr_bytes=32768 sts=0 ctx=ffff889589967d00 stack(10):
[0] nvme_cancel_request+0x52/0xb0 [nvme_core]
[1] bt_tags_iter+0x4b/0xa0
[2] blk_mq_tagset_busy_iter+0x186/0x2c0
[3] nvme_rdma_teardown_io_queues.part.0+0x73/0xf0 [nvme_rdma]
[4] nvme_rdma_error_recovery_work+0x41/0xe0 [nvme_rdma]
[5] process_one_work+0x133/0x380
[6] worker_thread+0x30d/0x420
[7] kthread+0xe5/0x110
[8] ret_from_fork+0x2d/0x50
[9] ret_from_fork_asm+0x11/0x20
(The dump CPU is C8; cpu=N inside each record is the producing
CPU. T came from cpu=8 with stack rooted at blk_mq_timeout_work;
C came from cpu=14 with stack rooted at
nvme_rdma_error_recovery_work. Two independent producers, no
re-entry, captured ~1us apart on the same rq.)
(2) Each chain proceeds through dm-rq's clone-end-io to call
blk_mq_complete_request() on the dm-mpath orig that owns this
clone via tio->clone. Our hook at blk_mq_complete_request()
captured *two* invocations on the same orig, one from each
upstream forced-completion chain (both age=4743us):
by-source[6] source=B age=4743us cpu=14 op=DRV_IN nr_bytes=32768 sts=0 ctx=ffff889589967d00 stack(14):
[0] nvme_rdma_dbg_blk_complete_hook+0xcd/0x130 [nvme_rdma]
[1] blk_mq_complete_request+0x17/0x70
[2] end_clone_request+0x1f/0x30
[3] __blk_mq_end_request+0x60/0x150
[4] nvme_cancel_request+0x5a/0xb0 [nvme_core]
[5] bt_tags_iter+0x4b/0xa0
[6] blk_mq_tagset_busy_iter+0x186/0x2c0
[7] nvme_rdma_teardown_io_queues.part.0+0x73/0xf0 [nvme_rdma]
[8] nvme_rdma_error_recovery_work+0x41/0xe0 [nvme_rdma]
[9] process_one_work+0x133/0x380
[10] worker_thread+0x30d/0x420
[11] kthread+0xe5/0x110
[12] ret_from_fork+0x2d/0x50
[13] ret_from_fork_asm+0x11/0x20
ring[4] source=B age=4743us cpu=8 op=DRV_IN nr_bytes=32768 sts=0 ctx=ffff889589967d00 stack(14):
[0] nvme_rdma_dbg_blk_complete_hook+0xcd/0x130 [nvme_rdma]
[1] blk_mq_complete_request+0x17/0x70
[2] end_clone_request+0x1f/0x30
[3] __blk_mq_end_request+0x60/0x150
[4] nvme_rdma_timeout+0xff/0x110 [nvme_rdma]
[5] blk_mq_handle_expired+0x5d/0x90
[6] bt_iter+0x7e/0x90
[7] blk_mq_queue_tag_busy_iter+0x2b3/0x590
[8] blk_mq_timeout_work+0x15f/0x1b0
(frames [9..13] truncated in our log dump by interleaved console
output during the dump; the lower frames are the standard
process_one_work/worker_thread/kthread tail.)
Two B records on the same orig at the same age=4743us, with two
*different* upstream chains (frame [4] is nvme_cancel_request in
the first, nvme_rdma_timeout in the second), is the direct
observation that the orig has been double-completed.
(3) Each B schedules a per-cpu BLOCK_SOFTIRQ for the orig on its
mq_ctx->cpu. The first softirq drain runs dm_softirq_done ->
dm_done -> dm_requeue_original_request -> release_clone_rq,
which releases this clone via multipath_release_clone ->
blk_mq_free_request. req_ref_put_and_test sees refcount=1,
dec to 0, free runs; "R" source captures this (age=4736us,
7us after the two B's):
by-source[5] source=R age=4736us cpu=8 op=DRV_IN nr_bytes=0 sts=0 ctx=ffff889589967d00 stack(10):
[0] nvme_rdma_dbg_release_clone_hook+0x3f/0x120 [nvme_rdma]
[1] multipath_release_clone+0x1f/0x60
[2] dm_requeue_original_request+0x61/0xd0
[3] blk_complete_reqs+0x3a/0x50
[4] handle_softirqs+0x1ee/0x4a0
[5] run_ksoftirqd+0x3c/0x70
[6] smpboot_thread_fn+0xdf/0x1a0
[7] kthread+0xe5/0x110
[8] ret_from_fork+0x2d/0x50
[9] ret_from_fork_asm+0x11/0x20
(4) The second softirq drain on the same orig (queued by the other
chain's blk_mq_complete_request) runs the same path. tio->clone
still points to the just-freed clone (dm-rq does not clear it),
so dm_done -> release_clone_rq -> blk_mq_free_request is called
again. This is the double-free: refcount is already 0 entering
req_ref_put_and_test, which trips the WARN_ON_ONCE:
[ 7122.320733] ------------[ cut here ]------------
[ 7122.320742] WARNING: CPU: 3 PID: 77382 at block/blk.h:508 \
blk_mq_free_request+0x64/0x90
[ 7122.320811] CPU: 3 PID: 77382 Comm: foed
[ 7122.320821] RIP: 0010:blk_mq_free_request+0x64/0x90
[ 7122.320942] Call Trace:
[ 7122.320952] ? __warn+0xa3/0x1d0
[ 7122.320956] ? blk_mq_free_request+0x64/0x90
[ 7122.320962] ? report_bug+0x200/0x230
[ 7122.320966] ? handle_bug+0x57/0x90
[ 7122.320984] dm_requeue_original_request+0x61/0xd0
[ 7122.320989] blk_complete_reqs+0x3a/0x50
[ 7122.320993] handle_softirqs+0x1ee/0x4a0
[ 7122.321003] __irq_exit_rcu+0x79/0xa0
[ 7122.321007] irq_exit_rcu+0xa/0x20
[ 7122.321011] common_interrupt+0x43/0xa0
[ 7122.321014] asm_common_interrupt+0x22/0x40
[ 7122.321050] ---[ end trace 0000000000000000 ]---
The premise for the race is that nvme_rdma_teardown_io_queues calls
nvme_quiesce_io_queues -> nvme_sync_io_queues BEFORE
nvme_rdma_cancel_tagset, so after sync returns we expect no
blk_mq_timeout_work can fire concurrent with cancel_tagset. With the
unpatched blk_sync_queue(), that expectation breaks:
nvme_rdma_timeout() returning BLK_EH_RESET_TIMER causes a per-request
blk_add_timer() to re-arm q->timeout from inside the timeout work
that blk_sync_queue is in the middle of draining. After sync's
del_timer_sync (which ran first in the old ordering) and
cancel_work_sync return, q->timeout is once again armed. It fires
shortly afterwards, queueing a new blk_mq_timeout_work that races
nvme_rdma_cancel_tagset on the same rqs.
The race also corrupts wire ordering, since the clone is released
by dm-rq while its SEND/RECV WRs are still outstanding (the forced
completions bypassed end_request's refcount drain gate). A late
recv CQE then lands on the freed slot, dereferences NULL mq_hctx
inside blk_mq_complete_request_remote, and panics the kernel:
[ 3079.146243] BUG: kernel NULL pointer dereference, address: 0000000000000136
[ 3079.150802] #PF: supervisor read access in kernel mode
[ 3079.150811] PGD 0 P4D 0
[ 3079.150813] Oops: 0000 [#1] SMP
[ 3079.150819] CPU: 1 PID: 10069 Comm: foed
[ 3079.150834] RIP: 0010:blk_mq_complete_request_remote+0x15/0x130
[ 3079.155506] CR2: 0000000000000136
[ 3079.155516] Call Trace:
[ 3079.166933] ? blk_mq_complete_request_remote+0x15/0x130
[ 3079.166953] ? nvme_should_fail+0x3e/0x90 [nvme_core]
[ 3079.166958] nvme_rdma_end_request+0x55/0x80 [nvme_rdma]
[ 3079.166962] nvme_rdma_recv_done+0x1a4/0x5e0 [nvme_rdma]
[ 3079.185939] __ib_process_cq+0x79/0x150 [ib_core]
[ 3079.193425] ib_poll_handler+0x2c/0xb0 [ib_core]
(CR2=0x136 = offsetof(struct blk_mq_hw_ctx, nr_ctx); the deref is
rq->mq_hctx->nr_ctx with rq->mq_hctx already nulled by
__blk_mq_free_request from the racy double release above.)
We backported your patch to 6.6 (QUEUE_FLAG_NOTIMEOUT is still in
the #define-with-slot-number style there; allocated slot 2). With
the patch applied, both upstream signals went silent across
multiple-hour torture runs:
- WARN_ON_ONCE at block/blk.h:508 (double-free): 0 occurrences.
- NULL deref panic at blk_mq_complete_request_remote+0x15 (stale
CQE on freed slot): 0 occurrences.
In response to Keith's earlier comment about this being an nvme-fc
problem:
- The deadlock you described (sync called from within timeout work)
is specific to nvme-fc's call pattern, agreed.
- The race in blk_sync_queue() is not. nvme-rdma does not sync from
within timeout work; sync is called from nvme_rdma_error_recovery_work
(a separate workqueue) under the assumption that after sync returns
no further timeout work for that queue can run. That assumption is
what blk_sync_queue's name and existing callers imply, and what
Mohamed's patch makes true.
- Any driver returning BLK_EH_RESET_TIMER from .timeout and having a
teardown path that calls blk_sync_queue is subject to the same
race. nvme-fc and nvme-rdma both qualify; we expect nvme-tcp does
too (haven't reproduced there yet).
A driver-local fix for each consumer doesn't address the contract:
blk_sync_queue() should provide what its name says it provides.
Tested-by: Casey Chen <cachen@purestorage.com>
Casey
^ permalink raw reply
* Re: [PATCH 21/27] null_blk: Enable lock context analysis
From: Damien Le Moal @ 2026-06-09 23:42 UTC (permalink / raw)
To: Bart Van Assche, Jens Axboe
Cc: linux-block, Christoph Hellwig, Marco Elver, Keith Busch,
Chaitanya Kulkarni, Johannes Thumshirn, Nilay Shroff,
Genjian Zhang, Kees Cook
In-Reply-To: <b70422c6f243b9275e336430071b52eb92dd40c4.1781042470.git.bvanassche@acm.org>
On 2026/06/10 6:05, Bart Van Assche wrote:
> Add __must_hold() annotations where these are missing. Annotate two
> functions that use conditional locking with __context_unsafe(). Enable lock
> context analysis in the Makefile.
>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Looks OK to me.
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply
* Re: [PATCH 23/27] ublk: Enable lock context analysis
From: Ming Lei @ 2026-06-10 2:24 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Marco Elver,
Nathan Chancellor
In-Reply-To: <0a98c4c6a776f88e8e300ef9b205c190a3857609.1781042470.git.bvanassche@acm.org>
On Tue, Jun 09, 2026 at 03:05:10PM -0700, Bart Van Assche wrote:
> Add the lock context annotations that are required by Clang.
>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Ming Lei <tom.leiming@gmail.com>
Thanks,
Ming
^ permalink raw reply
* [linus:master] [block] 3179a5f7f8: blktests.loop/010.fail
From: kernel test robot @ 2026-06-10 2:57 UTC (permalink / raw)
To: Li Chen
Cc: oe-lkp, lkp, linux-kernel, Jens Axboe, Chaitanya Kulkarni,
Bart Van Assche, linux-block, oliver.sang
Hello,
kernel test robot noticed "blktests.loop/010.fail" on:
commit: 3179a5f7f86bcc3acd5d6fb2a29f891ef5615852 ("block: rate-limit capacity change info log")
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master
[test failed on linus/master c10130c234c81f4a7a143edbf413080235f8d8ce]
[test failed on linux-next/master 6e845bcb78c95af935094040bd4edc3c2b6dd784]
in testcase: blktests
version: blktests-x86_64-9131687-1_20260529
with following parameters:
test: loop-010
config: x86_64-rhel-9.4-func
compiler: gcc-14
test machine: 16 threads Intel(R) Core(TM) i7-13620H (Raptor Lake) with 32G memory
(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202606100416.99599c18-lkp@intel.com
2026-06-05 21:40:15 echo loop/010
2026-06-05 21:40:15 ./check loop/010
loop/010 (check stale loop partition)
loop/010 (check stale loop partition) [failed]
runtime ... 92.093s
something found in dmesg:
[ 59.348633] [ T1202] run blktests loop/010 at 2026-06-05 21:40:15
[ 59.382173] [ T1223] loop0: detected capacity change from 0 to 2097152
[ 59.382922] [ T1005] loop0:
[ 59.385325] [ T1223] loop0:
[ 59.417927] [ T274] loop0: p1
[ 59.793846] [ T1267] loop0: detected capacity change from 0 to 2097152
[ 59.794835] [ T1268] loop0: p1
[ 59.796265] [ T1267] loop0: p1
[ 59.802753] [ T1272] loop0: detected capacity change from 0 to 2097152
[ 59.803800] [ T1272] loop0: p1
...
(See '/lkp/benchmarks/blktests/results/nodev/loop/010.dmesg' for the entire message)
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20260610/202606100416.99599c18-lkp@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* configurable block error injection v4
From: Christoph Hellwig @ 2026-06-10 5:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Jonathan Corbet, Damien Le Moal, Hannes Reinecke, Keith Busch,
linux-block, linux-doc
Hi all,
this series adds a new configurable block error injection facility.
We already have a few to inject block errors, but unfortunately most
of them are either not very useful or hard to use, or both:
- The fail_make_request failure injection point can't distinguish
different commands, different ranges in the file and can only injection
plain I/O errors.
- the should_fail_bio 'dynamic' failure injection has all the same issues
as fail_make_request
- dm-error can only fail all command in the table using BLK_STS_IOERR
and requires setting up a new block device
- dm-flakey and dm-dust allow all kinds of configurability, but still
don't have good error selection, no good support for non-read/write
commands and are limited to the dm table alignment requirements,
which for zoned devices enforces setting them up for an entire zone.
They also once again require setting up a stacked block device,
which is really annoying in harnesses like xfstests
This series adds a new debugfs-based block layer error injection
that allows to configure what operations and ranges the injection
applied to, and what status to return. It also allows to configure a
failure ratio similar to the xfs errortag injection.
Changes since v3:
- use a static branch to guard the new condition
- split out a new header so that jump_label.h doesn't get pulled into
blk.h
- more checking for impossible conditions in blk_status_to_tag
- more spelling fixes
Changes since v2:
- improve the documentation a bit
- fix a spelling mistake in a comment
Changes since v1:
- drop the should_fail_bio removal and cleanup depending on it, as it's
used by eBPF programs and thus a hidden UABI.
- as a result split the code out to it's own Kconfig symbol
- various error handling fixed pointed out by Keith
- documentation spelling fixes pointed out by Randy
Diffstat:
Documentation/block/error-injection.rst | 59 ++++++
Documentation/block/index.rst | 1
block/Kconfig | 8
block/Makefile | 1
block/blk-core.c | 87 ++++++--
block/blk-sysfs.c | 5
block/blk.h | 3
block/error-injection.c | 314 ++++++++++++++++++++++++++++++++
block/error-injection.h | 21 ++
block/genhd.c | 4
include/linux/blkdev.h | 6
11 files changed, 489 insertions(+), 20 deletions(-)
^ permalink raw reply
* [PATCH 1/4] block: add a macro to initialize the status table
From: Christoph Hellwig @ 2026-06-10 5:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Jonathan Corbet, Damien Le Moal, Hannes Reinecke, Keith Busch,
linux-block, linux-doc, Bart Van Assche, Hannes Reinecke
In-Reply-To: <20260610051015.1906799-1-hch@lst.de>
Prepare for adding a new value to the error table by adding a macro
to fill it.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
---
block/blk-core.c | 45 +++++++++++++++++++++++++--------------------
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 1c637db79e59..43121a9f99f0 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -132,39 +132,44 @@ inline const char *blk_op_str(enum req_op op)
}
EXPORT_SYMBOL_GPL(blk_op_str);
+#define ENT(_tag, _errno, _desc) \
+[BLK_STS_##_tag] = { \
+ .errno = _errno, \
+ .name = _desc, \
+}
static const struct {
int errno;
const char *name;
} blk_errors[] = {
- [BLK_STS_OK] = { 0, "" },
- [BLK_STS_NOTSUPP] = { -EOPNOTSUPP, "operation not supported" },
- [BLK_STS_TIMEOUT] = { -ETIMEDOUT, "timeout" },
- [BLK_STS_NOSPC] = { -ENOSPC, "critical space allocation" },
- [BLK_STS_TRANSPORT] = { -ENOLINK, "recoverable transport" },
- [BLK_STS_TARGET] = { -EREMOTEIO, "critical target" },
- [BLK_STS_RESV_CONFLICT] = { -EBADE, "reservation conflict" },
- [BLK_STS_MEDIUM] = { -ENODATA, "critical medium" },
- [BLK_STS_PROTECTION] = { -EILSEQ, "protection" },
- [BLK_STS_RESOURCE] = { -ENOMEM, "kernel resource" },
- [BLK_STS_DEV_RESOURCE] = { -EBUSY, "device resource" },
- [BLK_STS_AGAIN] = { -EAGAIN, "nonblocking retry" },
- [BLK_STS_OFFLINE] = { -ENODEV, "device offline" },
+ ENT(OK, 0, ""),
+ ENT(NOTSUPP, -EOPNOTSUPP, "operation not supported"),
+ ENT(TIMEOUT, -ETIMEDOUT, "timeout"),
+ ENT(NOSPC, -ENOSPC, "critical space allocation"),
+ ENT(TRANSPORT, -ENOLINK, "recoverable transport"),
+ ENT(TARGET, -EREMOTEIO, "critical target"),
+ ENT(RESV_CONFLICT, -EBADE, "reservation conflict"),
+ ENT(MEDIUM, -ENODATA, "critical medium"),
+ ENT(PROTECTION, -EILSEQ, "protection"),
+ ENT(RESOURCE, -ENOMEM, "kernel resource"),
+ ENT(DEV_RESOURCE, -EBUSY, "device resource"),
+ ENT(AGAIN, -EAGAIN, "nonblocking retry"),
+ ENT(OFFLINE, -ENODEV, "device offline"),
/* device mapper special case, should not leak out: */
- [BLK_STS_DM_REQUEUE] = { -EREMCHG, "dm internal retry" },
+ ENT(DM_REQUEUE, -EREMCHG, "dm internal retry"),
/* zone device specific errors */
- [BLK_STS_ZONE_OPEN_RESOURCE] = { -ETOOMANYREFS, "open zones exceeded" },
- [BLK_STS_ZONE_ACTIVE_RESOURCE] = { -EOVERFLOW, "active zones exceeded" },
+ ENT(ZONE_OPEN_RESOURCE, -ETOOMANYREFS, "open zones exceeded"),
+ ENT(ZONE_ACTIVE_RESOURCE, -EOVERFLOW, "active zones exceeded"),
/* Command duration limit device-side timeout */
- [BLK_STS_DURATION_LIMIT] = { -ETIME, "duration limit exceeded" },
-
- [BLK_STS_INVAL] = { -EINVAL, "invalid" },
+ ENT(DURATION_LIMIT, -ETIME, "duration limit exceeded"),
+ ENT(INVAL, -EINVAL, "invalid"),
/* everything else not covered above: */
- [BLK_STS_IOERR] = { -EIO, "I/O" },
+ ENT(IOERR, -EIO, "I/O"),
};
+#undef ENT
blk_status_t errno_to_blk_status(int errno)
{
--
2.53.0
^ permalink raw reply related
* [PATCH 2/4] block: add a "tag" for block status codes
From: Christoph Hellwig @ 2026-06-10 5:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Jonathan Corbet, Damien Le Moal, Hannes Reinecke, Keith Busch,
linux-block, linux-doc, Hannes Reinecke
In-Reply-To: <20260610051015.1906799-1-hch@lst.de>
The full name of the status codes is not good for user interfaces as it
can contain white spaces. Add the name of the status code without the
BLK_STS_ prefix as a tag so that it can be used for user interfaces.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
---
block/blk-core.c | 28 ++++++++++++++++++++++++++++
block/blk.h | 2 ++
2 files changed, 30 insertions(+)
diff --git a/block/blk-core.c b/block/blk-core.c
index 43121a9f99f0..842b5c6f2fb4 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -135,10 +135,12 @@ EXPORT_SYMBOL_GPL(blk_op_str);
#define ENT(_tag, _errno, _desc) \
[BLK_STS_##_tag] = { \
.errno = _errno, \
+ .tag = __stringify(_tag), \
.name = _desc, \
}
static const struct {
int errno;
+ const char *tag;
const char *name;
} blk_errors[] = {
ENT(OK, 0, ""),
@@ -203,6 +205,32 @@ const char *blk_status_to_str(blk_status_t status)
return blk_errors[idx].name;
}
+const char *blk_status_to_tag(blk_status_t status)
+{
+ int idx = (__force int)status;
+
+ if (WARN_ON_ONCE(idx >= ARRAY_SIZE(blk_errors) || !blk_errors[idx].tag))
+ return "<null>";
+ return blk_errors[idx].tag;
+}
+
+blk_status_t tag_to_blk_status(const char *tag)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(blk_errors); i++) {
+ if (blk_errors[i].tag &&
+ !strcmp(blk_errors[i].tag, tag))
+ return (__force blk_status_t)i;
+ }
+
+ /*
+ * Return BLK_STS_OK for mismatches as this function is intended to
+ * parse error status values.
+ */
+ return BLK_STS_OK;
+}
+
/**
* blk_sync_queue - cancel any pending callbacks on a queue
* @q: the queue
diff --git a/block/blk.h b/block/blk.h
index 7fdfb9012ce1..3ab2cdd6ed12 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -51,6 +51,8 @@ struct blk_flush_queue *blk_alloc_flush_queue(int node, int cmd_size,
void blk_free_flush_queue(struct blk_flush_queue *q);
const char *blk_status_to_str(blk_status_t status);
+const char *blk_status_to_tag(blk_status_t status);
+blk_status_t tag_to_blk_status(const char *tag);
bool __blk_mq_unfreeze_queue(struct request_queue *q, bool force_atomic);
bool blk_queue_start_drain(struct request_queue *q);
--
2.53.0
^ permalink raw reply related
* [PATCH 3/4] block: add a str_to_blk_op helper
From: Christoph Hellwig @ 2026-06-10 5:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Jonathan Corbet, Damien Le Moal, Hannes Reinecke, Keith Busch,
linux-block, linux-doc, Hannes Reinecke
In-Reply-To: <20260610051015.1906799-1-hch@lst.de>
Add a helper to find the REQ_OP_XYZ constant from the "XYZ" string.
This will be used for the error injection debugfs interface.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Keith Busch <kbusch@kernel.org>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
---
block/blk-core.c | 10 ++++++++++
block/blk.h | 1 +
2 files changed, 11 insertions(+)
diff --git a/block/blk-core.c b/block/blk-core.c
index 842b5c6f2fb4..beaab7a71fba 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -132,6 +132,16 @@ inline const char *blk_op_str(enum req_op op)
}
EXPORT_SYMBOL_GPL(blk_op_str);
+enum req_op str_to_blk_op(const char *op)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(blk_op_name); i++)
+ if (blk_op_name[i] && !strcmp(blk_op_name[i], op))
+ return (enum req_op)i;
+ return REQ_OP_LAST;
+}
+
#define ENT(_tag, _errno, _desc) \
[BLK_STS_##_tag] = { \
.errno = _errno, \
diff --git a/block/blk.h b/block/blk.h
index 3ab2cdd6ed12..507ab34a6e90 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -53,6 +53,7 @@ void blk_free_flush_queue(struct blk_flush_queue *q);
const char *blk_status_to_str(blk_status_t status);
const char *blk_status_to_tag(blk_status_t status);
blk_status_t tag_to_blk_status(const char *tag);
+enum req_op str_to_blk_op(const char *op);
bool __blk_mq_unfreeze_queue(struct request_queue *q, bool force_atomic);
bool blk_queue_start_drain(struct request_queue *q);
--
2.53.0
^ permalink raw reply related
* [PATCH 4/4] block: add configurable error injection
From: Christoph Hellwig @ 2026-06-10 5:08 UTC (permalink / raw)
To: Jens Axboe
Cc: Jonathan Corbet, Damien Le Moal, Hannes Reinecke, Keith Busch,
linux-block, linux-doc, Hannes Reinecke
In-Reply-To: <20260610051015.1906799-1-hch@lst.de>
Add a new block error injection interface that allows to inject specific
status code for specific ranges.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
---
Documentation/block/error-injection.rst | 59 +++++
Documentation/block/index.rst | 1 +
block/Kconfig | 8 +
block/Makefile | 1 +
block/blk-core.c | 4 +
block/blk-sysfs.c | 5 +
block/error-injection.c | 314 ++++++++++++++++++++++++
block/error-injection.h | 21 ++
block/genhd.c | 4 +
include/linux/blkdev.h | 6 +
10 files changed, 423 insertions(+)
create mode 100644 Documentation/block/error-injection.rst
create mode 100644 block/error-injection.c
create mode 100644 block/error-injection.h
diff --git a/Documentation/block/error-injection.rst b/Documentation/block/error-injection.rst
new file mode 100644
index 000000000000..81f31af82e65
--- /dev/null
+++ b/Documentation/block/error-injection.rst
@@ -0,0 +1,59 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============================
+Configurable Error Injection
+============================
+
+Overview
+--------
+
+Configurable error injection allows injecting specific block layer status codes
+for sector ranges of a block device. Errors can be injected unconditionally, or
+with a given probability.
+
+To use configurable error injection, CONFIG_BLK_ERROR_INJECTION must be enabled.
+
+The only interface is the error_injection debugfs file, which is created for
+each registered gendisk. Writes to this file are used to create or delete rules
+and reads return a list of the current error injection sites.
+
+Options
+-------
+
+The following options specify the operations:
+
+=================== =======================================================
+add add a new rule
+removeall remove all existing rules
+=================== =======================================================
+
+The following options specify the details of the rule for the add operation:
+
+=================== =======================================================
+op=<string> block layer operation this rule applies to. This uses
+ the XYZ for each REQ_OP_XYZ operation, e.g. READ, WRITE
+ or DISCARD. Mandatory.
+status=<string> Status to return. This uses XYZ for each BLK_STS_XYZ
+ code, e.g. IOERR or MEDIUM. Mandatory.
+start=<number> First block layer sector the rule applies to.
+ Optional, defaults to 0.
+nr_sectors=<number> Number of sectors this rule applies.
+ Optional, defaults to the remainder of the device.
+chance=<number> Only return a failure with a likelihood of 1/chance.
+ Optional, defaults to 1 (always).
+=================== =======================================================
+
+Example
+-------
+
+Return BLK_STS_IOERR for one in 10 reads of sector 0 of /dev/nvme0n1:
+
+ $ echo 'add,op=READ,start=0,status=IOERR,chance=10' > /sys/kernel/debug/block/nvme0n1/error_injection
+
+Return BLK_STS_MEDIUM for every write to /dev/nvme0n1:
+
+ $ echo 'add,op=WRITE,start=0,status=MEDIUM' > /sys/kernel/debug/block/nvme0n1/error_injection
+
+Remove all rules for /dev/nvme0n1:
+
+ $ echo 'removeall' > /sys/kernel/debug/block/nvme0n1/error_injection
diff --git a/Documentation/block/index.rst b/Documentation/block/index.rst
index 9fea696f9daa..bfa1bbd31ddf 100644
--- a/Documentation/block/index.rst
+++ b/Documentation/block/index.rst
@@ -22,3 +22,4 @@ Block
switching-sched
writeback_cache_control
ublk
+ error-injection
diff --git a/block/Kconfig b/block/Kconfig
index 15027963472d..70e4a66d941f 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -221,6 +221,14 @@ config BLOCK_HOLDER_DEPRECATED
config BLK_MQ_STACKING
bool
+config BLK_ERROR_INJECTION
+ bool "Enable block layer error injection"
+ select JUMP_LABEL if HAVE_ARCH_JUMP_LABEL
+ help
+ Enable inserting arbitrary block errors through a debugfs interface.
+
+ See Documentation/block/error-injection.rst for details.
+
source "block/Kconfig.iosched"
endif # BLOCK
diff --git a/block/Makefile b/block/Makefile
index 54130faacc21..e7bd320e3d69 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -13,6 +13,7 @@ obj-y := bdev.o fops.o bio.o elevator.o blk-core.o blk-sysfs.o \
genhd.o ioprio.o badblocks.o partitions/ blk-rq-qos.o \
disk-events.o blk-ia-ranges.o early-lookup.o
+obj-$(CONFIG_BLK_ERROR_INJECTION) += error-injection.o
obj-$(CONFIG_BLK_DEV_BSG_COMMON) += bsg.o
obj-$(CONFIG_BLK_DEV_BSGLIB) += bsg-lib.o
obj-$(CONFIG_BLK_CGROUP) += blk-cgroup.o
diff --git a/block/blk-core.c b/block/blk-core.c
index beaab7a71fba..73a41df98c9a 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -50,6 +50,7 @@
#include "blk-cgroup.h"
#include "blk-throttle.h"
#include "blk-ioprio.h"
+#include "error-injection.h"
struct dentry *blk_debugfs_root;
@@ -767,6 +768,9 @@ static void __submit_bio_noacct_mq(struct bio *bio)
void submit_bio_noacct_nocheck(struct bio *bio, bool split)
{
+ if (unlikely(blk_error_inject(bio)))
+ return;
+
blk_cgroup_bio_start(bio);
if (!bio_flagged(bio, BIO_TRACE_COMPLETION)) {
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index f22c1f253eb3..520972676ab4 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -19,6 +19,7 @@
#include "blk-wbt.h"
#include "blk-cgroup.h"
#include "blk-throttle.h"
+#include "error-injection.h"
struct queue_sysfs_entry {
struct attribute attr;
@@ -933,6 +934,8 @@ static void blk_debugfs_remove(struct gendisk *disk)
blk_debugfs_lock_nomemsave(q);
blk_trace_shutdown(q);
+ if (IS_ENABLED(CONFIG_BLK_ERROR_INJECTION))
+ blk_error_injection_exit(disk);
debugfs_remove_recursive(q->debugfs_dir);
q->debugfs_dir = NULL;
q->sched_debugfs_dir = NULL;
@@ -963,6 +966,8 @@ int blk_register_queue(struct gendisk *disk)
memflags = blk_debugfs_lock(q);
q->debugfs_dir = debugfs_create_dir(disk->disk_name, blk_debugfs_root);
+ if (IS_ENABLED(CONFIG_BLK_ERROR_INJECTION))
+ blk_error_injection_init(disk);
if (queue_is_mq(q))
blk_mq_debugfs_register(q);
blk_debugfs_unlock(q, memflags);
diff --git a/block/error-injection.c b/block/error-injection.c
new file mode 100644
index 000000000000..7f7f0d3327bc
--- /dev/null
+++ b/block/error-injection.c
@@ -0,0 +1,314 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2026 Christoph Hellwig.
+ */
+#include <linux/debugfs.h>
+#include <linux/blkdev.h>
+#include <linux/parser.h>
+#include <linux/seq_file.h>
+#include "blk.h"
+#include "error-injection.h"
+
+struct blk_error_inject {
+ struct list_head entry;
+ sector_t start;
+ sector_t end;
+ enum req_op op;
+ blk_status_t status;
+
+ /* only inject every 1 / chance times */
+ unsigned int chance;
+};
+
+DEFINE_STATIC_KEY_FALSE(blk_error_injection_enabled);
+
+bool __blk_error_inject(struct bio *bio)
+{
+ struct gendisk *disk = bio->bi_bdev->bd_disk;
+ struct blk_error_inject *inj;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(inj, &disk->error_injection_list, entry) {
+ if (bio->bi_iter.bi_sector <= inj->end &&
+ bio_end_sector(bio) > inj->start &&
+ bio_op(bio) == inj->op) {
+ blk_status_t status = inj->status;
+
+ if (inj->chance > 1 &&
+ (get_random_u32() % inj->chance) != 0)
+ continue;
+
+ pr_info_ratelimited("%pg: injecting %s error for %s at sector %llu:%u\n",
+ disk->part0,
+ blk_status_to_str(status),
+ blk_op_str(inj->op),
+ bio->bi_iter.bi_sector,
+ bio_sectors(bio));
+ rcu_read_unlock();
+ bio_endio_status(bio, status);
+ return true;
+ }
+ }
+ rcu_read_unlock();
+ return false;
+}
+
+static int error_inject_add(struct gendisk *disk, enum req_op op,
+ sector_t start, u64 nr_sectors, blk_status_t status,
+ unsigned int chance)
+{
+ struct blk_error_inject *inj;
+ int error = -EINVAL;
+
+ if (op == REQ_OP_LAST)
+ return -EINVAL;
+ if (status == BLK_STS_OK)
+ return -EINVAL;
+
+ inj = kzalloc_obj(*inj);
+ if (!inj)
+ return -ENOMEM;
+
+ if (nr_sectors) {
+ if (U64_MAX - nr_sectors < start)
+ goto out_free_inj;
+ inj->end = start + nr_sectors - 1;
+ } else {
+ inj->end = U64_MAX;
+ }
+
+ inj->op = op;
+ inj->start = start;
+ inj->status = status;
+ inj->chance = chance;
+
+ pr_debug_ratelimited("%pg: adding %s injection for %s at sector %llu:%llu\n",
+ disk->part0, blk_status_to_str(status),
+ blk_op_str(op),
+ start, nr_sectors);
+
+ /*
+ * Add to the front of the list so that newer entries can partially
+ * override other entries. This also intentionally allows duplicate
+ * entries as there is no real reason to reject them.
+ */
+ mutex_lock(&disk->error_injection_lock);
+ if (!disk_live(disk)) {
+ mutex_unlock(&disk->error_injection_lock);
+ error = -ENODEV;
+ goto out_free_inj;
+ }
+ if (list_empty(&disk->error_injection_list))
+ static_branch_inc(&blk_error_injection_enabled);
+ list_add_rcu(&inj->entry, &disk->error_injection_list);
+ set_bit(GD_ERROR_INJECT, &disk->state);
+ mutex_unlock(&disk->error_injection_lock);
+ return 0;
+
+out_free_inj:
+ kfree(inj);
+ return error;
+}
+
+static void error_inject_removeall(struct gendisk *disk)
+{
+ struct blk_error_inject *inj;
+
+ mutex_lock(&disk->error_injection_lock);
+ clear_bit(GD_ERROR_INJECT, &disk->state);
+ while ((inj = list_first_entry_or_null(&disk->error_injection_list,
+ struct blk_error_inject, entry))) {
+ list_del_rcu(&inj->entry);
+ mutex_unlock(&disk->error_injection_lock);
+
+ kfree_rcu_mightsleep(inj);
+
+ mutex_lock(&disk->error_injection_lock);
+ }
+ static_branch_dec(&blk_error_injection_enabled);
+ mutex_unlock(&disk->error_injection_lock);
+}
+
+enum options {
+ Opt_add = (1u << 0),
+ Opt_removeall = (1u << 1),
+
+ Opt_op = (1u << 16),
+ Opt_start = (1u << 17),
+ Opt_nr_sectors = (1u << 18),
+ Opt_status = (1u << 19),
+ Opt_chance = (1u << 20),
+
+ Opt_invalid,
+};
+
+static const match_table_t opt_tokens = {
+ { Opt_add, "add", },
+ { Opt_removeall, "removeall", },
+ { Opt_op, "op=%s", },
+ { Opt_start, "start=%u" },
+ { Opt_nr_sectors, "nr_sectors=%u" },
+ { Opt_status, "status=%s" },
+ { Opt_chance, "chance=%u" },
+ { Opt_invalid, NULL, },
+};
+
+static int match_op(substring_t *args, enum req_op *op)
+{
+ const char *tag;
+
+ tag = match_strdup(args);
+ if (!tag)
+ return -ENOMEM;
+ *op = str_to_blk_op(tag);
+ if (*op == REQ_OP_LAST)
+ pr_warn("invalid op '%s'\n", tag);
+ kfree(tag);
+ return 0;
+}
+
+static int match_status(substring_t *args, blk_status_t *status)
+{
+ const char *tag;
+
+ tag = match_strdup(args);
+ if (!tag)
+ return -ENOMEM;
+ *status = tag_to_blk_status(tag);
+ if (!*status)
+ pr_warn("invalid status '%s'\n", tag);
+ kfree(tag);
+ return 0;
+}
+
+static ssize_t blk_error_injection_parse_options(struct gendisk *disk,
+ char *options)
+{
+ enum { Unset, Add, Removeall } action = Unset;
+ unsigned int option_mask = 0, chance = 1;
+ enum req_op op = REQ_OP_LAST;
+ u64 start = 0, nr_sectors = 0;
+ blk_status_t status = BLK_STS_OK;
+ substring_t args[MAX_OPT_ARGS];
+ char *p;
+
+ while ((p = strsep(&options, ",\n")) != NULL) {
+ int error = 0;
+ ssize_t token;
+
+ if (!*p)
+ continue;
+ token = match_token(p, opt_tokens, args);
+ option_mask |= token;
+ switch (token) {
+ case Opt_add:
+ if (action != Unset)
+ return -EINVAL;
+ action = Add;
+ break;
+ case Opt_removeall:
+ if (action != Unset)
+ return -EINVAL;
+ action = Removeall;
+ break;
+ case Opt_op:
+ error = match_op(args, &op);
+ break;
+ case Opt_start:
+ error = match_u64(args, &start);
+ break;
+ case Opt_nr_sectors:
+ error = match_u64(args, &nr_sectors);
+ break;
+ case Opt_status:
+ error = match_status(args, &status);
+ break;
+ case Opt_chance:
+ error = match_uint(args, &chance);
+ if (!error && chance == 0)
+ error = -EINVAL;
+ break;
+ default:
+ pr_warn("unknown parameter or missing value '%s'\n", p);
+ error = -EINVAL;
+ }
+ if (error)
+ return error;
+ }
+
+ switch (action) {
+ case Add:
+ return error_inject_add(disk, op, start, nr_sectors, status,
+ chance);
+ case Removeall:
+ if (option_mask & ~Opt_removeall)
+ return -EINVAL;
+ error_inject_removeall(disk);
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+static ssize_t blk_error_injection_write(struct file *file,
+ const char __user *ubuf, size_t count, loff_t *pos)
+{
+ struct gendisk *disk = file_inode(file)->i_private;
+ char *options;
+ int error;
+
+ options = memdup_user_nul(ubuf, count);
+ if (IS_ERR(options))
+ return PTR_ERR(options);
+ error = blk_error_injection_parse_options(disk, options);
+ kfree(options);
+
+ if (error)
+ return error;
+ return count;
+}
+
+static int blk_error_injection_show(struct seq_file *s, void *private)
+{
+ struct gendisk *disk = s->private;
+ struct blk_error_inject *inj;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(inj, &disk->error_injection_list, entry) {
+ seq_printf(s, "%llu:%llu status=%s,chance=%u",
+ inj->start, inj->end,
+ blk_status_to_tag(inj->status), inj->chance);
+ seq_putc(s, '\n');
+ }
+ rcu_read_unlock();
+ return 0;
+}
+
+static int blk_error_injection_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, blk_error_injection_show, inode->i_private);
+}
+
+static int blk_error_injection_release(struct inode *inode, struct file *file)
+{
+ return single_release(inode, file);
+}
+
+static const struct file_operations blk_error_injection_fops = {
+ .owner = THIS_MODULE,
+ .write = blk_error_injection_write,
+ .read = seq_read,
+ .open = blk_error_injection_open,
+ .release = blk_error_injection_release,
+};
+
+void blk_error_injection_init(struct gendisk *disk)
+{
+ debugfs_create_file("error_injection", 0600, disk->queue->debugfs_dir,
+ disk, &blk_error_injection_fops);
+}
+
+void blk_error_injection_exit(struct gendisk *disk)
+{
+ error_inject_removeall(disk);
+}
diff --git a/block/error-injection.h b/block/error-injection.h
new file mode 100644
index 000000000000..9821d773abab
--- /dev/null
+++ b/block/error-injection.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _BLK_ERROR_INJECTION_H
+#define _BLK_ERROR_INJECTION_H 1
+
+#include <linux/jump_label.h>
+
+DECLARE_STATIC_KEY_FALSE(blk_error_injection_enabled);
+
+void blk_error_injection_init(struct gendisk *disk);
+void blk_error_injection_exit(struct gendisk *disk);
+bool __blk_error_inject(struct bio *bio);
+static inline bool blk_error_inject(struct bio *bio)
+{
+ if (IS_ENABLED(CONFIG_BLK_ERROR_INJECTION) &&
+ static_branch_unlikely(&blk_error_injection_enabled) &&
+ test_bit(GD_ERROR_INJECT, &bio->bi_bdev->bd_disk->state))
+ return __blk_error_inject(bio);
+ return false;
+}
+
+#endif /* _BLK_ERROR_INJECTION_H */
diff --git a/block/genhd.c b/block/genhd.c
index 7d6854fd28e9..f84b6a355b57 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1485,6 +1485,10 @@ struct gendisk *__alloc_disk_node(struct request_queue *q, int node_id,
lockdep_init_map(&disk->lockdep_map, "(bio completion)", lkclass, 0);
#ifdef CONFIG_BLOCK_HOLDER_DEPRECATED
INIT_LIST_HEAD(&disk->slave_bdevs);
+#endif
+#ifdef CONFIG_BLK_ERROR_INJECTION
+ mutex_init(&disk->error_injection_lock);
+ INIT_LIST_HEAD(&disk->error_injection_list);
#endif
mutex_init(&disk->rqos_state_mutex);
kobject_init(&disk->queue_kobj, &blk_queue_ktype);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 57e84d59a642..5070851cf924 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -176,6 +176,7 @@ struct gendisk {
#define GD_SUPPRESS_PART_SCAN 5
#define GD_OWNS_QUEUE 6
#define GD_ZONE_APPEND_USED 7
+#define GD_ERROR_INJECT 8
struct mutex open_mutex; /* open/close mutex */
unsigned open_partitions; /* number of open partitions */
@@ -227,6 +228,11 @@ struct gendisk {
*/
struct blk_independent_access_ranges *ia_ranges;
+#ifdef CONFIG_BLK_ERROR_INJECTION
+ struct mutex error_injection_lock;
+ struct list_head error_injection_list;
+#endif
+
struct mutex rqos_state_mutex; /* rqos state change mutex */
};
--
2.53.0
^ permalink raw reply related
* Re: [PATCH 02/27] drbd: Remove "extern" from function declarations
From: Christoph Hellwig @ 2026-06-10 5:17 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Marco Elver, Philipp Reisner,
Lars Ellenberg, Christoph Böhmwalder
In-Reply-To: <d9c2b6f320770bc0cb543d24f39fe5c9d28bcd99.1781042470.git.bvanassche@acm.org>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 03/27] drbd: Retain one _get_ldev_if_state() implementation
From: Christoph Hellwig @ 2026-06-10 5:18 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Marco Elver, Philipp Reisner,
Lars Ellenberg, Christoph Böhmwalder, Nathan Chancellor
In-Reply-To: <3777c00c7a6e8bf807ac9133cc9b89540c19bac1.1781042470.git.bvanassche@acm.org>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 04/27] drbd: Remove the get_ldev_if_state() macro
From: Christoph Hellwig @ 2026-06-10 5:19 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Marco Elver, Philipp Reisner,
Lars Ellenberg, Christoph Böhmwalder
In-Reply-To: <0f1141908524e2361a6c4921c668190c8f9a5831.1781042470.git.bvanassche@acm.org>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
> - ({ __acquire(x); true; }) : false)
> #define get_ldev(_device) get_ldev_if_state(_device, D_INCONSISTENT)
>
> static inline void put_ldev(struct drbd_device *device)
> @@ -2033,8 +2030,8 @@ static inline void put_ldev(struct drbd_device *device)
> }
> }
>
> -static inline int _get_ldev_if_state(struct drbd_device *device,
> - enum drbd_disk_state mins)
> +static inline int get_ldev_if_state(struct drbd_device *device,
> + enum drbd_disk_state mins)
> {
> int io_allowed;
>
---end quoted text---
^ permalink raw reply
* Re: [PATCH 05/27] drbd: Remove the 'local' lock context
From: Christoph Hellwig @ 2026-06-10 5:19 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Marco Elver, Philipp Reisner,
Lars Ellenberg, Christoph Böhmwalder
In-Reply-To: <0d220690717c802c4d2b684bf924d134dcd0a929.1781042470.git.bvanassche@acm.org>
On Tue, Jun 09, 2026 at 03:04:52PM -0700, Bart Van Assche wrote:
> The 'local' lock context represents code sections between get_ldev() and
> put_ldev() calls. Between these two calls the device->ldev pointer is
> valid. Since there are multiple functions with unpaired get_ldev() /
> put_ldev() calls, remove this lock context.
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 06/27] drbd: Simplify the bitmap locking functions.
From: Christoph Hellwig @ 2026-06-10 5:20 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Marco Elver, Philipp Reisner,
Lars Ellenberg, Christoph Böhmwalder
In-Reply-To: <b5a5c65d4f10572a112071033c105d54c9ad0974.1781042470.git.bvanassche@acm.org>
On Tue, Jun 09, 2026 at 03:04:53PM -0700, Bart Van Assche wrote:
> Call mutex_lock() instead of mutex_trylock() followed by mutex_lock().
> Remove the code that depends on device->bitmap == NULL because this
> can't happen. All drbd_bm_{lock,unlock}() callers guarantee that the
> device->bitmap pointer is valid.
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 07/27] drbd: Move two declarations
From: Christoph Hellwig @ 2026-06-10 5:20 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Marco Elver, Philipp Reisner,
Lars Ellenberg, Christoph Böhmwalder
In-Reply-To: <7e40961a092b149aa2e3b6fc3c510da650ee0401.1781042470.git.bvanassche@acm.org>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 08/27] drbd: Pass 'resource' directly to complete_conflicting_writes()
From: Christoph Hellwig @ 2026-06-10 5:21 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Marco Elver, Philipp Reisner,
Lars Ellenberg, Christoph Böhmwalder
In-Reply-To: <77d0fd75811d7f604fa80b5c93172b5653b52880.1781042470.git.bvanassche@acm.org>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 09/27] drbd: Split drbd_nl_get_connections_dumpit()
From: Christoph Hellwig @ 2026-06-10 5:22 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Marco Elver,
Philipp Reisner, Lars Ellenberg, Christoph Böhmwalder
In-Reply-To: <167a1fe9f633fe1d1a0d89a50f10c3185d9ddddd.1781042470.git.bvanassche@acm.org>
On Tue, Jun 09, 2026 at 03:04:56PM -0700, Bart Van Assche wrote:
> Move the code between the 'put_result' and 'out' labels into a new
> function. Prepare for making the following code unconditional:
>
> if (resource)
> mutex_unlock(&resource->conf_update);
>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/block/drbd/drbd_nl.c | 98 ++++++++++++++++++++----------------
> 1 file changed, 55 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
> index c44b92856da3..57ab1f1fb1bc 100644
> --- a/drivers/block/drbd/drbd_nl.c
> +++ b/drivers/block/drbd/drbd_nl.c
> @@ -3529,15 +3529,56 @@ int drbd_adm_dump_connections_done(struct netlink_callback *cb)
>
> enum { SINGLE_RESOURCE, ITERATE_RESOURCES };
>
> +static int drbd_nl_put_dump_connections_result(
> + struct sk_buff *skb, struct netlink_callback *cb,
> + struct drbd_resource *resource, struct drbd_connection *connection,
> + enum drbd_ret_code retcode)
Weird indenation here. The usual style is either lining up after the
opening brace or two tabs.
^ permalink raw reply
* Re: [PATCH 10/27] drbd: Make a mutex_unlock() call unconditional
From: Christoph Hellwig @ 2026-06-10 5:22 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Marco Elver, Philipp Reisner,
Lars Ellenberg, Christoph Böhmwalder
In-Reply-To: <803dcc7259a57e513e7ec06a82307e75acd187a6.1781042470.git.bvanassche@acm.org>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 11/27] drbd: Split drbd_req_state()
From: Christoph Hellwig @ 2026-06-10 5:24 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Marco Elver,
Philipp Reisner, Lars Ellenberg, Christoph Böhmwalder
In-Reply-To: <83d144f376580f491b42d7533e004aced7d26d7a.1781042470.git.bvanassche@acm.org>
> +{
> + enum drbd_state_rv rv;
> +
> + if (f & CS_SERIALIZE)
> + mutex_lock(device->state_mutex);
> + rv = __drbd_req_state(device, mask, val, f & ~CS_SERIALIZE);
> if (f & CS_SERIALIZE)
> mutex_unlock(device->state_mutex);
Wouldn't something like:
if (f & CS_SERIALIZE) {
mutex_lock(device->state_mutex);
rv = __drbd_req_state(device, mask, val, f & ~CS_SERIALIZE);
mutex_unlock(device->state_mutex);
} else {
rv = __drbd_req_state(device, mask, val, f & ~CS_SERIALIZE);
}
be either to follow? Is there much of a point in the CS_SERIALIZE
clearing here?
^ permalink raw reply
* Re: [PATCH 12/27] drbd: Convert drbd_req_state() to unconditional locking
From: Christoph Hellwig @ 2026-06-10 5:25 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Marco Elver, Philipp Reisner,
Lars Ellenberg, Christoph Böhmwalder
In-Reply-To: <f3f53e2c6d01b92091bfce447c9010b03a7debef.1781042470.git.bvanassche@acm.org>
On Tue, Jun 09, 2026 at 03:04:59PM -0700, Bart Van Assche wrote:
> Prepare for enabling lock context analysis.
>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/block/drbd/drbd_state.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
> index 5aa4b1889144..171c6488283d 100644
> --- a/drivers/block/drbd/drbd_state.c
> +++ b/drivers/block/drbd/drbd_state.c
> @@ -639,11 +639,12 @@ static enum drbd_state_rv drbd_req_state(struct drbd_device *device,
> {
> enum drbd_state_rv rv;
>
> - if (f & CS_SERIALIZE)
> - mutex_lock(device->state_mutex);
> + if (!(f & CS_SERIALIZE))
> + return __drbd_req_state(device, mask, val, f);
> +
> + mutex_lock(device->state_mutex);
> rv = __drbd_req_state(device, mask, val, f & ~CS_SERIALIZE);
> - if (f & CS_SERIALIZE)
> - mutex_unlock(device->state_mutex);
> + mutex_unlock(device->state_mutex);
It's not really unconditional, just different conditions. And really
belongs into the previous patch.
^ permalink raw reply
* Re: [PATCH 13/27] drbd: Annotate drbd_bm_{lock,unlock}()
From: Christoph Hellwig @ 2026-06-10 5:26 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Marco Elver, Philipp Reisner,
Lars Ellenberg, Christoph Böhmwalder
In-Reply-To: <b5346ca2d22de25c6a03a7b6477d4110c263e5c6.1781042470.git.bvanassche@acm.org>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH 15/27] loop: Split loop_change_fd()
From: Christoph Hellwig @ 2026-06-10 5:30 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, linux-block, Christoph Hellwig, Marco Elver
In-Reply-To: <f835863049bc0364d80c69a7eeb9514e2a698ae2.1781042470.git.bvanassche@acm.org>
On Tue, Jun 09, 2026 at 03:05:02PM -0700, Bart Van Assche wrote:
> Prepare for adding a second call of __loop_change_fd().
A bit confusing message. Yes, you latter add another call, but it's
from the same caller to sort out the locking mess, and that is directly
relevant to the scope of the factored out helper. So please state
that. Same for the next patch. All in merging the first free loop
patches would probably be easier to follow.
^ permalink raw reply
* Re: [PATCH 21/27] null_blk: Enable lock context analysis
From: Christoph Hellwig @ 2026-06-10 5:31 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Marco Elver,
Keith Busch, Damien Le Moal, Chaitanya Kulkarni,
Johannes Thumshirn, Nilay Shroff, Genjian Zhang, Kees Cook
In-Reply-To: <b70422c6f243b9275e336430071b52eb92dd40c4.1781042470.git.bvanassche@acm.org>
On Tue, Jun 09, 2026 at 03:05:08PM -0700, Bart Van Assche wrote:
> Add __must_hold() annotations where these are missing. Annotate two
> functions that use conditional locking with __context_unsafe().
Please explain why that is needed and there is no better way to have
proper annotations. Both here and in a comment in the code.
^ permalink raw reply
* Re: [PATCH 22/27] rbd: Enable lock context analysis
From: Christoph Hellwig @ 2026-06-10 5:32 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, linux-block, Marco Elver, Ilya Dryomov
In-Reply-To: <e239f06e82bb4e6f527a9be87d87dd58ffa265d1.1781042470.git.bvanassche@acm.org>
On Tue, Jun 09, 2026 at 03:05:09PM -0700, Bart Van Assche wrote:
> Add lock context annotations and enable lock context analysis.
It does part one, it does not do part 2 described here.
^ permalink raw reply
* Re: [PATCH 00/27] Enable lock context analysis in drivers/block/
From: Christoph Hellwig @ 2026-06-10 5:34 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, linux-block, Marco Elver
In-Reply-To: <cover.1781042470.git.bvanassche@acm.org>
Can you please split your series up in a saner way? A bunch of the
patches for drivers with their own subdirectories could be merged ASAP,
but you bundle them with lots of unrelated bits. Similarly having
a standalone series for drbd probably makes life easier as you don't
have to CC random other driver authors on it.
Marco: is there a way to enable the context analysis for parts of
a directory? I'm not sure that always converting a whole directory
is going to be feasible in the long run.
^ 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