On Fri, Apr 26, 2024 at 11:39:17AM +0200, Mattias Rönnblom wrote:
[ ... ]
> >
> >>
> >>The only reason for _Atomic being as it is, as far as I can see, is to
> >>accommodate for ISAs which does not have the appropriate atomic machine
> >>instructions, and thus require a lock or some other data associated with
> >>the actual user-data-carrying bits. Neither GCC nor DPDK supports any
> >>such ISAs, to my knowledge. I suspect neither never will. So the cast
> >>will continue to work.
> >
> >I tend to agree with you on this.
> >
> >We should officially decide that DPDK treats RTE_ATOMIC types as a union of
> >_Atomic and non-atomic, i.e. operations on RTE_ATOMIC types can be both
> >atomic and non-atomic.
> >
>
> I think this is a subject which needs to be further explored.
>
> Objects that can be accessed both atomically and non-atomically
> should be without _Atomic. With my current understanding of this
> issue, that seems like the best option.
i've been distracted by other work and while not in the scope of this
series i want to say +1 to having this discussion. utilizing a union for
this atomic vs non-atomic access that appears in practice is a good idea.
>
> You could turn it around as well, and have such marked _Atomic and
> have explicit casts to their non-_Atomic cousins when operated upon
> by non-atomic functions. Not sure how realistic that is, since
> non-atomicity is the norm. All generic selection-based "functions"
> must take this into account.
the problem with casts is they are actually different types and may have
different size and/or alignment relative to their non-atomic types.
for current non-locking atomics the implementations happen to be the
same (presumably because it was practical) but the union is definitely a
cleaner approach.
>
> >>
> >>>>+ unsigned int nr, int memory_order) \
> >>>>+ { \
> >>>>+ RTE_ASSERT(nr < size); \
> >>>>+ \
> >>>>+ const RTE_ATOMIC(uint ## size ## _t) *a_addr = \
> >>>>+ (const RTE_ATOMIC(uint ## size ## _t) *)addr; \
> >>>>+ uint ## size ## _t mask = (uint ## size ## _t)1 << nr; \
> >>>>+ return rte_atomic_load_explicit(a_addr, memory_order) &
> >>mask; \
> >>>>+ }
> >>>
> >>>
> >>>Similar considerations regarding volatile qualifier for the "once"
> >>operations.
> >>>
