Re: Spinlock assembly cleanup

Sounds good to me.  Consistencyis important because it lets us fix
problems across all cpu types.  I am not 100% excited about the memory
part because it invalidates all register memory values, not just the
shared memory location.  We are specifically accessing a memory address
as part of the ASM, so I don't see how it could pull that memory value
from a register behind our back.

---------------------------------------------------------------------------

Tom Lane wrote:
> Pursuant to the gripes raised by Martin Pitt ---
> 
> I've consulted some gcc experts within Red Hat and come to the following
> conclusions:
> 
> * We should consistently refer to the spinlock contents via a
> read/write operand declared like "+m"(*lock).  This is consistent
> with longstanding practice in the Linux kernel and therefore is unlikely
> to get broken in future gcc releases.  The existing ports that use
> matched input and output parameters will break, or at least draw nasty
> warnings, in upcoming gcc releases.
> 
> * Not all of the ports currently declare *lock as an input operand,
> but this seems rather dangerous to me; I think all should use "+m".
> 
> * Some but not all the ports list "memory" as a clobbered operand.
> The gcc manual saith
> 
> : If your assembler instruction modifies memory in an unpredictable
> : fashion, add `memory' to the list of clobbered registers.  This will
> : cause GNU CC to not keep memory values cached in registers across the
> : assembler instruction.
> 
> Now as far as I can see, none of the spinlock sequences directly clobber
> any memory other than the spinlock itself, and so (as long as the lock
> is stated to be an output operand) one might think the memory clobber
> marking to be excessive.  However, I am thinking it is actually a good
> idea and we ought to add the marking to all ports, not remove it.  The
> thought is that what we are actually using the spinlock for is to guard
> access to values in shared memory, and therefore the act of waiting for
> a spinlock can be seen as waiting for other memory variables to assume
> values they didn't necessarily have last time we looked.  If gcc caches
> shared variables in registers across a spinlock acquisition, the code is
> broken.
> 
> The alternative to doing this would be to always use volatile pointers
> to access shared memory, but I don't want to do that --- in the first
> place it's notationally cumbersome, and in the second place it would
> hurt performance unnecessarily.  Within straight-line code that holds a
> spinlock there is no reason to treat shared memory as volatile.  It's
> only when crossing a spinlock boundary that you must reload from memory,
> and that seems to be exactly what the "memory" modifier declares for us.
> 
> (I am assuming here that marking the asm fragment "volatile" does not
> necessarily do what the "memory" modifier does; I can't see anything in
> the gcc docs that claims "volatile" includes the effects of "memory".)
> 
> So I'd like to make all the gcc-asm fragments for spinlocks follow these
> rules.  Comments?
> 
>             regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
> 
>                http://www.postgresql.org/docs/faqs/FAQ.html
> 

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073