mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] selftests: ublk: fix the fault_inject delay
@ 2026-10-08 14:45 Qiliang Yuan
  2026-10-08 14:45 ` [PATCH 1/2] selftests: ublk: don't keep the fault_inject timespec on the stack Qiliang Yuan
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Qiliang Yuan @ 2026-10-08 14:45 UTC (permalink / raw)
  To: Ming Lei, Shuah Khan, Jens Axboe, Uday Shankar
  Cc: Ming Lei, linux-block, linux-kselftest, linux-kernel, Qiliang Yuan

The delay set by --delay_us on the fault_inject target is lost in two
ways. Its timespec is read after it has gone out of scope, which makes
generic_06 fail, and any other completion on the ring ends it early when
more than one I/O is in flight. Patch 1 keeps the timespec alive until
the SQE is submitted, and patch 2 makes the delay a pure timeout.

With both patches, a 1 s delay holds every I/O for 1.000 s at queue
depth 4.

Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
Qiliang Yuan (2):
      selftests: ublk: don't keep the fault_inject timespec on the stack
      selftests: ublk: make the fault_inject delay a pure timeout

 tools/testing/selftests/ublk/fault_inject.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
---
base-commit: e1274a6bc40a4618f6fbaebbdc51cca865d51bed
change-id: 20261008-bug-ublk-selftest-fault-inject-delay-46ea414ba226

Best regards,
-- 
Qiliang Yuan <odys.yuan@gmail.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] selftests: ublk: don't keep the fault_inject timespec on the stack
  2026-10-08 14:45 [PATCH 0/2] selftests: ublk: fix the fault_inject delay Qiliang Yuan
@ 2026-10-08 14:45 ` Qiliang Yuan
  2026-10-09 14:07   ` Ming Lei
  2026-10-08 14:45 ` [PATCH 2/2] selftests: ublk: make the fault_inject delay a pure timeout Qiliang Yuan
  2026-10-09 14:37 ` [PATCH 0/2] selftests: ublk: fix the fault_inject delay Jens Axboe
  2 siblings, 1 reply; 6+ messages in thread
From: Qiliang Yuan @ 2026-10-08 14:45 UTC (permalink / raw)
  To: Ming Lei, Shuah Khan, Jens Axboe, Uday Shankar
  Cc: Ming Lei, linux-block, linux-kselftest, linux-kernel, Qiliang Yuan

The fault_inject target delays each I/O with an IORING_OP_TIMEOUT, and
generic_06 relies on the delay to kill the server while an I/O is still
outstanding.

The timespec of the timeout is a local variable of
ublk_fault_inject_queue_io(). The SQE only records its address, and
io_uring reads it when the SQE is submitted, after the function has
returned. The timeout gets whatever the stack holds by then and expires
almost at once, so --delay_us has no effect, and generic_06 fails
because dd completes before the server is killed.

Store the timespec in the per-device fi_opts, and split the delay into
seconds and nanoseconds so that delays of a second or more give a valid
timespec.

fio 4k random reads for 10 s at queue depth 1 on a fault_inject device
with --delay_us 1000000:

                  before      after   expected
  I/Os            446927         10         10
  mean latency   18.3 us    1.000 s        1 s

Fixes: 81586652bb1f ("selftests: ublk: add generic_06 for covering fault inject")
Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
 tools/testing/selftests/ublk/fault_inject.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/ublk/fault_inject.c b/tools/testing/selftests/ublk/fault_inject.c
index 150896e02ff8b..e055f51a65447 100644
--- a/tools/testing/selftests/ublk/fault_inject.c
+++ b/tools/testing/selftests/ublk/fault_inject.c
@@ -11,7 +11,7 @@
 #include "kublk.h"
 
 struct fi_opts {
-	long long delay_ns;
+	struct __kernel_timespec delay;
 	bool die_during_fetch;
 };
 
@@ -47,7 +47,8 @@ static int ublk_fault_inject_tgt_init(const struct dev_ctx *ctx,
 		return -ENOMEM;
 	}
 
-	opts->delay_ns = ctx->fault_inject.delay_us * 1000;
+	opts->delay.tv_sec = ctx->fault_inject.delay_us / 1000000;
+	opts->delay.tv_nsec = ctx->fault_inject.delay_us % 1000000 * 1000;
 	opts->die_during_fetch = ctx->fault_inject.die_during_fetch;
 	dev->private_data = opts;
 
@@ -85,12 +86,13 @@ static int ublk_fault_inject_queue_io(struct ublk_thread *t,
 	const struct ublksrv_io_desc *iod = ublk_get_iod(q, tag);
 	struct io_uring_sqe *sqe;
 	struct fi_opts *opts = q->dev->private_data;
-	struct __kernel_timespec ts = {
-		.tv_nsec = opts->delay_ns,
-	};
 
+	/*
+	 * Pass a timespec that outlives this function, since io_uring
+	 * reads it only when the SQE is submitted.
+	 */
 	ublk_io_alloc_sqes(t, &sqe, 1);
-	io_uring_prep_timeout(sqe, &ts, 1, 0);
+	io_uring_prep_timeout(sqe, &opts->delay, 1, 0);
 	sqe->user_data = build_user_data(tag, ublksrv_get_op(iod), 0, q->q_id, 1);
 
 	ublk_queued_tgt_io(t, q, tag, 1);

-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] selftests: ublk: make the fault_inject delay a pure timeout
  2026-10-08 14:45 [PATCH 0/2] selftests: ublk: fix the fault_inject delay Qiliang Yuan
  2026-10-08 14:45 ` [PATCH 1/2] selftests: ublk: don't keep the fault_inject timespec on the stack Qiliang Yuan
