On Tue, Oct 24, 2023 at 11:56:11AM +0200, Morten Brørup wrote: > > From: Konstantin Ananyev [mailto:konstantin.v.anan...@yandex.ru] > > Sent: Tuesday, 24 October 2023 10.43 > > > > 17.10.2023 21:31, Tyler Retzlaff пишет: > > > Replace the use of gcc builtin __atomic_xxx intrinsics with > > > corresponding rte_atomic_xxx optional stdatomic API > > > > > > Signed-off-by: Tyler Retzlaff <roret...@linux.microsoft.com> > > > --- > > [...] > > > > if (!single) > > > - rte_wait_until_equal_32(&ht->tail, old_val, __ATOMIC_RELAXED); > > > + rte_wait_until_equal_32((volatile uint32_t *)(uintptr_t)&ht- > > >tail, old_val, > > > > I suppose we do need that double type conversion only for atomic types > > right? > > > > > + rte_memory_order_relaxed); > > > > > > ht->tail = new_val; > > > } > > This got me thinking... > > Do we want to cast away the value's atomic attribute like this, or should we > introduce new rte_atomic_wait_XX() functions with the parameters being > pointers to atomic values, instead of pointers to simple values?
just some notes here. so first let me start with it's okay to do this cast but only because we have knowledge of the internal implementation detail and this series has to do this in a few places. basically internally the actual atomic operation is fed back into an intrinsic/builtin that is either re-qualified as __rte_atomic or doesn't require qualification. i agree it isn't optimal since we have to take care should we ever alter the implementation to avoid compatibility problems but unlikely for it to be changed. we could provide new api but i'm not sure we can do that this late in the release cycle. notably i think it would be nicer if it *could* be made to be 'generic' as used literally in the atomics documentation which means it may operate on non-integer and non-pointer types. > > Just a thought. > > The initial rte_atomic_wait_XX() implementations could simply cast a away the > atomic attribute like here. >
