On 2014-01-05 12:56:05 -0500, Robert Haas wrote:
> Right now, storing spinlocks in dynamic shared memory *almost* works,
> but there are problems with --disable-spinlocks. In that
> configuration, we use semaphores to simulate spinlocks. Every time
> someone calls SpinLockInit(), it's going to allocate a new semaphore
> which will never be returned to the operating system, so you're pretty
> quickly going to run out. There are a couple of things we could do
> about this:
> 4. Drop support for --disable-spinlocks.
I very strongly vote 4). I think we're going to hit this more and more
often and it's a facility that benefits almost nobody. Just about every
new platform will be/is on gcc or clang and you can just duplicate the
compiler provided generic implementation we have for arm for there.
The atomics implementation make this an automatic fallback if there's
no compiler specific variant around.
> 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.
Agreed.
> 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.
My primary reason isn't dsm TBH but wanting to embed the buffer lwlocks
in the bufferdesc, on the same cacheline as the buffer headers
spinlock. All the embedded ones can be allocated without padding, while
the relatively low number of non-embedded ones can be padded to the full
cacheline size.
> A bigger problem is that I think we
> want to avoid having a large amount of notational churn. The obvious
> way to do that is to get rid of the LWLockId array and instead declare
> each fixed LWLock separately as e.g. LWLock *ProcArrayLock. However,
> creating a large number of new globals that will need to be
> initialized in every new EXEC_BACKEND process seems irritating. So
> maybe a better idea is to do something like this:
> #define BufFreelistLock (&fixedlwlocks[0])
> #define ShmemIndexLock (&fixedlwlocks[1])
> ...
> #define AutoFileLock (&fixedlwlocks[36])
> #define NUM_FIXED_LWLOCKS 37
>
> Comments, suggestions?
My idea here was to just have two APIs, a legacy one that works like the
current one, and a new one that locks the lwlocks passed in by a
pointer. After all, LWLockAssign() which you use in extensions currently
returns a LWLockId. Seems ugly to turn that into a pointer.
But perhaps your idea is better anyway, no matter the hackery of turning
LWLockId into a pointer.
Greetings,
Andres Freund