From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Atomic operations within spinlocks
Date: Fri, 05 Jun 2020 21:01:56 -0400
Message-ID: <1642804.1591405316@sss.pgh.pa.us> (raw)
In-Reply-To: <20200606001926.zin5getvvhqppnm2@alap3.anarazel.de>
References: <1141819.1591208385@sss.pgh.pa.us>
<20200603204542.reuladbmgmjqzoi6@alap3.anarazel.de>
<1253593.1591293439@sss.pgh.pa.us>
<20200604185525.wuyjtkluppj4eddr@alap3.anarazel.de>
<1256701.1591297654@sss.pgh.pa.us>
<20200605023302.g6v3ydozy5txifji@alap3.anarazel.de>
<20200606001926.zin5getvvhqppnm2@alap3.anarazel.de>
Andres Freund <andres@anarazel.de> writes:
> I wrote a patch for this, and when I got around to to testing it, I
> found that our tests currently don't pass when using both
> --disable-spinlocks and --disable-atomics. Turns out to not be related
> to the issue above, but the global barrier support added in 13.
> That *reads* two 64 bit atomics in a signal handler. Which is normally
> fine, but not at all cool when atomics (or just 64 bit atomics) are
> backed by spinlocks. Because we can "self interrupt" while already
> holding the spinlock.
This is the sort of weird platform-specific problem that I'd prefer to
avoid by minimizing our expectations of what spinlocks can be used for.
> I'm not really sure what to do about that issue. The easisest thing
> would probably be to change the barrier generation to 32bit (which
> doesn't have to use locks for reads in any situation).
Yeah, I think we need a hard rule that you can't use a spinlock in
an interrupt handler --- which means no atomics that don't have
non-spinlock implementations on every platform.
At some point I think we'll have to give up --disable-spinlocks;
it's really of pretty marginal use (how often does anyone port PG
to a new CPU type?) and the number of weird interactions it adds
in this area seems like more than it's worth. But of course
requiring 64-bit atomics is still a step too far.
> Randomly noticed while looking at the code:
> uint64 flagbit = UINT64CONST(1) << (uint64) type;
I'm surprised we didn't get any compiler warnings about that.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, andres@anarazel.de, robertmhaas@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Atomic operations within spinlocks
In-Reply-To: <1642804.1591405316@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox