Simon Riggs wrote: > On Sun, 2010-04-18 at 08:24 +0100, Simon Riggs wrote: >> On Sat, 2010-04-17 at 18:52 -0400, Tom Lane wrote: >>> Simon Riggs <si...@2ndquadrant.com> writes: >>>> What I'm not clear on is why you've used a spinlock everywhere when only >>>> weak-memory thang CPUs are a problem. Why not have a weak-memory-protect >>>> macro that does does nada when the hardware already protects us? (i.e. a >>>> spinlock only for the hardware that needs it). >>> Well, we could certainly consider that, if we had enough places where >>> there was a demonstrable benefit from it. I couldn't measure any real >>> slowdown from adding a spinlock in that sinval code, so I didn't propose >>> doing so at the time --- and I'm pretty dubious that this code is >>> sufficiently performance-critical to justify the work, either. >> OK, I'll put a spinlock around access to the head of the array. > > v2 patch attached
The locking seems overly complex to me. Looking at KnownAssignedXidsAdd(), isn't it possible for two processes to call it concurrently in exclusive_lock==false mode and get the same 'head' value, and step on each others toes? I guess KnownAssignedXidsAdd() is only called by the startup process, but it seems like an accident waiting to happen. Spinlocks are fast, if you have to add an if-branch to decide whether to acquire it, I suspect you've lost any performance gain to be had already. Let's keep it simple. And acquiring ProcArrayLock in exclusive mode while adding entries to the array seems OK to me as well. It only needs to be held very briefly, and getting this correct and keeping it simple is much more important at this point than squeezing out every possible CPU cycle from the system. Just require that the caller holds ProcArrayLock in exclusive mode when calling an operation that modifies the array, and in shared mode when reading it. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers