DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/4] use C11 atomic builtins for libs
@ 2020-09-11  3:29 Phil Yang
  2020-09-11  3:29 ` [dpdk-dev] [PATCH 1/4] eal: use C11 atomic builtins for already initialized check Phil Yang
                   ` (5 more replies)
  0 siblings, 6 replies; 27+ messages in thread
From: Phil Yang @ 2020-09-11  3:29 UTC (permalink / raw)
  To: dev; +Cc: Honnappa.Nagarahalli, Ruifeng.Wang, nd

Since rte_atomicXX APIs are not allowed to be used[1][2], use C11 atomic
builtins instead in eal, bbdev, power, and ethdev libs.

[1] http://code.dpdk.org/dpdk/latest/source/doc/guides/rel_notes/deprecation.rst#L87
[2] http://code.dpdk.org/dpdk/latest/source/devtools/checkpatches.sh#L80

Phil Yang (4):
  eal: use C11 atomic builtins for already initialized check
  bbdev: use C11 atomic builtins for device processing counter
  power: use C11 atomic builtins for power in use state update
  ethdev: use C11 atomic builtins for link status update

 lib/librte_bbdev/rte_bbdev.c            |  5 ++--
 lib/librte_bbdev/rte_bbdev.h            |  4 +--
 lib/librte_eal/freebsd/eal.c            | 18 +++++++------
 lib/librte_eal/linux/eal.c              | 20 ++++++++-------
 lib/librte_ethdev/rte_ethdev_driver.h   | 19 ++++----------
 lib/librte_power/power_acpi_cpufreq.c   | 45 +++++++++++++++++++++++++--------
 lib/librte_power/power_pstate_cpufreq.c | 45 +++++++++++++++++++++++++--------
 7 files changed, 100 insertions(+), 56 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH 1/4] eal: use C11 atomic builtins for already initialized check
  2020-09-11  3:29 [dpdk-dev] [PATCH 0/4] use C11 atomic builtins for libs Phil Yang
@ 2020-09-11  3:29 ` Phil Yang
  2020-09-11  3:29 ` [dpdk-dev] [PATCH 2/4] bbdev: use C11 atomic builtins for device processing counter Phil Yang
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 27+ messages in thread
From: Phil Yang @ 2020-09-11  3:29 UTC (permalink / raw)
  To: dev; +Cc: Honnappa.Nagarahalli, Ruifeng.Wang, nd, Bruce Richardson

Since rte_atomicXX APIs are not allowed to be used, use C11 builtins to
check if EAL is already initialized.

Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 lib/librte_eal/freebsd/eal.c | 18 ++++++++++--------
 lib/librte_eal/linux/eal.c   | 20 +++++++++++---------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
index 798add0..9f4c7bb 100644
--- a/lib/librte_eal/freebsd/eal.c
+++ b/lib/librte_eal/freebsd/eal.c
@@ -665,7 +665,8 @@ rte_eal_init(int argc, char **argv)
 {
 	int i, fctret, ret;
 	pthread_t thread_id;
-	static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0);
+	static uint32_t run_once;
+	uint32_t has_run = 0;
 	char cpuset[RTE_CPU_AFFINITY_STR_LEN];
 	char thread_name[RTE_MAX_THREAD_NAME_LEN];
 	const struct rte_config *config = rte_eal_get_configuration();
