Thread: [PATCH] Add support for TAS/S_UNLOCK for aarch64

[PATCH] Add support for TAS/S_UNLOCK for aarch64

From
Pavel Raiskup
Date:
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


Re: [PATCH] Add support for TAS/S_UNLOCK for aarch64

From
Pavel Raiskup
Date:
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




Re: [PATCH] Add support for TAS/S_UNLOCK for aarch64

From
Robert Haas
Date:
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



Re: [PATCH] Add support for TAS/S_UNLOCK for aarch64

From
Tom Lane
Date:
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



Re: [PATCH] Add support for TAS/S_UNLOCK for aarch64

From
Mark Salter
Date:
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





Re: [PATCH] Add support for TAS/S_UNLOCK for aarch64

From
Tom Lane
Date:
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



Re: [PATCH] Add support for TAS/S_UNLOCK for aarch64

From
Tom Lane
Date:
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