Re: [PATCH] io_uring/napi: enable even with a timeout of 0

[Jens Axboe <axboe@kernel.dk>]

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 <axboe@kernel.dk>
>>
>> ---
>>
>> 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