[PATCH] io_uring: avoid iowq again trap

[PATCH] io_uring: avoid iowq again trap

[Pavel Begunkov]

If an opcode handler semi-reliably returns -EAGAIN, io_wq_submit_work()
might continue busily hammer the same handler over and over again, which
is not ideal. The -EAGAIN handling in question was put there only for
IOPOLL, so restrict it to IOPOLL mode only.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 fs/io_uring.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index e01f595f5b7d..3af1905efc78 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -7319,6 +7319,8 @@ static void io_wq_submit_work(struct io_wq_work *work)
 		 * wait for request slots on the block side.
 		 */
 		if (!needs_poll) {
+			if (!(req->ctx->flags & IORING_SETUP_IOPOLL))
+				break;
 			cond_resched();
 			continue;
 		}
-- 
2.36.0

Re: [PATCH] io_uring: avoid iowq again trap

[Jens Axboe]

On 5/13/22 4:24 AM, Pavel Begunkov wrote:
> If an opcode handler semi-reliably returns -EAGAIN, io_wq_submit_work()
> might continue busily hammer the same handler over and over again, which
> is not ideal. The -EAGAIN handling in question was put there only for
> IOPOLL, so restrict it to IOPOLL mode only.

Looks good, needs:

Fixes: 90fa02883f06 ("io_uring: implement async hybrid mode for pollable requests")

unless I'm mistaken.

-- 
Jens Axboe

Re: [PATCH] io_uring: avoid iowq again trap

[Pavel Begunkov]

On 5/13/22 13:31, Jens Axboe wrote:
> On 5/13/22 4:24 AM, Pavel Begunkov wrote:
>> If an opcode handler semi-reliably returns -EAGAIN, io_wq_submit_work()
>> might continue busily hammer the same handler over and over again, which
>> is not ideal. The -EAGAIN handling in question was put there only for
>> IOPOLL, so restrict it to IOPOLL mode only.
> 
> Looks good, needs:
> 
> Fixes: 90fa02883f06 ("io_uring: implement async hybrid mode for pollable requests")
> 
> unless I'm mistaken.

It's probably more of

Fixes: def596e9557c9 ("io_uring: support for IO polling")

-- 
Pavel Begunkov

Re: [PATCH] io_uring: avoid iowq again trap

[Jens Axboe]

On 5/13/22 6:49 AM, Pavel Begunkov wrote:
> On 5/13/22 13:31, Jens Axboe wrote:
>> On 5/13/22 4:24 AM, Pavel Begunkov wrote:
>>> If an opcode handler semi-reliably returns -EAGAIN, io_wq_submit_work()
>>> might continue busily hammer the same handler over and over again, which
>>> is not ideal. The -EAGAIN handling in question was put there only for
>>> IOPOLL, so restrict it to IOPOLL mode only.
>>
>> Looks good, needs:
>>
>> Fixes: 90fa02883f06 ("io_uring: implement async hybrid mode for pollable requests")
>>
>> unless I'm mistaken.
> 
> It's probably more of
> 
> Fixes: def596e9557c9 ("io_uring: support for IO polling")

Yes, I think you are right and it goes back further. I'll get it queued
up, thanks!

-- 
Jens Axboe

Re: [PATCH] io_uring: avoid iowq again trap

[Jens Axboe]

On Fri, 13 May 2022 11:24:56 +0100, Pavel Begunkov wrote:
> If an opcode handler semi-reliably returns -EAGAIN, io_wq_submit_work()
> might continue busily hammer the same handler over and over again, which
> is not ideal. The -EAGAIN handling in question was put there only for
> IOPOLL, so restrict it to IOPOLL mode only.
> 
> 

Applied, thanks!

[1/1] io_uring: avoid iowq again trap
      (no commit info)

Best regards,
-- 
Jens Axboe

Re: [PATCH] io_uring: avoid iowq again trap

[Hao Xu]

On 5/13/22 18:24, Pavel Begunkov wrote:
> If an opcode handler semi-reliably returns -EAGAIN, io_wq_submit_work()

Hi Pavel,
When would it return -EAGAIN in non-IOPOLL mode?

> might continue busily hammer the same handler over and over again, which
> is not ideal. The -EAGAIN handling in question was put there only for
> IOPOLL, so restrict it to IOPOLL mode only.
> 
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>   fs/io_uring.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index e01f595f5b7d..3af1905efc78 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -7319,6 +7319,8 @@ static void io_wq_submit_work(struct io_wq_work *work)
>   		 * wait for request slots on the block side.
>   		 */
>   		if (!needs_poll) {
> +			if (!(req->ctx->flags & IORING_SETUP_IOPOLL))
> +				break;
>   			cond_resched();
>   			continue;
>   		}

Re: [PATCH] io_uring: avoid iowq again trap

[Pavel Begunkov]

On 5/15/22 08:31, Hao Xu wrote:
> On 5/13/22 18:24, Pavel Begunkov wrote:
>> If an opcode handler semi-reliably returns -EAGAIN, io_wq_submit_work()
> 
> Hi Pavel,
> When would it return -EAGAIN in non-IOPOLL mode?

I didn't see it in the wild but stumbled upon while preparing some
future patches. I hope it's not a real issue, but it's better to not
leave a way for some driver/etc. to abuse it.


>> might continue busily hammer the same handler over and over again, which
>> is not ideal. The -EAGAIN handling in question was put there only for
>> IOPOLL, so restrict it to IOPOLL mode only.
>>
>> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
>> ---
>>   fs/io_uring.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index e01f595f5b7d..3af1905efc78 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -7319,6 +7319,8 @@ static void io_wq_submit_work(struct io_wq_work *work)
>>            * wait for request slots on the block side.
>>            */
>>           if (!needs_poll) {
>> +            if (!(req->ctx->flags & IORING_SETUP_IOPOLL))
>> +                break;
>>               cond_resched();
>>               continue;
>>           }
> 

-- 
Pavel Begunkov

Re: [PATCH] io_uring: avoid iowq again trap

[Hao Xu]

On 5/15/22 22:21, Pavel Begunkov wrote:
> On 5/15/22 08:31, Hao Xu wrote:
>> On 5/13/22 18:24, Pavel Begunkov wrote:
>>> If an opcode handler semi-reliably returns -EAGAIN, io_wq_submit_work()
>>
>> Hi Pavel,
>> When would it return -EAGAIN in non-IOPOLL mode?
> 
> I didn't see it in the wild but stumbled upon while preparing some
> future patches. I hope it's not a real issue, but it's better to not
> leave a way for some driver/etc. to abuse it.
> 
> 
Gotcha, thanks.