[PATCH v3 5/7] net/ixgbe: replace rte atomics with GCC builtin atomics

[David Marchand]

Hello Qiming, Wenjun,

Review please.

On Thu, Mar 23, 2023 at 11:54 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.
>
> Signed-off-by: Tyler Retzlaff <roretzla at linux.microsoft.com>
> ---
>  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 -
>  5 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ixgbe/ixgbe_bypass.c b/drivers/net/ixgbe/ixgbe_bypass.c
> index 94f34a2..f615d18 100644
> --- a/drivers/net/ixgbe/ixgbe_bypass.c
> +++ b/drivers/net/ixgbe/ixgbe_bypass.c
> @@ -3,7 +3,6 @@
>   */
>
>  #include <time.h>
> -#include <rte_atomic.h>
>  #include <ethdev_driver.h>
>  #include "ixgbe_ethdev.h"
>  #include "ixgbe_bypass_api.h"
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 88118bc..4bb85af 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -1127,7 +1127,8 @@ struct rte_ixgbe_xstats_name_off {
>                 return 0;
>         }
>
> -       rte_atomic32_clear(&ad->link_thread_running);
> +       /* NOTE: review for potential ordering optimization */
> +       __atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
>         ixgbe_parse_devargs(eth_dev->data->dev_private,
>                             pci_dev->device.devargs);
>         rte_eth_copy_pci_info(eth_dev, pci_dev);
> @@ -1625,7 +1626,8 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
>                 return 0;
>         }
>
> -       rte_atomic32_clear(&ad->link_thread_running);
> +       /* NOTE: review for potential ordering optimization */
> +       __atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
>         ixgbevf_parse_devargs(eth_dev->data->dev_private,
>                               pci_dev->device.devargs);
>
> @@ -4186,7 +4188,8 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>         struct ixgbe_adapter *ad = dev->data->dev_private;
>         uint32_t timeout = timeout_ms ? timeout_ms : WARNING_TIMEOUT;
>
> -       while (rte_atomic32_read(&ad->link_thread_running)) {
> +       /* NOTE: review for potential ordering optimization */
> +       while (__atomic_load_n(&ad->link_thread_running, __ATOMIC_SEQ_CST)) {
>                 msec_delay(1);
>                 timeout--;
>
> @@ -4222,7 +4225,8 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>         ixgbe_setup_link(hw, speed, true);
>
>         intr->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG;
> -       rte_atomic32_clear(&ad->link_thread_running);
> +       /* NOTE: review for potential ordering optimization */
> +       __atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
>         return NULL;
>  }
>
> @@ -4317,7 +4321,8 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>         if (link_up == 0) {
>                 if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {
>                         ixgbe_dev_wait_setup_link_complete(dev, 0);
> -                       if (rte_atomic32_test_and_set(&ad->link_thread_running)) {
> +                       /* NOTE: review for potential ordering optimization */
> +                       if (__atomic_test_and_set(&ad->link_thread_running, __ATOMIC_SEQ_CST)) {
>                                 /* To avoid race condition between threads, set
>                                  * the IXGBE_FLAG_NEED_LINK_CONFIG flag only
>                                  * when there is no link thread running.
> @@ -4330,7 +4335,8 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
>                                         dev) < 0) {
>                                         PMD_DRV_LOG(ERR,
>                                                 "Create link thread failed!");
> -                                       rte_atomic32_clear(&ad->link_thread_running);
> +                                       /* NOTE: review for potential ordering optimization */
> +                                       __atomic_clear(&ad->link_thread_running, __ATOMIC_SEQ_CST);
>                                 }
>                         } else {
>                                 PMD_DRV_LOG(ERR,
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
> index 48290af..2ca6998 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -6,6 +6,7 @@
>  #define _IXGBE_ETHDEV_H_
>
>  #include <stdint.h>
> +#include <stdbool.h>
>  #include <sys/queue.h>
>
>  #include "base/ixgbe_type.h"
> @@ -510,7 +511,7 @@ struct ixgbe_adapter {
>          */
>         uint8_t pflink_fullchk;
>         uint8_t mac_ctrl_frame_fwd;
> -       rte_atomic32_t link_thread_running;
> +       bool link_thread_running;
>         pthread_t link_thread_tid;
>  };
>
> diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c
> index eac81ee..687341c 100644
> --- a/drivers/net/ixgbe/ixgbe_flow.c
> +++ b/drivers/net/ixgbe/ixgbe_flow.c
> @@ -18,7 +18,6 @@
>  #include <rte_log.h>
>  #include <rte_debug.h>
>  #include <rte_pci.h>
> -#include <rte_atomic.h>
>  #include <rte_branch_prediction.h>
>  #include <rte_memory.h>
>  #include <rte_eal.h>
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index c9d6ca9..8d7251d 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -27,7 +27,6 @@
>  #include <rte_eal.h>
>  #include <rte_per_lcore.h>
>  #include <rte_lcore.h>
> -#include <rte_atomic.h>
>  #include <rte_branch_prediction.h>
>  #include <rte_mempool.h>
>  #include <rte_malloc.h>
> --
> 1.8.3.1
>

-- 
David Marchand