[PATCH] Implements SPIN_LOCK on ARM

Started by David CARLIERabout 5 years ago2 messages
#1David CARLIER
devnexen@gmail.com
1 attachment(s)

Hi here a little update proposal for ARM architecture.

Kind regards.

Attachments:

0001-Implements-SPIN_LOCK-macro-for-ARM.patchapplication/octet-stream; name=0001-Implements-SPIN_LOCK-macro-for-ARM.patchDownload
From 89b310c5192e75cfd1844c324cb8a92e2bc414c8 Mon Sep 17 00:00:00 2001
From: David Carlier <devnexen@gmail.com>
Date: Fri, 18 Dec 2020 21:48:35 +0000
Subject: [PATCH] Implements SPIN_LOCK macro for ARM.

using yield instruction which is the x86's "pause" equivalent.

Signed-off-by: David Carlier <devnexen@gmail.com>
---
 src/include/storage/s_lock.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index 31a5ca6fb3..b4d1d1f515 100644
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -336,6 +336,29 @@ tas(volatile slock_t *lock)
 
 #define S_UNLOCK(lock) __sync_lock_release(lock)
 
+#if defined(__aarch64__) || __ARM_ARCH__ >= 7
+
+#define SPIN_LOCK spin_delay
+
+static __inline__ void
+spin_delay(void)
+{
+	/*
+	 * With spin locks, the yield hint
+	 * improves CPU usage and performance
+	 * on ARM hardwares implemented on 64 bits
+	 * but is a NOP on ARM11 for example thus
+	 * enabling this instruction when useful.
+	 */
+	__asm__ __volatile__(
+		"       yield     \n"
+:
+:
+:		"memory");
+
+}
+
+#endif   /* __aarch64__ || __ARM_ARCH__ >= 7 */
 #endif	 /* HAVE_GCC__SYNC_INT32_TAS */
 #endif	 /* __arm__ || __arm || __aarch64__ || __aarch64 */
 
-- 
2.20.1

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: David CARLIER (#1)
Re: [PATCH] Implements SPIN_LOCK on ARM

David CARLIER <devnexen@gmail.com> writes:

Hi here a little update proposal for ARM architecture.

This sort of thing is not a "little proposal" where you can just
send in an unsupported patch and expect it to be accepted.
You need to provide some evidence that (a) it actually does anything
useful and (b) it isn't a net loss on some ARM architectures.

For comparison's sake, see

/messages/by-id/CAB10pyamDkTFWU_BVGeEVmkc8=EhgCjr6QBk02SCdJtKpHkdFw@mail.gmail.com

where we still haven't pulled the trigger despite a great deal
more than zero testing.

FWIW, some casual googling suggests that ARM "yield" is not
all that much like x86 "pause": it supposedly encourages
the system to swap control away from the thread altogether,
exactly what we *don't* want in a spinloop. So I'm a little
doubtful whether there's a case to be made for this at all.
But for sure, you haven't tried to make a case.

regards, tom lane