On Fri, Nov 28, 2014 at 9:07 PM, Fujii Masao <masao.fu...@gmail.com> wrote: > On Wed, Nov 26, 2014 at 4:05 PM, Michael Paquier > <michael.paqu...@gmail.com> wrote: >> On Fri, Aug 15, 2014 at 8:17 PM, Fujii Masao <masao.fu...@gmail.com> wrote: >>> On Fri, Aug 15, 2014 at 3:40 AM, Andres Freund <and...@2ndquadrant.com> >>> wrote: >>>> 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. >>> >>> So what about applying the attached patch first, which adds the macros >>> to load and store the changecount with the memory barries, and changes >>> pgstat.c use them. Maybe this patch needs to be back-patch to at least 9.4? >>> >>> After applying the patch, I will rebase the pg_last_xact_insert_timestamp >>> patch and post it again. >> Hm, what's the status on this patch? The addition of those macros to >> control count increment with a memory barrier seems like a good thing >> at least. > > Thanks for reminding me of that! Barring any objection, I will commit it.
Applied. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
