Re: better atomics - v0.5 - Mailing list pgsql-hackers
| From | Amit Kapila |
|---|---|
| Subject | Re: better atomics - v0.5 |
| Date | |
| Msg-id | CAA4eK1LSzPPiBfJVPC2dLuuvgdgz2sWO_pBwpdkgC=75OP2XeA@mail.gmail.com Whole thread Raw |
| In response to | Re: better atomics - v0.5 (Robert Haas <robertmhaas@gmail.com>) |
| Responses |
Re: better atomics - v0.5
(Martijn van Oosterhout <kleptog@svana.org>)
Re: better atomics - v0.5 (Andres Freund <andres@2ndquadrant.com>) |
| List | pgsql-hackers |
On Wed, Jul 9, 2014 at 8:58 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Tue, Jul 8, 2014 at 2:21 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Thu, Jul 3, 2014 at 10:56 AM, Amit Kapila <amit.kapila16@gmail.com>
> > wrote:
> >
> > Further review of patch:
> > 1.
> > /*
> > * pg_atomic_test_and_set_flag - TAS()
> > *
> > * Acquire/read barrier semantics.
> > */
> > STATIC_IF_INLINE_DECLARE bool
> > pg_atomic_test_set_flag(volatile pg_atomic_flag *ptr);
> >
> > a. I think Acquire and read barrier semantics aren't equal.
> > With acquire semantics, "the results of the operation are available before
> > the
> > results of any operation that appears after it in code" which means it
> > applies
> > for both load and stores. Definition of read barrier just ensures loads.
> >
> > So will be right to declare like above in comments?
>
> Yeah. Barriers can be by the operations they keep from being
> reordered (read, write, or both) and by the direction in which they
> prevent reordering (acquire, release, or both). So in theory there
> are 9 combinations:
>
> read barrier (both directions)
> read barrier (acquire)
> read barrier (release)
> write barrier (both directions)
> write barrier (acquire)
> write barrier (both directions)
> full barrier (both directions)
> full barrier (acquire)
> full barrier (release)
With these definitions, I think it will fall into *full barrier (acquire)*
category as it needs to ensure that no reordering should happen
> On Tue, Jul 8, 2014 at 2:21 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Thu, Jul 3, 2014 at 10:56 AM, Amit Kapila <amit.kapila16@gmail.com>
> > wrote:
> >
> > Further review of patch:
> > 1.
> > /*
> > * pg_atomic_test_and_set_flag - TAS()
> > *
> > * Acquire/read barrier semantics.
> > */
> > STATIC_IF_INLINE_DECLARE bool
> > pg_atomic_test_set_flag(volatile pg_atomic_flag *ptr);
> >
> > a. I think Acquire and read barrier semantics aren't equal.
> > With acquire semantics, "the results of the operation are available before
> > the
> > results of any operation that appears after it in code" which means it
> > applies
> > for both load and stores. Definition of read barrier just ensures loads.
> >
> > So will be right to declare like above in comments?
>
> Yeah. Barriers can be by the operations they keep from being
> reordered (read, write, or both) and by the direction in which they
> prevent reordering (acquire, release, or both). So in theory there
> are 9 combinations:
>
> read barrier (both directions)
> read barrier (acquire)
> read barrier (release)
> write barrier (both directions)
> write barrier (acquire)
> write barrier (both directions)
> full barrier (both directions)
> full barrier (acquire)
> full barrier (release)
With these definitions, I think it will fall into *full barrier (acquire)*
category as it needs to ensure that no reordering should happen
for both load and store. As an example, we can refer its
implementation for msvc which ensures that it follows full memory
barrier semantics.
> To make things more confusing, a read barrier plus a write barrier
> need not equal a full barrier. Read barriers prevent reads from being
> reordered with other reads, and write barriers keep writes from being
> reordered with other writes, but neither prevents a read from being
> reordered relative to a write. A full barrier, however, must prevent
> all reordering: read/read, read/write, write/read, and write/write.
>
> Clarity here is essential.
>
> barrier.h only proposed to implement the following:
>
> read barrier (both directions), write barrier (both directions), full
> barrier (both directions)
As per my understanding of the general theory around barriers,
> The reason I did that is because it seemed to be more or less what
> Linux was doing, and it's generally suitable for lock-less algorithms.
> The acquire and release semantics come into play specifically when
> you're using barriers to implement locking primitives, which is isn't
> what I was trying to enable.
#define TAS(lock) (InterlockedCompareExchange(lock, 1, 0))
> To make things more confusing, a read barrier plus a write barrier
> need not equal a full barrier. Read barriers prevent reads from being
> reordered with other reads, and write barriers keep writes from being
> reordered with other writes, but neither prevents a read from being
> reordered relative to a write. A full barrier, however, must prevent
> all reordering: read/read, read/write, write/read, and write/write.
>
> Clarity here is essential.
>
> barrier.h only proposed to implement the following:
>
> read barrier (both directions), write barrier (both directions), full
> barrier (both directions)
As per my understanding of the general theory around barriers,
read and write are defined to avoid reordering due to compiler and
full memory barriers are defined to avoid reordering due to processors.
There are some processors that support instructions for memory
barriers with acquire, release and fence semantics.
I think the current definitions of barriers is as per needs of usage in
PostgreSQL and not by referring standard (am I missing something
here), however as you explained the definitions are quite clear.
> The reason I did that is because it seemed to be more or less what
> Linux was doing, and it's generally suitable for lock-less algorithms.
> The acquire and release semantics come into play specifically when
> you're using barriers to implement locking primitives, which is isn't
> what I was trying to enable.
Now by implementing atomic-ops, I think we are moving more towards
lock-less algorithms, so I am not sure if we just want to adhere to
existing definitions as you listed above (what current barrier.h
implements) and implement accordingly or we can extend the existing
definitions.
pgsql-hackers by date: