[v2,00/16] replace __atomic operations returning new value

  This series replaces uses of __atomic_{add,and,or,sub,xor}_fetch with
__atomic_fetch_{add,and,or,sub,xor} intrinsics where the new value
is used.

This series is being separated from the other similar series in
an effort to reduce the chance of mistakes being spotted in review
since the usages in this case consume the returned / new value.

v2:
    * remove unnecessary casts of signed to unsigned arguments when
      using generic __atomic builtins.
    * remove inappropriate cast of signed negative value on addend.

Tyler Retzlaff (16):
  app/test: use previous value atomic fetch operations
  common/cnxk: use previous value atomic fetch operations
  common/mlx5: use previous value atomic fetch operations
  drivers/event: use previous value atomic fetch operations
  net/af_xdp: use previous value atomic fetch operations
  net/cnxk: use previous value atomic fetch operations
  net/cxgbe: use previous value atomic fetch operations
  net/iavf: use previous value atomic fetch operations
  net/mlx5: use previous value atomic fetch operations
  net/octeontx: use previous value atomic fetch operations
  raw/ifpga: use previous value atomic fetch operations
  bbdev: use previous value atomic fetch operations
  eal: use previous value atomic fetch operations
  ipsec: use previous value atomic fetch operations
  mbuf: use previous value atomic fetch operations
  rcu: use previous value atomic fetch operations

 app/test/test_ring_perf.c               |  2 +-
 drivers/common/cnxk/roc_ae.c            |  2 +-
 drivers/common/cnxk/roc_ae_fpm_tables.c |  2 +-
 drivers/common/cnxk/roc_npa.c           |  2 +-
 drivers/common/mlx5/linux/mlx5_nl.c     |  2 +-
 drivers/common/mlx5/mlx5_common_mr.c    |  8 ++++----
 drivers/common/mlx5/mlx5_common_utils.c |  8 ++++----
 drivers/event/cnxk/cnxk_tim_worker.h    |  2 +-
 drivers/event/dsw/dsw_event.c           |  4 ++--
 drivers/event/octeontx/timvf_worker.h   |  2 +-
 drivers/net/af_xdp/rte_eth_af_xdp.c     |  4 ++--
 drivers/net/cnxk/cn10k_tx.h             |  4 ++--
 drivers/net/cxgbe/clip_tbl.c            |  2 +-
 drivers/net/cxgbe/mps_tcam.c            |  2 +-
 drivers/net/iavf/iavf_vchnl.c           |  8 ++++----
 drivers/net/mlx5/linux/mlx5_verbs.c     |  2 +-
 drivers/net/mlx5/mlx5.c                 |  4 ++--
 drivers/net/mlx5/mlx5_flow.c            |  8 ++++----
 drivers/net/mlx5/mlx5_flow_dv.c         | 12 ++++++------
 drivers/net/mlx5/mlx5_flow_hw.c         | 14 +++++++-------
 drivers/net/mlx5/mlx5_hws_cnt.c         |  4 ++--
 drivers/net/mlx5/mlx5_rxq.c             |  6 +++---
 drivers/net/mlx5/mlx5_txq.c             |  2 +-
 drivers/net/octeontx/octeontx_ethdev.c  |  2 +-
 drivers/raw/ifpga/ifpga_rawdev.c        |  2 +-
 lib/bbdev/rte_bbdev.c                   |  4 ++--
 lib/eal/include/generic/rte_rwlock.h    |  8 ++++----
 lib/eal/ppc/include/rte_atomic.h        | 16 ++++++++--------
 lib/ipsec/ipsec_sqn.h                   |  2 +-
 lib/mbuf/rte_mbuf.h                     | 12 ++++++------
 lib/rcu/rte_rcu_qsbr.h                  |  2 +-
 31 files changed, 77 insertions(+), 77 deletions(-)
  

