Horton

Thread

Commits

  1. Allow use of __sync_lock_test_and_set for spinlocks on any machine.

  1. spinlock support on loongarch64

    吴亚飞 <wuyf41619@hundsun.com> — 2022-11-02T05:56:36Z

    add spinlock support on loongarch64.

  2. Re: spinlock support on loongarch64

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-11-02T15:37:35Z

    =?gb2312?B?zuLRx7fJ?= <wuyf41619@hundsun.com> writes:
> add spinlock support on loongarch64.

I wonder if we shouldn't just do that (ie, try to use
__sync_lock_test_and_set) as a generic fallback on any unsupported
architecture.  We could get rid of the separate stanza for RISC-V
that way.  The main thing that an arch-specific stanza could bring
is knowledge of the best data type width to use for a spinlock;
but I don't see a big problem with defaulting to "int".  We can
always add arch-specific stanzas for any machines where that's
shown to be a seriously poor choice.

			regards, tom lane

  3. Re: spinlock support on loongarch64

    Andres Freund <andres@anarazel.de> — 2022-11-02T17:27:06Z

    Hi,

On 2022-11-02 11:37:35 -0400, Tom Lane wrote:
> =?gb2312?B?zuLRx7fJ?= <wuyf41619@hundsun.com> writes:
> > add spinlock support on loongarch64.
> 
> I wonder if we shouldn't just do that (ie, try to use
> __sync_lock_test_and_set) as a generic fallback on any unsupported
> architecture.  We could get rid of the separate stanza for RISC-V
> that way.  The main thing that an arch-specific stanza could bring
> is knowledge of the best data type width to use for a spinlock;
> but I don't see a big problem with defaulting to "int".  We can
> always add arch-specific stanzas for any machines where that's
> shown to be a seriously poor choice.

Yes, please. It might not be perfect for all architectures, and it might not
be good for some very old architectures. But for anything new it'll be vastly
better than not having spinlocks at all.

Greetings,

Andres Freund

  4. Re: spinlock support on loongarch64

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-11-02T18:29:44Z

    Andres Freund <andres@anarazel.de> writes:
> On 2022-11-02 11:37:35 -0400, Tom Lane wrote:
>> I wonder if we shouldn't just do that (ie, try to use
>> __sync_lock_test_and_set) as a generic fallback on any unsupported
>> architecture.  We could get rid of the separate stanza for RISC-V
>> that way.  The main thing that an arch-specific stanza could bring
>> is knowledge of the best data type width to use for a spinlock;
>> but I don't see a big problem with defaulting to "int".  We can
>> always add arch-specific stanzas for any machines where that's
>> shown to be a seriously poor choice.

> Yes, please. It might not be perfect for all architectures, and it might not
> be good for some very old architectures. But for anything new it'll be vastly
> better than not having spinlocks at all.

So about like this, then.

			regards, tom lane

  5. Re: spinlock support on loongarch64

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-11-02T18:55:04Z

    I wrote:
> So about like this, then.

After actually testing (by removing the ARM stanza on a macOS machine),
it seems that placement doesn't work, because of the default definition
of S_UNLOCK at the bottom of the "#if defined(__GNUC__)" stuff.  Putting
it inside that test works, and seems like it should be fine, since this
is a GCC-ism.

			regards, tom lane

  6. Re: spinlock support on loongarch64

    Andres Freund <andres@anarazel.de> — 2022-11-02T21:04:52Z

    Hi,

On 2022-11-02 14:55:04 -0400, Tom Lane wrote:
> I wrote:
> > So about like this, then.
> 
> After actually testing (by removing the ARM stanza on a macOS machine),
> it seems that placement doesn't work, because of the default definition
> of S_UNLOCK at the bottom of the "#if defined(__GNUC__)" stuff.  Putting
> it inside that test works, and seems like it should be fine, since this
> is a GCC-ism.

Looks reasonable. I tested it on x86-64 by disabling that section and it
works.

FWIW, In a heavily spinlock-contending workload it's a tad slower, largely due
to to loosing spin_delay. If I define that it's very close. Not that it
matters hugely, I just thought it'd be good to validate.

I wonder if it's worth keeing the full copy of this in the arm section? We
could just define SPIN_DELAY() for aarch64?

Greetings,

Andres Freund

  7. Re: spinlock support on loongarch64

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-11-02T21:37:04Z

    Andres Freund <andres@anarazel.de> writes:
> On 2022-11-02 14:55:04 -0400, Tom Lane wrote:
>> After actually testing (by removing the ARM stanza on a macOS machine),
>> it seems that placement doesn't work, because of the default definition
>> of S_UNLOCK at the bottom of the "#if defined(__GNUC__)" stuff.  Putting
>> it inside that test works, and seems like it should be fine, since this
>> is a GCC-ism.

> Looks reasonable. I tested it on x86-64 by disabling that section and it
> works.

Thanks for looking.

> I wonder if it's worth keeing the full copy of this in the arm section? We
> could just define SPIN_DELAY() for aarch64?

I thought about that, but given the increasing popularity of ARM
I bet that that stanza is going to accrete more special-case knowledge
over time.  It's probably simplest to keep it separate.

			regards, tom lane

  8. Re: spinlock support on loongarch64

    Andres Freund <andres@anarazel.de> — 2022-11-02T23:22:16Z

    On 2022-11-02 17:37:04 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2022-11-02 14:55:04 -0400, Tom Lane wrote:
> >> After actually testing (by removing the ARM stanza on a macOS machine),
> >> it seems that placement doesn't work, because of the default definition
> >> of S_UNLOCK at the bottom of the "#if defined(__GNUC__)" stuff.  Putting
> >> it inside that test works, and seems like it should be fine, since this
> >> is a GCC-ism.
> 
> > Looks reasonable. I tested it on x86-64 by disabling that section and it
> > works.
> 
> Thanks for looking.
> 
> > I wonder if it's worth keeing the full copy of this in the arm section? We
> > could just define SPIN_DELAY() for aarch64?
> 
> I thought about that, but given the increasing popularity of ARM
> I bet that that stanza is going to accrete more special-case knowledge
> over time.  It's probably simplest to keep it separate.

WFM.