On 8/11/24 7:00 PM, Olivier Langlois wrote:
> On Sun, 2024-08-11 at 20:34 -0400, Olivier Langlois wrote:
>> io_napi_entry() has 2 calling sites. One of them is unlikely to find
>> an
>> entry and if it does, the timeout should arguable not be updated.
>>
>> The other io_napi_entry() calling site is overwriting the update made
>> by io_napi_entry() so the io_napi_entry() timeout value update has no
>> or
>> little value and therefore is removed.
>>
>> Signed-off-by: Olivier Langlois <olivier@xxxxxxxxxxxxxx>
>> ---
>> io_uring/napi.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/io_uring/napi.c b/io_uring/napi.c
>> index 73c4159e8405..1de1d4d62925 100644
>> --- a/io_uring/napi.c
>> +++ b/io_uring/napi.c
>> @@ -26,7 +26,6 @@ static struct io_napi_entry
>> *io_napi_hash_find(struct hlist_head *hash_list,
>> hlist_for_each_entry_rcu(e, hash_list, node) {
>> if (e->napi_id != napi_id)
>> continue;
>> - e->timeout = jiffies + NAPI_TIMEOUT;
>> return e;
>> }
>>
> I am commenting my own patch because I found something curious that I
> was not sure about when I was reviewing the code.
>
> Should the remaining e->timeout assignation be wrapped with a
> WRITE_ONCE() macro to ensure an atomic store?
I think that makes sense to do as lookup can be within rcu, and
hence we have nothing serializing it. Not for torn writes, but to
ensure that the memory sanitizer doesn't complain. I can just make
this change while applying, or send a v2.
--
Jens Axboe