On Wed, Mar 15, 2023 at 02:15:29PM -0700, Tyler Retzlaff wrote:
> This series replaces uses of __atomic_{add,and,or,sub,xor}_fetch with
> __atomic_fetch_{add,and,or,sub,xor} intrinsics where the new value
> is used.
> 
> This series is being separated from the other similar series in
> an effort to reduce the chance of mistakes being spotted in review
> since the usages in this case consume the returned / new value.
> 
> v2:
>     * remove unnecessary casts of signed to unsigned arguments when
>       using generic __atomic builtins.
>     * remove inappropriate cast of signed negative value on addend.
> 
> Tyler Retzlaff (16):
>   app/test: use previous value atomic fetch operations
>   common/cnxk: use previous value atomic fetch operations
>   common/mlx5: use previous value atomic fetch operations
>   drivers/event: use previous value atomic fetch operations
>   net/af_xdp: use previous value atomic fetch operations
>   net/cnxk: use previous value atomic fetch operations
>   net/cxgbe: use previous value atomic fetch operations
>   net/iavf: use previous value atomic fetch operations
>   net/mlx5: use previous value atomic fetch operations
>   net/octeontx: use previous value atomic fetch operations
>   raw/ifpga: use previous value atomic fetch operations
>   bbdev: use previous value atomic fetch operations
>   eal: use previous value atomic fetch operations
>   ipsec: use previous value atomic fetch operations
>   mbuf: use previous value atomic fetch operations
>   rcu: use previous value atomic fetch operations
> 
I am wondering how we go about ensuring that we don't introduce any more of
these atomic_X_fetch intrinsics. Is there some way we can add a compiler
warning for them or have a checkpatch check, for example?
  

16/03/2023 11:03, Bruce Richardson:
> On Wed, Mar 15, 2023 at 02:15:29PM -0700, Tyler Retzlaff wrote:
> > This series replaces uses of __atomic_{add,and,or,sub,xor}_fetch with
> > __atomic_fetch_{add,and,or,sub,xor} intrinsics where the new value
> > is used.
[...]
> > Tyler Retzlaff (16):
> >   app/test: use previous value atomic fetch operations
> >   common/cnxk: use previous value atomic fetch operations
> >   common/mlx5: use previous value atomic fetch operations
> >   drivers/event: use previous value atomic fetch operations
> >   net/af_xdp: use previous value atomic fetch operations
> >   net/cnxk: use previous value atomic fetch operations
> >   net/cxgbe: use previous value atomic fetch operations
> >   net/iavf: use previous value atomic fetch operations
> >   net/mlx5: use previous value atomic fetch operations
> >   net/octeontx: use previous value atomic fetch operations
> >   raw/ifpga: use previous value atomic fetch operations
> >   bbdev: use previous value atomic fetch operations
> >   eal: use previous value atomic fetch operations
> >   ipsec: use previous value atomic fetch operations
> >   mbuf: use previous value atomic fetch operations
> >   rcu: use previous value atomic fetch operations
> > 
> I am wondering how we go about ensuring that we don't introduce any more of
> these atomic_X_fetch intrinsics. Is there some way we can add a compiler
> warning for them or have a checkpatch check, for example?

