Re: [PATCH blktests 0/5] nvme: detect ABA ghost writes on multipath fabrics

[Mohamed Khalfella <mkhalfella@purestorage.com>]

On Tue 2026-10-20 11:02:47 -0700, Mohamed Khalfella wrote:
> On Fri 2026-10-02 05:03:20 +0200, Shin'ichiro Kawasaki wrote:
> > On Sep 23, 2026 / 10:02, Mohamed Khalfella wrote:
> > > On Wed 2026-09-23 17:18:21 +0900, Shin'ichiro Kawasaki wrote:
> > > > On Sep 16, 2026 / 20:06, Mohamed Khalfella wrote:
> > > > 
> > > > Mohamad, thanks for the series. Before I look in the details of the patches,
> > > > I would like make a comment below.
> > > > 
> > > > > Reproducing the race needs an IO that can be held at the target for
> > > > > longer than the host's io_timeout without being failed. The first three
> > > > > patches give miniublk that ability, since there was previously no way to
> > > > > talk to a running ublk server at all:
> > > > > 
> > > > >   1/5  adds a loopback UDP control channel to the daemon, served by a
> > > > >        detached thread, one reply datagram per request
> > > > >   2/5  adds an INJECT_DELAY command which delays a given number of reads
> > > > >        or writes as an io_uring timeout on the queue's own ring
> > > > >   3/5  exposes it as "miniublk inject", which returns only once the
> > > > >        daemon has armed the delay, so a test can start IO immediately
> > > > >        without racing it
> > > > 
> > > > It is interesting to implement the delay feature to miniublk, and use it as the
> > > > testing tool. But I'm not sure if this is the best approach because of two
> > > > points. First, scsi_debug has jdelay and ndelay options. I wonder if this
> > > > existing feature can fulfill the requirements of the new test case. Second, when
> > > > rublk [*] is available in the PATH of the test system, blktests detect it and
> > > > use it instead of miniublk. I guess the added delay feature is not available in
> > > > rublk, and the test case won't work well. What do you think of these points?
> > > > 
> > > 
> > > The two points make sense to me. Let me look at scsi_debug delay
> > > injection mechanism first and see if it can be used here. I will also
> > > look at rublk. Thanks for the pointers.
> > 
> > I've learned that now delay injection is being added to the block layer [*].
> > This looks more generic and could be the best way for this use case.
> > 
> > [*] https://lore.kernel.org/linux-block/20260928221634.43239-1-haris.iqbal@linux.dev/
> 
> Thank you for pointing me to this patchset. The change as in v3 in the
> link above is not sufficient for this case. Block layer "error_inject"
> does not support an option to set the maximum number of times a rule
> should be executed. I want only the first write to be delayed for the
> test to be deterministic.
> 
> That is said, it should not be difficult to close this gap. I will give
> it a shot.

Adding a limit to error injection rule worked.

[ 1244.485435] run blktests nvme/070 at 2026-10-22 11:40:52.361981983
[ 1245.355103] nvmet_tcp: enabling port 0 (127.0.0.1:4420)
[ 1245.401761] nvmet_tcp: enabling port 1 (127.0.0.1:4421)
[ 1245.443655] nvmet: adding nsid 1 to subsystem blktests-subsystem-1
[ 1245.579679] nvmet: Created nvm controller 1 for subsystem blktests-subsystem-1 for NQN nqn.2014-08.org.nvmexpress:uui.
[ 1245.587802] nvme nvme4: creating 8 I/O queues.
[ 1245.599665] nvme nvme4: mapped 8/0/0 default/read/poll queues.
[ 1245.612520] nvme nvme4: new ctrl: NQN "blktests-subsystem-1", addr 127.0.0.1:4420, hostnqn: nqn.2014-08.org.nvmexpres9
[ 1245.993765] nvmet: Created nvm controller 2 for subsystem blktests-subsystem-1 for NQN nqn.2014-08.org.nvmexpress:uui.
[ 1246.001615] nvme nvme5: creating 8 I/O queues.
[ 1246.016800] nvme nvme5: mapped 8/0/0 default/read/poll queues.
[ 1246.031323] nvme nvme5: new ctrl: NQN "blktests-subsystem-1", addr 127.0.0.1:4421, hostnqn: nqn.2014-08.org.nvmexpres9
[ 1246.561862] ublkb0: delaying WRITE at sector 0:8 by 4000000us
[ 1249.908509] nvme nvme4: I/O tag 81 (0051) type 4 opcode 0x1 (Write) QID 7 timeout
[ 1249.910523] nvme nvme4: starting controller fencing
[ 1249.911588] nvme nvme4: attempting CCR, timeout 15000ms
[ 1249.913245] nvmet: ctrl 1 fatal error occurred!
[ 1250.631188] nvme nvme4: CCR succeeded using nvme5
[ 1250.647822] nvme nvme4: Reconnecting in 10 seconds...
[ 1260.710957] nvmet: Created nvm controller 1 for subsystem blktests-subsystem-1 for NQN nqn.2014-08.org.nvmexpress:uui.
[ 1260.718851] nvme nvme4: creating 8 I/O queues.
[ 1260.793810] nvme nvme4: mapped 8/0/0 default/read/poll queues.
[ 1260.805832] nvme nvme4: Successfully reconnected (attempt 1/60)
[ 1330.829938] nvme nvme5: Removing ctrl: NQN "blktests-subsystem-1"
[ 1331.316251] nvme nvme4: Removing ctrl: NQN "blktests-subsystem-1"

