asm-generic: ticket-lock: Optimize arch_spin_value_unlocked

From: Guo Ren <[email protected]>

Using arch_spinlock_is_locked would cause another unnecessary memory access to the contended value. Although it won't cause a significant performance gap in most architectures, the arch_spin_value_unlocked argument contains enough information. Thus, remove unnecessary atomic_read in arch_spin_value_unlocked().

Signed-off-by Guo Ren <[email protected]>

Signed-off-by Guo Ren <[email protected]>

Cc David Laight <[email protected]>

Cc Peter Zijlstra <[email protected]>

---
Changelog:
This patch is separate from:
https://lore.kernel.org/linux-riscv/[email protected]/

Peter & David have commented on it:
https://lore.kernel.org/linux-riscv/[email protected]/

---
  include/asm-generic/spinlock.h | 16 +++++++++-------
  1 file changed, 9 insertions(+), 7 deletions(-)

include/asm-generic/spinlock.h +9 -2

index fdfebcb050f4..90803a826ba0 100644
@@ -68,11 +68,18 @@ static __always_inline void arch_spin_unlock(arch_spinlock_t *lock)
  	smp_store_release(ptr, (u16)val + 1);
  }
 
+static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
+{
+	u32 val = lock.counter;
+
+	return ((val >> 16) == (val & 0xffff));
+}
+
  static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock)
  {
-	u32 val = atomic_read(lock);
+	arch_spinlock_t val = READ_ONCE(*lock);
 
-	return ((val >> 16) != (val & 0xffff));
+	return !arch_spin_value_unlocked(val);
  }
 
  static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock)

@@ -82,11 +89,6 @@ static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock)
  	return (s16)((val >> 16) - (val & 0xffff)) > 1;

Quoted 7 lines · On 7/19/23 03:00, [email protected] wrote:

From: Guo Ren <[email protected]>

Using arch_spinlock_is_locked would cause another unnecessary memory access to the contended value. Although it won't cause a significant performance gap in most architectures, the arch_spin_value_unlocked argument contains enough information. Thus, remove unnecessary atomic_read in arch_spin_value_unlocked().

AFAICS, only one memory access is needed for the current arch_spinlock_is_locked(). So your description isn't quite right. OTOH,

caller of arch_spin_value_unlocked() could benefit from this change. Currently, the only caller is lockref.

Other than that, the patch looks good to me.

Cheers,
Longman

Quoted 46 lines

Signed-off-by Guo Ren <[email protected]>

Signed-off-by Guo Ren <[email protected]>

Cc David Laight <[email protected]>

Cc Peter Zijlstra <[email protected]>

---
Changelog:
This patch is separate from:
https://lore.kernel.org/linux-riscv/[email protected]/

Peter & David have commented on it:
https://lore.kernel.org/linux-riscv/[email protected]/

---
  include/asm-generic/spinlock.h | 16 +++++++++-------
  1 file changed, 9 insertions(+), 7 deletions(-)

include/asm-generic/spinlock.h +9 -7

index fdfebcb050f4..90803a826ba0 100644
@@ -68,11 +68,18 @@ static __always_inline void arch_spin_unlock(arch_spinlock_t *lock)
  	smp_store_release(ptr, (u16)val + 1);
  }
+static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
+{
+	u32 val = lock.counter;
+
+	return ((val >> 16) == (val & 0xffff));
+}
+
  static __always_inline int arch_spin_is_locked(arch_spinlock_t *lock)
  {
-	u32 val = atomic_read(lock);
+	arch_spinlock_t val = READ_ONCE(*lock);
-	return ((val >> 16) != (val & 0xffff));
+	return !arch_spin_value_unlocked(val);
  }
  static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock)

@@ -82,11 +89,6 @@ static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock)
  	return (s16)((val >> 16) - (val & 0xffff)) > 1;
  }
-static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
-{
-	return !arch_spin_is_locked(&lock);
-}
-

include/asm-generic/spinlock.h static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock) · line 94

Quoted 2 lines

include/asm-generic/spinlock.h +0 -0

  #include <asm/qrwlock.h>
  #endif /* __ASM_GENERIC_SPINLOCK_H */

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv