> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Thursday, 17 August 2023 21.10
>
> On Thu, Aug 17, 2023 at 01:45:21PM +0200, Morten Brørup wrote:
> > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > Sent: Wednesday, 16 August 2023 23.39
> > >
> > > Provide API for atomic operations in the rte namespace that may
> > > optionally be configured to use C11 atomics with meson
> > > option enable_stdatomics=true
> > >
> > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
> > > ---
> >
> > Speed blindness during my review... I have now spotted a couple of copy-
> paste typos:
> >
> > > +#define rte_atomic_compare_exchange_weak_explicit( \
> > > + ptr, expected, desired, succ_memorder, fail_memorder) \
> > > + atomic_compare_exchange_strong_explicit( \
> >
> > atomic_compare_exchange_weak_explicit, not strong.
>
> yikes, thanks for catching that cut & past error
>
> >
> > > + ptr, expected, desired, succ_memorder, fail_memorder)
> > > +
> >
> > [...]
> >
> > > +#define rte_atomic_flag_clear_explicit(ptr, memorder) \
> > > + atomic_flag_clear(ptr, memorder)
> >
> > atomic_flag_clear_explicit(ptr, memorder), missing _explicit.
>
> yes, currently unused otherwise it would have failed to compie
Yes, I guessed something similar, when I spotted this... two parameters being passed to a single-parameter function.
>
> i'll correct this too.
>
> thank you for the careful review i look at the diffs over and over and
> still it's hard to spot subtle swaps/exchanges of things.
Yes, when reviewing many similar lines of code, the probability of overlooking something increases rapidly, even for external reviewers.
About 20 years ago, a Danish consulting company realized this, and turned it to something positive. They set up a highly specialized organization, Specialisterne (https://specialisterne.com/), to offer autistic consultants for tasks like this. These autistics enjoy repetitive tasks, and are excellent at spotting subtle differences normal people would likely overlook.
@@ -303,6 +303,7 @@ endforeach
# set other values pulled from the build options
dpdk_conf.set('RTE_MAX_ETHPORTS', get_option('max_ethports'))
dpdk_conf.set('RTE_LIBEAL_USE_HPET', get_option('use_hpet'))
+dpdk_conf.set('RTE_ENABLE_STDATOMIC', get_option('enable_stdatomic'))
dpdk_conf.set('RTE_ENABLE_TRACE_FP', get_option('enable_trace_fp'))
# values which have defaults which may be overridden
dpdk_conf.set('RTE_MAX_VFIO_GROUPS', 64)
@@ -15,6 +15,7 @@
#include <stdint.h>
#include <rte_compat.h>
#include <rte_common.h>
+#include <rte_stdatomic.h>
#ifdef __DOXYGEN__
@@ -16,6 +16,7 @@
#include <assert.h>
#include <rte_common.h>
#include <rte_atomic.h>
+#include <rte_stdatomic.h>
/**
* Pause CPU execution for a short while
@@ -32,6 +32,7 @@
#include <rte_common.h>
#include <rte_lock_annotations.h>
#include <rte_pause.h>
+#include <rte_stdatomic.h>
/**
* The rte_rwlock_t type.
@@ -23,6 +23,7 @@
#endif
#include <rte_lock_annotations.h>
#include <rte_pause.h>
+#include <rte_stdatomic.h>
/**
* The rte_spinlock_t type.
@@ -42,6 +42,7 @@ headers += files(
'rte_seqlock.h',
'rte_service.h',
'rte_service_component.h',
+ 'rte_stdatomic.h',
'rte_string_fns.h',
'rte_tailq.h',
'rte_thread.h',
@@ -27,6 +27,7 @@
#include <rte_common.h>
#include <rte_pause.h>
#include <rte_branch_prediction.h>
+#include <rte_stdatomic.h>
/**
* The rte_mcslock_t type.
@@ -34,6 +34,7 @@
#include <rte_compat.h>
#include <rte_common.h>
#include <rte_pause.h>
+#include <rte_stdatomic.h>
/**
* The rte_pflock_t type.
@@ -26,6 +26,7 @@
#include <rte_atomic.h>
#include <rte_branch_prediction.h>
#include <rte_compat.h>
+#include <rte_stdatomic.h>
/**
* The RTE seqcount type.
new file mode 100644
@@ -0,0 +1,198 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2023 Microsoft Corporation
+ */
+
+#ifndef _RTE_STDATOMIC_H_
+#define _RTE_STDATOMIC_H_
+
+#include <assert.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#ifdef RTE_ENABLE_STDATOMIC
+#ifdef __STDC_NO_ATOMICS__
+#error enable_stdatomics=true but atomics not supported by toolchain
+#endif
+
+#include <stdatomic.h>
+
+/* RTE_ATOMIC(type) is provided for use as a type specifier
+ * permitting designation of an rte atomic type.
+ */
+#define RTE_ATOMIC(type) _Atomic(type)
+
+/* __rte_atomic is provided for type qualification permitting
+ * designation of an rte atomic qualified type-name.
+ */
+#define __rte_atomic _Atomic
+
+/* The memory order is an enumerated type in C11. */
+typedef memory_order rte_memory_order;
+
+#define rte_memory_order_relaxed memory_order_relaxed
+#ifdef __ATOMIC_RELAXED
+static_assert(rte_memory_order_relaxed == __ATOMIC_RELAXED,
+ "rte_memory_order_relaxed == __ATOMIC_RELAXED");
+#endif
+
+#define rte_memory_order_consume memory_order_consume
+#ifdef __ATOMIC_CONSUME
+static_assert(rte_memory_order_consume == __ATOMIC_CONSUME,
+ "rte_memory_order_consume == __ATOMIC_CONSUME");
+#endif
+
+#define rte_memory_order_acquire memory_order_acquire
+#ifdef __ATOMIC_ACQUIRE
+static_assert(rte_memory_order_acquire == __ATOMIC_ACQUIRE,
+ "rte_memory_order_acquire == __ATOMIC_ACQUIRE");
+#endif
+
+#define rte_memory_order_release memory_order_release
+#ifdef __ATOMIC_RELEASE
+static_assert(rte_memory_order_release == __ATOMIC_RELEASE,
+ "rte_memory_order_release == __ATOMIC_RELEASE");
+#endif
+
+#define rte_memory_order_acq_rel memory_order_acq_rel
+#ifdef __ATOMIC_ACQ_REL
+static_assert(rte_memory_order_acq_rel == __ATOMIC_ACQ_REL,
+ "rte_memory_order_acq_rel == __ATOMIC_ACQ_REL");
+#endif
+
+#define rte_memory_order_seq_cst memory_order_seq_cst
+#ifdef __ATOMIC_SEQ_CST
+static_assert(rte_memory_order_seq_cst == __ATOMIC_SEQ_CST,
+ "rte_memory_order_seq_cst == __ATOMIC_SEQ_CST");
+#endif
+
+#define rte_atomic_load_explicit(ptr, memorder) \
+ atomic_load_explicit(ptr, memorder)
+
+#define rte_atomic_store_explicit(ptr, val, memorder) \
+ atomic_store_explicit(ptr, val, memorder)
+
+#define rte_atomic_exchange_explicit(ptr, val, memorder) \
+ atomic_exchange_explicit(ptr, val, memorder)
+
+#define rte_atomic_compare_exchange_strong_explicit( \
+ ptr, expected, desired, succ_memorder, fail_memorder) \
+ atomic_compare_exchange_strong_explicit( \
+ ptr, expected, desired, succ_memorder, fail_memorder)
+
+#define rte_atomic_compare_exchange_weak_explicit( \
+ ptr, expected, desired, succ_memorder, fail_memorder) \
+ atomic_compare_exchange_strong_explicit( \
+ ptr, expected, desired, succ_memorder, fail_memorder)
+
+#define rte_atomic_fetch_add_explicit(ptr, val, memorder) \
+ atomic_fetch_add_explicit(ptr, val, memorder)
+
+#define rte_atomic_fetch_sub_explicit(ptr, val, memorder) \
+ atomic_fetch_sub_explicit(ptr, val, memorder)
+
+#define rte_atomic_fetch_and_explicit(ptr, val, memorder) \
+ atomic_fetch_and_explicit(ptr, val, memorder)
+
+#define rte_atomic_fetch_xor_explicit(ptr, val, memorder) \
+ atomic_fetch_xor_explicit(ptr, val, memorder)
+
+#define rte_atomic_fetch_or_explicit(ptr, val, memorder) \
+ atomic_fetch_or_explicit(ptr, val, memorder)
+
+#define rte_atomic_fetch_nand_explicit(ptr, val, memorder) \
+ atomic_fetch_nand_explicit(ptr, val, memorder)
+
+#define rte_atomic_flag_test_and_set_explicit(ptr, memorder) \
+ atomic_flag_test_and_set_explicit(ptr, memorder)
+
+#define rte_atomic_flag_clear_explicit(ptr, memorder) \
+ atomic_flag_clear(ptr, memorder)
+
+/* We provide internal macro here to allow conditional expansion
+ * in the body of the per-arch rte_atomic_thread_fence inline functions.
+ */
+#define __rte_atomic_thread_fence(memorder) \
+ atomic_thread_fence(memorder)
+
+#else
+
+/* RTE_ATOMIC(type) is provided for use as a type specifier
+ * permitting designation of an rte atomic type.
+ */
+#define RTE_ATOMIC(type) type
+
+/* __rte_atomic is provided for type qualification permitting
+ * designation of an rte atomic qualified type-name.
+ */
+#define __rte_atomic
+
+/* The memory order is an integer type in GCC built-ins,
+ * not an enumerated type like in C11.
+ */
+typedef int rte_memory_order;
+
+#define rte_memory_order_relaxed __ATOMIC_RELAXED
+#define rte_memory_order_consume __ATOMIC_CONSUME
+#define rte_memory_order_acquire __ATOMIC_ACQUIRE
+#define rte_memory_order_release __ATOMIC_RELEASE
+#define rte_memory_order_acq_rel __ATOMIC_ACQ_REL
+#define rte_memory_order_seq_cst __ATOMIC_SEQ_CST
+
+#define rte_atomic_load_explicit(ptr, memorder) \
+ __atomic_load_n(ptr, memorder)
+
+#define rte_atomic_store_explicit(ptr, val, memorder) \
+ __atomic_store_n(ptr, val, memorder)
+
+#define rte_atomic_exchange_explicit(ptr, val, memorder) \
+ __atomic_exchange_n(ptr, val, memorder)
+
+#define rte_atomic_compare_exchange_strong_explicit( \
+ ptr, expected, desired, succ_memorder, fail_memorder) \
+ __atomic_compare_exchange_n( \
+ ptr, expected, desired, 0, succ_memorder, fail_memorder)
+
+#define rte_atomic_compare_exchange_weak_explicit( \
+ ptr, expected, desired, succ_memorder, fail_memorder) \
+ __atomic_compare_exchange_n( \
+ ptr, expected, desired, 1, succ_memorder, fail_memorder)
+
+#define rte_atomic_fetch_add_explicit(ptr, val, memorder) \
+ __atomic_fetch_add(ptr, val, memorder)
+
+#define rte_atomic_fetch_sub_explicit(ptr, val, memorder) \
+ __atomic_fetch_sub(ptr, val, memorder)
+
+#define rte_atomic_fetch_and_explicit(ptr, val, memorder) \
+ __atomic_fetch_and(ptr, val, memorder)
+
+#define rte_atomic_fetch_xor_explicit(ptr, val, memorder) \
+ __atomic_fetch_xor(ptr, val, memorder)
+
+#define rte_atomic_fetch_or_explicit(ptr, val, memorder) \
+ __atomic_fetch_or(ptr, val, memorder)
+
+#define rte_atomic_fetch_nand_explicit(ptr, val, memorder) \
+ __atomic_fetch_nand(ptr, val, memorder)
+
+#define rte_atomic_flag_test_and_set_explicit(ptr, memorder) \
+ __atomic_test_and_set(ptr, memorder)
+
+#define rte_atomic_flag_clear_explicit(ptr, memorder) \
+ __atomic_clear(ptr, memorder)
+
+/* We provide internal macro here to allow conditional expansion
+ * in the body of the per-arch rte_atomic_thread_fence inline functions.
+ */
+#define __rte_atomic_thread_fence(memorder) \
+ __atomic_thread_fence(memorder)
+
+#endif
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_STDATOMIC_H_ */
@@ -24,6 +24,7 @@
#include <rte_common.h>
#include <rte_lcore.h>
#include <rte_pause.h>
+#include <rte_stdatomic.h>
/**
* The rte_ticketlock_t type.
@@ -30,6 +30,7 @@
#include <rte_per_lcore.h>
#include <rte_string_fns.h>
#include <rte_uuid.h>
+#include <rte_stdatomic.h>
/** The tracepoint object. */
typedef uint64_t rte_trace_point_t;
@@ -46,6 +46,8 @@ option('mbuf_refcnt_atomic', type: 'boolean', value: true, description:
'Atomically access the mbuf refcnt.')
option('platform', type: 'string', value: 'native', description:
'Platform to build, either "native", "generic" or a SoC. Please refer to the Linux build guide for more information.')
+option('enable_stdatomic', type: 'boolean', value: false, description:
+ 'enable use of C11 stdatomic')
option('enable_trace_fp', type: 'boolean', value: false, description:
'enable fast path trace points.')
option('tests', type: 'boolean', value: true, description: