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]<mailto:[email protected]>> wrote: From: EXT Ola Liljedahl [mailto:[email protected]<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]<mailto:[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? 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. 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