@@ -679,7 +680,8 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
-	if (!rte_atomic32_test_and_set(&run_once)) {
+	if (!__atomic_compare_exchange_n(&run_once, &has_run, 1, 0,
+					__ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
 		rte_eal_init_alert("already called initialization.");
 		rte_errno = EALREADY;
 		return -1;
@@ -705,7 +707,7 @@ rte_eal_init(int argc, char **argv)
 	if (fctret < 0) {
 		rte_eal_init_alert("Invalid 'command line' arguments.");
 		rte_errno = EINVAL;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -715,20 +717,20 @@ rte_eal_init(int argc, char **argv)
 	if (eal_plugins_init() < 0) {
 		rte_eal_init_alert("Cannot init plugins");
 		rte_errno = EINVAL;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
 	if (eal_trace_init() < 0) {
 		rte_eal_init_alert("Cannot init trace");
 		rte_errno = EFAULT;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
 	if (eal_option_device_parse()) {
 		rte_errno = ENODEV;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -762,7 +764,7 @@ rte_eal_init(int argc, char **argv)
 	if (rte_bus_scan()) {
 		rte_eal_init_alert("Cannot scan the buses for devices");
 		rte_errno = ENODEV;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -790,7 +792,7 @@ rte_eal_init(int argc, char **argv)
 		if (ret < 0) {
 			rte_eal_init_alert("Cannot get hugepage information.");
 			rte_errno = EACCES;
-			rte_atomic32_clear(&run_once);
+			__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 			return -1;
 		}
 	}
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 0960f01..82a73ed 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -960,7 +960,8 @@ rte_eal_init(int argc, char **argv)
 {
 	int i, fctret, ret;
 	pthread_t thread_id;
-	static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0);
+	static uint32_t run_once;
+	uint32_t has_run = 0;
 	const char *p;
 	static char logid[PATH_MAX];
 	char cpuset[RTE_CPU_AFFINITY_STR_LEN];
@@ -977,7 +978,8 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
-	if (!rte_atomic32_test_and_set(&run_once)) {
+	if (!__atomic_compare_exchange_n(&run_once, &has_run, 1, 0,
+					__ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
 		rte_eal_init_alert("already called initialization.");
 		rte_errno = EALREADY;
 		return -1;
@@ -1005,14 +1007,14 @@ rte_eal_init(int argc, char **argv)
 	if (fctret < 0) {
 		rte_eal_init_alert("Invalid 'command line' arguments.");
 		rte_errno = EINVAL;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
 	if (eal_plugins_init() < 0) {
 		rte_eal_init_alert("Cannot init plugins");
 		rte_errno = EINVAL;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -1024,7 +1026,7 @@ rte_eal_init(int argc, char **argv)
 
 	if (eal_option_device_parse()) {
 		rte_errno = ENODEV;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -1064,7 +1066,7 @@ rte_eal_init(int argc, char **argv)
 	if (rte_bus_scan()) {
 		rte_eal_init_alert("Cannot scan the buses for devices");
 		rte_errno = ENODEV;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -1138,7 +1140,7 @@ rte_eal_init(int argc, char **argv)
 		if (ret < 0) {
 			rte_eal_init_alert("Cannot get hugepage information.");
 			rte_errno = EACCES;
-			rte_atomic32_clear(&run_once);
+			__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 			return -1;
 		}
 	}
@@ -1162,7 +1164,7 @@ rte_eal_init(int argc, char **argv)
 	if (rte_eal_log_init(logid, internal_conf->syslog_facility) < 0) {
 		rte_eal_init_alert("Cannot init logging.");
 		rte_errno = ENOMEM;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -1170,7 +1172,7 @@ rte_eal_init(int argc, char **argv)
 	if (rte_eal_vfio_setup() < 0) {
 		rte_eal_init_alert("Cannot init VFIO");
 		rte_errno = EAGAIN;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 #endif
-- 
2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH 2/4] bbdev: use C11 atomic builtins for device processing counter
  2020-09-11  3:29 [dpdk-dev] [PATCH 0/4] use C11 atomic builtins for libs Phil Yang
  2020-09-11  3:29 ` [dpdk-dev] [PATCH 1/4] eal: use C11 atomic builtins for already initialized check Phil Yang
@ 2020-09-11  3:29 ` Phil Yang
  2020-09-11  3:29 ` [dpdk-dev] [PATCH 3/4] power: use C11 atomic builtins for power in use state update Phil Yang
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 27+ messages in thread
From: Phil Yang @ 2020-09-11  3:29 UTC (permalink / raw)
  To: dev; +Cc: Honnappa.Nagarahalli, Ruifeng.Wang, nd, Nicolas Chautru

Since rte_atomicXX APIs are not allowed to be used, use C11 atomic builtins
for device processing counter.

Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 lib/librte_bbdev/rte_bbdev.c | 5 +++--
 lib/librte_bbdev/rte_bbdev.h | 4 +---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/librte_bbdev/rte_bbdev.c b/lib/librte_bbdev/rte_bbdev.c
index a4fdb69..5ba891c 100644
--- a/lib/librte_bbdev/rte_bbdev.c
+++ b/lib/librte_bbdev/rte_bbdev.c
@@ -210,7 +210,7 @@ rte_bbdev_allocate(const char *name)
 		return NULL;
 	}
 
-	rte_atomic16_inc(&bbdev->data->process_cnt);
+	__atomic_add_fetch(&bbdev->data->process_cnt, 1, __ATOMIC_RELAXED);
 	bbdev->data->dev_id = dev_id;
 	bbdev->state = RTE_BBDEV_INITIALIZED;
 
@@ -252,7 +252,8 @@ rte_bbdev_release(struct rte_bbdev *bbdev)
 	}
 
 	/* clear shared BBDev Data if no process is using the device anymore */
-	if (rte_atomic16_dec_and_test(&bbdev->data->process_cnt))
+	if (__atomic_sub_fetch(&bbdev->data->process_cnt, 1,
+			      __ATOMIC_RELAXED) == 0)
 		memset(bbdev->data, 0, sizeof(*bbdev->data));
 
 	memset(bbdev, 0, sizeof(*bbdev));
diff --git a/lib/librte_bbdev/rte_bbdev.h b/lib/librte_bbdev/rte_bbdev.h
index 5729137..7017124 100644
--- a/lib/librte_bbdev/rte_bbdev.h
+++ b/lib/librte_bbdev/rte_bbdev.h
@@ -33,7 +33,6 @@ extern "C" {
 #include <string.h>
 
 #include <rte_compat.h>
-#include <rte_atomic.h>
 #include <rte_bus.h>
 #include <rte_cpuflags.h>
 #include <rte_memory.h>
@@ -426,8 +425,7 @@ struct rte_bbdev_data {
 	uint16_t dev_id;  /**< Device ID */
 	int socket_id;  /**< NUMA socket that device is on */
 	bool started;  /**< Device run-time state */
-	/** Counter of processes using the device */
-	rte_atomic16_t process_cnt;
+	uint16_t process_cnt;  /** Counter of processes using the device */
 };
 
 /* Forward declarations */
-- 
2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH 3/4] power: use C11 atomic builtins for power in use state update
  2020-09-11  3:29 [dpdk-dev] [PATCH 0/4] use C11 atomic builtins for libs Phil Yang
  2020-09-11  3:29 ` [dpdk-dev] [PATCH 1/4] eal: use C11 atomic builtins for already initialized check Phil Yang
  2020-09-11  3:29 ` [dpdk-dev] [PATCH 2/4] bbdev: use C11 atomic builtins for device processing counter Phil Yang
@ 2020-09-11  3:29 ` Phil Yang
  2020-09-11  3:29 ` [dpdk-dev] [PATCH 4/4] ethdev: use C11 atomic builtins for link status update Phil Yang
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 27+ messages in thread
From: Phil Yang @ 2020-09-11  3:29 UTC (permalink / raw)
  To: dev; +Cc: Honnappa.Nagarahalli, Ruifeng.Wang, nd, David Hunt

Since rte_atomicXX APIs are not allowed to be used, use C11 atomic
builtins for power in use state update.

Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 lib/librte_power/power_acpi_cpufreq.c   | 45 +++++++++++++++++++++++++--------
 lib/librte_power/power_pstate_cpufreq.c | 45 +++++++++++++++++++++++++--------
 2 files changed, 70 insertions(+), 20 deletions(-)

diff --git a/lib/librte_power/power_acpi_cpufreq.c b/lib/librte_power/power_acpi_cpufreq.c
index 583815a..84a9d75 100644
--- a/lib/librte_power/power_acpi_cpufreq.c
+++ b/lib/librte_power/power_acpi_cpufreq.c
@@ -12,7 +12,6 @@
 #include <signal.h>
 #include <limits.h>
 
-#include <rte_atomic.h>
 #include <rte_memcpy.h>
 #include <rte_memory.h>
 #include <rte_string_fns.h>
@@ -86,7 +85,7 @@ struct rte_power_info {
 	FILE *f;                             /**< FD of scaling_setspeed */
 	char governor_ori[32];               /**< Original governor name */
 	uint32_t curr_idx;                   /**< Freq index in freqs array */
-	volatile uint32_t state;             /**< Power in use state */
+	uint32_t state;                      /**< Power in use state */
 	uint16_t turbo_available;            /**< Turbo Boost available */
 	uint16_t turbo_enable;               /**< Turbo Boost enable/disable */
 } __rte_cache_aligned;
@@ -300,6 +299,7 @@ int
 power_acpi_cpufreq_init(unsigned int lcore_id)
 {
 	struct rte_power_info *pi;
+	uint32_t exp_state;
 
 	if (lcore_id >= RTE_MAX_LCORE) {
 		RTE_LOG(ERR, POWER, "Lcore id %u can not exceeds %u\n",
@@ -308,8 +308,16 @@ power_acpi_cpufreq_init(unsigned int lcore_id)
 	}
 
 	pi = &lcore_power_info[lcore_id];
-	if (rte_atomic32_cmpset(&(pi->state), POWER_IDLE, POWER_ONGOING)
-			== 0) {
+	exp_state = POWER_IDLE;
+	/* The power in use state works as a guard variable between
+	 * the CPU frequency control initialization and exit process.
+	 * The ACQUIRE memory ordering here pairs with the RELEASE
+	 * ordering below as lock to make sure the frequency operations
+	 * in the critical section are done under the correct state.
+	 */
+	if (!__atomic_compare_exchange_n(&(pi->state), &exp_state,
+					POWER_ONGOING, 0,
+					__ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
 		RTE_LOG(INFO, POWER, "Power management of lcore %u is "
 				"in use\n", lcore_id);
 		return -1;
@@ -346,12 +354,16 @@ power_acpi_cpufreq_init(unsigned int lcore_id)
 
 	RTE_LOG(INFO, POWER, "Initialized successfully for lcore %u "
 			"power management\n", lcore_id);
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_USED);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_USED,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return 0;
 
 fail:
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_UNKNOWN);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_UNKNOWN,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return -1;
 }
@@ -408,6 +420,7 @@ int
 power_acpi_cpufreq_exit(unsigned int lcore_id)
 {
 	struct rte_power_info *pi;
+	uint32_t exp_state;
 
 	if (lcore_id >= RTE_MAX_LCORE) {
 		RTE_LOG(ERR, POWER, "Lcore id %u can not exceeds %u\n",
@@ -415,8 +428,16 @@ power_acpi_cpufreq_exit(unsigned int lcore_id)
 		return -1;
 	}
 	pi = &lcore_power_info[lcore_id];
-	if (rte_atomic32_cmpset(&(pi->state), POWER_USED, POWER_ONGOING)
-			== 0) {
+	exp_state = POWER_USED;
+	/* The power in use state works as a guard variable between
+	 * the CPU frequency control initialization and exit process.
+	 * The ACQUIRE memory ordering here pairs with the RELEASE
+	 * ordering below as lock to make sure the frequency operations
+	 * in the critical section are done under the correct state.
+	 */
+	if (!__atomic_compare_exchange_n(&(pi->state), &exp_state,
+					POWER_ONGOING, 0,
+					__ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
 		RTE_LOG(INFO, POWER, "Power management of lcore %u is "
 				"not used\n", lcore_id);
 		return -1;
@@ -436,12 +457,16 @@ power_acpi_cpufreq_exit(unsigned int lcore_id)
 	RTE_LOG(INFO, POWER, "Power management of lcore %u has exited from "
 			"'userspace' mode and been set back to the "
 			"original\n", lcore_id);
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_IDLE);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_IDLE,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return 0;
 
 fail:
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_UNKNOWN);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_UNKNOWN,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return -1;
 }
diff --git a/lib/librte_power/power_pstate_cpufreq.c b/lib/librte_power/power_pstate_cpufreq.c
index 2526441..e3126d3 100644
--- a/lib/librte_power/power_pstate_cpufreq.c
+++ b/lib/librte_power/power_pstate_cpufreq.c
@@ -14,7 +14,6 @@
 #include <errno.h>
 #include <inttypes.h>
 
-#include <rte_atomic.h>
 #include <rte_memcpy.h>
 #include <rte_memory.h>
 #include <rte_string_fns.h>
@@ -100,7 +99,7 @@ struct pstate_power_info {
 	uint32_t non_turbo_max_ratio;        /**< Non Turbo Max ratio  */
 	uint32_t sys_max_freq;               /**< system wide max freq  */
 	uint32_t core_base_freq;             /**< core base freq  */
-	volatile uint32_t state;             /**< Power in use state */
+	uint32_t state;                      /**< Power in use state */
 	uint16_t turbo_available;            /**< Turbo Boost available */
 	uint16_t turbo_enable;               /**< Turbo Boost enable/disable */
 	uint16_t priority_core;              /**< High Performance core */
@@ -542,6 +541,7 @@ int
 power_pstate_cpufreq_init(unsigned int lcore_id)
 {
 	struct pstate_power_info *pi;
+	uint32_t exp_state;
 
 	if (lcore_id >= RTE_MAX_LCORE) {
 		RTE_LOG(ERR, POWER, "Lcore id %u can not exceed %u\n",
@@ -550,8 +550,16 @@ power_pstate_cpufreq_init(unsigned int lcore_id)
 	}
 
 	pi = &lcore_power_info[lcore_id];
-	if (rte_atomic32_cmpset(&(pi->state), POWER_IDLE, POWER_ONGOING)
-			== 0) {
+	exp_state = POWER_IDLE;
+	/* The power in use state works as a guard variable between
+	 * the CPU frequency control initialization and exit process.
+	 * The ACQUIRE memory ordering here pairs with the RELEASE
+	 * ordering below as lock to make sure the frequency operations
+	 * in the critical section are done under the correct state.
+	 */
+	if (!__atomic_compare_exchange_n(&(pi->state), &exp_state,
+					POWER_ONGOING, 0,
+					__ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
 		RTE_LOG(INFO, POWER, "Power management of lcore %u is "
 				"in use\n", lcore_id);
 		return -1;
@@ -588,12 +596,16 @@ power_pstate_cpufreq_init(unsigned int lcore_id)
 
 	RTE_LOG(INFO, POWER, "Initialized successfully for lcore %u "
 			"power management\n", lcore_id);
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_USED);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_USED,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return 0;
 
 fail:
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_UNKNOWN);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_UNKNOWN,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return -1;
 }
@@ -602,6 +614,7 @@ int
 power_pstate_cpufreq_exit(unsigned int lcore_id)
 {
 	struct pstate_power_info *pi;
+	uint32_t exp_state;
 
 	if (lcore_id >= RTE_MAX_LCORE) {
 		RTE_LOG(ERR, POWER, "Lcore id %u can not exceeds %u\n",
@@ -610,8 +623,16 @@ power_pstate_cpufreq_exit(unsigned int lcore_id)
 	}
 	pi = &lcore_power_info[lcore_id];
 
-	if (rte_atomic32_cmpset(&(pi->state), POWER_USED, POWER_ONGOING)
-			== 0) {
+	exp_state = POWER_USED;
+	/* The power in use state works as a guard variable between
+	 * the CPU frequency control initialization and exit process.
+	 * The ACQUIRE memory ordering here pairs with the RELEASE
+	 * ordering below as lock to make sure the frequency operations
+	 * in the critical section are under done the correct state.
+	 */
+	if (!__atomic_compare_exchange_n(&(pi->state), &exp_state,
+					POWER_ONGOING, 0,
+					__ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
 		RTE_LOG(INFO, POWER, "Power management of lcore %u is "
 				"not used\n", lcore_id);
 		return -1;
@@ -633,12 +654,16 @@ power_pstate_cpufreq_exit(unsigned int lcore_id)
 	RTE_LOG(INFO, POWER, "Power management of lcore %u has exited from "
 			"'performance' mode and been set back to the "
 			"original\n", lcore_id);
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_IDLE);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_IDLE,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return 0;
 
 fail:
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_UNKNOWN);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_UNKNOWN,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return -1;
 }
-- 
2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH 4/4] ethdev: use C11 atomic builtins for link status update
  2020-09-11  3:29 [dpdk-dev] [PATCH 0/4] use C11 atomic builtins for libs Phil Yang
                   ` (2 preceding siblings ...)
  2020-09-11  3:29 ` [dpdk-dev] [PATCH 3/4] power: use C11 atomic builtins for power in use state update Phil Yang
@ 2020-09-11  3:29 ` Phil Yang
  2020-09-15 15:12 ` [dpdk-dev] [PATCH 0/4] use C11 atomic builtins for libs David Marchand
  2020-09-16  8:23 ` [dpdk-dev] [PATCH v2 " Phil Yang
  5 siblings, 0 replies; 27+ messages in thread
From: Phil Yang @ 2020-09-11  3:29 UTC (permalink / raw)
  To: dev
  Cc: Honnappa.Nagarahalli, Ruifeng.Wang, nd, Thomas Monjalon,
	Ferruh Yigit, Andrew Rybchenko

Since rte_atomicXX APIs are not allowed to be used, use C11 atomic
builtins for link status update.

Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 lib/librte_ethdev/rte_ethdev_driver.h | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
index 13fd049..78590dd 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -216,8 +216,7 @@ static inline int
 rte_eth_linkstatus_set(struct rte_eth_dev *dev,
 		       const struct rte_eth_link *new_link)
 {
-	volatile uint64_t *dev_link
-		 = (volatile uint64_t *)&(dev->data->dev_link);
+	uint64_t *dev_link = (uint64_t *)&(dev->data->dev_link);
 	union {
 		uint64_t val64;
 		struct rte_eth_link link;
@@ -225,8 +224,8 @@ rte_eth_linkstatus_set(struct rte_eth_dev *dev,
 
 	RTE_BUILD_BUG_ON(sizeof(*new_link) != sizeof(uint64_t));
 
-	orig.val64 = rte_atomic64_exchange(dev_link,
-					   *(const uint64_t *)new_link);
+	orig.val64 = __atomic_exchange_n(dev_link, (const uint64_t *)new_link,
+					__ATOMIC_SEQ_CST);
 
 	return (orig.link.link_status == new_link->link_status) ? -1 : 0;
 }
@@ -244,20 +243,12 @@ static inline void
 rte_eth_linkstatus_get(const struct rte_eth_dev *dev,
 		       struct rte_eth_link *link)
 {
-	volatile uint64_t *src = (uint64_t *)&(dev->data->dev_link);
+	uint64_t *src = (uint64_t *)&(dev->data->dev_link);
 	uint64_t *dst = (uint64_t *)link;
 
 	RTE_BUILD_BUG_ON(sizeof(*link) != sizeof(uint64_t));
 
-#ifdef __LP64__
-	/* if cpu arch has 64 bit unsigned lon then implicitly atomic */
-	*dst = *src;
-#else
-	/* can't use rte_atomic64_read because it returns signed int */
-	do {
-		*dst = *src;
-	} while (!rte_atomic64_cmpset(src, *dst, *dst));
-#endif
+	*dst = __atomic_load_n(src, __ATOMIC_SEQ_CST);
 }
 
 /**
-- 
2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH 0/4] use C11 atomic builtins for libs
  2020-09-11  3:29 [dpdk-dev] [PATCH 0/4] use C11 atomic builtins for libs Phil Yang
                   ` (3 preceding siblings ...)
  2020-09-11  3:29 ` [dpdk-dev] [PATCH 4/4] ethdev: use C11 atomic builtins for link status update Phil Yang
@ 2020-09-15 15:12 ` David Marchand
  2020-09-16  7:32   ` Phil Yang
  2020-09-16  8:23 ` [dpdk-dev] [PATCH v2 " Phil Yang
  5 siblings, 1 reply; 27+ messages in thread
From: David Marchand @ 2020-09-15 15:12 UTC (permalink / raw)
  To: Phil Yang
  Cc: dev, Honnappa Nagarahalli, Ruifeng Wang (Arm Technology China), nd

On Fri, Sep 11, 2020 at 5:29 AM Phil Yang <phil.yang@arm.com> wrote:
>
> Since rte_atomicXX APIs are not allowed to be used[1][2], use C11 atomic
> builtins instead in eal, bbdev, power, and ethdev libs.
>
> [1] http://code.dpdk.org/dpdk/latest/source/doc/guides/rel_notes/deprecation.rst#L87
> [2] http://code.dpdk.org/dpdk/latest/source/devtools/checkpatches.sh#L80
>
> Phil Yang (4):
>   eal: use C11 atomic builtins for already initialized check
>   bbdev: use C11 atomic builtins for device processing counter
>   power: use C11 atomic builtins for power in use state update
>   ethdev: use C11 atomic builtins for link status update

It breaks build with clang (Travis + FreeBSD vm at UNH).


-- 
David Marchand


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH 0/4] use C11 atomic builtins for libs
  2020-09-15 15:12 ` [dpdk-dev] [PATCH 0/4] use C11 atomic builtins for libs David Marchand
@ 2020-09-16  7:32   ` Phil Yang
  0 siblings, 0 replies; 27+ messages in thread
From: Phil Yang @ 2020-09-16  7:32 UTC (permalink / raw)
  To: David Marchand; +Cc: dev, Honnappa Nagarahalli, Ruifeng Wang, nd, nd

David Marchand <david.marchand@redhat.com> writes:

> Subject: Re: [dpdk-dev] [PATCH 0/4] use C11 atomic builtins for libs
> 
> On Fri, Sep 11, 2020 at 5:29 AM Phil Yang <phil.yang@arm.com> wrote:
> >
> > Since rte_atomicXX APIs are not allowed to be used[1][2], use C11 atomic
> > builtins instead in eal, bbdev, power, and ethdev libs.
> >
> > [1]
> http://code.dpdk.org/dpdk/latest/source/doc/guides/rel_notes/deprecatio
> n.rst#L87
> > [2]
> http://code.dpdk.org/dpdk/latest/source/devtools/checkpatches.sh#L80
> >
> > Phil Yang (4):
> >   eal: use C11 atomic builtins for already initialized check
> >   bbdev: use C11 atomic builtins for device processing counter
> >   power: use C11 atomic builtins for power in use state update
> >   ethdev: use C11 atomic builtins for link status update
> 
> It breaks build with clang (Travis + FreeBSD vm at UNH).

Yes. It is an 'int-conversion' warning in clang.
Problem resolved. Will update the patch soon.

Thanks,
Phil

^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v2 0/4] use C11 atomic builtins for libs
  2020-09-11  3:29 [dpdk-dev] [PATCH 0/4] use C11 atomic builtins for libs Phil Yang
                   ` (4 preceding siblings ...)
  2020-09-15 15:12 ` [dpdk-dev] [PATCH 0/4] use C11 atomic builtins for libs David Marchand
@ 2020-09-16  8:23 ` Phil Yang
  2020-09-16  8:23   ` [dpdk-dev] [PATCH v2 1/4] eal: use C11 atomic builtins for already initialized check Phil Yang
                     ` (5 more replies)
  5 siblings, 6 replies; 27+ messages in thread
From: Phil Yang @ 2020-09-16  8:23 UTC (permalink / raw)
  To: david.marchand, dev; +Cc: Honnappa.Nagarahalli, Ruifeng.Wang, nd

Since rte_atomicXX APIs are not allowed to be used[1][2], use C11 atomic
builtins instead in eal, bbdev, power, and ethdev libs.

[1] http://code.dpdk.org/dpdk/latest/source/doc/guides/rel_notes/deprecation.rst#L87
[2] http://code.dpdk.org/dpdk/latest/source/devtools/checkpatches.sh#L80

v2:
Fix Clang int-conversion warning.

v1:
Initial version.

Phil Yang (4):
  eal: use C11 atomic builtins for already initialized check
  bbdev: use C11 atomic builtins for device processing counter
  power: use C11 atomic builtins for power in use state update
  ethdev: use C11 atomic builtins for link status update

 lib/librte_bbdev/rte_bbdev.c            |  5 ++--
 lib/librte_bbdev/rte_bbdev.h            |  4 +--
 lib/librte_eal/freebsd/eal.c            | 18 +++++++------
 lib/librte_eal/linux/eal.c              | 20 ++++++++-------
 lib/librte_ethdev/rte_ethdev_driver.h   | 19 ++++----------
 lib/librte_power/power_acpi_cpufreq.c   | 45 +++++++++++++++++++++++++--------
 lib/librte_power/power_pstate_cpufreq.c | 45 +++++++++++++++++++++++++--------
 7 files changed, 100 insertions(+), 56 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v2 1/4] eal: use C11 atomic builtins for already initialized check
  2020-09-16  8:23 ` [dpdk-dev] [PATCH v2 " Phil Yang
@ 2020-09-16  8:23   ` Phil Yang
  2020-09-23 13:06     ` David Marchand
  2020-09-16  8:23   ` [dpdk-dev] [PATCH v2 2/4] bbdev: use C11 atomic builtins for device processing counter Phil Yang
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 27+ messages in thread
From: Phil Yang @ 2020-09-16  8:23 UTC (permalink / raw)
  To: david.marchand, dev
  Cc: Honnappa.Nagarahalli, Ruifeng.Wang, nd, Bruce Richardson

Since rte_atomicXX APIs are not allowed to be used, use C11 builtins to
check if EAL is already initialized.

Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 lib/librte_eal/freebsd/eal.c | 18 ++++++++++--------
 lib/librte_eal/linux/eal.c   | 20 +++++++++++---------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
index 798add0..9f4c7bb 100644
--- a/lib/librte_eal/freebsd/eal.c
+++ b/lib/librte_eal/freebsd/eal.c
@@ -665,7 +665,8 @@ rte_eal_init(int argc, char **argv)
 {
 	int i, fctret, ret;
 	pthread_t thread_id;
-	static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0);
+	static uint32_t run_once;
+	uint32_t has_run = 0;
 	char cpuset[RTE_CPU_AFFINITY_STR_LEN];
 	char thread_name[RTE_MAX_THREAD_NAME_LEN];
 	const struct rte_config *config = rte_eal_get_configuration();
@@ -679,7 +680,8 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
-	if (!rte_atomic32_test_and_set(&run_once)) {
+	if (!__atomic_compare_exchange_n(&run_once, &has_run, 1, 0,
+					__ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
 		rte_eal_init_alert("already called initialization.");
 		rte_errno = EALREADY;
 		return -1;
@@ -705,7 +707,7 @@ rte_eal_init(int argc, char **argv)
 	if (fctret < 0) {
 		rte_eal_init_alert("Invalid 'command line' arguments.");
 		rte_errno = EINVAL;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -715,20 +717,20 @@ rte_eal_init(int argc, char **argv)
 	if (eal_plugins_init() < 0) {
 		rte_eal_init_alert("Cannot init plugins");
 		rte_errno = EINVAL;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
 	if (eal_trace_init() < 0) {
 		rte_eal_init_alert("Cannot init trace");
 		rte_errno = EFAULT;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
 	if (eal_option_device_parse()) {
 		rte_errno = ENODEV;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -762,7 +764,7 @@ rte_eal_init(int argc, char **argv)
 	if (rte_bus_scan()) {
 		rte_eal_init_alert("Cannot scan the buses for devices");
 		rte_errno = ENODEV;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -790,7 +792,7 @@ rte_eal_init(int argc, char **argv)
 		if (ret < 0) {
 			rte_eal_init_alert("Cannot get hugepage information.");
 			rte_errno = EACCES;
-			rte_atomic32_clear(&run_once);
+			__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 			return -1;
 		}
 	}
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 0960f01..82a73ed 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -960,7 +960,8 @@ rte_eal_init(int argc, char **argv)
 {
 	int i, fctret, ret;
 	pthread_t thread_id;
-	static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0);
+	static uint32_t run_once;
+	uint32_t has_run = 0;
 	const char *p;
 	static char logid[PATH_MAX];
 	char cpuset[RTE_CPU_AFFINITY_STR_LEN];
@@ -977,7 +978,8 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
-	if (!rte_atomic32_test_and_set(&run_once)) {
+	if (!__atomic_compare_exchange_n(&run_once, &has_run, 1, 0,
+					__ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
 		rte_eal_init_alert("already called initialization.");
 		rte_errno = EALREADY;
 		return -1;
@@ -1005,14 +1007,14 @@ rte_eal_init(int argc, char **argv)
 	if (fctret < 0) {
 		rte_eal_init_alert("Invalid 'command line' arguments.");
 		rte_errno = EINVAL;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
 	if (eal_plugins_init() < 0) {
 		rte_eal_init_alert("Cannot init plugins");
 		rte_errno = EINVAL;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -1024,7 +1026,7 @@ rte_eal_init(int argc, char **argv)
 
 	if (eal_option_device_parse()) {
 		rte_errno = ENODEV;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -1064,7 +1066,7 @@ rte_eal_init(int argc, char **argv)
 	if (rte_bus_scan()) {
 		rte_eal_init_alert("Cannot scan the buses for devices");
 		rte_errno = ENODEV;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -1138,7 +1140,7 @@ rte_eal_init(int argc, char **argv)
 		if (ret < 0) {
 			rte_eal_init_alert("Cannot get hugepage information.");
 			rte_errno = EACCES;
-			rte_atomic32_clear(&run_once);
+			__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 			return -1;
 		}
 	}
@@ -1162,7 +1164,7 @@ rte_eal_init(int argc, char **argv)
 	if (rte_eal_log_init(logid, internal_conf->syslog_facility) < 0) {
 		rte_eal_init_alert("Cannot init logging.");
 		rte_errno = ENOMEM;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -1170,7 +1172,7 @@ rte_eal_init(int argc, char **argv)
 	if (rte_eal_vfio_setup() < 0) {
 		rte_eal_init_alert("Cannot init VFIO");
 		rte_errno = EAGAIN;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 #endif
-- 
2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v2 2/4] bbdev: use C11 atomic builtins for device processing counter
  2020-09-16  8:23 ` [dpdk-dev] [PATCH v2 " Phil Yang
  2020-09-16  8:23   ` [dpdk-dev] [PATCH v2 1/4] eal: use C11 atomic builtins for already initialized check Phil Yang
@ 2020-09-16  8:23   ` Phil Yang
  2020-09-16  8:23   ` [dpdk-dev] [PATCH v2 3/4] power: use C11 atomic builtins for power in use state update Phil Yang
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 27+ messages in thread
From: Phil Yang @ 2020-09-16  8:23 UTC (permalink / raw)
  To: david.marchand, dev
  Cc: Honnappa.Nagarahalli, Ruifeng.Wang, nd, Nicolas Chautru

Since rte_atomicXX APIs are not allowed to be used, use C11 atomic builtins
for device processing counter.

Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 lib/librte_bbdev/rte_bbdev.c | 5 +++--
 lib/librte_bbdev/rte_bbdev.h | 4 +---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/librte_bbdev/rte_bbdev.c b/lib/librte_bbdev/rte_bbdev.c
index a4fdb69..5ba891c 100644
--- a/lib/librte_bbdev/rte_bbdev.c
+++ b/lib/librte_bbdev/rte_bbdev.c
@@ -210,7 +210,7 @@ rte_bbdev_allocate(const char *name)
 		return NULL;
 	}
 
-	rte_atomic16_inc(&bbdev->data->process_cnt);
+	__atomic_add_fetch(&bbdev->data->process_cnt, 1, __ATOMIC_RELAXED);
 	bbdev->data->dev_id = dev_id;
 	bbdev->state = RTE_BBDEV_INITIALIZED;
 
@@ -252,7 +252,8 @@ rte_bbdev_release(struct rte_bbdev *bbdev)
 	}
 
 	/* clear shared BBDev Data if no process is using the device anymore */
-	if (rte_atomic16_dec_and_test(&bbdev->data->process_cnt))
+	if (__atomic_sub_fetch(&bbdev->data->process_cnt, 1,
+			      __ATOMIC_RELAXED) == 0)
 		memset(bbdev->data, 0, sizeof(*bbdev->data));
 
 	memset(bbdev, 0, sizeof(*bbdev));
diff --git a/lib/librte_bbdev/rte_bbdev.h b/lib/librte_bbdev/rte_bbdev.h
index 5729137..7017124 100644
--- a/lib/librte_bbdev/rte_bbdev.h
+++ b/lib/librte_bbdev/rte_bbdev.h
@@ -33,7 +33,6 @@ extern "C" {
 #include <string.h>
 
 #include <rte_compat.h>
-#include <rte_atomic.h>
 #include <rte_bus.h>
 #include <rte_cpuflags.h>
 #include <rte_memory.h>
@@ -426,8 +425,7 @@ struct rte_bbdev_data {
 	uint16_t dev_id;  /**< Device ID */
 	int socket_id;  /**< NUMA socket that device is on */
 	bool started;  /**< Device run-time state */
-	/** Counter of processes using the device */
-	rte_atomic16_t process_cnt;
+	uint16_t process_cnt;  /** Counter of processes using the device */
 };
 
 /* Forward declarations */
-- 
2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v2 3/4] power: use C11 atomic builtins for power in use state update
  2020-09-16  8:23 ` [dpdk-dev] [PATCH v2 " Phil Yang
  2020-09-16  8:23   ` [dpdk-dev] [PATCH v2 1/4] eal: use C11 atomic builtins for already initialized check Phil Yang
  2020-09-16  8:23   ` [dpdk-dev] [PATCH v2 2/4] bbdev: use C11 atomic builtins for device processing counter Phil Yang
@ 2020-09-16  8:23   ` Phil Yang
  2020-09-16  8:23   ` [dpdk-dev] [PATCH v2 4/4] ethdev: use C11 atomic builtins for link status update Phil Yang
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 27+ messages in thread
From: Phil Yang @ 2020-09-16  8:23 UTC (permalink / raw)
  To: david.marchand, dev; +Cc: Honnappa.Nagarahalli, Ruifeng.Wang, nd, David Hunt

Since rte_atomicXX APIs are not allowed to be used, use C11 atomic
builtins for power in use state update.

Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 lib/librte_power/power_acpi_cpufreq.c   | 45 +++++++++++++++++++++++++--------
 lib/librte_power/power_pstate_cpufreq.c | 45 +++++++++++++++++++++++++--------
 2 files changed, 70 insertions(+), 20 deletions(-)

diff --git a/lib/librte_power/power_acpi_cpufreq.c b/lib/librte_power/power_acpi_cpufreq.c
index 583815a..84a9d75 100644
--- a/lib/librte_power/power_acpi_cpufreq.c
+++ b/lib/librte_power/power_acpi_cpufreq.c
@@ -12,7 +12,6 @@
 #include <signal.h>
 #include <limits.h>
 
-#include <rte_atomic.h>
 #include <rte_memcpy.h>
 #include <rte_memory.h>
 #include <rte_string_fns.h>
@@ -86,7 +85,7 @@ struct rte_power_info {
 	FILE *f;                             /**< FD of scaling_setspeed */
 	char governor_ori[32];               /**< Original governor name */
 	uint32_t curr_idx;                   /**< Freq index in freqs array */
-	volatile uint32_t state;             /**< Power in use state */
+	uint32_t state;                      /**< Power in use state */
 	uint16_t turbo_available;            /**< Turbo Boost available */
 	uint16_t turbo_enable;               /**< Turbo Boost enable/disable */
 } __rte_cache_aligned;
@@ -300,6 +299,7 @@ int
 power_acpi_cpufreq_init(unsigned int lcore_id)
 {
 	struct rte_power_info *pi;
+	uint32_t exp_state;
 
 	if (lcore_id >= RTE_MAX_LCORE) {
 		RTE_LOG(ERR, POWER, "Lcore id %u can not exceeds %u\n",
@@ -308,8 +308,16 @@ power_acpi_cpufreq_init(unsigned int lcore_id)
 	}
 
 	pi = &lcore_power_info[lcore_id];
-	if (rte_atomic32_cmpset(&(pi->state), POWER_IDLE, POWER_ONGOING)
-			== 0) {
+	exp_state = POWER_IDLE;
+	/* The power in use state works as a guard variable between
+	 * the CPU frequency control initialization and exit process.
+	 * The ACQUIRE memory ordering here pairs with the RELEASE
+	 * ordering below as lock to make sure the frequency operations
+	 * in the critical section are done under the correct state.
+	 */
+	if (!__atomic_compare_exchange_n(&(pi->state), &exp_state,
+					POWER_ONGOING, 0,
+					__ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
 		RTE_LOG(INFO, POWER, "Power management of lcore %u is "
 				"in use\n", lcore_id);
 		return -1;
@@ -346,12 +354,16 @@ power_acpi_cpufreq_init(unsigned int lcore_id)
 
 	RTE_LOG(INFO, POWER, "Initialized successfully for lcore %u "
 			"power management\n", lcore_id);
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_USED);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_USED,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return 0;
 
 fail:
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_UNKNOWN);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_UNKNOWN,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return -1;
 }
@@ -408,6 +420,7 @@ int
 power_acpi_cpufreq_exit(unsigned int lcore_id)
 {
 	struct rte_power_info *pi;
+	uint32_t exp_state;
 
 	if (lcore_id >= RTE_MAX_LCORE) {
 		RTE_LOG(ERR, POWER, "Lcore id %u can not exceeds %u\n",
@@ -415,8 +428,16 @@ power_acpi_cpufreq_exit(unsigned int lcore_id)
 		return -1;
 	}
 	pi = &lcore_power_info[lcore_id];
-	if (rte_atomic32_cmpset(&(pi->state), POWER_USED, POWER_ONGOING)
-			== 0) {
+	exp_state = POWER_USED;
+	/* The power in use state works as a guard variable between
+	 * the CPU frequency control initialization and exit process.
+	 * The ACQUIRE memory ordering here pairs with the RELEASE
+	 * ordering below as lock to make sure the frequency operations
+	 * in the critical section are done under the correct state.
+	 */
+	if (!__atomic_compare_exchange_n(&(pi->state), &exp_state,
+					POWER_ONGOING, 0,
+					__ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
 		RTE_LOG(INFO, POWER, "Power management of lcore %u is "
 				"not used\n", lcore_id);
 		return -1;
@@ -436,12 +457,16 @@ power_acpi_cpufreq_exit(unsigned int lcore_id)
 	RTE_LOG(INFO, POWER, "Power management of lcore %u has exited from "
 			"'userspace' mode and been set back to the "
 			"original\n", lcore_id);
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_IDLE);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_IDLE,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return 0;
 
 fail:
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_UNKNOWN);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_UNKNOWN,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return -1;
 }
diff --git a/lib/librte_power/power_pstate_cpufreq.c b/lib/librte_power/power_pstate_cpufreq.c
index 2526441..e3126d3 100644
--- a/lib/librte_power/power_pstate_cpufreq.c
+++ b/lib/librte_power/power_pstate_cpufreq.c
@@ -14,7 +14,6 @@
 #include <errno.h>
 #include <inttypes.h>
 
-#include <rte_atomic.h>
 #include <rte_memcpy.h>
 #include <rte_memory.h>
 #include <rte_string_fns.h>
@@ -100,7 +99,7 @@ struct pstate_power_info {
 	uint32_t non_turbo_max_ratio;        /**< Non Turbo Max ratio  */
 	uint32_t sys_max_freq;               /**< system wide max freq  */
 	uint32_t core_base_freq;             /**< core base freq  */
-	volatile uint32_t state;             /**< Power in use state */
+	uint32_t state;                      /**< Power in use state */
 	uint16_t turbo_available;            /**< Turbo Boost available */
 	uint16_t turbo_enable;               /**< Turbo Boost enable/disable */
 	uint16_t priority_core;              /**< High Performance core */
@@ -542,6 +541,7 @@ int
 power_pstate_cpufreq_init(unsigned int lcore_id)
 {
 	struct pstate_power_info *pi;
+	uint32_t exp_state;
 
 	if (lcore_id >= RTE_MAX_LCORE) {
 		RTE_LOG(ERR, POWER, "Lcore id %u can not exceed %u\n",
@@ -550,8 +550,16 @@ power_pstate_cpufreq_init(unsigned int lcore_id)
 	}
 
 	pi = &lcore_power_info[lcore_id];
-	if (rte_atomic32_cmpset(&(pi->state), POWER_IDLE, POWER_ONGOING)
-			== 0) {
+	exp_state = POWER_IDLE;
+	/* The power in use state works as a guard variable between
+	 * the CPU frequency control initialization and exit process.
+	 * The ACQUIRE memory ordering here pairs with the RELEASE
+	 * ordering below as lock to make sure the frequency operations
+	 * in the critical section are done under the correct state.
+	 */
+	if (!__atomic_compare_exchange_n(&(pi->state), &exp_state,
+					POWER_ONGOING, 0,
+					__ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
 		RTE_LOG(INFO, POWER, "Power management of lcore %u is "
 				"in use\n", lcore_id);
 		return -1;
@@ -588,12 +596,16 @@ power_pstate_cpufreq_init(unsigned int lcore_id)
 
 	RTE_LOG(INFO, POWER, "Initialized successfully for lcore %u "
 			"power management\n", lcore_id);
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_USED);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_USED,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return 0;
 
 fail:
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_UNKNOWN);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_UNKNOWN,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return -1;
 }
@@ -602,6 +614,7 @@ int
 power_pstate_cpufreq_exit(unsigned int lcore_id)
 {
 	struct pstate_power_info *pi;
+	uint32_t exp_state;
 
 	if (lcore_id >= RTE_MAX_LCORE) {
 		RTE_LOG(ERR, POWER, "Lcore id %u can not exceeds %u\n",
@@ -610,8 +623,16 @@ power_pstate_cpufreq_exit(unsigned int lcore_id)
 	}
 	pi = &lcore_power_info[lcore_id];
 
-	if (rte_atomic32_cmpset(&(pi->state), POWER_USED, POWER_ONGOING)
-			== 0) {
+	exp_state = POWER_USED;
+	/* The power in use state works as a guard variable between
+	 * the CPU frequency control initialization and exit process.
+	 * The ACQUIRE memory ordering here pairs with the RELEASE
+	 * ordering below as lock to make sure the frequency operations
+	 * in the critical section are under done the correct state.
+	 */
+	if (!__atomic_compare_exchange_n(&(pi->state), &exp_state,
+					POWER_ONGOING, 0,
+					__ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
 		RTE_LOG(INFO, POWER, "Power management of lcore %u is "
 				"not used\n", lcore_id);
 		return -1;
@@ -633,12 +654,16 @@ power_pstate_cpufreq_exit(unsigned int lcore_id)
 	RTE_LOG(INFO, POWER, "Power management of lcore %u has exited from "
 			"'performance' mode and been set back to the "
 			"original\n", lcore_id);
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_IDLE);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_IDLE,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return 0;
 
 fail:
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_UNKNOWN);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_UNKNOWN,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return -1;
 }
-- 
2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v2 4/4] ethdev: use C11 atomic builtins for link status update
  2020-09-16  8:23 ` [dpdk-dev] [PATCH v2 " Phil Yang
                     ` (2 preceding siblings ...)
  2020-09-16  8:23   ` [dpdk-dev] [PATCH v2 3/4] power: use C11 atomic builtins for power in use state update Phil Yang
@ 2020-09-16  8:23   ` Phil Yang
  2020-09-17 16:08     ` Andrew Rybchenko
  2020-09-23 13:18   ` [dpdk-dev] [PATCH v2 0/4] use C11 atomic builtins for libs David Marchand
  2020-09-24  5:39   ` [dpdk-dev] [PATCH v3 " Phil Yang
  5 siblings, 1 reply; 27+ messages in thread
From: Phil Yang @ 2020-09-16  8:23 UTC (permalink / raw)
  To: david.marchand, dev
  Cc: Honnappa.Nagarahalli, Ruifeng.Wang, nd, Thomas Monjalon,
	Ferruh Yigit, Andrew Rybchenko

Since rte_atomicXX APIs are not allowed to be used, use C11 atomic
builtins for link status update.

Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 lib/librte_ethdev/rte_ethdev_driver.h | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
index 13fd049..ebaf6da 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -216,8 +216,7 @@ static inline int
 rte_eth_linkstatus_set(struct rte_eth_dev *dev,
 		       const struct rte_eth_link *new_link)
 {
-	volatile uint64_t *dev_link
-		 = (volatile uint64_t *)&(dev->data->dev_link);
+	uint64_t *dev_link = (uint64_t *)&(dev->data->dev_link);
 	union {
 		uint64_t val64;
 		struct rte_eth_link link;
@@ -225,8 +224,8 @@ rte_eth_linkstatus_set(struct rte_eth_dev *dev,
 
 	RTE_BUILD_BUG_ON(sizeof(*new_link) != sizeof(uint64_t));
 
-	orig.val64 = rte_atomic64_exchange(dev_link,
-					   *(const uint64_t *)new_link);
+	orig.val64 = __atomic_exchange_n(dev_link, *(const uint64_t *)new_link,
+					__ATOMIC_SEQ_CST);
 
 	return (orig.link.link_status == new_link->link_status) ? -1 : 0;
 }
@@ -244,20 +243,12 @@ static inline void
 rte_eth_linkstatus_get(const struct rte_eth_dev *dev,
 		       struct rte_eth_link *link)
 {
-	volatile uint64_t *src = (uint64_t *)&(dev->data->dev_link);
+	uint64_t *src = (uint64_t *)&(dev->data->dev_link);
 	uint64_t *dst = (uint64_t *)link;
 
 	RTE_BUILD_BUG_ON(sizeof(*link) != sizeof(uint64_t));
 
-#ifdef __LP64__
-	/* if cpu arch has 64 bit unsigned lon then implicitly atomic */
-	*dst = *src;
-#else
-	/* can't use rte_atomic64_read because it returns signed int */
-	do {
-		*dst = *src;
-	} while (!rte_atomic64_cmpset(src, *dst, *dst));
-#endif
+	*dst = __atomic_load_n(src, __ATOMIC_SEQ_CST);
 }
 
 /**
-- 
2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v2 4/4] ethdev: use C11 atomic builtins for link status update
  2020-09-16  8:23   ` [dpdk-dev] [PATCH v2 4/4] ethdev: use C11 atomic builtins for link status update Phil Yang
@ 2020-09-17 16:08     ` Andrew Rybchenko
  0 siblings, 0 replies; 27+ messages in thread
From: Andrew Rybchenko @ 2020-09-17 16:08 UTC (permalink / raw)
  To: Phil Yang, david.marchand, dev
  Cc: Honnappa.Nagarahalli, Ruifeng.Wang, nd, Thomas Monjalon, Ferruh Yigit

On 9/16/20 11:23 AM, Phil Yang wrote:
> Since rte_atomicXX APIs are not allowed to be used, use C11 atomic
> builtins for link status update.
> 
> Signed-off-by: Phil Yang <phil.yang@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/4] eal: use C11 atomic builtins for already initialized check
  2020-09-16  8:23   ` [dpdk-dev] [PATCH v2 1/4] eal: use C11 atomic builtins for already initialized check Phil Yang
@ 2020-09-23 13:06     ` David Marchand
  2020-09-24  3:44       ` Phil Yang
  0 siblings, 1 reply; 27+ messages in thread
From: David Marchand @ 2020-09-23 13:06 UTC (permalink / raw)
  To: Phil Yang
  Cc: dev, Honnappa Nagarahalli, Ruifeng Wang (Arm Technology China),
	nd, Bruce Richardson

On Wed, Sep 16, 2020 at 10:24 AM Phil Yang <phil.yang@arm.com> wrote:
>
> Since rte_atomicXX APIs are not allowed to be used, use C11 builtins to
> check if EAL is already initialized.
>
> Signed-off-by: Phil Yang <phil.yang@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>  lib/librte_eal/freebsd/eal.c | 18 ++++++++++--------
>  lib/librte_eal/linux/eal.c   | 20 +++++++++++---------
>  2 files changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
> index 798add0..9f4c7bb 100644
> --- a/lib/librte_eal/freebsd/eal.c
> +++ b/lib/librte_eal/freebsd/eal.c
> @@ -665,7 +665,8 @@ rte_eal_init(int argc, char **argv)
>  {
>         int i, fctret, ret;
>         pthread_t thread_id;
> -       static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0);
> +       static uint32_t run_once;
> +       uint32_t has_run = 0;
>         char cpuset[RTE_CPU_AFFINITY_STR_LEN];
>         char thread_name[RTE_MAX_THREAD_NAME_LEN];
>         const struct rte_config *config = rte_eal_get_configuration();
> @@ -679,7 +680,8 @@ rte_eal_init(int argc, char **argv)
>                 return -1;
>         }
>
> -       if (!rte_atomic32_test_and_set(&run_once)) {
> +       if (!__atomic_compare_exchange_n(&run_once, &has_run, 1, 0,
> +                                       __ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
>                 rte_eal_init_alert("already called initialization.");
>                 rte_errno = EALREADY;
>                 return -1;
> @@ -705,7 +707,7 @@ rte_eal_init(int argc, char **argv)
>         if (fctret < 0) {
>                 rte_eal_init_alert("Invalid 'command line' arguments.");
>                 rte_errno = EINVAL;
> -               rte_atomic32_clear(&run_once);
> +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
>                 return -1;
>         }
>
> @@ -715,20 +717,20 @@ rte_eal_init(int argc, char **argv)
>         if (eal_plugins_init() < 0) {
>                 rte_eal_init_alert("Cannot init plugins");
>                 rte_errno = EINVAL;
> -               rte_atomic32_clear(&run_once);
> +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
>                 return -1;
>         }
>
>         if (eal_trace_init() < 0) {
>                 rte_eal_init_alert("Cannot init trace");
>                 rte_errno = EFAULT;
> -               rte_atomic32_clear(&run_once);
> +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
>                 return -1;
>         }
>
>         if (eal_option_device_parse()) {
>                 rte_errno = ENODEV;
> -               rte_atomic32_clear(&run_once);
> +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
>                 return -1;
>         }
>
> @@ -762,7 +764,7 @@ rte_eal_init(int argc, char **argv)
>         if (rte_bus_scan()) {
>                 rte_eal_init_alert("Cannot scan the buses for devices");
>                 rte_errno = ENODEV;
> -               rte_atomic32_clear(&run_once);
> +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
>                 return -1;
>         }
>
> @@ -790,7 +792,7 @@ rte_eal_init(int argc, char **argv)
>                 if (ret < 0) {
>                         rte_eal_init_alert("Cannot get hugepage information.");
>                         rte_errno = EACCES;
> -                       rte_atomic32_clear(&run_once);
> +                       __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
>                         return -1;
>                 }
>         }
> diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
> index 0960f01..82a73ed 100644
> --- a/lib/librte_eal/linux/eal.c
> +++ b/lib/librte_eal/linux/eal.c
> @@ -960,7 +960,8 @@ rte_eal_init(int argc, char **argv)
>  {
>         int i, fctret, ret;
>         pthread_t thread_id;
> -       static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0);
> +       static uint32_t run_once;
> +       uint32_t has_run = 0;
>         const char *p;
>         static char logid[PATH_MAX];
>         char cpuset[RTE_CPU_AFFINITY_STR_LEN];
> @@ -977,7 +978,8 @@ rte_eal_init(int argc, char **argv)
>                 return -1;
>         }
>
> -       if (!rte_atomic32_test_and_set(&run_once)) {
> +       if (!__atomic_compare_exchange_n(&run_once, &has_run, 1, 0,
> +                                       __ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
>                 rte_eal_init_alert("already called initialization.");
>                 rte_errno = EALREADY;
>                 return -1;
> @@ -1005,14 +1007,14 @@ rte_eal_init(int argc, char **argv)
>         if (fctret < 0) {
>                 rte_eal_init_alert("Invalid 'command line' arguments.");
>                 rte_errno = EINVAL;
> -               rte_atomic32_clear(&run_once);
> +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
>                 return -1;
>         }
>
>         if (eal_plugins_init() < 0) {
>                 rte_eal_init_alert("Cannot init plugins");
>                 rte_errno = EINVAL;
> -               rte_atomic32_clear(&run_once);
> +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
>                 return -1;
>         }
>
> @@ -1024,7 +1026,7 @@ rte_eal_init(int argc, char **argv)
>
>         if (eal_option_device_parse()) {
>                 rte_errno = ENODEV;
> -               rte_atomic32_clear(&run_once);
> +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
>                 return -1;
>         }
>
> @@ -1064,7 +1066,7 @@ rte_eal_init(int argc, char **argv)
>         if (rte_bus_scan()) {
>                 rte_eal_init_alert("Cannot scan the buses for devices");
>                 rte_errno = ENODEV;
> -               rte_atomic32_clear(&run_once);
> +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
>                 return -1;
>         }
>
> @@ -1138,7 +1140,7 @@ rte_eal_init(int argc, char **argv)
>                 if (ret < 0) {
>                         rte_eal_init_alert("Cannot get hugepage information.");
>                         rte_errno = EACCES;
> -                       rte_atomic32_clear(&run_once);
> +                       __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
>                         return -1;
>                 }
>         }
> @@ -1162,7 +1164,7 @@ rte_eal_init(int argc, char **argv)
>         if (rte_eal_log_init(logid, internal_conf->syslog_facility) < 0) {
>                 rte_eal_init_alert("Cannot init logging.");
>                 rte_errno = ENOMEM;
> -               rte_atomic32_clear(&run_once);
> +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
>                 return -1;
>         }
>
> @@ -1170,7 +1172,7 @@ rte_eal_init(int argc, char **argv)
>         if (rte_eal_vfio_setup() < 0) {
>                 rte_eal_init_alert("Cannot init VFIO");
>                 rte_errno = EAGAIN;
> -               rte_atomic32_clear(&run_once);
> +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
>                 return -1;
>         }
>  #endif
> --
> 2.7.4
>

I see no reason to include rte_atomic.h in those files after this patch.
Did I miss something?


-- 
David Marchand


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/4] use C11 atomic builtins for libs
  2020-09-16  8:23 ` [dpdk-dev] [PATCH v2 " Phil Yang
                     ` (3 preceding siblings ...)
  2020-09-16  8:23   ` [dpdk-dev] [PATCH v2 4/4] ethdev: use C11 atomic builtins for link status update Phil Yang
@ 2020-09-23 13:18   ` David Marchand
  2020-09-24  3:47     ` Phil Yang
  2020-09-24  5:39   ` [dpdk-dev] [PATCH v3 " Phil Yang
  5 siblings, 1 reply; 27+ messages in thread
From: David Marchand @ 2020-09-23 13:18 UTC (permalink / raw)
  To: Phil Yang, Nicolas Chautru, David Hunt
  Cc: dev, Honnappa Nagarahalli, Ruifeng Wang (Arm Technology China), nd

On Wed, Sep 16, 2020 at 10:24 AM Phil Yang <phil.yang@arm.com> wrote:
>
> Since rte_atomicXX APIs are not allowed to be used[1][2], use C11 atomic
> builtins instead in eal, bbdev, power, and ethdev libs.
>
> [1] http://code.dpdk.org/dpdk/latest/source/doc/guides/rel_notes/deprecation.rst#L87
> [2] http://code.dpdk.org/dpdk/latest/source/devtools/checkpatches.sh#L80
>
> v2:
> Fix Clang int-conversion warning.
>
> v1:
> Initial version.
>
> Phil Yang (4):
>   eal: use C11 atomic builtins for already initialized check
>   bbdev: use C11 atomic builtins for device processing counter
>   power: use C11 atomic builtins for power in use state update
>   ethdev: use C11 atomic builtins for link status update

Thanks Phil.
Just a small comment on the first patch, the rest lgtm.

Nicolas, David H., could you have a look at (resp.) patch 2, 3?


-- 
David Marchand


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/4] eal: use C11 atomic builtins for already initialized check
  2020-09-23 13:06     ` David Marchand
@ 2020-09-24  3:44       ` Phil Yang
  0 siblings, 0 replies; 27+ messages in thread
From: Phil Yang @ 2020-09-24  3:44 UTC (permalink / raw)
  To: David Marchand
  Cc: dev, Honnappa Nagarahalli, Ruifeng Wang, nd, Bruce Richardson, nd

David Marchand <david.marchand@redhat.com> writes:

> Subject: Re: [PATCH v2 1/4] eal: use C11 atomic builtins for already initialized
> check
> 
> On Wed, Sep 16, 2020 at 10:24 AM Phil Yang <phil.yang@arm.com> wrote:
> >
> > Since rte_atomicXX APIs are not allowed to be used, use C11 builtins to
> > check if EAL is already initialized.
> >
> > Signed-off-by: Phil Yang <phil.yang@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > ---
> >  lib/librte_eal/freebsd/eal.c | 18 ++++++++++--------
> >  lib/librte_eal/linux/eal.c   | 20 +++++++++++---------
> >  2 files changed, 21 insertions(+), 17 deletions(-)
> >
> > diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
> > index 798add0..9f4c7bb 100644
> > --- a/lib/librte_eal/freebsd/eal.c
> > +++ b/lib/librte_eal/freebsd/eal.c
> > @@ -665,7 +665,8 @@ rte_eal_init(int argc, char **argv)
> >  {
> >         int i, fctret, ret;
> >         pthread_t thread_id;
> > -       static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0);
> > +       static uint32_t run_once;
> > +       uint32_t has_run = 0;
> >         char cpuset[RTE_CPU_AFFINITY_STR_LEN];
> >         char thread_name[RTE_MAX_THREAD_NAME_LEN];
> >         const struct rte_config *config = rte_eal_get_configuration();
> > @@ -679,7 +680,8 @@ rte_eal_init(int argc, char **argv)
> >                 return -1;
> >         }
> >
> > -       if (!rte_atomic32_test_and_set(&run_once)) {
> > +       if (!__atomic_compare_exchange_n(&run_once, &has_run, 1, 0,
> > +                                       __ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
> >                 rte_eal_init_alert("already called initialization.");
> >                 rte_errno = EALREADY;
> >                 return -1;
> > @@ -705,7 +707,7 @@ rte_eal_init(int argc, char **argv)
> >         if (fctret < 0) {
> >                 rte_eal_init_alert("Invalid 'command line' arguments.");
> >                 rte_errno = EINVAL;
> > -               rte_atomic32_clear(&run_once);
> > +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
> >                 return -1;
> >         }
> >
> > @@ -715,20 +717,20 @@ rte_eal_init(int argc, char **argv)
> >         if (eal_plugins_init() < 0) {
> >                 rte_eal_init_alert("Cannot init plugins");
> >                 rte_errno = EINVAL;
> > -               rte_atomic32_clear(&run_once);
> > +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
> >                 return -1;
> >         }
> >
> >         if (eal_trace_init() < 0) {
> >                 rte_eal_init_alert("Cannot init trace");
> >                 rte_errno = EFAULT;
> > -               rte_atomic32_clear(&run_once);
> > +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
> >                 return -1;
> >         }
> >
> >         if (eal_option_device_parse()) {
> >                 rte_errno = ENODEV;
> > -               rte_atomic32_clear(&run_once);
> > +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
> >                 return -1;
> >         }
> >
> > @@ -762,7 +764,7 @@ rte_eal_init(int argc, char **argv)
> >         if (rte_bus_scan()) {
> >                 rte_eal_init_alert("Cannot scan the buses for devices");
> >                 rte_errno = ENODEV;
> > -               rte_atomic32_clear(&run_once);
> > +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
> >                 return -1;
> >         }
> >
> > @@ -790,7 +792,7 @@ rte_eal_init(int argc, char **argv)
> >                 if (ret < 0) {
> >                         rte_eal_init_alert("Cannot get hugepage information.");
> >                         rte_errno = EACCES;
> > -                       rte_atomic32_clear(&run_once);
> > +                       __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
> >                         return -1;
> >                 }
> >         }
> > diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
> > index 0960f01..82a73ed 100644
> > --- a/lib/librte_eal/linux/eal.c
> > +++ b/lib/librte_eal/linux/eal.c
> > @@ -960,7 +960,8 @@ rte_eal_init(int argc, char **argv)
> >  {
> >         int i, fctret, ret;
> >         pthread_t thread_id;
> > -       static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0);
> > +       static uint32_t run_once;
> > +       uint32_t has_run = 0;
> >         const char *p;
> >         static char logid[PATH_MAX];
> >         char cpuset[RTE_CPU_AFFINITY_STR_LEN];
> > @@ -977,7 +978,8 @@ rte_eal_init(int argc, char **argv)
> >                 return -1;
> >         }
> >
> > -       if (!rte_atomic32_test_and_set(&run_once)) {
> > +       if (!__atomic_compare_exchange_n(&run_once, &has_run, 1, 0,
> > +                                       __ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
> >                 rte_eal_init_alert("already called initialization.");
> >                 rte_errno = EALREADY;
> >                 return -1;
> > @@ -1005,14 +1007,14 @@ rte_eal_init(int argc, char **argv)
> >         if (fctret < 0) {
> >                 rte_eal_init_alert("Invalid 'command line' arguments.");
> >                 rte_errno = EINVAL;
> > -               rte_atomic32_clear(&run_once);
> > +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
> >                 return -1;
> >         }
> >
> >         if (eal_plugins_init() < 0) {
> >                 rte_eal_init_alert("Cannot init plugins");
> >                 rte_errno = EINVAL;
> > -               rte_atomic32_clear(&run_once);
> > +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
> >                 return -1;
> >         }
> >
> > @@ -1024,7 +1026,7 @@ rte_eal_init(int argc, char **argv)
> >
> >         if (eal_option_device_parse()) {
> >                 rte_errno = ENODEV;
> > -               rte_atomic32_clear(&run_once);
> > +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
> >                 return -1;
> >         }
> >
> > @@ -1064,7 +1066,7 @@ rte_eal_init(int argc, char **argv)
> >         if (rte_bus_scan()) {
> >                 rte_eal_init_alert("Cannot scan the buses for devices");
> >                 rte_errno = ENODEV;
> > -               rte_atomic32_clear(&run_once);
> > +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
> >                 return -1;
> >         }
> >
> > @@ -1138,7 +1140,7 @@ rte_eal_init(int argc, char **argv)
> >                 if (ret < 0) {
> >                         rte_eal_init_alert("Cannot get hugepage information.");
> >                         rte_errno = EACCES;
> > -                       rte_atomic32_clear(&run_once);
> > +                       __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
> >                         return -1;
> >                 }
> >         }
> > @@ -1162,7 +1164,7 @@ rte_eal_init(int argc, char **argv)
> >         if (rte_eal_log_init(logid, internal_conf->syslog_facility) < 0) {
> >                 rte_eal_init_alert("Cannot init logging.");
> >                 rte_errno = ENOMEM;
> > -               rte_atomic32_clear(&run_once);
> > +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
> >                 return -1;
> >         }
> >
> > @@ -1170,7 +1172,7 @@ rte_eal_init(int argc, char **argv)
> >         if (rte_eal_vfio_setup() < 0) {
> >                 rte_eal_init_alert("Cannot init VFIO");
> >                 rte_errno = EAGAIN;
> > -               rte_atomic32_clear(&run_once);
> > +               __atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
> >                 return -1;
> >         }
> >  #endif
> > --
> > 2.7.4
> >
> 
> I see no reason to include rte_atomic.h in those files after this patch.
> Did I miss something?

No, it is not needed anymore. 
Will remove them.


Thanks,
Phil

> 
> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v2 0/4] use C11 atomic builtins for libs
  2020-09-23 13:18   ` [dpdk-dev] [PATCH v2 0/4] use C11 atomic builtins for libs David Marchand
@ 2020-09-24  3:47     ` Phil Yang
  0 siblings, 0 replies; 27+ messages in thread
From: Phil Yang @ 2020-09-24  3:47 UTC (permalink / raw)
  To: David Marchand, Nicolas Chautru, David Hunt
  Cc: dev, Honnappa Nagarahalli, Ruifeng Wang, nd, nd

David Marchand <david.marchand@redhat.com> writes:


> Subject: Re: [PATCH v2 0/4] use C11 atomic builtins for libs
> 
> On Wed, Sep 16, 2020 at 10:24 AM Phil Yang <phil.yang@arm.com> wrote:
> >
> > Since rte_atomicXX APIs are not allowed to be used[1][2], use C11 atomic
> > builtins instead in eal, bbdev, power, and ethdev libs.
> >
> > [1]
> http://code.dpdk.org/dpdk/latest/source/doc/guides/rel_notes/deprecatio
> n.rst#L87
> > [2]
> http://code.dpdk.org/dpdk/latest/source/devtools/checkpatches.sh#L80
> >
> > v2:
> > Fix Clang int-conversion warning.
> >
> > v1:
> > Initial version.
> >
> > Phil Yang (4):
> >   eal: use C11 atomic builtins for already initialized check
> >   bbdev: use C11 atomic builtins for device processing counter
> >   power: use C11 atomic builtins for power in use state update
> >   ethdev: use C11 atomic builtins for link status update
> 
> Thanks Phil.
> Just a small comment on the first patch, the rest lgtm.
> 
> Nicolas, David H., could you have a look at (resp.) patch 2, 3?

Thanks for your comments.
I will address them in the next version.


Thanks,
Phil
> 
> 
> --
> David Marchand


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v3 0/4] use C11 atomic builtins for libs
  2020-09-16  8:23 ` [dpdk-dev] [PATCH v2 " Phil Yang
                     ` (4 preceding siblings ...)
  2020-09-23 13:18   ` [dpdk-dev] [PATCH v2 0/4] use C11 atomic builtins for libs David Marchand
@ 2020-09-24  5:39   ` Phil Yang
  2020-09-24  5:39     ` [dpdk-dev] [PATCH v3 1/4] eal: use C11 atomic builtins for already initialized check Phil Yang
                       ` (4 more replies)
  5 siblings, 5 replies; 27+ messages in thread
From: Phil Yang @ 2020-09-24  5:39 UTC (permalink / raw)
  To: dev, david.marchand, nicolas.chautru, david.hunt
  Cc: Ruifeng.Wang, Honnappa.Nagarahalli, nd

Since rte_atomicXX APIs are not allowed to be used[1][2], use C11 atomic
builtins instead in eal, bbdev, power, and ethdev libs.

[1] http://code.dpdk.org/dpdk/latest/source/doc/guides/rel_notes/deprecation.rst#L87
[2] http://code.dpdk.org/dpdk/latest/source/devtools/checkpatches.sh#L80

v3:
remove unnecessary rte_atomic.h headers. (David)

v2:
Fix Clang int-conversion warning.

v1:
Initial version.

Phil Yang (4):
  eal: use C11 atomic builtins for already initialized check
  bbdev: use C11 atomic builtins for device processing counter
  power: use C11 atomic builtins for power in use state update
  ethdev: use C11 atomic builtins for link status update

 lib/librte_bbdev/rte_bbdev.c            |  5 ++--
 lib/librte_bbdev/rte_bbdev.h            |  4 +--
 lib/librte_eal/freebsd/eal.c            | 19 +++++++-------
 lib/librte_eal/linux/eal.c              | 21 +++++++--------
 lib/librte_ethdev/rte_ethdev_driver.h   | 19 ++++----------
 lib/librte_power/power_acpi_cpufreq.c   | 45 +++++++++++++++++++++++++--------
 lib/librte_power/power_pstate_cpufreq.c | 45 +++++++++++++++++++++++++--------
 7 files changed, 100 insertions(+), 58 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v3 1/4] eal: use C11 atomic builtins for already initialized check
  2020-09-24  5:39   ` [dpdk-dev] [PATCH v3 " Phil Yang
@ 2020-09-24  5:39     ` Phil Yang
  2020-09-24  5:39     ` [dpdk-dev] [PATCH v3 2/4] bbdev: use C11 atomic builtins for device processing counter Phil Yang
                       ` (3 subsequent siblings)
  4 siblings, 0 replies; 27+ messages in thread
From: Phil Yang @ 2020-09-24  5:39 UTC (permalink / raw)
  To: dev, david.marchand, nicolas.chautru, david.hunt
  Cc: Ruifeng.Wang, Honnappa.Nagarahalli, nd, Bruce Richardson

Since rte_atomicXX APIs are not allowed to be used, use C11 builtins to
check if EAL is already initialized.

Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 lib/librte_eal/freebsd/eal.c | 19 ++++++++++---------
 lib/librte_eal/linux/eal.c   | 21 +++++++++++----------
 2 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
index 798add0..ccea60a 100644
--- a/lib/librte_eal/freebsd/eal.c
+++ b/lib/librte_eal/freebsd/eal.c
@@ -41,7 +41,6 @@
 #include <rte_devargs.h>
 #include <rte_version.h>
 #include <rte_vfio.h>
-#include <rte_atomic.h>
 #include <malloc_heap.h>
 #include <rte_telemetry.h>
 
@@ -665,7 +664,8 @@ rte_eal_init(int argc, char **argv)
 {
 	int i, fctret, ret;
 	pthread_t thread_id;
-	static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0);
+	static uint32_t run_once;
+	uint32_t has_run = 0;
 	char cpuset[RTE_CPU_AFFINITY_STR_LEN];
 	char thread_name[RTE_MAX_THREAD_NAME_LEN];
 	const struct rte_config *config = rte_eal_get_configuration();
@@ -679,7 +679,8 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
-	if (!rte_atomic32_test_and_set(&run_once)) {
+	if (!__atomic_compare_exchange_n(&run_once, &has_run, 1, 0,
+					__ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
 		rte_eal_init_alert("already called initialization.");
 		rte_errno = EALREADY;
 		return -1;
@@ -705,7 +706,7 @@ rte_eal_init(int argc, char **argv)
 	if (fctret < 0) {
 		rte_eal_init_alert("Invalid 'command line' arguments.");
 		rte_errno = EINVAL;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -715,20 +716,20 @@ rte_eal_init(int argc, char **argv)
 	if (eal_plugins_init() < 0) {
 		rte_eal_init_alert("Cannot init plugins");
 		rte_errno = EINVAL;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
 	if (eal_trace_init() < 0) {
 		rte_eal_init_alert("Cannot init trace");
 		rte_errno = EFAULT;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
 	if (eal_option_device_parse()) {
 		rte_errno = ENODEV;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -762,7 +763,7 @@ rte_eal_init(int argc, char **argv)
 	if (rte_bus_scan()) {
 		rte_eal_init_alert("Cannot scan the buses for devices");
 		rte_errno = ENODEV;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -790,7 +791,7 @@ rte_eal_init(int argc, char **argv)
 		if (ret < 0) {
 			rte_eal_init_alert("Cannot get hugepage information.");
 			rte_errno = EACCES;
-			rte_atomic32_clear(&run_once);
+			__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 			return -1;
 		}
 	}
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index 0960f01..9cf0e2e 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -47,7 +47,6 @@
 #include <rte_dev.h>
 #include <rte_devargs.h>
 #include <rte_version.h>
-#include <rte_atomic.h>
 #include <malloc_heap.h>
 #include <rte_vfio.h>
 #include <rte_telemetry.h>
@@ -960,7 +959,8 @@ rte_eal_init(int argc, char **argv)
 {
 	int i, fctret, ret;
 	pthread_t thread_id;
-	static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0);
+	static uint32_t run_once;
+	uint32_t has_run = 0;
 	const char *p;
 	static char logid[PATH_MAX];
 	char cpuset[RTE_CPU_AFFINITY_STR_LEN];
@@ -977,7 +977,8 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
-	if (!rte_atomic32_test_and_set(&run_once)) {
+	if (!__atomic_compare_exchange_n(&run_once, &has_run, 1, 0,
+					__ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
 		rte_eal_init_alert("already called initialization.");
 		rte_errno = EALREADY;
 		return -1;
@@ -1005,14 +1006,14 @@ rte_eal_init(int argc, char **argv)
 	if (fctret < 0) {
 		rte_eal_init_alert("Invalid 'command line' arguments.");
 		rte_errno = EINVAL;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
 	if (eal_plugins_init() < 0) {
 		rte_eal_init_alert("Cannot init plugins");
 		rte_errno = EINVAL;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -1024,7 +1025,7 @@ rte_eal_init(int argc, char **argv)
 
 	if (eal_option_device_parse()) {
 		rte_errno = ENODEV;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -1064,7 +1065,7 @@ rte_eal_init(int argc, char **argv)
 	if (rte_bus_scan()) {
 		rte_eal_init_alert("Cannot scan the buses for devices");
 		rte_errno = ENODEV;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -1138,7 +1139,7 @@ rte_eal_init(int argc, char **argv)
 		if (ret < 0) {
 			rte_eal_init_alert("Cannot get hugepage information.");
 			rte_errno = EACCES;
-			rte_atomic32_clear(&run_once);
+			__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 			return -1;
 		}
 	}
@@ -1162,7 +1163,7 @@ rte_eal_init(int argc, char **argv)
 	if (rte_eal_log_init(logid, internal_conf->syslog_facility) < 0) {
 		rte_eal_init_alert("Cannot init logging.");
 		rte_errno = ENOMEM;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 
@@ -1170,7 +1171,7 @@ rte_eal_init(int argc, char **argv)
 	if (rte_eal_vfio_setup() < 0) {
 		rte_eal_init_alert("Cannot init VFIO");
 		rte_errno = EAGAIN;
-		rte_atomic32_clear(&run_once);
+		__atomic_store_n(&run_once, 0, __ATOMIC_RELAXED);
 		return -1;
 	}
 #endif
-- 
2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v3 2/4] bbdev: use C11 atomic builtins for device processing counter
  2020-09-24  5:39   ` [dpdk-dev] [PATCH v3 " Phil Yang
  2020-09-24  5:39     ` [dpdk-dev] [PATCH v3 1/4] eal: use C11 atomic builtins for already initialized check Phil Yang
@ 2020-09-24  5:39     ` Phil Yang
  2020-09-24 22:01       ` Chautru, Nicolas
  2020-09-24  5:39     ` [dpdk-dev] [PATCH v3 3/4] power: use C11 atomic builtins for power in use state update Phil Yang
                       ` (2 subsequent siblings)
  4 siblings, 1 reply; 27+ messages in thread
From: Phil Yang @ 2020-09-24  5:39 UTC (permalink / raw)
  To: dev, david.marchand, nicolas.chautru, david.hunt
  Cc: Ruifeng.Wang, Honnappa.Nagarahalli, nd

Since rte_atomicXX APIs are not allowed to be used, use C11 atomic builtins
for device processing counter.

Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 lib/librte_bbdev/rte_bbdev.c | 5 +++--
 lib/librte_bbdev/rte_bbdev.h | 4 +---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/lib/librte_bbdev/rte_bbdev.c b/lib/librte_bbdev/rte_bbdev.c
index a4fdb69..5ba891c 100644
--- a/lib/librte_bbdev/rte_bbdev.c
+++ b/lib/librte_bbdev/rte_bbdev.c
@@ -210,7 +210,7 @@ rte_bbdev_allocate(const char *name)
 		return NULL;
 	}
 
-	rte_atomic16_inc(&bbdev->data->process_cnt);
+	__atomic_add_fetch(&bbdev->data->process_cnt, 1, __ATOMIC_RELAXED);
 	bbdev->data->dev_id = dev_id;
 	bbdev->state = RTE_BBDEV_INITIALIZED;
 
@@ -252,7 +252,8 @@ rte_bbdev_release(struct rte_bbdev *bbdev)
 	}
 
 	/* clear shared BBDev Data if no process is using the device anymore */
-	if (rte_atomic16_dec_and_test(&bbdev->data->process_cnt))
+	if (__atomic_sub_fetch(&bbdev->data->process_cnt, 1,
+			      __ATOMIC_RELAXED) == 0)
 		memset(bbdev->data, 0, sizeof(*bbdev->data));
 
 	memset(bbdev, 0, sizeof(*bbdev));
diff --git a/lib/librte_bbdev/rte_bbdev.h b/lib/librte_bbdev/rte_bbdev.h
index 5729137..7017124 100644
--- a/lib/librte_bbdev/rte_bbdev.h
+++ b/lib/librte_bbdev/rte_bbdev.h
@@ -33,7 +33,6 @@ extern "C" {
 #include <string.h>
 
 #include <rte_compat.h>
-#include <rte_atomic.h>
 #include <rte_bus.h>
 #include <rte_cpuflags.h>
 #include <rte_memory.h>
@@ -426,8 +425,7 @@ struct rte_bbdev_data {
 	uint16_t dev_id;  /**< Device ID */
 	int socket_id;  /**< NUMA socket that device is on */
 	bool started;  /**< Device run-time state */
-	/** Counter of processes using the device */
-	rte_atomic16_t process_cnt;
+	uint16_t process_cnt;  /** Counter of processes using the device */
 };
 
 /* Forward declarations */
-- 
2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v3 3/4] power: use C11 atomic builtins for power in use state update
  2020-09-24  5:39   ` [dpdk-dev] [PATCH v3 " Phil Yang
  2020-09-24  5:39     ` [dpdk-dev] [PATCH v3 1/4] eal: use C11 atomic builtins for already initialized check Phil Yang
  2020-09-24  5:39     ` [dpdk-dev] [PATCH v3 2/4] bbdev: use C11 atomic builtins for device processing counter Phil Yang
@ 2020-09-24  5:39     ` Phil Yang
  2020-09-24  8:34       ` David Hunt
  2020-09-24  5:39     ` [dpdk-dev] [PATCH v3 4/4] ethdev: use C11 atomic builtins for link status update Phil Yang
  2020-09-25 13:59     ` [dpdk-dev] [PATCH v3 0/4] use C11 atomic builtins for libs David Marchand
  4 siblings, 1 reply; 27+ messages in thread
From: Phil Yang @ 2020-09-24  5:39 UTC (permalink / raw)
  To: dev, david.marchand, nicolas.chautru, david.hunt
  Cc: Ruifeng.Wang, Honnappa.Nagarahalli, nd

Since rte_atomicXX APIs are not allowed to be used, use C11 atomic
builtins for power in use state update.

Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 lib/librte_power/power_acpi_cpufreq.c   | 45 +++++++++++++++++++++++++--------
 lib/librte_power/power_pstate_cpufreq.c | 45 +++++++++++++++++++++++++--------
 2 files changed, 70 insertions(+), 20 deletions(-)

diff --git a/lib/librte_power/power_acpi_cpufreq.c b/lib/librte_power/power_acpi_cpufreq.c
index 583815a..84a9d75 100644
--- a/lib/librte_power/power_acpi_cpufreq.c
+++ b/lib/librte_power/power_acpi_cpufreq.c
@@ -12,7 +12,6 @@
 #include <signal.h>
 #include <limits.h>
 
-#include <rte_atomic.h>
 #include <rte_memcpy.h>
 #include <rte_memory.h>
 #include <rte_string_fns.h>
@@ -86,7 +85,7 @@ struct rte_power_info {
 	FILE *f;                             /**< FD of scaling_setspeed */
 	char governor_ori[32];               /**< Original governor name */
 	uint32_t curr_idx;                   /**< Freq index in freqs array */
-	volatile uint32_t state;             /**< Power in use state */
+	uint32_t state;                      /**< Power in use state */
 	uint16_t turbo_available;            /**< Turbo Boost available */
 	uint16_t turbo_enable;               /**< Turbo Boost enable/disable */
 } __rte_cache_aligned;
@@ -300,6 +299,7 @@ int
 power_acpi_cpufreq_init(unsigned int lcore_id)
 {
 	struct rte_power_info *pi;
+	uint32_t exp_state;
 
 	if (lcore_id >= RTE_MAX_LCORE) {
 		RTE_LOG(ERR, POWER, "Lcore id %u can not exceeds %u\n",
@@ -308,8 +308,16 @@ power_acpi_cpufreq_init(unsigned int lcore_id)
 	}
 
 	pi = &lcore_power_info[lcore_id];
-	if (rte_atomic32_cmpset(&(pi->state), POWER_IDLE, POWER_ONGOING)
-			== 0) {
+	exp_state = POWER_IDLE;
+	/* The power in use state works as a guard variable between
+	 * the CPU frequency control initialization and exit process.
+	 * The ACQUIRE memory ordering here pairs with the RELEASE
+	 * ordering below as lock to make sure the frequency operations
+	 * in the critical section are done under the correct state.
+	 */
+	if (!__atomic_compare_exchange_n(&(pi->state), &exp_state,
+					POWER_ONGOING, 0,
+					__ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
 		RTE_LOG(INFO, POWER, "Power management of lcore %u is "
 				"in use\n", lcore_id);
 		return -1;
@@ -346,12 +354,16 @@ power_acpi_cpufreq_init(unsigned int lcore_id)
 
 	RTE_LOG(INFO, POWER, "Initialized successfully for lcore %u "
 			"power management\n", lcore_id);
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_USED);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_USED,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return 0;
 
 fail:
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_UNKNOWN);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_UNKNOWN,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return -1;
 }
@@ -408,6 +420,7 @@ int
 power_acpi_cpufreq_exit(unsigned int lcore_id)
 {
 	struct rte_power_info *pi;
+	uint32_t exp_state;
 
 	if (lcore_id >= RTE_MAX_LCORE) {
 		RTE_LOG(ERR, POWER, "Lcore id %u can not exceeds %u\n",
@@ -415,8 +428,16 @@ power_acpi_cpufreq_exit(unsigned int lcore_id)
 		return -1;
 	}
 	pi = &lcore_power_info[lcore_id];
-	if (rte_atomic32_cmpset(&(pi->state), POWER_USED, POWER_ONGOING)
-			== 0) {
+	exp_state = POWER_USED;
+	/* The power in use state works as a guard variable between
+	 * the CPU frequency control initialization and exit process.
+	 * The ACQUIRE memory ordering here pairs with the RELEASE
+	 * ordering below as lock to make sure the frequency operations
+	 * in the critical section are done under the correct state.
+	 */
+	if (!__atomic_compare_exchange_n(&(pi->state), &exp_state,
+					POWER_ONGOING, 0,
+					__ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
 		RTE_LOG(INFO, POWER, "Power management of lcore %u is "
 				"not used\n", lcore_id);
 		return -1;
@@ -436,12 +457,16 @@ power_acpi_cpufreq_exit(unsigned int lcore_id)
 	RTE_LOG(INFO, POWER, "Power management of lcore %u has exited from "
 			"'userspace' mode and been set back to the "
 			"original\n", lcore_id);
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_IDLE);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_IDLE,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return 0;
 
 fail:
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_UNKNOWN);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_UNKNOWN,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return -1;
 }
diff --git a/lib/librte_power/power_pstate_cpufreq.c b/lib/librte_power/power_pstate_cpufreq.c
index 2526441..e3126d3 100644
--- a/lib/librte_power/power_pstate_cpufreq.c
+++ b/lib/librte_power/power_pstate_cpufreq.c
@@ -14,7 +14,6 @@
 #include <errno.h>
 #include <inttypes.h>
 
-#include <rte_atomic.h>
 #include <rte_memcpy.h>
 #include <rte_memory.h>
 #include <rte_string_fns.h>
@@ -100,7 +99,7 @@ struct pstate_power_info {
 	uint32_t non_turbo_max_ratio;        /**< Non Turbo Max ratio  */
 	uint32_t sys_max_freq;               /**< system wide max freq  */
 	uint32_t core_base_freq;             /**< core base freq  */
-	volatile uint32_t state;             /**< Power in use state */
+	uint32_t state;                      /**< Power in use state */
 	uint16_t turbo_available;            /**< Turbo Boost available */
 	uint16_t turbo_enable;               /**< Turbo Boost enable/disable */
 	uint16_t priority_core;              /**< High Performance core */
@@ -542,6 +541,7 @@ int
 power_pstate_cpufreq_init(unsigned int lcore_id)
 {
 	struct pstate_power_info *pi;
+	uint32_t exp_state;
 
 	if (lcore_id >= RTE_MAX_LCORE) {
 		RTE_LOG(ERR, POWER, "Lcore id %u can not exceed %u\n",
@@ -550,8 +550,16 @@ power_pstate_cpufreq_init(unsigned int lcore_id)
 	}
 
 	pi = &lcore_power_info[lcore_id];
-	if (rte_atomic32_cmpset(&(pi->state), POWER_IDLE, POWER_ONGOING)
-			== 0) {
+	exp_state = POWER_IDLE;
+	/* The power in use state works as a guard variable between
+	 * the CPU frequency control initialization and exit process.
+	 * The ACQUIRE memory ordering here pairs with the RELEASE
+	 * ordering below as lock to make sure the frequency operations
+	 * in the critical section are done under the correct state.
+	 */
+	if (!__atomic_compare_exchange_n(&(pi->state), &exp_state,
+					POWER_ONGOING, 0,
+					__ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
 		RTE_LOG(INFO, POWER, "Power management of lcore %u is "
 				"in use\n", lcore_id);
 		return -1;
@@ -588,12 +596,16 @@ power_pstate_cpufreq_init(unsigned int lcore_id)
 
 	RTE_LOG(INFO, POWER, "Initialized successfully for lcore %u "
 			"power management\n", lcore_id);
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_USED);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_USED,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return 0;
 
 fail:
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_UNKNOWN);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_UNKNOWN,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return -1;
 }
@@ -602,6 +614,7 @@ int
 power_pstate_cpufreq_exit(unsigned int lcore_id)
 {
 	struct pstate_power_info *pi;
+	uint32_t exp_state;
 
 	if (lcore_id >= RTE_MAX_LCORE) {
 		RTE_LOG(ERR, POWER, "Lcore id %u can not exceeds %u\n",
@@ -610,8 +623,16 @@ power_pstate_cpufreq_exit(unsigned int lcore_id)
 	}
 	pi = &lcore_power_info[lcore_id];
 
-	if (rte_atomic32_cmpset(&(pi->state), POWER_USED, POWER_ONGOING)
-			== 0) {
+	exp_state = POWER_USED;
+	/* The power in use state works as a guard variable between
+	 * the CPU frequency control initialization and exit process.
+	 * The ACQUIRE memory ordering here pairs with the RELEASE
+	 * ordering below as lock to make sure the frequency operations
+	 * in the critical section are under done the correct state.
+	 */
+	if (!__atomic_compare_exchange_n(&(pi->state), &exp_state,
+					POWER_ONGOING, 0,
+					__ATOMIC_ACQUIRE, __ATOMIC_RELAXED)) {
 		RTE_LOG(INFO, POWER, "Power management of lcore %u is "
 				"not used\n", lcore_id);
 		return -1;
@@ -633,12 +654,16 @@ power_pstate_cpufreq_exit(unsigned int lcore_id)
 	RTE_LOG(INFO, POWER, "Power management of lcore %u has exited from "
 			"'performance' mode and been set back to the "
 			"original\n", lcore_id);
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_IDLE);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_IDLE,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return 0;
 
 fail:
-	rte_atomic32_cmpset(&(pi->state), POWER_ONGOING, POWER_UNKNOWN);
+	exp_state = POWER_ONGOING;
+	__atomic_compare_exchange_n(&(pi->state), &exp_state, POWER_UNKNOWN,
+				    0, __ATOMIC_RELEASE, __ATOMIC_RELAXED);
 
 	return -1;
 }
-- 
2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [dpdk-dev] [PATCH v3 4/4] ethdev: use C11 atomic builtins for link status update
  2020-09-24  5:39   ` [dpdk-dev] [PATCH v3 " Phil Yang
                       ` (2 preceding siblings ...)
  2020-09-24  5:39     ` [dpdk-dev] [PATCH v3 3/4] power: use C11 atomic builtins for power in use state update Phil Yang
@ 2020-09-24  5:39     ` Phil Yang
  2020-09-25 13:59     ` [dpdk-dev] [PATCH v3 0/4] use C11 atomic builtins for libs David Marchand
  4 siblings, 0 replies; 27+ messages in thread
From: Phil Yang @ 2020-09-24  5:39 UTC (permalink / raw)
  To: dev, david.marchand, nicolas.chautru, david.hunt
  Cc: Ruifeng.Wang, Honnappa.Nagarahalli, nd, Thomas Monjalon,
	Ferruh Yigit, Andrew Rybchenko

Since rte_atomicXX APIs are not allowed to be used, use C11 atomic
builtins for link status update.

Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Acked-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_ethdev/rte_ethdev_driver.h | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
index 23cc1e0..04ac8e9 100644
--- a/lib/librte_ethdev/rte_ethdev_driver.h
+++ b/lib/librte_ethdev/rte_ethdev_driver.h
@@ -920,8 +920,7 @@ static inline int
 rte_eth_linkstatus_set(struct rte_eth_dev *dev,
 		       const struct rte_eth_link *new_link)
 {
-	volatile uint64_t *dev_link
-		 = (volatile uint64_t *)&(dev->data->dev_link);
+	uint64_t *dev_link = (uint64_t *)&(dev->data->dev_link);
 	union {
 		uint64_t val64;
 		struct rte_eth_link link;
@@ -929,8 +928,8 @@ rte_eth_linkstatus_set(struct rte_eth_dev *dev,
 
 	RTE_BUILD_BUG_ON(sizeof(*new_link) != sizeof(uint64_t));
 
-	orig.val64 = rte_atomic64_exchange(dev_link,
-					   *(const uint64_t *)new_link);
+	orig.val64 = __atomic_exchange_n(dev_link, *(const uint64_t *)new_link,
+					__ATOMIC_SEQ_CST);
 
 	return (orig.link.link_status == new_link->link_status) ? -1 : 0;
 }
@@ -948,20 +947,12 @@ static inline void
 rte_eth_linkstatus_get(const struct rte_eth_dev *dev,
 		       struct rte_eth_link *link)
 {
-	volatile uint64_t *src = (uint64_t *)&(dev->data->dev_link);
+	uint64_t *src = (uint64_t *)&(dev->data->dev_link);
 	uint64_t *dst = (uint64_t *)link;
 
 	RTE_BUILD_BUG_ON(sizeof(*link) != sizeof(uint64_t));
 
-#ifdef __LP64__
-	/* if cpu arch has 64 bit unsigned lon then implicitly atomic */
-	*dst = *src;
-#else
-	/* can't use rte_atomic64_read because it returns signed int */
-	do {
-		*dst = *src;
-	} while (!rte_atomic64_cmpset(src, *dst, *dst));
-#endif
+	*dst = __atomic_load_n(src, __ATOMIC_SEQ_CST);
 }
 
 /**
-- 
2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3 3/4] power: use C11 atomic builtins for power in use state update
  2020-09-24  5:39     ` [dpdk-dev] [PATCH v3 3/4] power: use C11 atomic builtins for power in use state update Phil Yang
@ 2020-09-24  8:34       ` David Hunt
  0 siblings, 0 replies; 27+ messages in thread
From: David Hunt @ 2020-09-24  8:34 UTC (permalink / raw)
  To: Phil Yang, dev, david.marchand, nicolas.chautru
  Cc: Ruifeng.Wang, Honnappa.Nagarahalli, nd

Hi Phil,

On 24/9/2020 6:39 AM, Phil Yang wrote:
> Since rte_atomicXX APIs are not allowed to be used, use C11 atomic
> builtins for power in use state update.
>
> Signed-off-by: Phil Yang <phil.yang@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> ---
>   lib/librte_power/power_acpi_cpufreq.c   | 45 +++++++++++++++++++++++++--------
>   lib/librte_power/power_pstate_cpufreq.c | 45 +++++++++++++++++++++++++--------
>   2 files changed, 70 insertions(+), 20 deletions(-)
>

Looks good to me. Code looks good, and I applied it locally to have a 
test, and it's entering and exiting power manangemnt state fine on my 
systems here.
Thanks.

Acked-by: David Hunt <david.hunt@intel.com>



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/4] bbdev: use C11 atomic builtins for device processing counter
  2020-09-24  5:39     ` [dpdk-dev] [PATCH v3 2/4] bbdev: use C11 atomic builtins for device processing counter Phil Yang
@ 2020-09-24 22:01       ` Chautru, Nicolas
  2020-09-24 22:44         ` Honnappa Nagarahalli
  0 siblings, 1 reply; 27+ messages in thread
From: Chautru, Nicolas @ 2020-09-24 22:01 UTC (permalink / raw)
  To: Phil Yang, dev, david.marchand, Hunt, David
  Cc: Ruifeng.Wang, Honnappa.Nagarahalli, nd

Hi Phil, 
Naïve question but the deprecation document was stating that "DPDK will adopt C11 atomic operations semantics and provide wrappers using C11 atomic built-ins."
Here you are using directly the C11 atomic built-ins and not providing and using a DPDK wrapper. 
Wasn't the intent to have a new rte_... wrapper here? Ie. the same way as the __sync_fetch_and_add() were called before behind the rte_atomicNN_XX wrapper. 

Thanks
Nic


> -----Original Message-----
> From: Phil Yang <phil.yang@arm.com>
> Sent: Wednesday, September 23, 2020 10:39 PM
> To: dev@dpdk.org; david.marchand@redhat.com; Chautru, Nicolas
> <nicolas.chautru@intel.com>; Hunt, David <david.hunt@intel.com>
> Cc: Ruifeng.Wang@arm.com; Honnappa.Nagarahalli@arm.com;
> nd@arm.com
> Subject: [PATCH v3 2/4] bbdev: use C11 atomic builtins for device processing
> counter
> 
> Since rte_atomicXX APIs are not allowed to be used, use C11 atomic builtins
> for device processing counter.
> 
> Signed-off-by: Phil Yang <phil.yang@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> ---
>  lib/librte_bbdev/rte_bbdev.c | 5 +++--
>  lib/librte_bbdev/rte_bbdev.h | 4 +---
>  2 files changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/librte_bbdev/rte_bbdev.c b/lib/librte_bbdev/rte_bbdev.c
> index a4fdb69..5ba891c 100644
> --- a/lib/librte_bbdev/rte_bbdev.c
> +++ b/lib/librte_bbdev/rte_bbdev.c
> @@ -210,7 +210,7 @@ rte_bbdev_allocate(const char *name)
>  		return NULL;
>  	}
> 
> -	rte_atomic16_inc(&bbdev->data->process_cnt);
> +	__atomic_add_fetch(&bbdev->data->process_cnt, 1,
> __ATOMIC_RELAXED);
>  	bbdev->data->dev_id = dev_id;
>  	bbdev->state = RTE_BBDEV_INITIALIZED;
> 
> @@ -252,7 +252,8 @@ rte_bbdev_release(struct rte_bbdev *bbdev)
>  	}
> 
>  	/* clear shared BBDev Data if no process is using the device anymore
> */
> -	if (rte_atomic16_dec_and_test(&bbdev->data->process_cnt))
> +	if (__atomic_sub_fetch(&bbdev->data->process_cnt, 1,
> +			      __ATOMIC_RELAXED) == 0)
>  		memset(bbdev->data, 0, sizeof(*bbdev->data));
> 
>  	memset(bbdev, 0, sizeof(*bbdev));
> diff --git a/lib/librte_bbdev/rte_bbdev.h b/lib/librte_bbdev/rte_bbdev.h
> index 5729137..7017124 100644
> --- a/lib/librte_bbdev/rte_bbdev.h
> +++ b/lib/librte_bbdev/rte_bbdev.h
> @@ -33,7 +33,6 @@ extern "C" {
>  #include <string.h>
> 
>  #include <rte_compat.h>
> -#include <rte_atomic.h>
>  #include <rte_bus.h>
>  #include <rte_cpuflags.h>
>  #include <rte_memory.h>
> @@ -426,8 +425,7 @@ struct rte_bbdev_data {
>  	uint16_t dev_id;  /**< Device ID */
>  	int socket_id;  /**< NUMA socket that device is on */
>  	bool started;  /**< Device run-time state */
> -	/** Counter of processes using the device */
> -	rte_atomic16_t process_cnt;
> +	uint16_t process_cnt;  /** Counter of processes using the device */
>  };
> 
>  /* Forward declarations */
> --
> 2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/4] bbdev: use C11 atomic builtins for device processing counter
  2020-09-24 22:01       ` Chautru, Nicolas
@ 2020-09-24 22:44         ` Honnappa Nagarahalli
  2020-09-24 23:20           ` Chautru, Nicolas
  0 siblings, 1 reply; 27+ messages in thread
From: Honnappa Nagarahalli @ 2020-09-24 22:44 UTC (permalink / raw)
  To: Chautru, Nicolas, Phil Yang, dev, david.marchand, Hunt, David
  Cc: Ruifeng Wang, nd, Honnappa Nagarahalli, nd

<snip>

> 
> Hi Phil,
> Naïve question but the deprecation document was stating that "DPDK will
> adopt C11 atomic operations semantics and provide wrappers using C11
> atomic built-ins."
At the time of writing the deprecation notice, that was the thinking. However, through further discussions [1] in the community, it was decided to use atomic built-ins.

[1] http://mails.dpdk.org/archives/dev/2020-May/167416.html

> Here you are using directly the C11 atomic built-ins and not providing and
> using a DPDK wrapper.
> Wasn't the intent to have a new rte_... wrapper here? Ie. the same way as
> the __sync_fetch_and_add() were called before behind the rte_atomicNN_XX
> wrapper.
> 
> Thanks
> Nic
> 
> 
> > -----Original Message-----
> > From: Phil Yang <phil.yang@arm.com>
> > Sent: Wednesday, September 23, 2020 10:39 PM
> > To: dev@dpdk.org; david.marchand@redhat.com; Chautru, Nicolas
> > <nicolas.chautru@intel.com>; Hunt, David <david.hunt@intel.com>
> > Cc: Ruifeng.Wang@arm.com; Honnappa.Nagarahalli@arm.com;
> nd@arm.com
> > Subject: [PATCH v3 2/4] bbdev: use C11 atomic builtins for device
> > processing counter
> >
> > Since rte_atomicXX APIs are not allowed to be used, use C11 atomic
> > builtins for device processing counter.
> >
> > Signed-off-by: Phil Yang <phil.yang@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > ---
> >  lib/librte_bbdev/rte_bbdev.c | 5 +++--  lib/librte_bbdev/rte_bbdev.h
> > | 4 +---
> >  2 files changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/librte_bbdev/rte_bbdev.c
> > b/lib/librte_bbdev/rte_bbdev.c index a4fdb69..5ba891c 100644
> > --- a/lib/librte_bbdev/rte_bbdev.c
> > +++ b/lib/librte_bbdev/rte_bbdev.c
> > @@ -210,7 +210,7 @@ rte_bbdev_allocate(const char *name)
> >  		return NULL;
> >  	}
> >
> > -	rte_atomic16_inc(&bbdev->data->process_cnt);
> > +	__atomic_add_fetch(&bbdev->data->process_cnt, 1,
> > __ATOMIC_RELAXED);
> >  	bbdev->data->dev_id = dev_id;
> >  	bbdev->state = RTE_BBDEV_INITIALIZED;
> >
> > @@ -252,7 +252,8 @@ rte_bbdev_release(struct rte_bbdev *bbdev)
> >  	}
> >
> >  	/* clear shared BBDev Data if no process is using the device anymore
> > */
> > -	if (rte_atomic16_dec_and_test(&bbdev->data->process_cnt))
> > +	if (__atomic_sub_fetch(&bbdev->data->process_cnt, 1,
> > +			      __ATOMIC_RELAXED) == 0)
> >  		memset(bbdev->data, 0, sizeof(*bbdev->data));
> >
> >  	memset(bbdev, 0, sizeof(*bbdev));
> > diff --git a/lib/librte_bbdev/rte_bbdev.h
> > b/lib/librte_bbdev/rte_bbdev.h index 5729137..7017124 100644
> > --- a/lib/librte_bbdev/rte_bbdev.h
> > +++ b/lib/librte_bbdev/rte_bbdev.h
> > @@ -33,7 +33,6 @@ extern "C" {
> >  #include <string.h>
> >
> >  #include <rte_compat.h>
> > -#include <rte_atomic.h>
> >  #include <rte_bus.h>
> >  #include <rte_cpuflags.h>
> >  #include <rte_memory.h>
> > @@ -426,8 +425,7 @@ struct rte_bbdev_data {
> >  	uint16_t dev_id;  /**< Device ID */
> >  	int socket_id;  /**< NUMA socket that device is on */
> >  	bool started;  /**< Device run-time state */
> > -	/** Counter of processes using the device */
> > -	rte_atomic16_t process_cnt;
> > +	uint16_t process_cnt;  /** Counter of processes using the device */
> >  };
> >
> >  /* Forward declarations */
> > --
> > 2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3 2/4] bbdev: use C11 atomic builtins for device processing counter
  2020-09-24 22:44         ` Honnappa Nagarahalli
@ 2020-09-24 23:20           ` Chautru, Nicolas
  0 siblings, 0 replies; 27+ messages in thread
From: Chautru, Nicolas @ 2020-09-24 23:20 UTC (permalink / raw)
  To: Honnappa Nagarahalli, Phil Yang, dev, david.marchand, Hunt, David
  Cc: Ruifeng Wang, nd, nd

Thanks Honnappa, 

Acked-By: Nicolas Chautru <nicolas.chautru@intel.com>

> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Thursday, September 24, 2020 3:45 PM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>; Phil Yang
> <Phil.Yang@arm.com>; dev@dpdk.org; david.marchand@redhat.com; Hunt,
> David <david.hunt@intel.com>
> Cc: Ruifeng Wang <Ruifeng.Wang@arm.com>; nd <nd@arm.com>;
> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; nd
> <nd@arm.com>
> Subject: RE: [PATCH v3 2/4] bbdev: use C11 atomic builtins for device
> processing counter
> 
> <snip>
> 
> >
> > Hi Phil,
> > Naïve question but the deprecation document was stating that "DPDK
> > will adopt C11 atomic operations semantics and provide wrappers using
> > C11 atomic built-ins."
> At the time of writing the deprecation notice, that was the thinking.
> However, through further discussions [1] in the community, it was decided
> to use atomic built-ins.
> 
> [1] http://mails.dpdk.org/archives/dev/2020-May/167416.html
> 
> > Here you are using directly the C11 atomic built-ins and not providing
> > and using a DPDK wrapper.
> > Wasn't the intent to have a new rte_... wrapper here? Ie. the same way
> > as the __sync_fetch_and_add() were called before behind the
> > rte_atomicNN_XX wrapper.
> >
> > Thanks
> > Nic
> >
> >
> > > -----Original Message-----
> > > From: Phil Yang <phil.yang@arm.com>
> > > Sent: Wednesday, September 23, 2020 10:39 PM
> > > To: dev@dpdk.org; david.marchand@redhat.com; Chautru, Nicolas
> > > <nicolas.chautru@intel.com>; Hunt, David <david.hunt@intel.com>
> > > Cc: Ruifeng.Wang@arm.com; Honnappa.Nagarahalli@arm.com;
> > nd@arm.com
> > > Subject: [PATCH v3 2/4] bbdev: use C11 atomic builtins for device
> > > processing counter
> > >
> > > Since rte_atomicXX APIs are not allowed to be used, use C11 atomic
> > > builtins for device processing counter.
> > >
> > > Signed-off-by: Phil Yang <phil.yang@arm.com>
> > > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > ---
> > >  lib/librte_bbdev/rte_bbdev.c | 5 +++--
> > > lib/librte_bbdev/rte_bbdev.h
> > > | 4 +---
> > >  2 files changed, 4 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/lib/librte_bbdev/rte_bbdev.c
> > > b/lib/librte_bbdev/rte_bbdev.c index a4fdb69..5ba891c 100644
> > > --- a/lib/librte_bbdev/rte_bbdev.c
> > > +++ b/lib/librte_bbdev/rte_bbdev.c
> > > @@ -210,7 +210,7 @@ rte_bbdev_allocate(const char *name)
> > >  		return NULL;
> > >  	}
> > >
> > > -	rte_atomic16_inc(&bbdev->data->process_cnt);
> > > +	__atomic_add_fetch(&bbdev->data->process_cnt, 1,
> > > __ATOMIC_RELAXED);
> > >  	bbdev->data->dev_id = dev_id;
> > >  	bbdev->state = RTE_BBDEV_INITIALIZED;
> > >
> > > @@ -252,7 +252,8 @@ rte_bbdev_release(struct rte_bbdev *bbdev)
> > >  	}
> > >
> > >  	/* clear shared BBDev Data if no process is using the device
> > > anymore */
> > > -	if (rte_atomic16_dec_and_test(&bbdev->data->process_cnt))
> > > +	if (__atomic_sub_fetch(&bbdev->data->process_cnt, 1,
> > > +			      __ATOMIC_RELAXED) == 0)
> > >  		memset(bbdev->data, 0, sizeof(*bbdev->data));
> > >
> > >  	memset(bbdev, 0, sizeof(*bbdev));
> > > diff --git a/lib/librte_bbdev/rte_bbdev.h
> > > b/lib/librte_bbdev/rte_bbdev.h index 5729137..7017124 100644
> > > --- a/lib/librte_bbdev/rte_bbdev.h
> > > +++ b/lib/librte_bbdev/rte_bbdev.h
> > > @@ -33,7 +33,6 @@ extern "C" {
> > >  #include <string.h>
> > >
> > >  #include <rte_compat.h>
> > > -#include <rte_atomic.h>
> > >  #include <rte_bus.h>
> > >  #include <rte_cpuflags.h>
> > >  #include <rte_memory.h>
> > > @@ -426,8 +425,7 @@ struct rte_bbdev_data {
> > >  	uint16_t dev_id;  /**< Device ID */
> > >  	int socket_id;  /**< NUMA socket that device is on */
> > >  	bool started;  /**< Device run-time state */
> > > -	/** Counter of processes using the device */
> > > -	rte_atomic16_t process_cnt;
> > > +	uint16_t process_cnt;  /** Counter of processes using the device
> > > +*/
> > >  };
> > >
> > >  /* Forward declarations */
> > > --
> > > 2.7.4


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [dpdk-dev] [PATCH v3 0/4] use C11 atomic builtins for libs
  2020-09-24  5:39   ` [dpdk-dev] [PATCH v3 " Phil Yang
                       ` (3 preceding siblings ...)
  2020-09-24  5:39     ` [dpdk-dev] [PATCH v3 4/4] ethdev: use C11 atomic builtins for link status update Phil Yang
@ 2020-09-25 13:59     ` David Marchand
  4 siblings, 0 replies; 27+ messages in thread
From: David Marchand @ 2020-09-25 13:59 UTC (permalink / raw)
  To: Phil Yang
  Cc: dev, Nicolas Chautru, David Hunt,
	Ruifeng Wang (Arm Technology China),
	Honnappa Nagarahalli, nd, Andrew Rybchenko

On Thu, Sep 24, 2020 at 7:40 AM Phil Yang <phil.yang@arm.com> wrote:
>
> Since rte_atomicXX APIs are not allowed to be used[1][2], use C11 atomic
> builtins instead in eal, bbdev, power, and ethdev libs.
>
> [1] http://code.dpdk.org/dpdk/latest/source/doc/guides/rel_notes/deprecation.rst#L87
> [2] http://code.dpdk.org/dpdk/latest/source/devtools/checkpatches.sh#L80
>
> v3:
> remove unnecessary rte_atomic.h headers. (David)
>
> v2:
> Fix Clang int-conversion warning.
>
> v1:
> Initial version.
>
> Phil Yang (4):
>   eal: use C11 atomic builtins for already initialized check
>   bbdev: use C11 atomic builtins for device processing counter
>   power: use C11 atomic builtins for power in use state update
>   ethdev: use C11 atomic builtins for link status update
>
>  lib/librte_bbdev/rte_bbdev.c            |  5 ++--
>  lib/librte_bbdev/rte_bbdev.h            |  4 +--
>  lib/librte_eal/freebsd/eal.c            | 19 +++++++-------
>  lib/librte_eal/linux/eal.c              | 21 +++++++--------
>  lib/librte_ethdev/rte_ethdev_driver.h   | 19 ++++----------
>  lib/librte_power/power_acpi_cpufreq.c   | 45 +++++++++++++++++++++++++--------
>  lib/librte_power/power_pstate_cpufreq.c | 45 +++++++++++++++++++++++++--------
>  7 files changed, 100 insertions(+), 58 deletions(-)
>

Series applied, thanks Phil.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2020-09-25 14:00 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-11  3:29 [dpdk-dev] [PATCH 0/4] use C11 atomic builtins for libs Phil Yang
2020-09-11  3:29 ` [dpdk-dev] [PATCH 1/4] eal: use C11 atomic builtins for already initialized check Phil Yang
2020-09-11  3:29 ` [dpdk-dev] [PATCH 2/4] bbdev: use C11 atomic builtins for device processing counter Phil Yang
2020-09-11  3:29 ` [dpdk-dev] [PATCH 3/4] power: use C11 atomic builtins for power in use state update Phil Yang
2020-09-11  3:29 ` [dpdk-dev] [PATCH 4/4] ethdev: use C11 atomic builtins for link status update Phil Yang
2020-09-15 15:12 ` [dpdk-dev] [PATCH 0/4] use C11 atomic builtins for libs David Marchand
2020-09-16  7:32   ` Phil Yang
2020-09-16  8:23 ` [dpdk-dev] [PATCH v2 " Phil Yang
2020-09-16  8:23   ` [dpdk-dev] [PATCH v2 1/4] eal: use C11 atomic builtins for already initialized check Phil Yang
2020-09-23 13:06     ` David Marchand
2020-09-24  3:44       ` Phil Yang
2020-09-16  8:23   ` [dpdk-dev] [PATCH v2 2/4] bbdev: use C11 atomic builtins for device processing counter Phil Yang
2020-09-16  8:23   ` [dpdk-dev] [PATCH v2 3/4] power: use C11 atomic builtins for power in use state update Phil Yang
2020-09-16  8:23   ` [dpdk-dev] [PATCH v2 4/4] ethdev: use C11 atomic builtins for link status update Phil Yang
2020-09-17 16:08     ` Andrew Rybchenko
2020-09-23 13:18   ` [dpdk-dev] [PATCH v2 0/4] use C11 atomic builtins for libs David Marchand
2020-09-24  3:47     ` Phil Yang
2020-09-24  5:39   ` [dpdk-dev] [PATCH v3 " Phil Yang
2020-09-24  5:39     ` [dpdk-dev] [PATCH v3 1/4] eal: use C11 atomic builtins for already initialized check Phil Yang
2020-09-24  5:39     ` [dpdk-dev] [PATCH v3 2/4] bbdev: use C11 atomic builtins for device processing counter Phil Yang
2020-09-24 22:01       ` Chautru, Nicolas
2020-09-24 22:44         ` Honnappa Nagarahalli
2020-09-24 23:20           ` Chautru, Nicolas
2020-09-24  5:39     ` [dpdk-dev] [PATCH v3 3/4] power: use C11 atomic builtins for power in use state update Phil Yang
2020-09-24  8:34       ` David Hunt
2020-09-24  5:39     ` [dpdk-dev] [PATCH v3 4/4] ethdev: use C11 atomic builtins for link status update Phil Yang
2020-09-25 13:59     ` [dpdk-dev] [PATCH v3 0/4] use C11 atomic builtins for libs David Marchand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).