Re: Latches with weak memory ordering (Re: max_wal_senders must die)

On Mon, Nov 15, 2010 at 11:12 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
>> In SetLatch, is it enough to add the SpinLockAcquire() call *after*
>> checking that is_set is not already set? Ie. still do the quick exit
>> without holding a lock. Or do we need a memory barrier operation before
>> the fetch, to ensure that we see if the other process has just cleared
>> the flag with ResetLatch() ? Presumable ResetLatch() needs to call
>> SpinLockAcquire() anyway to ensure that other processes see the clearing
>> of the flag.
>
> Hmm ... I just remembered the reason why we didn't use a spinlock in
> these functions already.  Namely, that it's unsafe for a signal handler
> to try to acquire a spinlock that the interrupted code might be holding.
> So I think a bit more thought is needed here.  Maybe we need to bite the
> bullet and do memory barriers ...

The signal handler just checks a process-local, volatile variable
called "waiting" (which should be fine) and then sends a self-pipe
byte.  It deliberately *doesn't* take a spinlock.  So unless I'm
missing something (which is perfectly possible) protecting a few more
things with a spinlock should be safe enough.

Of course, there's still a potential *performance* problem if we end
up doing a kernel call while holding a spin lock, but I'm uncertain
whether we should worry about that.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company