* [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 related [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 related [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 related [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 related [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 related [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 related [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 related [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 related [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 related [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 related [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 related [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 related [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.