Re: [PATCH v3 0/7] replace rte atomics with GCC builtin atomics

[Tyler Retzlaff <roretzla@linux.microsoft.com>]

On Wed, May 24, 2023 at 02:40:43PM +0200, David Marchand wrote:
> Hello Tyler,
> 
> On Thu, Mar 23, 2023 at 11:54 PM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> >
> > Replace the use of rte_atomic.h types and functions, instead use GCC
> > supplied C++11 memory model builtins.
> >
> > This series covers the libraries and drivers that are built on Windows.
> >
> > The code has be converted to use the __atomic builtins but there are
> > additional during conversion i notice that there may be some issues
> > that need to be addressed.
> >
> > I'll comment in the patches where my concerns are so the maintainers
> > may comment.
> >
> > v3:
> >   * style, don't use c99 comments
> >
> > v2:
> >   * comment code where optimizations may be possible now that memory
> >     order can be specified.
> >   * comment code where operations should potentially be atomic so that
> >     maintainers can review.
> >   * change a couple of variables labeled as counters to be unsigned.
> >
> > Tyler Retzlaff (7):
> >   ring: replace rte atomics with GCC builtin atomics
> >   stack: replace rte atomics with GCC builtin atomics
> >   dma/idxd: replace rte atomics with GCC builtin atomics
> >   net/ice: replace rte atomics with GCC builtin atomics
> >   net/ixgbe: replace rte atomics with GCC builtin atomics
> >   net/null: replace rte atomics with GCC builtin atomics
> >   net/ring: replace rte atomics with GCC builtin atomics
> >
> >  drivers/dma/idxd/idxd_internal.h |  3 +--
> >  drivers/dma/idxd/idxd_pci.c      |  8 +++++---
> >  drivers/net/ice/ice_dcf.c        |  1 -
> >  drivers/net/ice/ice_dcf_ethdev.c |  1 -
> >  drivers/net/ice/ice_ethdev.c     | 12 ++++++++----
> >  drivers/net/ixgbe/ixgbe_bypass.c |  1 -
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 18 ++++++++++++------
> >  drivers/net/ixgbe/ixgbe_ethdev.h |  3 ++-
> >  drivers/net/ixgbe/ixgbe_flow.c   |  1 -
> >  drivers/net/ixgbe/ixgbe_rxtx.c   |  1 -
> >  drivers/net/null/rte_eth_null.c  | 28 ++++++++++++++++++----------
> >  drivers/net/ring/rte_eth_ring.c  | 26 ++++++++++++++++----------
> >  lib/ring/rte_ring_core.h         |  1 -
> >  lib/ring/rte_ring_generic_pvt.h  | 12 ++++++++----
> >  lib/stack/rte_stack_lf_generic.h | 16 +++++++++-------
> >  15 files changed, 79 insertions(+), 53 deletions(-)
> >
> 
> There is still some code using the DPDK "legacy" atomic API, but I
> guess this will be converted later.

Yes, it will be converted later.

If I did it correctly... the series was an attempt to move away
from the legacy API where there was a dependency on EAL that would
change when moving to stdatomic. I'm hoping that the remaining use of
the legacy API are not sensitive to the theoretical ABI surface
changing when that move is complete.

> As you proposed, I dropped patch 1 on the ring library (waiting for
> ARM to provide an alternative) and applied this series, thanks.
> 
> Note: Thomas, Ferruh, we will have to be careful when merging subtrees
> to make sure we are not reintroducing those again (like for example
> net/ice).
> 
> -- 
> David Marchand