Re: dynamic shared memory and locks

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: dynamic shared memory and locks
Date: 2014-01-05 19:47:28
Message-ID: 20140105194728.GC18220@alap2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2014-01-05 14:06:52 -0500, Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > For what it's worth, my vote is currently for #2. I can't think of
> > many interesting to do with dynamic shared memory without having at
> > least spinlocks, so I don't think we'd be losing much. #1 seems
> > needlessly unfriendly, #3 seems like a lot of work for not much, and
> > #4 seems excessive at least as a solution to this particular problem,
> > though there may be other arguments for it. What do others think?
>
> I agree with this position. There may be some good reason to drop
> --disable-spinlocks altogether in future, but DSM isn't a sufficient
> excuse.

Agreed that DSM isn't sufficient cause. The reasons for removing it for
future reasons I see are:
* It's not tested at all and it has been partially broken for
significants of time. Afair Heikki just noticed the breakage
accidentally when adding enough new spinlocks recently.
* It's showed up as problematic in a couple of patches while adding not
much value (at least dsm, atomic ops, afair some others)
* It's slow enough that it's not of a practical value.
* Implementing simple support for spinlocks on realistic platforms isn't
hard these days due to compiler intrinsics.
* The platforms that don't have a barrier implementation will rely on
spinlocks. And for correctness those spinlocks should employ
barriers. That might be more of an argument for getting rid of that
fallback tho.

> > I think we're also going to want to be able to create LWLocks in
> > dynamic shared memory: some critical sections won't be short enough or
> > safe enough to be protected by spinlocks. At some level this seems
> > easy: change LWLockAcquire and friends to accept an LWLock * rather
> > than an LWLockId, and similarly change held_lwlocks[] to hold LWLock
> > pointers rather than LWLockIds.
>
> I seem to recall that there was some good reason for keeping all the
> LWLocks in an array, back when the facility was first designed.
> I'm too lazy to research the point right now, but you might want to
> go back and look at the archives around when lwlock.c was written.

Your proposal is at
http://www.postgresql.org/message-id/1054.1001520600@sss.pgh.pa.us -
afaics there hasn't been much discussion about implementation details at
that level.

> In the end, though, that decision was taken before we were concerned
> about being able to add new LWLocks on the fly, which is what this is
> really about (whether they're stored in DSM or not is a secondary point).
> The pressure for that has gotten strong enough that it may be time to
> change the tradeoff.

I personally find the sharing of a cacheline between a buffer headers
spinlock and the lwlock much more interesting than DSM...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Mark Dilger 2014-01-05 19:51:16 Re: In-core regression tests for replication, cascading, archiving, PITR, etc. Michael Paquier
Previous Message Fabrizio Mello 2014-01-05 19:46:14 Re: [PATCH] Support for pg_stat_archiver view