Peter Eisentraut <[EMAIL PROTECTED]> writes: > The SuSE PPC guru said that the PPC spinlock code we currently use may > behave erroneously on multiprocessor systems. Attached is the proposed > patch, suggested for inclusion in 7.4. Comments?
I looked into this some more. The current CVS tip is identical to the TAS code used in 7.3.* except for being gcc inlined assembler instead of an out-of-line subroutine. The 7.3 code is known to work, cf http://archives.postgresql.org/pgsql-bugs/2002-09/msg00252.php Given that testing and the lack of any bug reports against 7.3.*, I think the burden of proof is on the person who thinks we should change it. AFAICS the proposed change for TAS() simply amounts to reversing the sense of the test following stwcx., so that the "success" path corresponds to "branch not taken" rather than "branch taken". I cannot see anything in the IBM PPC architecture manual http://www-3.ibm.com/chips/techlib/techlib.nsf/techdocs/852569B20050FF778525699600682CC7 to justify thinking that this changes anything. If there is any difference in behavior then I'd think that having the isync in the forward branch path is safer than not. The TAS example in the manual (p. 398) looks like loop: lwarx ... ... stwcx. ... bne loop isync which might be read as saying that the isync should be in the "fall through" path, but I think it is more correctly read as putting the isync in the "not predicted to be taken" path. Branch backward will be statically predicted to be taken, branch forward not. In any case there's no mention here of needing to code the branch in one particular way. The proposed change from sync to lwsync during S_UNLOCK is interesting, but we have to keep in mind that that is *not* a bug fix but an attempt at performance improvement --- lwsync is a weaker constraint than sync. I am not convinced that this change is safe for our usage, and I think it would be folly to stick it into 7.4 at the RC stage of the cycle. In short, my vote is "show me why" for the TAS change, and "no way for 7.4, but we can look at it later" for the S_UNLOCK change. regards, tom lane ---------------------------(end of broadcast)--------------------------- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
