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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers