rqspinlock: Enclose lock/unlock within lock entry acquisitions #6419
+43
−38
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We currently have a tiny window between the fast-path cmpxchg and the grabbing of the lock entry where an NMI could land, attempt the same lock that was just acquired, and end up timing out. This is not ideal. Instead, move the lock entry acquisition from the fast path to before the cmpxchg, and remove the grabbing of the lock entry in the slow path, assuming it was already taken by the fast path. There is a similar case when unlocking the lock. If the NMI lands between the WRITE_ONCE and smp_store_release, it is possible that we end up in a situation where the NMI fails to diagnose the AA condition, leading to a timeout. The TAS fallback is invoked directly without being preceded by the typical fast path, therefore we must continue to grab the deadlock detection entry in that case. Note the changes to the comments in release_held_lock_entry and res_spin_unlock. They talk about prevention of the following scenario, which is introduced by this commit, and was avoided by placing smp_store_release after WRITE_ONCE (the case before this commit): grab entry A lock A grab entry B lock B unlock B smp_store_release(B->locked, 0) grab entry B lock B grab entry A lock A ! <detect ABBA> WRITE_ONCE(rqh->locks[rqh->cnt - 1], NULL) If the store release were placed after the WRITE_ONCE, the other CPU would not observe B in the table of the CPU unlocking the lock B. Avoiding this while it was convenient was a prudent choice, but since it leads to missed diagnosis of AA deadlocks in case of NMIs, it does not make sense to keep such ordering any further. Moreover, while this particular schedule is a misdiagnosis, the CPUs are obviously participating in an ABBA deadlock otherwise, and are only lucky to avoid an error before due to the aforementioned race. Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
Author
|
Upstream branch: 22d70d4 |
Author
|
At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=1027534 expired. Closing PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request for series with
subject: rqspinlock: Enclose lock/unlock within lock entry acquisitions
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1027534