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. -- 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