Thread (5 messages) 5 messages, 3 authors, 2024-08-28

Re: [PATCH] powerpc/qspinlock: Fix deadlock in MCS queue

From: Nysal Jan K.A. <hidden>
Date: 2024-08-28 04:28:16
Also in: lkml, stable

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help