Spinlock assembly cleanup
От | Tom Lane |
---|---|
Тема | Spinlock assembly cleanup |
Дата | |
Msg-id | 5988.1087244304@sss.pgh.pa.us обсуждение исходный текст |
Ответы |
Re: Spinlock assembly cleanup
(Bruce Momjian <pgman@candle.pha.pa.us>)
|
Список | pgsql-hackers |
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
В списке pgsql-hackers по дате отправления: