On Wed, Aug 28, 2024 at 01:19:46PM GMT, Nicholas Piggin wrote: <snip>
What probably makes it really difficult to hit is that I think both locks A and B need contention from other sources to push them into queueing slow path. I guess that's omitted for brevity in the flow above, which is fine.
I'll mention that in the commit message, just so that it is clear.
AFAIKS this fix works.
There is one complication which is those two stores could be swapped by the compiler. So we could take an IRQ here that sees the node has been freed, but node->lock has not yet been cleared. Basically equivalent to the problem solved by the barrier() on the count++ side.
This reordering would not cause a problem in your scenario AFAIKS because when the lock call returns, node->lock *will* be cleared so it can not cause a problem later.
Still, should we put a barrier() between these just to make things a bit cleaner? I.e., when count is decremented, we definitely won't do any other stores to node. Otherwise,
Agree, that will make it cleaner.
Reviewed-by: Nicholas Piggin npiggin@gmail.com
Thanks, Nick
Thanks for the review Nick, I'll send a v2 with these changes.
Regards --Nysal