This full kernel logs for the test run with block layer delay injection.
A delay of 4 second injected -> io_timeout -> CCR.

diff --git a/tests/nvme/070 b/tests/nvme/070
index e9a4690..85604e7 100755
--- a/tests/nvme/070
+++ b/tests/nvme/070
@@ -51,7 +51,7 @@ set_io_timeout_of_subsystem() {
 test() {
        echo "Running ${TEST_NAME}"

-       local ns port nr_paths
+       local ns port nr_paths einj
        local -a ports

        if ! _init_ublk; then
@@ -84,9 +84,8 @@ test() {

        set_io_timeout_of_subsystem "${def_subsysnqn}" 2000

-       if ! ${UBLK_PROG} inject -n 0 -o write -d 4 -c 1 >> "$FULL" 2>&1; then
-               echo "FAIL: can not inject write delay"
-       fi
+       einj="/sys/kernel/debug/block/ublkb0/error_injection"
+       echo 'add,op=WRITE,status=OK,chance=1,delay_us=4000000,limit=1' > "${einj}"

        ns=$(_find_nvme_ns "${def_subsys_uuid}")
        "$SRCDIR/nvme-ghost-write-detector" "/dev/${ns}"

Changes to the test to use block layer error injection.

diff --git a/block/error-injection.c b/block/error-injection.c
index a35fe41a43e4..4d0150958cff 100644
--- a/block/error-injection.c
+++ b/block/error-injection.c
@@ -29,6 +29,9 @@ struct blk_error_inject {
 
 	/* hold the bio for this long before submitting or failing it */
 	unsigned int			delay_us;
+
+	/* maximum number of times to execute the rule, -1 no limit */
+	atomic_t			limit;
 };
 
 /*
@@ -143,6 +146,8 @@ bool __blk_error_inject(struct bio *bio)
 		if (inj->chance > 1 && (get_random_u32() % inj->chance) != 0)
 			continue;
 
+		if (atomic_dec_if_positive(&inj->limit) == -1)
+			continue;
 		status = inj->status;
 		delay_us = inj->delay_us;
 		break;
@@ -165,7 +170,7 @@ bool __blk_error_inject(struct bio *bio)
 
 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, unsigned int delay_us)
+		unsigned int chance, unsigned int delay_us, int limit)
 {
 	struct blk_error_inject *inj;
 	int error = -EINVAL;
@@ -194,6 +199,7 @@ static int error_inject_add(struct gendisk *disk, enum req_op op,
 	inj->status = status;
 	inj->chance = chance;
 	inj->delay_us = delay_us;
+	atomic_set(&inj->limit, limit);
 
 	pr_debug_ratelimited("%pg: adding %s injection for %s at sector %llu:%llu\n",
 			disk->part0, blk_status_to_str(status),
@@ -248,6 +254,7 @@ enum options {
 	Opt_status		= (1u << 19),
 	Opt_chance		= (1u << 20),
 	Opt_delay_us		= (1u << 21),
+	Opt_limit		= (1u << 22),
 
 	Opt_invalid,
 };
@@ -261,6 +268,7 @@ static const match_table_t opt_tokens = {
 	{ Opt_status,			"status=%s"		},
 	{ Opt_chance,			"chance=%u"		},
 	{ Opt_delay_us,			"delay_us=%u"		},
+	{ Opt_limit,			"limit=%d"		},
 	{ Opt_invalid,			NULL,			},
 };
 
@@ -302,6 +310,7 @@ static ssize_t blk_error_injection_parse_options(struct gendisk *disk,
 	u64 start = 0, nr_sectors = 0;
 	blk_status_t status = BLK_STS_OK;
 	substring_t args[MAX_OPT_ARGS];
+	int limit = -1;
 	char *p;
 
 	while ((p = strsep(&options, ",\n")) != NULL) {
@@ -343,6 +352,9 @@ static ssize_t blk_error_injection_parse_options(struct gendisk *disk,
 		case Opt_delay_us:
 			error = match_uint(args, &delay_us);
 			break;
+		case Opt_limit:
+			error = match_int(args, &limit);
+			break;
 		default:
 			pr_warn("unknown parameter or missing value '%s'\n", p);
 			error = -EINVAL;
@@ -354,7 +366,7 @@ static ssize_t blk_error_injection_parse_options(struct gendisk *disk,
 	switch (action) {
 	case Add:
 		return error_inject_add(disk, op, start, nr_sectors, status,
-				chance, delay_us);
+				chance, delay_us, limit);
 	case Removeall:
 		if (option_mask & ~Opt_removeall)
 			return -EINVAL;
@@ -390,11 +402,11 @@ static int blk_error_injection_show(struct seq_file *s, void *private)
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(inj, &disk->error_injection_list, entry) {
-		seq_printf(s, "%llu:%llu op=%s,status=%s,chance=%u,delay_us=%u",
+		seq_printf(s, "%llu:%llu op=%s,status=%s,chance=%u,delay_us=%u,limit=%d",
 			   inj->start, inj->end,
 			   blk_op_str(inj->op),
 			   blk_status_to_tag(inj->status), inj->chance,
-			   inj->delay_us);
+			   inj->delay_us, atomic_read(&inj->limit));
 		seq_putc(s, '\n');
 	}
 	rcu_read_unlock();

And here is the change to block error injection to add a limit to error
injection rules. It missing documentation change but that should be
trivial to add.

Now, I need to follow with the patchset adding delay injection to see if
that can get merged.