Similar to commit b4b29f94856a ("locking/osq: Fix ordering of node initialisation in osq_lock") the use of xchg_acquire() is fundamentally broken with MCS like constructs. Furthermore, it turns out we rely on the global transitivity of this operation because the unlock path observes the pointer with a READ_ONCE(), not an smp_load_acquire(). This is non-critical because the MCS code isn't actually used and mostly serves as documentation, a stepping stone to the more complex things we've build on top of the idea.
Unified · Split
| @@ -67,7 +67,13 @@ void mcs_spin_lock(struct mcs_spinlock **lock, struct mcs_spinlock *node) · 66 unmodified lines | |||
| 67 | node->locked = 0; | 67 | node->locked = 0; |
| 68 | node->next = NULL; | 68 | node->next = NULL; |
| 69 | 69 | ||
| 70 | prev = xchg_acquire(lock, node); | 70 | /* |
| 71 | * We rely on the full barrier with global transitivity implied by the | ||
| 72 | * below xchg() to order the initialization stores above against any | ||
| 73 | * observation of @node. And to provide the ACQUIRE ordering associated | ||
| 74 | * with a LOCK primitive. | ||
| 75 | */ | ||
| 76 | prev = xchg(lock, node); | ||
| 71 | if (likely(prev == NULL)) { | 77 | if (likely(prev == NULL)) { |
| 72 | /* | 78 | /* |
| 73 | * Lock acquired, don't need to set node->locked to 1. Threads | 79 | * Lock acquired, don't need to set node->locked to 1. Threads |