* [PATCH] io_uring/napi: enable even with a timeout of 0
@ 2024-02-15 22:32 Jens Axboe
2024-02-15 22:37 ` David Wei
0 siblings, 1 reply; 3+ messages in thread
From: Jens Axboe @ 2024-02-15 22:32 UTC (permalink / raw)
To: io-uring, David Wei
1 usec is not as short as it used to be, and it makes sense to allow 0
for a busy poll timeout - this means just do one loop to check if we
have anything available. Add a separate ->napi_enabled to check if napi
has been enabled or not.
While at it, move the writing of the ctx napi values after we've copied
the old values back to userspace. This ensures that if the call fails,
we'll be in the same state as we were before, rather than some
indeterminate state.
Signed-off-by: Jens Axboe <[email protected]>
---
diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
index 4fe7af8a4907..bd7071aeec5d 100644
--- a/include/linux/io_uring_types.h
+++ b/include/linux/io_uring_types.h
@@ -420,6 +420,7 @@ struct io_ring_ctx {
/* napi busy poll default timeout */
unsigned int napi_busy_poll_to;
bool napi_prefer_busy_poll;
+ bool napi_enabled;
DECLARE_HASHTABLE(napi_ht, 4);
#endif
diff --git a/io_uring/napi.c b/io_uring/napi.c
index b234adda7dfd..e653927a376e 100644
--- a/io_uring/napi.c
+++ b/io_uring/napi.c
@@ -227,12 +227,12 @@ int io_register_napi(struct io_ring_ctx *ctx, void __user *arg)
if (napi.pad[0] || napi.pad[1] || napi.pad[2] || napi.resv)
return -EINVAL;
- WRITE_ONCE(ctx->napi_busy_poll_to, napi.busy_poll_to);
- WRITE_ONCE(ctx->napi_prefer_busy_poll, !!napi.prefer_busy_poll);
-
if (copy_to_user(arg, &curr, sizeof(curr)))
return -EFAULT;
+ WRITE_ONCE(ctx->napi_busy_poll_to, napi.busy_poll_to);
+ WRITE_ONCE(ctx->napi_prefer_busy_poll, !!napi.prefer_busy_poll);
+ WRITE_ONCE(ctx->napi_enabled, true);
return 0;
}
@@ -256,6 +256,7 @@ int io_unregister_napi(struct io_ring_ctx *ctx, void __user *arg)
WRITE_ONCE(ctx->napi_busy_poll_to, 0);
WRITE_ONCE(ctx->napi_prefer_busy_poll, false);
+ WRITE_ONCE(ctx->napi_enabled, true);
return 0;
}
@@ -300,7 +301,7 @@ void __io_napi_busy_loop(struct io_ring_ctx *ctx, struct io_wait_queue *iowq)
{
iowq->napi_prefer_busy_poll = READ_ONCE(ctx->napi_prefer_busy_poll);
- if (!(ctx->flags & IORING_SETUP_SQPOLL) && iowq->napi_busy_poll_to)
+ if (!(ctx->flags & IORING_SETUP_SQPOLL) && ctx->napi_enabled)
io_napi_blocking_busy_loop(ctx, iowq);
}
--
Jens Axboe
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] io_uring/napi: enable even with a timeout of 0
2024-02-15 22:32 [PATCH] io_uring/napi: enable even with a timeout of 0 Jens Axboe
@ 2024-02-15 22:37 ` David Wei
2024-02-15 22:37 ` Jens Axboe
0 siblings, 1 reply; 3+ messages in thread
From: David Wei @ 2024-02-15 22:37 UTC (permalink / raw)
To: Jens Axboe, io-uring
On 2024-02-15 15:32, Jens Axboe wrote:
> 1 usec is not as short as it used to be, and it makes sense to allow 0
> for a busy poll timeout - this means just do one loop to check if we
> have anything available. Add a separate ->napi_enabled to check if napi
> has been enabled or not.
>
> While at it, move the writing of the ctx napi values after we've copied
> the old values back to userspace. This ensures that if the call fails,
> we'll be in the same state as we were before, rather than some
> indeterminate state.
>
> Signed-off-by: Jens Axboe <[email protected]>
>
> ---
>
> diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
> index 4fe7af8a4907..bd7071aeec5d 100644
> --- a/include/linux/io_uring_types.h
> +++ b/include/linux/io_uring_types.h
> @@ -420,6 +420,7 @@ struct io_ring_ctx {
> /* napi busy poll default timeout */
> unsigned int napi_busy_poll_to;
> bool napi_prefer_busy_poll;
> + bool napi_enabled;
>
> DECLARE_HASHTABLE(napi_ht, 4);
> #endif
> diff --git a/io_uring/napi.c b/io_uring/napi.c
> index b234adda7dfd..e653927a376e 100644
> --- a/io_uring/napi.c
> +++ b/io_uring/napi.c
> @@ -227,12 +227,12 @@ int io_register_napi(struct io_ring_ctx *ctx, void __user *arg)
> if (napi.pad[0] || napi.pad[1] || napi.pad[2] || napi.resv)
> return -EINVAL;
>
> - WRITE_ONCE(ctx->napi_busy_poll_to, napi.busy_poll_to);
> - WRITE_ONCE(ctx->napi_prefer_busy_poll, !!napi.prefer_busy_poll);
> -
> if (copy_to_user(arg, &curr, sizeof(curr)))
> return -EFAULT;
>
> + WRITE_ONCE(ctx->napi_busy_poll_to, napi.busy_poll_to);
> + WRITE_ONCE(ctx->napi_prefer_busy_poll, !!napi.prefer_busy_poll);
> + WRITE_ONCE(ctx->napi_enabled, true);
> return 0;
> }
>
> @@ -256,6 +256,7 @@ int io_unregister_napi(struct io_ring_ctx *ctx, void __user *arg)
>
> WRITE_ONCE(ctx->napi_busy_poll_to, 0);
> WRITE_ONCE(ctx->napi_prefer_busy_poll, false);
> + WRITE_ONCE(ctx->napi_enabled, true);
Should this be false?
> return 0;
> }
>
> @@ -300,7 +301,7 @@ void __io_napi_busy_loop(struct io_ring_ctx *ctx, struct io_wait_queue *iowq)
> {
> iowq->napi_prefer_busy_poll = READ_ONCE(ctx->napi_prefer_busy_poll);
>
> - if (!(ctx->flags & IORING_SETUP_SQPOLL) && iowq->napi_busy_poll_to)
> + if (!(ctx->flags & IORING_SETUP_SQPOLL) && ctx->napi_enabled)
> io_napi_blocking_busy_loop(ctx, iowq);
> }
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] io_uring/napi: enable even with a timeout of 0
2024-02-15 22:37 ` David Wei
@ 2024-02-15 22:37 ` Jens Axboe
0 siblings, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2024-02-15 22:37 UTC (permalink / raw)
To: David Wei, io-uring
On 2/15/24 3:37 PM, David Wei wrote:
> On 2024-02-15 15:32, Jens Axboe wrote:
>> 1 usec is not as short as it used to be, and it makes sense to allow 0
>> for a busy poll timeout - this means just do one loop to check if we
>> have anything available. Add a separate ->napi_enabled to check if napi
>> has been enabled or not.
>>
>> While at it, move the writing of the ctx napi values after we've copied
>> the old values back to userspace. This ensures that if the call fails,
>> we'll be in the same state as we were before, rather than some
>> indeterminate state.
>>
>> Signed-off-by: Jens Axboe <[email protected]>
>>
>> ---
>>
>> diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
>> index 4fe7af8a4907..bd7071aeec5d 100644
>> --- a/include/linux/io_uring_types.h
>> +++ b/include/linux/io_uring_types.h
>> @@ -420,6 +420,7 @@ struct io_ring_ctx {
>> /* napi busy poll default timeout */
>> unsigned int napi_busy_poll_to;
>> bool napi_prefer_busy_poll;
>> + bool napi_enabled;
>>
>> DECLARE_HASHTABLE(napi_ht, 4);
>> #endif
>> diff --git a/io_uring/napi.c b/io_uring/napi.c
>> index b234adda7dfd..e653927a376e 100644
>> --- a/io_uring/napi.c
>> +++ b/io_uring/napi.c
>> @@ -227,12 +227,12 @@ int io_register_napi(struct io_ring_ctx *ctx, void __user *arg)
>> if (napi.pad[0] || napi.pad[1] || napi.pad[2] || napi.resv)
>> return -EINVAL;
>>
>> - WRITE_ONCE(ctx->napi_busy_poll_to, napi.busy_poll_to);
>> - WRITE_ONCE(ctx->napi_prefer_busy_poll, !!napi.prefer_busy_poll);
>> -
>> if (copy_to_user(arg, &curr, sizeof(curr)))
>> return -EFAULT;
>>
>> + WRITE_ONCE(ctx->napi_busy_poll_to, napi.busy_poll_to);
>> + WRITE_ONCE(ctx->napi_prefer_busy_poll, !!napi.prefer_busy_poll);
>> + WRITE_ONCE(ctx->napi_enabled, true);
>> return 0;
>> }
>>
>> @@ -256,6 +256,7 @@ int io_unregister_napi(struct io_ring_ctx *ctx, void __user *arg)
>>
>> WRITE_ONCE(ctx->napi_busy_poll_to, 0);
>> WRITE_ONCE(ctx->napi_prefer_busy_poll, false);
>> + WRITE_ONCE(ctx->napi_enabled, true);
>
> Should this be false?
It should indeed... Updated, thanks!
--
Jens Axboe
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-02-15 22:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-15 22:32 [PATCH] io_uring/napi: enable even with a timeout of 0 Jens Axboe
2024-02-15 22:37 ` David Wei
2024-02-15 22:37 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox