Thread: [PATCH] Add support for TAS/S_UNLOCK for aarch64
Hi, I was asked [1] to add following patch downstream, could it be considered upstream also? Thanks, Pavel. [1] https://bugzilla.redhat.com/show_bug.cgi?id=970661
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. > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=970661 Oh, I see now it was already consulted here: http://www.postgresql.org/message-id/1368448758.23422.12.camel@t520.redhat.com Sorry for noise, it can be discussed there. Pavel
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. >> >> [1] https://bugzilla.redhat.com/show_bug.cgi?id=970661 > > Oh, I see now it was already consulted here: > > http://www.postgresql.org/message-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
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: >> http://www.postgresql.org/message-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
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: > >> http://www.postgresql.org/message-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. 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
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
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