@ 2026-10-08 14:45 ` Qiliang Yuan
  2026-10-09 14:08   ` Ming Lei
  2026-10-09 14:37 ` [PATCH 0/2] selftests: ublk: fix the fault_inject delay Jens Axboe
  2 siblings, 1 reply; 6+ messages in thread
From: Qiliang Yuan @ 2026-10-08 14:45 UTC (permalink / raw)
  To: Ming Lei, Shuah Khan, Jens Axboe, Uday Shankar
  Cc: Ming Lei, linux-block, linux-kselftest, linux-kernel, Qiliang Yuan

ublk_fault_inject_queue_io() queues the delay of each I/O as an
IORING_OP_TIMEOUT with a completion count of 1. Such a timeout also
completes as soon as any other CQE is posted on the ring.

With more than one I/O in flight, the completion of one I/O ends the
delay of the others, and ublk_fault_inject_tgt_io_done() reports every
early completion as "unexpected cqe res 0".

Pass a count of 0 so that only the expiry of the timer completes the
timeout.

fio 4k random reads for 10 s at queue depth 4 on a fault_inject device
with --delay_us 1000000:

                            before      after   expected
  I/Os                     2263170         40         40
  mean latency             16.3 us    1.000 s        1 s
  "unexpected cqe res 0"   2263169          0          0

Fixes: 81586652bb1f ("selftests: ublk: add generic_06 for covering fault inject")
Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
 tools/testing/selftests/ublk/fault_inject.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/ublk/fault_inject.c b/tools/testing/selftests/ublk/fault_inject.c
index e055f51a65447..4b59f17a918c0 100644
--- a/tools/testing/selftests/ublk/fault_inject.c
+++ b/tools/testing/selftests/ublk/fault_inject.c
@@ -92,7 +92,7 @@ static int ublk_fault_inject_queue_io(struct ublk_thread *t,
 	 * reads it only when the SQE is submitted.
 	 */
 	ublk_io_alloc_sqes(t, &sqe, 1);
-	io_uring_prep_timeout(sqe, &opts->delay, 1, 0);
+	io_uring_prep_timeout(sqe, &opts->delay, 0, 0);
 	sqe->user_data = build_user_data(tag, ublksrv_get_op(iod), 0, q->q_id, 1);
 
 	ublk_queued_tgt_io(t, q, tag, 1);

-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] selftests: ublk: don't keep the fault_inject timespec on the stack
  2026-10-08 14:45 ` [PATCH 1/2] selftests: ublk: don't keep the fault_inject timespec on the stack Qiliang Yuan
@ 2026-10-09 14:07   ` Ming Lei
  0 siblings, 0 replies; 6+ messages in thread
From: Ming Lei @ 2026-10-09 14:07 UTC (permalink / raw)
  To: Qiliang Yuan
  Cc: Shuah Khan, Jens Axboe, Uday Shankar, linux-block,
	linux-kselftest, linux-kernel

On Thu, Oct 08, 2026 at 10:45:39PM +0800, Qiliang Yuan wrote:
> The fault_inject target delays each I/O with an IORING_OP_TIMEOUT, and
> generic_06 relies on the delay to kill the server while an I/O is still
> outstanding.
> 
> The timespec of the timeout is a local variable of
> ublk_fault_inject_queue_io(). The SQE only records its address, and
> io_uring reads it when the SQE is submitted, after the function has
> returned. The timeout gets whatever the stack holds by then and expires
> almost at once, so --delay_us has no effect, and generic_06 fails
> because dd completes before the server is killed.
> 
> Store the timespec in the per-device fi_opts, and split the delay into
> seconds and nanoseconds so that delays of a second or more give a valid
> timespec.
> 
> fio 4k random reads for 10 s at queue depth 1 on a fault_inject device
> with --delay_us 1000000:
> 
>                   before      after   expected
>   I/Os            446927         10         10
>   mean latency   18.3 us    1.000 s        1 s
> 
> Fixes: 81586652bb1f ("selftests: ublk: add generic_06 for covering fault inject")
> Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>

Reviewed-by: Ming Lei <tom.leiming@gmail.com>

Thanks, 
Ming

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] selftests: ublk: make the fault_inject delay a pure timeout
  2026-10-08 14:45 ` [PATCH 2/2] selftests: ublk: make the fault_inject delay a pure timeout Qiliang Yuan
