[PATCH] Add support for TAS/S_UNLOCK for aarch64
Hi, I was asked [1]https://bugzilla.redhat.com/show_bug.cgi?id=970661 to add following patch downstream, could it be
considered upstream also? Thanks, Pavel.
[1]: https://bugzilla.redhat.com/show_bug.cgi?id=970661
From ed791f40aa117d4fc273e4b96d9295ee9571fc96 Mon Sep 17 00:00:00 2001
From: Mark Salter <msalter@redhat.com>
Date: Tue, 4 Jun 2013 17:23:01 +0200
Subject: [PATCH] Add support for TAS/S_UNLOCK for aarch64
---
src/include/storage/s_lock.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index ce45ffe..a2d6f63 100644
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -336,6 +336,25 @@ tas(volatile slock_t *lock)
#endif /* __arm__ */+/*
+ * Use gcc builtins for AArch64.
+ */
+#if defined(__aarch64__) && defined(HAVE_GCC_INT_ATOMICS)
+#define HAS_TEST_AND_SET
+
+#define TAS(lock) tas(lock)
+
+typedef int slock_t;
+
+static __inline__ int
+tas(volatile slock_t *lock)
+{
+ return __sync_lock_test_and_set(lock, 1);
+}
+
+#define S_UNLOCK(lock) __sync_lock_release(lock)
+#endif /* __aarch64__ */
+
/* S/390 and S/390x Linux (32- and 64-bit zSeries) */
#if defined(__s390__) || defined(__s390x__)
#define HAS_TEST_AND_SET
--
1.8.2.1--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tuesday, June 04, 2013 05:28:09 PM Pavel Raiskup wrote:
Hi, I was asked [1] to add following patch downstream, could it be
considered upstream also? Thanks, Pavel.
Oh, I see now it was already consulted here:
/messages/by-id/1368448758.23422.12.camel@t520.redhat.com
Sorry for noise, it can be discussed there.
Pavel
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Jun 4, 2013 at 11:48 AM, Pavel Raiskup <praiskup@redhat.com> wrote:
On Tuesday, June 04, 2013 05:28:09 PM Pavel Raiskup wrote:
Hi, I was asked [1] to add following patch downstream, could it be
considered upstream also? Thanks, Pavel.Oh, I see now it was already consulted here:
/messages/by-id/1368448758.23422.12.camel@t520.redhat.com
Sorry for noise, it can be discussed there.
I think we should go ahead and commit this patch, or some variant of
it. Having a buildfarm machine would be good... but I don't think
that should be a prerequisite for this sort of support. We certainly
have spinlock support for other platforms for which we don't have
buildfarm machines.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
On Tue, Jun 4, 2013 at 11:48 AM, Pavel Raiskup <praiskup@redhat.com> wrote:
Oh, I see now it was already consulted here:
/messages/by-id/1368448758.23422.12.camel@t520.redhat.com
I think we should go ahead and commit this patch, or some variant of
it. Having a buildfarm machine would be good... but I don't think
that should be a prerequisite for this sort of support. We certainly
have spinlock support for other platforms for which we don't have
buildfarm machines.
We got no response to the question of whether it couldn't be merged with
the existing ARM32 code block. I'd prefer not to have essentially
duplicate sections in s_lock.h if it's not necessary.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, 2013-06-04 at 13:40 -0400, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Tue, Jun 4, 2013 at 11:48 AM, Pavel Raiskup <praiskup@redhat.com> wrote:
Oh, I see now it was already consulted here:
/messages/by-id/1368448758.23422.12.camel@t520.redhat.comI think we should go ahead and commit this patch, or some variant of
it. Having a buildfarm machine would be good... but I don't think
that should be a prerequisite for this sort of support. We certainly
have spinlock support for other platforms for which we don't have
buildfarm machines.We got no response to the question of whether it couldn't be merged with
the existing ARM32 code block. I'd prefer not to have essentially
duplicate sections in s_lock.h if it's not necessary.
Of course it could be. I don't think it should be. Aarch64 is a
completely new architecture with faint resemblance to 32bit arm. I went
back and read the previous thread concerning the problems with builtin
gcc atomics with certain tool/hardware combinations. Would it be okay to
add a default implementation based on gcc builtins to be used if no
arch-specific definitions exist? If so, I could work up a patch for
review.
--Mark
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Mark Salter <msalter@redhat.com> writes:
On Tue, 2013-06-04 at 13:40 -0400, Tom Lane wrote:
We got no response to the question of whether it couldn't be merged with
the existing ARM32 code block. I'd prefer not to have essentially
duplicate sections in s_lock.h if it's not necessary.
Of course it could be. I don't think it should be. Aarch64 is a
completely new architecture with faint resemblance to 32bit arm.
OK, fair enough.
I went
back and read the previous thread concerning the problems with builtin
gcc atomics with certain tool/hardware combinations. Would it be okay to
add a default implementation based on gcc builtins to be used if no
arch-specific definitions exist? If so, I could work up a patch for
review.
We had pretty much rejected that concept in the previous discussion,
I thought. In the first place, there is no good reason to assume
that somebody on a nonstandard platform is using gcc, and in the second,
our review of the available info suggested that the implementation
quality of gcc's builtins is ... um ... variable. Blindly falling back
to that on a platform we haven't tested does not seem very safe.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I wrote:
Mark Salter <msalter@redhat.com> writes:
On Tue, 2013-06-04 at 13:40 -0400, Tom Lane wrote:
We got no response to the question of whether it couldn't be merged with
the existing ARM32 code block. I'd prefer not to have essentially
duplicate sections in s_lock.h if it's not necessary.
Of course it could be. I don't think it should be. Aarch64 is a
completely new architecture with faint resemblance to 32bit arm.
OK, fair enough.
Applied in HEAD and 9.2. If there's demand, we could patch further
back, but we don't have aarch64 in config.guess/config.sub before 9.2,
so we'd have to change that too. Given that aarch64 is barely past the
vaporware stage, I'm not sure there's demand to install back-rev
Postgres on it.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers