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 <axboe@xxxxxxxxx>
>
> ---
>
> 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);
> }
>
