Re: rte_atomic_*_explicit

["Mattias Rönnblom" <hofors@lysator.liu.se>]

On 2024-01-26 11:52, Morten Brørup wrote:
>> From: Mattias Rönnblom [mailto:hofors@lysator.liu.se]
>> Sent: Friday, 26 January 2024 09.07
>>
>> On 2024-01-25 23:10, Morten Brørup wrote:
>>>> From: Mattias Rönnblom [mailto:hofors@lysator.liu.se]
>>>> Sent: Thursday, 25 January 2024 19.54
>>>>
>>>> Why do rte_stdatomic.h functions have the suffix "_explicit"?
>>>> Especially
>>>> since there aren't any wrappers for the implicit variants.
>>>>
>>>> More to type, more to read.
>>>
>>> They have the "_explicit" suffix to make their names similar to those
>> in stdatomic.h.
>>>
>>
>> OK, so to avoid a situation where someone accidentally misinterpret
>> rte_atomic_fetch_add(&counter, 1, rte_memory_order_relaxed);
>> as what, exactly?
>>
>> If you have a wrapper, why not take the opportunity and reap some
>> benefits from this and fix/extend the standard API, making it a better
>> fit for your application. Like removing the redundant "_explicit",
>> "fetch"-less add/sub, maybe and a function for single-writer atomic add
>> (non-atomic read + non-atomic add + atomic store), etc.
>>
>> Prohibiting implicit operations was done already, so it's already now
>> not a straight-off copy of the standard API.
>>
>>> You might consider their existence somewhat temporary until C11
>> stdatomics can be fully phased in, so there's another argument for
>> similar names. (This probably does not happen as long as compilers
>> generate slower code for C11 stdatomics than with their atomic built-
>> ins.)
>>>
>>
>> To me, it seems reasonable a wrapper API should stay as long as it
>> provide a benefit over the standard API. One could imagine that being a
>> long time.
>>
>> I imagine some DPDK developers being tired of migrating from one
>> atomics
>> API to another. <rte_atomic.h> -> GCC built-ins (-> attempted C11
>> stdatomics?) -> <rte_stdatomic.h>. Now you are adding a future "-> CXY
>> atomics" move as well, and with that it also seems natural to add a
>> change back to a wrapper or complementary API, when CXY didn't turned
>> out good enough for some particular platform, or when some non-
>> complaint
>> compiler comes along.
> 
> Yes, more migrations seem to be on the roadmap.
> 
> We can take the opportunity to change direction now, and decide to keep the <rte_stdatomic.h> API long term.
> Then it would need more documentation (basically copying function descriptions from <stdatomic.h>), and the functions could have the "_explicit" suffix removed (macros with the suffix could be added for backwards compatibility), and more functions - like the ones you suggested above - could be added.
> 
> What do people think?
> 1. Keep considering <rte_stdatomic.h> a temporary wrapper for <stdatomic.h> until compilers reach some undefined level of maturity, or
> 2. Consider <rte_stdatomic.h> stable, clean it up (remove "_explicit" suffix), add documentation to the macros, and extend it.
> 
> Living in a DPDK-only environment, I would prefer option 2; but if mixing DPDK code with non-DPDK code (that uses <stdatomic.h>) it might be weird.
> 

I think DPDK should have its own atomics API, with C11-style naming and 
semantics, where there is an overlap, and keep it until the heat death 
of the universe.

Is there an example of a large-scale project of the OS kernel or network 
stack kind that *doesn't* have its own atomics API (which wraps/extends 
C11 atomics, and/or uses compiler built-ins or in-line assembler)?

fd.io VPP, OpenDataPlane (ODP) and the Linux kernel all have custom APIs.

>>
>> I suggested fixing the original <rte_atomic.h> API, or at least have a
>> wrapper API, already at the point DPDK moved to direct GCC built-in
>> calls. Then we wouldn't have had this atomics API ping-pong.
> 
> The decision back then might have been too hasty, and based on incomplete assumptions.
> Please shout louder next time you think a mistake is in the making.
> 
>>
>>>>
>>>> When was this API introduced? Shouldn't it say "experimental"
>>>> somewhere?
>>>
>>> They were introduced as part of the migration to C11.
>>> I suppose they were not marked experimental because they replaced
>> something we didn't want anymore (the compiler built-ins for atomics,
>> e.g. __atomic_load_n()). I don't recall if we discussed experimental
>> marking or not.
> 
> In hindsight, they should probably have been marked "experimental".
>