From: Andres Freund <andres@anarazel.de>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Detect double-release of spinlock
Date: Mon, 29 Jul 2024 11:12:19 -0700
Message-ID: <20240729181219.vmrzzo7y52ui3pco@awork3.anarazel.de> (raw)
In-Reply-To: <790b901f-c0f6-4e97-b59a-97d8f921d57e@iki.fi>
References: <E1sYSF2-001lEB-D1@gemulon.postgresql.org>
<2600757.1722267116@sss.pgh.pa.us>
<20240729161846.kgiafbe2dtgabxgb@awork3.anarazel.de>
<2802449.1722270793@sss.pgh.pa.us>
<20240729164026.yurg37tej34o76uk@awork3.anarazel.de>
<20240729165154.56zqyg34x2ywkpsh@awork3.anarazel.de>
<e8d4ef94-b8aa-4cda-8f31-081f78194275@iki.fi>
<2809049.1722273922@sss.pgh.pa.us>
<20240729174853.uo5qjqkxlacjg6vv@awork3.anarazel.de>
<790b901f-c0f6-4e97-b59a-97d8f921d57e@iki.fi>
Hi,
On 2024-07-29 21:00:35 +0300, Heikki Linnakangas wrote:
> On 29/07/2024 20:48, Andres Freund wrote:
> > On 2024-07-29 13:25:22 -0400, Tom Lane wrote:
> > > Heikki Linnakangas <hlinnaka@iki.fi> writes:
> > > > Yeah I'm not worried about that at all. Also, the assert is made when
> > > > you have already released the spinlock; you are already out of the
> > > > critical section.
> > >
> > > Not in the patch Andres posted.
> >
> > Which seems fairly fundamental - once outside of the critical section, we
> > can't actually assert that the lock isn't acquired, somebody else *validly*
> > might have acquired it by then.
>
> You could do:
>
> bool was_free = S_LOCK_FREE(lock);
>
> S_UNLOCK(lock);
> Assert(!was_free);
I don't really see the point - we're about to crash with an assertion failure,
why would we want to do that outside of the critical section? If anything that
will make it harder to debug the issue in a core dump, because other backends
might "destroy evidence" due to being able to acquire the spinlock.
> Depending on the underlying implementation, you could also use
> compare-and-exchange.
That'd scale a lot worse, at least on x86-64, as it requires the unlock to be
an atomic op, whereas today it's a simple store (+ compiler barrier).
I've experimented with replacing all spinlocks with lwlocks, and the fact that
you need an atomic op for an rwlock release is one of the two major reasons
they have a higher overhead (the remainder is boring stuff like the overhead
of external function calls and ownership management).
Greetings,
Andres Freund
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-committers@postgresql.org
Cc: andres@anarazel.de, hlinnaka@iki.fi, tgl@sss.pgh.pa.us, pgsql-hackers@postgresql.org
Subject: Re: Detect double-release of spinlock
In-Reply-To: <20240729181219.vmrzzo7y52ui3pco@awork3.anarazel.de>
* 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