[v3,5/7] net/ixgbe: replace rte atomics with GCC builtin atomics
Checks
Commit Message
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@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(-)
Comments
Hello Qiming, Wenjun,
Review please.
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.
>
> Signed-off-by: Tyler Retzlaff <roretzla@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
>
@@ -3,7 +3,6 @@
*/
#include <time.h>
-#include <rte_atomic.h>
#include <ethdev_driver.h>
#include "ixgbe_ethdev.h"
#include "ixgbe_bypass_api.h"
@@ -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,
@@ -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;
};
@@ -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>
@@ -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>