On 20 October 2015 at 11:52, Savolainen, Petri (Nokia - FI/Espoo) <
[email protected]> wrote:

>
>
>
>
> *From:* EXT Ola Liljedahl [mailto:[email protected]]
> *Sent:* Tuesday, October 20, 2015 11:44 AM
> *To:* Savolainen, Petri (Nokia - FI/Espoo)
> *Cc:* LNG ODP Mailman List
> *Subject:* Re: [lng-odp] [API-NEXT PATCH 6/6] api: atomic: added 32 bit
> acquire and release
>
>
>
> On 16 October 2015 at 10:11, Savolainen, Petri (Nokia - FI/Espoo) <
> [email protected]> wrote:
>
>
>
>
>
> *From:* EXT Ola Liljedahl [mailto:[email protected]]
> *Sent:* Thursday, October 15, 2015 10:55 PM
> *To:* Savolainen, Petri (Nokia - FI/Espoo)
> *Cc:* LNG ODP Mailman List
> *Subject:* Re: [lng-odp] [API-NEXT PATCH 6/6] api: atomic: added 32 bit
> acquire and release
>
>
>
> On 15 October 2015 at 10:45, Petri Savolainen <[email protected]>
> wrote:
>
> Added 32 bit acquire load/cas and release store/add/sub calls.
>
> You only have cas_acquire. Don't you see a need for cas_release? I use
> that in one lockless design.
>
>
>
> I see the beginning of a combinatorial explosion here. Why not just expose
> the atomic operations that take the memory ordering as a parameter?
>
>
>
> -- Ola
>
>
>
> I’m trying to limit the combinatory explosion here by defining only those
> combinations that are mostly used, which are: load – acquire and store –
> release, right?
>
> And then you have e.g. cas_acquire. By specifying the ordering as part of
> the function name, you will have to add a lot of new calls (e.g.
> cas_release which I use) in the future.
>
>
>
> Yes, we have e.g. cas_acq_u32() and maybe cas_rel_ptr(), but not e.g.
> inc_seq_cst_32(), cas_consume_u32(), store_acq_u32() … Especially the last
>  one does not make sense (and e.g. GCC __atomic_store() don’t support it).
>
>
>
> It’s enough that we have a well defined API that serve 99% of use cases
> and leave corner cases out. I believe that there is a small set of
> operations that are used to implement most of the (actually used)
> lock/lockless algorithms.
>
>
>
>
>
> A memory order parameter would open the can of all kind of combinations
> that would then need more documentation telling which ones are actually
> legal, useful or high/low performance.
>
>  Anyone using these calls need to understand memory consistency. Thats a
> separate subject, not invented by ODP. We don't have to provide a complete
> documentation of memory consistency. Other API's and concepts in ODP are
> ODP specific so we need complete documentation. But atomics and memory
> consistency is a different case, there's nothing ODP specific about it.
>
>
>
>
>
> Yes, that’s why the patch set starts the atomic module documentation with
> this reference to C11 spec: “If not otherwise documented, operations in
> this API are implemented using <b> RELAXED memory ordering </b> (see memory
> order descriptions in the C11 specification).”
>
>
>
>
>
>
>
> Cas_release is kind of odd combination – try to release something with cas
> that you own already?
>
> It's not odd at all. The release ordering just makes sure that the pointer
> I (conditionally) swap in can be safely dereferenced by someone which
> acquires (load_acquire or cas_acquire) the same pointer. I cannot use
> store_release in this specific case.
>
> Acquire and release must pair, using a relaxed cas does not work (my prior
> writes, e.g. by dereferencing the pointer, are not guaranteed to by
> observable).
>
>
>
>
>
> Maybe it then generally useful only with pointer types?
>
Use with pointer types seems plausible but my design actually does a
cas_release on an uint64_t (for 64-bit targets) or uint32_t  (for 32-bit
targets) (the value consists of two (32- or 16-bit) indexes which refer to
stuff in an array of pointers). Now this code might not be of immediate
interest to ODP, it is just an example of the need for a cas_release
operation.


>
> int odp_atomic_cas_rel_ptr(odp_atomic_ptr_t *atom, void *old_ptr, void
> *new_ptr);
>
>
>
> We should just define the set of API calls that are actually needed. It
> provides less possible combinations and bugs.
>
Is this API design rule valid also for the counter capabilities we
discussed in another email thread?


>
>
>
> We can leave it out from API, if it’s a rarely needed/used combination.
>
> I think we need to add e.g. cas_release to the API when it is needed (not
> necessarily now) because it is a valid combination even though it might not
> be very frequently used. It cannot be replaced with something else.
>
>
>
> Agree. See above, maybe it’s only for the pointer type.
>
>
>
>
>
> -Petri
>
>
>
_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to