> From: Tyler Retzlaff [mailto:roret...@linux.microsoft.com] > Sent: Tuesday, 24 October 2023 17.59 > > 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.
I agree with all of the above, incl. the conclusion: Future proofing this (for a very distant future) is not worth the effort - and added APIs - at this time. Thank you for elaborating, Tyler. > > > > > Just a thought. > > > > The initial rte_atomic_wait_XX() implementations could simply cast a > away the atomic attribute like here. > >