In devtools/checkpatches.sh, we are checking for these patterns:
	rte_atomic[0-9][0-9]_.*\(
	__atomic_thread_fence\(

Feel free to add more "forbidden patterns".
  

On Thu, Mar 16, 2023 at 04:25:41PM +0100, Thomas Monjalon wrote:
> 16/03/2023 11:03, Bruce Richardson:
> > On Wed, Mar 15, 2023 at 02:15:29PM -0700, Tyler Retzlaff wrote:
> > > This series replaces uses of __atomic_{add,and,or,sub,xor}_fetch with
> > > __atomic_fetch_{add,and,or,sub,xor} intrinsics where the new value
> > > is used.
> [...]
> > > Tyler Retzlaff (16):
> > >   app/test: use previous value atomic fetch operations
> > >   common/cnxk: use previous value atomic fetch operations
> > >   common/mlx5: use previous value atomic fetch operations
> > >   drivers/event: use previous value atomic fetch operations
> > >   net/af_xdp: use previous value atomic fetch operations
> > >   net/cnxk: use previous value atomic fetch operations
> > >   net/cxgbe: use previous value atomic fetch operations
> > >   net/iavf: use previous value atomic fetch operations
> > >   net/mlx5: use previous value atomic fetch operations
> > >   net/octeontx: use previous value atomic fetch operations
> > >   raw/ifpga: use previous value atomic fetch operations
> > >   bbdev: use previous value atomic fetch operations
> > >   eal: use previous value atomic fetch operations
> > >   ipsec: use previous value atomic fetch operations
> > >   mbuf: use previous value atomic fetch operations
> > >   rcu: use previous value atomic fetch operations
> > > 
> > I am wondering how we go about ensuring that we don't introduce any more of
> > these atomic_X_fetch intrinsics. Is there some way we can add a compiler
> > warning for them or have a checkpatch check, for example?
> 
> In devtools/checkpatches.sh, we are checking for these patterns:
> 	rte_atomic[0-9][0-9]_.*\(
> 	__atomic_thread_fence\(
> 
> Feel free to add more "forbidden patterns".
> 
> 

yes, i was going to do this before end of week but got interrupted by
other work. i will introduce a patch for checkpatches.sh standalone
asap that can be merged before these changes.
  

> -----Original Message-----
> From: Tyler Retzlaff <roretzla@linux.microsoft.com>
> Sent: Thursday, March 16, 2023 5:15 AM
> To: dev@dpdk.org
> Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; thomas@monjalon.net; Tyler Retzlaff <roretzla@linux.microsoft.com>
> Subject: [PATCH v2 00/16] replace __atomic operations returning new value
> 
> This series replaces uses of __atomic_{add,and,or,sub,xor}_fetch with
> __atomic_fetch_{add,and,or,sub,xor} intrinsics where the new value is used.
> 
> This series is being separated from the other similar series in an effort to reduce the
> chance of mistakes being spotted in review since the usages in this case consume the
> returned / new value.
> 
> v2:
>     * remove unnecessary casts of signed to unsigned arguments when
>       using generic __atomic builtins.
>     * remove inappropriate cast of signed negative value on addend.
> 
> Tyler Retzlaff (16):
>   app/test: use previous value atomic fetch operations
>   common/cnxk: use previous value atomic fetch operations
>   common/mlx5: use previous value atomic fetch operations
>   drivers/event: use previous value atomic fetch operations
>   net/af_xdp: use previous value atomic fetch operations
>   net/cnxk: use previous value atomic fetch operations
>   net/cxgbe: use previous value atomic fetch operations
>   net/iavf: use previous value atomic fetch operations
>   net/mlx5: use previous value atomic fetch operations
>   net/octeontx: use previous value atomic fetch operations
>   raw/ifpga: use previous value atomic fetch operations
>   bbdev: use previous value atomic fetch operations
>   eal: use previous value atomic fetch operations
>   ipsec: use previous value atomic fetch operations
>   mbuf: use previous value atomic fetch operations
>   rcu: use previous value atomic fetch operations
> 
>  app/test/test_ring_perf.c               |  2 +-
>  drivers/common/cnxk/roc_ae.c            |  2 +-
>  drivers/common/cnxk/roc_ae_fpm_tables.c |  2 +-
>  drivers/common/cnxk/roc_npa.c           |  2 +-
>  drivers/common/mlx5/linux/mlx5_nl.c     |  2 +-
>  drivers/common/mlx5/mlx5_common_mr.c    |  8 ++++----
>  drivers/common/mlx5/mlx5_common_utils.c |  8 ++++----
>  drivers/event/cnxk/cnxk_tim_worker.h    |  2 +-
>  drivers/event/dsw/dsw_event.c           |  4 ++--
>  drivers/event/octeontx/timvf_worker.h   |  2 +-
>  drivers/net/af_xdp/rte_eth_af_xdp.c     |  4 ++--
>  drivers/net/cnxk/cn10k_tx.h             |  4 ++--
>  drivers/net/cxgbe/clip_tbl.c            |  2 +-
>  drivers/net/cxgbe/mps_tcam.c            |  2 +-
>  drivers/net/iavf/iavf_vchnl.c           |  8 ++++----
>  drivers/net/mlx5/linux/mlx5_verbs.c     |  2 +-
>  drivers/net/mlx5/mlx5.c                 |  4 ++--
>  drivers/net/mlx5/mlx5_flow.c            |  8 ++++----
>  drivers/net/mlx5/mlx5_flow_dv.c         | 12 ++++++------
>  drivers/net/mlx5/mlx5_flow_hw.c         | 14 +++++++-------
>  drivers/net/mlx5/mlx5_hws_cnt.c         |  4 ++--
>  drivers/net/mlx5/mlx5_rxq.c             |  6 +++---
>  drivers/net/mlx5/mlx5_txq.c             |  2 +-
>  drivers/net/octeontx/octeontx_ethdev.c  |  2 +-
>  drivers/raw/ifpga/ifpga_rawdev.c        |  2 +-
>  lib/bbdev/rte_bbdev.c                   |  4 ++--
>  lib/eal/include/generic/rte_rwlock.h    |  8 ++++----
>  lib/eal/ppc/include/rte_atomic.h        | 16 ++++++++--------
>  lib/ipsec/ipsec_sqn.h                   |  2 +-
>  lib/mbuf/rte_mbuf.h                     | 12 ++++++------
>  lib/rcu/rte_rcu_qsbr.h                  |  2 +-
>  31 files changed, 77 insertions(+), 77 deletions(-)
> 
> --
> 1.8.3.1
Series-reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
  

On Thu, Mar 16, 2023 at 09:17:05AM -0700, Tyler Retzlaff wrote:
> On Thu, Mar 16, 2023 at 04:25:41PM +0100, Thomas Monjalon wrote:
> > 16/03/2023 11:03, Bruce Richardson:
> > > On Wed, Mar 15, 2023 at 02:15:29PM -0700, Tyler Retzlaff wrote:
> > > > This series replaces uses of __atomic_{add,and,or,sub,xor}_fetch with
> > > > __atomic_fetch_{add,and,or,sub,xor} intrinsics where the new value
> > > > is used.
> > [...]
> > > > Tyler Retzlaff (16):
> > > >   app/test: use previous value atomic fetch operations
> > > >   common/cnxk: use previous value atomic fetch operations
> > > >   common/mlx5: use previous value atomic fetch operations
> > > >   drivers/event: use previous value atomic fetch operations
> > > >   net/af_xdp: use previous value atomic fetch operations
> > > >   net/cnxk: use previous value atomic fetch operations
> > > >   net/cxgbe: use previous value atomic fetch operations
> > > >   net/iavf: use previous value atomic fetch operations
> > > >   net/mlx5: use previous value atomic fetch operations
> > > >   net/octeontx: use previous value atomic fetch operations
> > > >   raw/ifpga: use previous value atomic fetch operations
> > > >   bbdev: use previous value atomic fetch operations
> > > >   eal: use previous value atomic fetch operations
> > > >   ipsec: use previous value atomic fetch operations
> > > >   mbuf: use previous value atomic fetch operations
> > > >   rcu: use previous value atomic fetch operations
> > > > 
> > > I am wondering how we go about ensuring that we don't introduce any more of
> > > these atomic_X_fetch intrinsics. Is there some way we can add a compiler
> > > warning for them or have a checkpatch check, for example?
> > 
> > In devtools/checkpatches.sh, we are checking for these patterns:
> > 	rte_atomic[0-9][0-9]_.*\(
> > 	__atomic_thread_fence\(
> > 
> > Feel free to add more "forbidden patterns".
> > 
> > 
> 
> yes, i was going to do this before end of week but got interrupted by
> other work. i will introduce a patch for checkpatches.sh standalone
> asap that can be merged before these changes.

just fyi, there is a series up for this.

https://patchwork.dpdk.org/project/dpdk/list/?series=27613