Since (a successful) CAS operation is both a read and a write, you might need to specify both acquire and release ordering. It depends on the usage of the old (read) and new (written) values.
On 20 October 2015 at 13:50, Ola Liljedahl <[email protected]> wrote: > > > 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
