[PATCH v5 0/6] replace rte atomics with GCC builtin atomics

[David Marchand]

On Tue, Jun 6, 2023 at 11:45 PM Tyler Retzlaff
<roretzla at 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.
>
> v5:
>   * use relaxed ordering for counter increments in net/ring patch
>   * remove note comments from net/ring patch
>
> v4:
>
>   * drop patch for lib/ring it will be provided by ARM / Honnappa
>   * rebase for changes in dma/idxd merge
>   * adapt __atomic_fetch_sub(...) - 1 == 0 to be (__atomic_fetch_sub(...) == 1)
>     as per feedback.
>   * drop one /* NOTE: review for potential ordering optimization */ since
>     the note reference non-critical to perf control path.
>
>   note:
>
>   Remainder of the NOTE comments have been retained since there
>   seems to be no consensus but stronger opinion/argument to keep
>   expressed. while I generally agree that changes should not
>   include ``TODO'' style comments I also agree that without these
>   comments in your face people are very unlikely to feel compelled
>   to make the review they are trying to solicit without them. if
>   it is absolute that the series won't be merged with them then I
>   will remove them, but please be explicit soon.
>
> 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 (6):
>   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      | 11 ++++++-----
>  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  | 20 ++++++++++----------
>  lib/stack/rte_stack_lf_generic.h | 16 +++++++++-------
>  13 files changed, 66 insertions(+), 50 deletions(-)

I am not really enthousiastic about those NOTE:.
I would prefer we get an explicit go/nogo from each maintainers, but
this did not happen.
I think that this indicates that those NOTE: will rot in the code now.

Thomas proposed to track those NOTE: in the release announce mail and
that we ping maintainers regularly.
Let's see how it goes.

I am merging this series so we can progress on the $SUBJECT.
Series applied, thanks.


Tyler, about the patch on the ring library that was dropped by got no
viable alternative, I'll wait for a decision from ARM and you.

-- 
David Marchand