@ 2026-10-09 14:08   ` Ming Lei
  0 siblings, 0 replies; 6+ messages in thread
From: Ming Lei @ 2026-10-09 14:08 UTC (permalink / raw)
  To: Qiliang Yuan
  Cc: Shuah Khan, Jens Axboe, Uday Shankar, linux-block,
	linux-kselftest, linux-kernel

On Thu, Oct 08, 2026 at 10:45:40PM +0800, Qiliang Yuan wrote:
> ublk_fault_inject_queue_io() queues the delay of each I/O as an
> IORING_OP_TIMEOUT with a completion count of 1. Such a timeout also
> completes as soon as any other CQE is posted on the ring.
> 
> With more than one I/O in flight, the completion of one I/O ends the
> delay of the others, and ublk_fault_inject_tgt_io_done() reports every
> early completion as "unexpected cqe res 0".
> 
> Pass a count of 0 so that only the expiry of the timer completes the
> timeout.
> 
> fio 4k random reads for 10 s at queue depth 4 on a fault_inject device
> with --delay_us 1000000:
> 
>                             before      after   expected
>   I/Os                     2263170         40         40
>   mean latency             16.3 us    1.000 s        1 s
>   "unexpected cqe res 0"   2263169          0          0
> 
> Fixes: 81586652bb1f ("selftests: ublk: add generic_06 for covering fault inject")
> Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>

Reviewed-by: Ming Lei <tom.leiming@gmail.com>

Thanks,
Ming

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] selftests: ublk: fix the fault_inject delay
  2026-10-08 14:45 [PATCH 0/2] selftests: ublk: fix the fault_inject delay Qiliang Yuan
  2026-10-08 14:45 ` [PATCH 1/2] selftests: ublk: don't keep the fault_inject timespec on the stack Qiliang Yuan
  2026-10-08 14:45 ` [PATCH 2/2] selftests: ublk: make the fault_inject delay a pure timeout Qiliang Yuan
@ 2026-10-09 14:37 ` Jens Axboe
  2 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2026-10-09 14:37 UTC (permalink / raw)
  To: Ming Lei, Shuah Khan, Uday Shankar, Qiliang Yuan
  Cc: Ming Lei, linux-block, linux-kselftest, linux-kernel


On Thu, 08 Oct 2026 22:45:38 +0800, Qiliang Yuan wrote:
> The delay set by --delay_us on the fault_inject target is lost in two
> ways. Its timespec is read after it has gone out of scope, which makes
> generic_06 fail, and any other completion on the ring ends it early when
> more than one I/O is in flight. Patch 1 keeps the timespec alive until
> the SQE is submitted, and patch 2 makes the delay a pure timeout.
> 
> With both patches, a 1 s delay holds every I/O for 1.000 s at queue
> depth 4.
> 
> [...]

Applied, thanks!

[1/2] selftests: ublk: don't keep the fault_inject timespec on the stack
      commit: 955cba87d9ed334fbe0b669b35b4abcddd7f9929
[2/2] selftests: ublk: make the fault_inject delay a pure timeout
      commit: 69ae59173a5668d015ab2733c8628ed542f00ccc

Best regards,
-- 
Jens Axboe




^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-09 14:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-08 14:45 [PATCH 0/2] selftests: ublk: fix the fault_inject delay Qiliang Yuan
2026-10-08 14:45 ` [PATCH 1/2] selftests: ublk: don't keep the fault_inject timespec on the stack Qiliang Yuan
2026-10-09 14:07   ` Ming Lei
2026-10-08 14:45 ` [PATCH 2/2] selftests: ublk: make the fault_inject delay a pure timeout Qiliang Yuan
2026-10-09 14:08   ` Ming Lei
2026-10-09 14:37 ` [PATCH 0/2] selftests: ublk: fix the fault_inject delay Jens Axboe

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

all inboxes | Powered by JetHome®