On 8/12/24 12:10 PM, Jens Axboe wrote:
> 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.
As a separate patch I mean, not a v2. That part can wait until 6.12.
--
Jens Axboe
