On Mon, Aug 22, 2022 at 05:51:00PM +0300, Vitaliy Makkoveev wrote: > On Mon, Aug 22, 2022 at 03:59:39PM +0200, Alexander Bluhm wrote: > > On Sun, Aug 21, 2022 at 10:10:53PM +0300, Vitaliy Makkoveev wrote: > > > On Sun, Aug 21, 2022 at 07:19:23PM +0200, Alexander Bluhm wrote: > > > > When testing parallel UDP input, I see contention on the netlock > > > > from the protocol timer. There is a check to avoid loop traversal > > > > if igmp and mld6 are not active. Move the shortcut to avoid grabbing > > > > netlock. This should be save as variables igmp_timers_are_running > > > > and mld_timers_are_running are read atomically. In case we miss > > > > an fast timer due to MP races, we just run it next time. > > > > > > Could we have a commentary like this before unlocked check? The rest of > > > diff looks ok by me. > > > > > > > This should be save as variables igmp_timers_are_running > > > > and mld_timers_are_running are read atomically. In case we miss > > > > an fast timer due to MP races, we just run it next time. > > > > I commited it with comment. I wonder if we should use read once. > > It is an unprotected atomic acces. Read once makes this explicit. > > > > READ_ONCE() is useless here. It will be loaded in any case. But since > this check and `*_timers_are_running' modifications are not serialized, > we could miss this timer run. > > > * Variable igmp_timers_are_running is read atomically. In case we > > * miss a fast timer due to MP races, just run it next time. > > */ > > - if (!igmp_timers_are_running) > > + if (!READ_ONCE(igmp_timers_are_running)) > > return; >
May be it was not clean, I asked to put this comment for mark, that this lockless check was made intentionally and this is not the error.
