On Tue, Nov 5, 2013 at 5:51 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> So what I've previously proposed did use plain types for the
> to-be-manipulated atomic integers. I am not sure about that anymore
> though, C11 includes support for atomic operations, but it assumes that
> the to-be-manipulated variable is of a special "atomic" type. While I am
> not propose that we try to emulate C11's API completely (basically
> impossible as it uses type agnostic functions, it also exposes too
> much), it seems to make sense to allow PG's atomics to be implemented by
> C11's.
>
> The API is described at: http://en.cppreference.com/w/c/atomic
>
> The fundamental difference to what I've suggested so far is that the
> atomic variable isn't a plain integer/type but a distinct datatype that
> can *only* be manipulated via the atomic operators. That has the nice
> property that it cannot be accidentally manipulated via plain operators.
>
> E.g. it wouldn't be
> uint32 pg_atomic_fetch_add_u32(uint32 *ptr, uint32 add_);
> but
> uint32 pg_atomic_fetch_add_u32(pg_atomic_uint32 *ptr, uint32 add_);
>
> where, for now, atomic_uint32 is something like:
> typedef struct pg_atomic_uint32
> {
> volatile uint32 value;
> } pg_atomic_uint32;
> a future C11 implementation would just typedef C11's respective atomic
> type to pg_atomic_uint32.
>
> Comments?
C11 atomics need to be initialized through atomic_init(), so a simple
typedef will not be correct here. Also, C11 atomics are designed to
have the compiler take care of memory barriers - so they might not
always be a perfect match to our API's, or any API implementable
without compiler support.
However I'm mildly supportive on having a separate type for variables
accessed by atomics. It can result in some unnecessary code churn, but
in general if an atomic access to a variable is added, then all other
accesses to it need to be audited for memory barriers, even if they
were previously correctly synchronized by a lock.
I guess the best approach for deciding would be to try to convert a
couple of the existing unlocked accesses to the API and see what the
patch looks like.
Regards,
Ants Aasma
--
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: http://www.postgresql-support.de