public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH v1] block: blk_queue_enter() / __bio_queue_enter() must return -EAGAIN for nowait
@ 2022-09-12 16:53 Stefan Roesch
  2022-09-12 17:14 ` Bart Van Assche
  2022-09-13 21:08 ` Jens Axboe
  0 siblings, 2 replies; 4+ messages in thread
From: Stefan Roesch @ 2022-09-12 16:53 UTC (permalink / raw)
  To: kernel-team, axboe, linux-block; +Cc: shr, io-uring

Today blk_queue_enter() and __bio_queue_enter() return -EBUSY for the
nowait code path. This is not correct: they should return -EAGAIN
instead.

This problem was detected by fio. The following command exposed the
above problem:

t/io_uring -p0 -d128 -b4096 -s32 -c32 -F1 -B0 -R0 -X1 -n24 -P1 -u1 -O0 /dev/ng0n1

By applying the patch, the retry case is handled correctly in the slow
path.

Signed-off-by: Stefan Roesch <[email protected]>
---
 block/blk-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index a0d1104c5590..651057c4146b 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -295,7 +295,7 @@ int blk_queue_enter(struct request_queue *q, blk_mq_req_flags_t flags)
 
 	while (!blk_try_enter_queue(q, pm)) {
 		if (flags & BLK_MQ_REQ_NOWAIT)
-			return -EBUSY;
+			return -EAGAIN;
 
 		/*
 		 * read pair of barrier in blk_freeze_queue_start(), we need to
@@ -325,7 +325,7 @@ int __bio_queue_enter(struct request_queue *q, struct bio *bio)
 			if (test_bit(GD_DEAD, &disk->state))
 				goto dead;
 			bio_wouldblock_error(bio);
-			return -EBUSY;
+			return -EAGAIN;
 		}
 
 		/*

base-commit: 80e78fcce86de0288793a0ef0f6acf37656ee4cf
-- 
2.30.2


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

* Re: [PATCH v1] block: blk_queue_enter() / __bio_queue_enter() must return -EAGAIN for nowait
  2022-09-12 16:53 [PATCH v1] block: blk_queue_enter() / __bio_queue_enter() must return -EAGAIN for nowait Stefan Roesch
@ 2022-09-12 17:14 ` Bart Van Assche
  2022-09-12 17:22   ` Jens Axboe
  2022-09-13 21:08 ` Jens Axboe
  1 sibling, 1 reply; 4+ messages in thread
From: Bart Van Assche @ 2022-09-12 17:14 UTC (permalink / raw)
  To: Stefan Roesch, kernel-team, axboe, linux-block; +Cc: io-uring

On 9/12/22 09:53, Stefan Roesch wrote:
> Today blk_queue_enter() and __bio_queue_enter() return -EBUSY for the
> nowait code path. This is not correct: they should return -EAGAIN
> instead.

Why is this considered incorrect? Please explain.

Since this patch also affects other code, e.g. NVMe pass-through, can 
this patch break existing user space code by changing the value returned 
to user space?

Thanks,

Bart.

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

* Re: [PATCH v1] block: blk_queue_enter() / __bio_queue_enter() must return -EAGAIN for nowait
  2022-09-12 17:14 ` Bart Van Assche
@ 2022-09-12 17:22   ` Jens Axboe
  0 siblings, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2022-09-12 17:22 UTC (permalink / raw)
  To: Bart Van Assche, Stefan Roesch, kernel-team, linux-block; +Cc: io-uring

On 9/12/22 11:14 AM, Bart Van Assche wrote:
> On 9/12/22 09:53, Stefan Roesch wrote:
>> Today blk_queue_enter() and __bio_queue_enter() return -EBUSY for the
>> nowait code path. This is not correct: they should return -EAGAIN
>> instead.
> 
> Why is this considered incorrect? Please explain.
> 
> Since this patch also affects other code, e.g. NVMe pass-through, can
> this patch break existing user space code by changing the value
> returned to user space?

It is currently broken because we always return EAGAIN/EWOULDBLOCK for
these cases, if a non-block flag is set of some sort. I strongly suspect
that nobody has really being doing non-blocking IO to devices before
io_uring made it a lot more common. EAGAIN means "try again later, or
without nonblock said".

We should be consistent here.

-- 
Jens Axboe

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

* Re: [PATCH v1] block: blk_queue_enter() / __bio_queue_enter() must return -EAGAIN for nowait
  2022-09-12 16:53 [PATCH v1] block: blk_queue_enter() / __bio_queue_enter() must return -EAGAIN for nowait Stefan Roesch
  2022-09-12 17:14 ` Bart Van Assche
@ 2022-09-13 21:08 ` Jens Axboe
  1 sibling, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2022-09-13 21:08 UTC (permalink / raw)
  To: linux-block, kernel-team, Stefan Roesch; +Cc: io-uring

On Mon, 12 Sep 2022 09:53:25 -0700, Stefan Roesch wrote:
> Today blk_queue_enter() and __bio_queue_enter() return -EBUSY for the
> nowait code path. This is not correct: they should return -EAGAIN
> instead.
> 
> This problem was detected by fio. The following command exposed the
> above problem:
> 
> [...]

Applied, thanks!

[1/1] block: blk_queue_enter() / __bio_queue_enter() must return -EAGAIN for nowait
      commit: 56f99b8d06ef1ed1c9730948f9f05ac2b930a20b

Best regards,
-- 
Jens Axboe



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

end of thread, other threads:[~2022-09-13 21:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-12 16:53 [PATCH v1] block: blk_queue_enter() / __bio_queue_enter() must return -EAGAIN for nowait Stefan Roesch
2022-09-12 17:14 ` Bart Van Assche
2022-09-12 17:22   ` Jens Axboe
2022-09-13 21:08 ` Jens Axboe

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