Skip to contentSkip to conversation

MCS spinlock: Use smp_cond_load_acquire()

PATCH In review Jason Low 2 participants 4 messages Apr 13, 2016 – Apr 14, 2016 Locking primitives

lore
Share & tools
lore

Jason Low

author
#0 lore raw

PATCH v2 1/1 MCS spinlock: Use smp_cond_load_acquire()

For qspinlocks on ARM64, we would like to use WFE instead of purely spinning. Qspinlocks internally have lock contenders spin on an MCS lock.

Update arch_mcs_spin_lock_contended() such that it uses the new smp_cond_load_acquire() so that ARM64 can also override this spin loop with its own implementation using WFE.

On x86, it can also cheaper to use this than spinning on smp_load_acquire().

---
Changes from v1:
- Pass l instead of &l to smp_cond_load_acquire() since l is already a pointer to the lock variable.

 kernel/locking/mcs_spinlock.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
kernel/locking/mcs_spinlock.h +4 -4
index c835270..1c55987 100644
@@ -22,13 +22,13 @@ struct mcs_spinlock {
 
 #ifndef arch_mcs_spin_lock_contended
 /*
- * Using smp_load_acquire() provides a memory barrier that ensures
- * subsequent operations happen after the lock is acquired.
+ * Using smp_cond_load_acquire() provides the acquire semantics
+ * required so that subsequent operations happen after the
+ * lock is acquired.
  */
 #define arch_mcs_spin_lock_contended(l)					\
 do {									\
-	while (!(smp_load_acquire(l)))					\
-		cpu_relax_lowlatency();					\
+	smp_cond_load_acquire(l, VAL);					\
 } while (0)
 #endif
 
Signature · 1 line
2.1.4
#1 lore raw
Quoted 4 lines · Jason Low

For qspinlocks on ARM64, we would like to use WFE instead of purely spinning. Qspinlocks internally have lock contenders spin on an MCS lock.

Quoted 8 lines

Update arch_mcs_spin_lock_contended() such that it uses the new smp_cond_load_acquire() so that ARM64 can also override this spin loop with its own implementation using WFE.

On x86, it can also cheaper to use this than spinning on smp_load_acquire().

FWIW, we just override arch_mcs_spin_lock_contended entirely for arch/arm/ and use wfe there so we could do the same for arm64 in mainline already.

Will

author
#2 lore raw
Quoted 9 lines · Will Deacon
Quoted 8 lines · Jason Low

For qspinlocks on ARM64, we would like to use WFE instead of purely spinning. Qspinlocks internally have lock contenders spin on an MCS lock.

Update arch_mcs_spin_lock_contended() such that it uses the new smp_cond_load_acquire() so that ARM64 can also override this spin loop with its own implementation using WFE.

Quoted 7 lines
Quoted 4 lines

On x86, it can also cheaper to use this than spinning on smp_load_acquire().

FWIW, we just override arch_mcs_spin_lock_contended entirely for arch/arm/ and use wfe there so we could do the same for arm64 in mainline already.

Right, I was also thinking about that, although when we use smp_cond_load_acquire() in the generic implementation, would we just end up overriding it for the arch/arm64 version with the same thing? :)

Jason

#3 lore raw
Quoted 13 lines · Jason Low
Quoted 12 lines · Will Deacon
Quoted 11 lines · Jason Low

For qspinlocks on ARM64, we would like to use WFE instead of purely spinning. Qspinlocks internally have lock contenders spin on an MCS lock.

Update arch_mcs_spin_lock_contended() such that it uses the new smp_cond_load_acquire() so that ARM64 can also override this spin loop with its own implementation using WFE.

On x86, it can also cheaper to use this than spinning on smp_load_acquire().

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

FWIW, we just override arch_mcs_spin_lock_contended entirely for arch/arm/ and use wfe there so we could do the same for arm64 in mainline already.

Right, I was also thinking about that, although when we use smp_cond_load_acquire() in the generic implementation, would we just end up overriding it for the arch/arm64 version with the same thing? :)

Sure, and we can convert to smp_cond_load_acquire then. I was just thinking that you can avoid the immediate dependency on Peter's stuff, that's all.

Will