> -----Original Message----- > From: EXT Ola Liljedahl [mailto:[email protected]] > Sent: Monday, November 16, 2015 6:01 PM > To: Savolainen, Petri (Nokia - FI/Espoo) > Cc: LNG ODP Mailman List > Subject: Re: [lng-odp] [API-NEXT PATCH v2] api: atomic: added > atomic_lock_free_u64 > > On 12 November 2015 at 09:40, Savolainen, Petri (Nokia - FI/Espoo) > <[email protected]> wrote: > > > > > > > > > > From: EXT Ola Liljedahl [mailto:[email protected]] > > Sent: Wednesday, November 11, 2015 6:50 PM > > To: Savolainen, Petri (Nokia - FI/Espoo) > > Cc: LNG ODP Mailman List > > Subject: Re: [lng-odp] [API-NEXT PATCH v2] api: atomic: added > > atomic_lock_free_u64 > > > > > > > > On 11 November 2015 at 15:24, Petri Savolainen > <[email protected]> > > wrote: > > > > Platforms may support some uint64 operations lock-free and > > others not. For example, inc_64 can be natively supported but > > cas_64 (or max_64/min_64) not. User may be able to switch to > > 32 bit variables when a 64 bit operation uses locks. The same > > atomic operation struture could be used for platform init to guide > > > > structure? > > > > > > > > lock vs. lock-free implementation (e.g. if cas_64 is never > > called, inc_64 can be lock-free). > > > > I think this also should be added to the patch. It feels incomplete now. > > > > > > > > This is topic of another patch (set). This patch provides the type, > other > > patches can make use of it. It’s a separate discussion if this type > should > > be used in general or platform specific init parameters. > OK > > > > > > > > > > > > > > > Signed-off-by: Petri Savolainen <[email protected]> > > --- > > include/odp/api/atomic.h | 51 > > +++++++++++++++++++++++++++++++++++++ > > platform/linux-generic/Makefile.am | 1 + > > platform/linux-generic/odp_atomic.c | 26 +++++++++++++++++++ > > 3 files changed, 78 insertions(+) > > create mode 100644 platform/linux-generic/odp_atomic.c > > > > diff --git a/include/odp/api/atomic.h b/include/odp/api/atomic.h > > index 316f13a..17a38fb 100644 > > --- a/include/odp/api/atomic.h > > +++ b/include/odp/api/atomic.h > > @@ -388,6 +388,57 @@ void odp_atomic_add_rls_u32(odp_atomic_u32_t *atom, > > uint32_t val); > > void odp_atomic_sub_rls_u32(odp_atomic_u32_t *atom, uint32_t val); > > > > /** > > + * Atomic operations > > + * > > + * Atomic operations listed in a bit field structure. > > + */ > > +typedef union odp_atomic_op_t { > > + /** Operation flags */ > > + struct { > > + uint32_t init : 1; /**< Atomic init */ > > > > Init doesn't have to be atomic. No other thread is supposed to operate > on an > > atomic/shared variable before being signalled that this is OK (and > > initialisation has been done). The signalling will include the necessary > > ordering. > > > > > > > > Comment here is merely the name of the operation. Anyway, changed it in > v3. > What I mean is that since init does not have to be atomic, it doesn't > have to be included in this list of (supported) atomic operations. It > is not relevant whether init is atomic or not. It does not belong > here.
This type list's all u64 API calls. It depends on the context which calls are relevant. For example, implementation can ignore checking "init", when it does not matter if user has set that bit or not. This can be even documented per API call that uses this type. I just would not want to define several types, one for all calls, one for all truly atomic calls, etc. > > > > > > > > > > > > > + uint32_t load : 1; /**< Atomic load */ > > + uint32_t store : 1; /**< Atomic store */ > > + uint32_t fetch_add : 1; /**< Atomic fetch and add */ > > + uint32_t add : 1; /**< Atomic add */ > > + uint32_t fetch_sub : 1; /**< Atomic fetch and substract > */ > > + uint32_t sub : 1; /**< Atomic substract */ > > + uint32_t fetch_inc : 1; /**< Atomic fetch and increment > */ > > + uint32_t inc : 1; /**< Atomic increment */ > > + uint32_t fetch_dec : 1; /**< Atomic fetch and decrement > */ > > + uint32_t dec : 1; /**< Atomic decrement */ > > + uint32_t min : 1; /**< Atomic minimum */ > > + uint32_t max : 1; /**< Atomic maximum */ > > > > I am missing atomic exchange here. > > > > > > > > It’s not here, because it’s not currently in the API. > Please add it. I see it as a basic atomic operation. In some > situations, you don't want to have to use load and CAS when exchange > would be sufficient. Especially not if there is a "far" implementation > of atomic exchange, load would defeat the purpose of such a "far" > implementation. It cannot be added in the list before it's in the API yet. Is an another patch to add it into the API (and simultaneously into this type). I think v3 (12th Nov) has everything needed right now. Could you review it. Exchange can be then added then on top of it. -Petri _______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
