On 2014-08-14 14:37:22 -0400, Robert Haas wrote: > On Thu, Aug 14, 2014 at 2:21 PM, Andres Freund <and...@2ndquadrant.com> wrote: > > On 2014-08-14 14:19:13 -0400, Robert Haas wrote: > >> That's about the idea. However, what you've got there is actually > >> unsafe, because shmem->counter++ is not an atomic operation. It reads > >> the counter (possibly even as two separate 4-byte loads if the counter > >> is an 8-byte value), increments it inside the CPU, and then writes the > >> resulting value back to memory. If two backends do this concurrently, > >> one of the updates might be lost. > > > > All these are only written by one backend, so it should be safe. Note > > that that coding pattern, just without memory barriers, is all over > > pgstat.c > > Ah, OK. If there's a separate slot for each backend, I agree that it's safe. > > We should probably add barriers to pgstat.c, too.
Yea, definitely. I think this is rather borked on "weaker" architectures. It's just that the consequences of an out of date/torn value are rather low, so it's unlikely to be noticed. Imo we should encapsulate the changecount modifications/checks somehow instead of repeating the barriers, Asserts, comments et al everywhere. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
