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: >> On Thu, Aug 14, 2014 at 1:51 PM, Fujii Masao <masao.fu...@gmail.com> wrote: >> > On Fri, Aug 15, 2014 at 1:55 AM, Robert Haas <robertmh...@gmail.com> wrote: >> >> On Mon, Aug 11, 2014 at 3:20 AM, Fujii Masao <masao.fu...@gmail.com> >> >> wrote: >> >>> There is no extra spinlock. >> >> >> >> The version I reviewed had one; that's what I was objecting to. >> > >> > Sorry for confusing you. I posted the latest patch to other thread. >> > This version doesn't use any spinlock. >> > >> > http://www.postgresql.org/message-id/CAHGQGwEwuh5CC3ib6wd0fxs9LAWme=ko09s4moxnynafn7n...@mail.gmail.com >> > >> >> Might need to add some pg_read_barrier() and pg_write_barrier() calls, >> >> since we have those now. >> > >> > Yep, memory barries might be needed as follows. >> > >> > * Set the commit timestamp to shared memory. >> > >> > shmem->counter++; >> > pg_write_barrier(); >> > shmem->timestamp = my_timestamp; >> > pg_write_barrier(); >> > shmem->count++; >> > >> > * Read the commit timestamp from shared memory >> > >> > my_count = shmem->counter; >> > pg_read_barrier(); >> > my_timestamp = shmem->timestamp; >> > pg_read_barrier(); >> > my_count = shmem->counter; >> > >> > Is this way to use memory barriers right? >> >> 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. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
