eal: introduce atomics abstraction

  Introduce atomics abstraction that permits optional use of standard C11
atomics when meson is provided the new enable_stdatomics=true option.

Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 config/meson.build                     |  11 +++
 lib/eal/arm/include/rte_atomic_32.h    |   6 +-
 lib/eal/arm/include/rte_atomic_64.h    |   6 +-
 lib/eal/include/generic/rte_atomic.h   | 156 ++++++++++++++++++++++++++++++++-
 lib/eal/loongarch/include/rte_atomic.h |   6 +-
 lib/eal/ppc/include/rte_atomic.h       |   6 +-
 lib/eal/riscv/include/rte_atomic.h     |   6 +-
 lib/eal/x86/include/rte_atomic.h       |   8 +-
 meson_options.txt                      |   2 +
 9 files changed, 199 insertions(+), 8 deletions(-)
  

Honnappa, please could you give your view on the future of atomics in DPDK?


12/01/2023 22:26, Tyler Retzlaff:
> Introduce atomics abstraction that permits optional use of standard C11
> atomics when meson is provided the new enable_stdatomics=true option.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>  config/meson.build                     |  11 +++
>  lib/eal/arm/include/rte_atomic_32.h    |   6 +-
>  lib/eal/arm/include/rte_atomic_64.h    |   6 +-
>  lib/eal/include/generic/rte_atomic.h   | 156 ++++++++++++++++++++++++++++++++-
>  lib/eal/loongarch/include/rte_atomic.h |   6 +-
>  lib/eal/ppc/include/rte_atomic.h       |   6 +-
>  lib/eal/riscv/include/rte_atomic.h     |   6 +-
>  lib/eal/x86/include/rte_atomic.h       |   8 +-
>  meson_options.txt                      |   2 +
>  9 files changed, 199 insertions(+), 8 deletions(-)
> 
> diff --git a/config/meson.build b/config/meson.build
> index 6d9ffd4..9515ce9 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -254,6 +254,17 @@ endif
>  # add -include rte_config to cflags
>  add_project_arguments('-include', 'rte_config.h', language: 'c')
>  
> +stdc_atomics_enabled = get_option('enable_stdatomics')
> +dpdk_conf.set('RTE_STDC_ATOMICS', stdc_atomics_enabled)
> +
> +if stdc_atomics_enabled
> +if cc.get_id() == 'gcc' or cc.get_id() == 'clang'
> +    add_project_arguments('-std=gnu11', language: 'c')
> +else
> +    add_project_arguments('-std=c11', language: 'c')
> +endif
> +endif
> +
>  # enable extra warnings and disable any unwanted warnings
>  # -Wall is added by default at warning level 1, and -Wextra
>  # at warning level 2 (DPDK default)
> diff --git a/lib/eal/arm/include/rte_atomic_32.h b/lib/eal/arm/include/rte_atomic_32.h
> index c00ab78..7088a12 100644
> --- a/lib/eal/arm/include/rte_atomic_32.h
> +++ b/lib/eal/arm/include/rte_atomic_32.h
> @@ -34,9 +34,13 @@
>  #define rte_io_rmb() rte_rmb()
>  
>  static __rte_always_inline void
> -rte_atomic_thread_fence(int memorder)
> +rte_atomic_thread_fence(rte_memory_order memorder)
>  {
> +#ifdef RTE_STDC_ATOMICS
> +	atomic_thread_fence(memorder);
> +#else
>  	__atomic_thread_fence(memorder);
> +#endif
>  }
>  
>  #ifdef __cplusplus
> diff --git a/lib/eal/arm/include/rte_atomic_64.h b/lib/eal/arm/include/rte_atomic_64.h
> index 6047911..7f02c57 100644
> --- a/lib/eal/arm/include/rte_atomic_64.h
> +++ b/lib/eal/arm/include/rte_atomic_64.h
> @@ -38,9 +38,13 @@
>  #define rte_io_rmb() rte_rmb()
>  
>  static __rte_always_inline void
> -rte_atomic_thread_fence(int memorder)
> +rte_atomic_thread_fence(rte_memory_order memorder)
>  {
> +#ifdef RTE_STDC_ATOMICS
> +	atomic_thread_fence(memorder);
> +#else
>  	__atomic_thread_fence(memorder);
> +#endif
>  }
>  
>  /*------------------------ 128 bit atomic operations -------------------------*/
> diff --git a/lib/eal/include/generic/rte_atomic.h b/lib/eal/include/generic/rte_atomic.h
> index f5c49a9..cb71c0d 100644
> --- a/lib/eal/include/generic/rte_atomic.h
> +++ b/lib/eal/include/generic/rte_atomic.h
> @@ -110,6 +110,160 @@
>  
>  #endif /* __DOXYGEN__ */
>  
> +#ifdef RTE_STDC_ATOMICS
> +
> +#if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 201112L || defined(__STDC_NO_ATOMICS__)
> +#error compiler does not support C11 standard atomics
> +#else
> +#include <stdatomic.h>
> +#endif
> +
> +#define __rte_atomic _Atomic
> +
> +typedef int rte_memory_order;
> +
> +#define rte_memory_order_relaxed memory_order_relaxed
> +#define rte_memory_order_consume memory_order_consume
> +#define rte_memory_order_acquire memory_order_acquire
> +#define rte_memory_order_release memory_order_release
> +#define rte_memory_order_acq_rel memory_order_acq_rel
> +#define rte_memory_order_seq_cst memory_order_seq_cst
> +
> +#define rte_atomic_store(obj, desired) \
> +	atomic_store_explicit(obj, desired)
> +
> +#define rte_atomic_store_explicit(obj, desired, order) \
> +	atomic_store_explicit(obj, desired, order)
> +
> +#define rte_atomic_load(obj) \
> +	atomic_load_explicit(obj)
> +
> +#define rte_atomic_load_explicit(obj, order) \
> +	atomic_load_explicit(obj, order)
> +
> +#define rte_atomic_exchange(obj, desired) \
> +	atomic_exchange(obj, desired)
> +
> +#define rte_atomic_exchange_explicit(obj, desired, order) \
> +	atomic_exchange_explicit(obj, desired, order)
> +
> +#define rte_atomic_compare_exchange_strong(obj, expected, desired) \
> +	atomic_compare_exchange_strong(obj, expected, desired)
> +
> +#define rte_atomic_compare_exchange_strong_explicit(obj, expected, desired, success, fail) \
> +	atomic_compare_exchange_strong_explicit(obj, expected, desired, success, fail)
> +
> +#define rte_atomic_compare_exchange_weak(obj, expected, desired) \
> +	atomic_compare_exchange_weak(obj, expected, desired)
> +
> +#define rte_atomic_compare_exchange_weak_explicit(obj, expected, desired, success, fail) \
> +	atomic_compare_exchange_weak_explicit(obj, expected, desired, success, fail)
> +
> +#define rte_atomic_fetch_add(obj, arg) \
> +	atomic_fetch_add(obj, arg)
> +
> +#define rte_atomic_fetch_add_explicit(obj, arg, order) \
> +	atomic_fetch_add_explicit(obj, arg, order)
> +
> +#define rte_atomic_fetch_sub(obj, arg) \
> +	atomic_fetch_sub(obj, arg)
> +
> +#define rte_atomic_fetch_sub_explicit(obj, arg, order) \
> +	atomic_fetch_sub_explicit(obj, arg, order)
> +
> +#define rte_atomic_fetch_or(obj, arg) \
> +	atomic_fetch_or(obj, arg)
> +
> +#define rte_atomic_fetch_or_explicit(obj, arg, order) \
> +	atomic_fetch_or_explicit(obj, arg, order)
> +
> +#define rte_atomic_fetch_xor(obj, arg) \
> +	atomic_fetch_xor(obj, arg)
> +
> +#define rte_atomic_fetch_xor_explicit(obj, arg, order) \
> +	atomic_fetch_xor_explicit(obj, arg, order)
> +
> +#define rte_atomic_fetch_and(obj, arg) \
> +	atomic_fetch_and(obj, arg)
> +
> +#define rte_atomic_fetch_and_explicit(obj, arg, order) \
> +	atomic_fetch_and_explicit(obj, arg, order)
> +
> +#else
> +
> +#define __rte_atomic
> +
> +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_store(obj, desired) \
> +	__atomic_store_n(obj, desired, rte_memory_order_seq_cst)
> +
> +#define rte_atomic_store_explicit(obj, desired, order) \
> +	__atomic_store_n(obj, desired, order)
> +
> +#define rte_atomic_load(obj) \
> +	__atomic_load_n(obj, rte_memory_order_seq_cst)
> +
> +#define rte_atomic_load_explicit(obj, order) \
> +	__atomic_load_n(obj, order)
> +
> +#define rte_atomic_exchange(obj, desired) \
> +	__atomic_exchange_n(obj, desired, rte_memory_order_seq_cst)
> +
> +#define rte_atomic_exchange_explicit(obj, desired, order) \
> +	__atomic_exchange_n(obj, desired, order)
> +
> +#define rte_atomic_compare_exchange_strong(obj, expected, desired) \
> +	__sync_bool_compare_and_swap(obj, expected, desired)
> +
> +#define rte_atomic_compare_exchange_strong_explicit(obj, expected, desired, success, fail) \
> +	__atomic_compare_exchange_n(obj, expected, desired, 0, success, fail)
> +
> +#define rte_atomic_compare_exchange_weak(obj, expected, desired) \
> +	__atomic_compare_exchange_n(obj, expected, desired, 1, rte_memory_order_seq_cst, rte_memory_order_seq_cst)
> +
> +#define rte_atomic_compare_exchange_weak_explicit(obj, expected, desired, success, fail) \
> +	__atomic_compare_exchange_n(obj, expected, desired, 1, success, fail)
> +
> +#define rte_atomic_fetch_add(obj, arg) \
> +	__atomic_fetch_add(obj, arg, rte_memory_order_seq_cst)
> +
> +#define rte_atomic_fetch_add_explicit(obj, arg, order) \
> +	__atomic_fetch_add(obj, arg, order)
> +
> +#define rte_atomic_fetch_sub(obj, arg) \
> +	__atomic_fetch_sub(obj, arg, rte_memory_order_seq_cst)
> +
> +#define rte_atomic_fetch_sub_explicit(obj, arg, order) \
> +	__atomic_fetch_sub(obj, arg, order)
> +
> +#define rte_atomic_fetch_or(obj, arg) \
> +	__atomic_fetch_or(obj, arg, rte_memory_order_seq_cst)
> +
> +#define rte_atomic_fetch_or_explicit(obj, arg, order) \
> +	__atomic_fetch_or(obj, arg, order)
> +
> +#define rte_atomic_fetch_xor(obj, arg) \
> +	__atomic_fetch_xor(obj, arg, rte_memory_order_seq_cst)
> +
> +#define rte_atomic_fetch_xor_explicit(obj, arg, order) \
> +	__atomic_fetch_xor(obj, arg, order)
> +
> +#define rte_atomic_fetch_and(obj, arg) \
> +	__atomic_fetch_and(obj, arg, rte_memory_order_seq_cst)
> +
> +#define rte_atomic_fetch_and_explicit(obj, arg, order) \
> +	__atomic_fetch_and(obj, arg, order)
> +
> +#endif
> +
>  /**
>   * Compiler barrier.
>   *
> @@ -123,7 +277,7 @@
>  /**
>   * Synchronization fence between threads based on the specified memory order.
>   */
> -static inline void rte_atomic_thread_fence(int memorder);
> +static inline void rte_atomic_thread_fence(rte_memory_order memorder);
>  
>  /*------------------------- 16 bit atomic operations -------------------------*/
>  
> diff --git a/lib/eal/loongarch/include/rte_atomic.h b/lib/eal/loongarch/include/rte_atomic.h
> index 3c82845..66aa0c8 100644
> --- a/lib/eal/loongarch/include/rte_atomic.h
> +++ b/lib/eal/loongarch/include/rte_atomic.h
> @@ -35,9 +35,13 @@
>  #define rte_io_rmb()	rte_mb()
>  
>  static __rte_always_inline void
> -rte_atomic_thread_fence(int memorder)
> +rte_atomic_thread_fence(rte_memory_order memorder)
>  {
> +#ifdef RTE_STDC_ATOMICS
> +	atomic_thread_fence(memorder);
> +#else
>  	__atomic_thread_fence(memorder);
> +#endif
>  }
>  
>  #ifdef __cplusplus
> diff --git a/lib/eal/ppc/include/rte_atomic.h b/lib/eal/ppc/include/rte_atomic.h
> index 663b4d3..a428a83 100644
> --- a/lib/eal/ppc/include/rte_atomic.h
> +++ b/lib/eal/ppc/include/rte_atomic.h
> @@ -38,9 +38,13 @@
>  #define rte_io_rmb() rte_rmb()
>  
>  static __rte_always_inline void
> -rte_atomic_thread_fence(int memorder)
> +rte_atomic_thread_fence(rte_memory_order memorder)
>  {
> +#ifdef RTE_STDC_ATOMICS
> +	atomic_thread_fence(memorder);
> +#else
>  	__atomic_thread_fence(memorder);
> +#endif
>  }
>  
>  /*------------------------- 16 bit atomic operations -------------------------*/
> diff --git a/lib/eal/riscv/include/rte_atomic.h b/lib/eal/riscv/include/rte_atomic.h
> index 4b4633c..3c203a9 100644
> --- a/lib/eal/riscv/include/rte_atomic.h
> +++ b/lib/eal/riscv/include/rte_atomic.h
> @@ -40,9 +40,13 @@
>  #define rte_io_rmb()	asm volatile("fence ir, ir" : : : "memory")
>  
>  static __rte_always_inline void
> -rte_atomic_thread_fence(int memorder)
> +rte_atomic_thread_fence(rte_memory_order memorder)
>  {
> +#ifdef RTE_STDC_ATOMICS
> +	atomic_thread_fence(memorder);
> +#else
>  	__atomic_thread_fence(memorder);
> +#endif
>  }
>  
>  #ifdef __cplusplus
> diff --git a/lib/eal/x86/include/rte_atomic.h b/lib/eal/x86/include/rte_atomic.h
> index f2ee1a9..02d8b12 100644
> --- a/lib/eal/x86/include/rte_atomic.h
> +++ b/lib/eal/x86/include/rte_atomic.h
> @@ -87,12 +87,16 @@
>   * used instead.
>   */
>  static __rte_always_inline void
> -rte_atomic_thread_fence(int memorder)
> +rte_atomic_thread_fence(rte_memory_order memorder)
>  {
> -	if (memorder == __ATOMIC_SEQ_CST)
> +	if (memorder == rte_memory_order_seq_cst)
>  		rte_smp_mb();
>  	else
> +#ifdef RTE_STDC_ATOMICS
> +		atomic_thread_fence(memorder);
> +#else
>  		__atomic_thread_fence(memorder);
> +#endif
>  }
>  
>  /*------------------------- 16 bit atomic operations -------------------------*/
> diff --git a/meson_options.txt b/meson_options.txt
> index 0852849..acbcbb8 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -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_stdatomics', type: 'boolean', value: false, description:
> +       'enable use of standard C11 atomics.')
>  option('enable_trace_fp', type: 'boolean', value: false, description:
>         'enable fast path trace points.')
>  option('tests', type: 'boolean', value: true, description:
>
  

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, January 31, 2023 4:42 PM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Cc: dev@dpdk.org; bruce.richardson@intel.com; mb@smartsharesystems.com;
> Tyler Retzlaff <roretzla@linux.microsoft.com>; david.marchand@redhat.com;
> jerinj@marvell.com; konstantin.ananyev@huawei.com; ferruh.yigit@amd.com
> Subject: Re: [PATCH] eal: introduce atomics abstraction
> 
> Honnappa, please could you give your view on the future of atomics in DPDK?
Thanks Thomas, apologies it has taken me a while to get to this discussion.

IMO, we do not need DPDK's own abstractions. APIs from stdatomic.h (stdatomics as is called here) already serve the purpose. These APIs are well understood and documented.

For environments where stdatomics are not supported, we could have a stdatomic.h in DPDK implementing the same APIs (we have to support only _explicit APIs). This allows the code to use stdatomics APIs and when we move to minimum supported standard C11, we just need to get rid of the file in DPDK repo.

> 
> 
> 12/01/2023 22:26, Tyler Retzlaff:
> > Introduce atomics abstraction that permits optional use of standard
> > C11 atomics when meson is provided the new enable_stdatomics=true
> option.
> >
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > ---
> >  config/meson.build                     |  11 +++
> >  lib/eal/arm/include/rte_atomic_32.h    |   6 +-
> >  lib/eal/arm/include/rte_atomic_64.h    |   6 +-
> >  lib/eal/include/generic/rte_atomic.h   | 156
> ++++++++++++++++++++++++++++++++-
> >  lib/eal/loongarch/include/rte_atomic.h |   6 +-
> >  lib/eal/ppc/include/rte_atomic.h       |   6 +-
> >  lib/eal/riscv/include/rte_atomic.h     |   6 +-
> >  lib/eal/x86/include/rte_atomic.h       |   8 +-
> >  meson_options.txt                      |   2 +
> >  9 files changed, 199 insertions(+), 8 deletions(-)
> >
> > diff --git a/config/meson.build b/config/meson.build index
> > 6d9ffd4..9515ce9 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -254,6 +254,17 @@ endif
> >  # add -include rte_config to cflags
> >  add_project_arguments('-include', 'rte_config.h', language: 'c')
> >
> > +stdc_atomics_enabled = get_option('enable_stdatomics')
> > +dpdk_conf.set('RTE_STDC_ATOMICS', stdc_atomics_enabled)
> > +
> > +if stdc_atomics_enabled
> > +if cc.get_id() == 'gcc' or cc.get_id() == 'clang'
> > +    add_project_arguments('-std=gnu11', language: 'c') else
> > +    add_project_arguments('-std=c11', language: 'c') endif endif
> > +
> >  # enable extra warnings and disable any unwanted warnings  # -Wall is
> > added by default at warning level 1, and -Wextra  # at warning level 2
> > (DPDK default) diff --git a/lib/eal/arm/include/rte_atomic_32.h
> > b/lib/eal/arm/include/rte_atomic_32.h
> > index c00ab78..7088a12 100644
> > --- a/lib/eal/arm/include/rte_atomic_32.h
> > +++ b/lib/eal/arm/include/rte_atomic_32.h
> > @@ -34,9 +34,13 @@
> >  #define rte_io_rmb() rte_rmb()
> >
> >  static __rte_always_inline void
> > -rte_atomic_thread_fence(int memorder)
> > +rte_atomic_thread_fence(rte_memory_order memorder)
> >  {
> > +#ifdef RTE_STDC_ATOMICS
> > +	atomic_thread_fence(memorder);
> > +#else
> >  	__atomic_thread_fence(memorder);
> > +#endif
> >  }
> >
> >  #ifdef __cplusplus
> > diff --git a/lib/eal/arm/include/rte_atomic_64.h
> > b/lib/eal/arm/include/rte_atomic_64.h
> > index 6047911..7f02c57 100644
> > --- a/lib/eal/arm/include/rte_atomic_64.h
> > +++ b/lib/eal/arm/include/rte_atomic_64.h
> > @@ -38,9 +38,13 @@
> >  #define rte_io_rmb() rte_rmb()
> >
> >  static __rte_always_inline void
> > -rte_atomic_thread_fence(int memorder)
> > +rte_atomic_thread_fence(rte_memory_order memorder)
> >  {
> > +#ifdef RTE_STDC_ATOMICS
> > +	atomic_thread_fence(memorder);
> > +#else
> >  	__atomic_thread_fence(memorder);
> > +#endif
> >  }
> >
> >  /*------------------------ 128 bit atomic operations
> > -------------------------*/ diff --git
> > a/lib/eal/include/generic/rte_atomic.h
> > b/lib/eal/include/generic/rte_atomic.h
> > index f5c49a9..cb71c0d 100644
> > --- a/lib/eal/include/generic/rte_atomic.h
> > +++ b/lib/eal/include/generic/rte_atomic.h
> > @@ -110,6 +110,160 @@
> >
> >  #endif /* __DOXYGEN__ */
> >
> > +#ifdef RTE_STDC_ATOMICS
> > +
> > +#if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 201112L ||
> > +defined(__STDC_NO_ATOMICS__) #error compiler does not support C11
> > +standard atomics #else #include <stdatomic.h> #endif
> > +
> > +#define __rte_atomic _Atomic
> > +
> > +typedef int rte_memory_order;
> > +
> > +#define rte_memory_order_relaxed memory_order_relaxed #define
> > +rte_memory_order_consume memory_order_consume #define
> > +rte_memory_order_acquire memory_order_acquire #define
> > +rte_memory_order_release memory_order_release #define
> > +rte_memory_order_acq_rel memory_order_acq_rel #define
> > +rte_memory_order_seq_cst memory_order_seq_cst
> > +
> > +#define rte_atomic_store(obj, desired) \
> > +	atomic_store_explicit(obj, desired)
> > +
> > +#define rte_atomic_store_explicit(obj, desired, order) \
> > +	atomic_store_explicit(obj, desired, order)
> > +
> > +#define rte_atomic_load(obj) \
> > +	atomic_load_explicit(obj)
> > +
> > +#define rte_atomic_load_explicit(obj, order) \
> > +	atomic_load_explicit(obj, order)
> > +
> > +#define rte_atomic_exchange(obj, desired) \
> > +	atomic_exchange(obj, desired)
> > +
> > +#define rte_atomic_exchange_explicit(obj, desired, order) \
> > +	atomic_exchange_explicit(obj, desired, order)
> > +
> > +#define rte_atomic_compare_exchange_strong(obj, expected, desired) \
> > +	atomic_compare_exchange_strong(obj, expected, desired)
> > +
> > +#define rte_atomic_compare_exchange_strong_explicit(obj, expected,
> desired, success, fail) \
> > +	atomic_compare_exchange_strong_explicit(obj, expected, desired,
> > +success, fail)
> > +
> > +#define rte_atomic_compare_exchange_weak(obj, expected, desired) \
> > +	atomic_compare_exchange_weak(obj, expected, desired)
> > +
> > +#define rte_atomic_compare_exchange_weak_explicit(obj, expected,
> desired, success, fail) \
> > +	atomic_compare_exchange_weak_explicit(obj, expected, desired,
> > +success, fail)
> > +
> > +#define rte_atomic_fetch_add(obj, arg) \
> > +	atomic_fetch_add(obj, arg)
> > +
> > +#define rte_atomic_fetch_add_explicit(obj, arg, order) \
> > +	atomic_fetch_add_explicit(obj, arg, order)
> > +
> > +#define rte_atomic_fetch_sub(obj, arg) \
> > +	atomic_fetch_sub(obj, arg)
> > +
> > +#define rte_atomic_fetch_sub_explicit(obj, arg, order) \
> > +	atomic_fetch_sub_explicit(obj, arg, order)
> > +
> > +#define rte_atomic_fetch_or(obj, arg) \
> > +	atomic_fetch_or(obj, arg)
> > +
> > +#define rte_atomic_fetch_or_explicit(obj, arg, order) \
> > +	atomic_fetch_or_explicit(obj, arg, order)
> > +
> > +#define rte_atomic_fetch_xor(obj, arg) \
> > +	atomic_fetch_xor(obj, arg)
> > +
> > +#define rte_atomic_fetch_xor_explicit(obj, arg, order) \
> > +	atomic_fetch_xor_explicit(obj, arg, order)
> > +
> > +#define rte_atomic_fetch_and(obj, arg) \
> > +	atomic_fetch_and(obj, arg)
> > +
> > +#define rte_atomic_fetch_and_explicit(obj, arg, order) \
> > +	atomic_fetch_and_explicit(obj, arg, order)
> > +
> > +#else
> > +
> > +#define __rte_atomic
> > +
> > +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_store(obj, desired) \
> > +	__atomic_store_n(obj, desired, rte_memory_order_seq_cst)
> > +
> > +#define rte_atomic_store_explicit(obj, desired, order) \
> > +	__atomic_store_n(obj, desired, order)
> > +
> > +#define rte_atomic_load(obj) \
> > +	__atomic_load_n(obj, rte_memory_order_seq_cst)
> > +
> > +#define rte_atomic_load_explicit(obj, order) \
> > +	__atomic_load_n(obj, order)
> > +
> > +#define rte_atomic_exchange(obj, desired) \
> > +	__atomic_exchange_n(obj, desired, rte_memory_order_seq_cst)
> > +
> > +#define rte_atomic_exchange_explicit(obj, desired, order) \
> > +	__atomic_exchange_n(obj, desired, order)
> > +
> > +#define rte_atomic_compare_exchange_strong(obj, expected, desired) \
> > +	__sync_bool_compare_and_swap(obj, expected, desired)
> > +
> > +#define rte_atomic_compare_exchange_strong_explicit(obj, expected,
> desired, success, fail) \
> > +	__atomic_compare_exchange_n(obj, expected, desired, 0, success,
> > +fail)
> > +
> > +#define rte_atomic_compare_exchange_weak(obj, expected, desired) \
> > +	__atomic_compare_exchange_n(obj, expected, desired, 1,
> > +rte_memory_order_seq_cst, rte_memory_order_seq_cst)
> > +
> > +#define rte_atomic_compare_exchange_weak_explicit(obj, expected,
> desired, success, fail) \
> > +	__atomic_compare_exchange_n(obj, expected, desired, 1, success,
> > +fail)
> > +
> > +#define rte_atomic_fetch_add(obj, arg) \
> > +	__atomic_fetch_add(obj, arg, rte_memory_order_seq_cst)
> > +
> > +#define rte_atomic_fetch_add_explicit(obj, arg, order) \
> > +	__atomic_fetch_add(obj, arg, order)
> > +
> > +#define rte_atomic_fetch_sub(obj, arg) \
> > +	__atomic_fetch_sub(obj, arg, rte_memory_order_seq_cst)
> > +
> > +#define rte_atomic_fetch_sub_explicit(obj, arg, order) \
> > +	__atomic_fetch_sub(obj, arg, order)
> > +
> > +#define rte_atomic_fetch_or(obj, arg) \
> > +	__atomic_fetch_or(obj, arg, rte_memory_order_seq_cst)
> > +
> > +#define rte_atomic_fetch_or_explicit(obj, arg, order) \
> > +	__atomic_fetch_or(obj, arg, order)
> > +
> > +#define rte_atomic_fetch_xor(obj, arg) \
> > +	__atomic_fetch_xor(obj, arg, rte_memory_order_seq_cst)
> > +
> > +#define rte_atomic_fetch_xor_explicit(obj, arg, order) \
> > +	__atomic_fetch_xor(obj, arg, order)
> > +
> > +#define rte_atomic_fetch_and(obj, arg) \
> > +	__atomic_fetch_and(obj, arg, rte_memory_order_seq_cst)
> > +
> > +#define rte_atomic_fetch_and_explicit(obj, arg, order) \
> > +	__atomic_fetch_and(obj, arg, order)
> > +
> > +#endif
> > +
> >  /**
> >   * Compiler barrier.
> >   *
> > @@ -123,7 +277,7 @@
> >  /**
> >   * Synchronization fence between threads based on the specified memory
> order.
> >   */
> > -static inline void rte_atomic_thread_fence(int memorder);
> > +static inline void rte_atomic_thread_fence(rte_memory_order
> > +memorder);
> >
> >  /*------------------------- 16 bit atomic operations
> > -------------------------*/
> >
> > diff --git a/lib/eal/loongarch/include/rte_atomic.h
> > b/lib/eal/loongarch/include/rte_atomic.h
> > index 3c82845..66aa0c8 100644
> > --- a/lib/eal/loongarch/include/rte_atomic.h
> > +++ b/lib/eal/loongarch/include/rte_atomic.h
> > @@ -35,9 +35,13 @@
> >  #define rte_io_rmb()	rte_mb()
> >
> >  static __rte_always_inline void
> > -rte_atomic_thread_fence(int memorder)
> > +rte_atomic_thread_fence(rte_memory_order memorder)
> >  {
> > +#ifdef RTE_STDC_ATOMICS
> > +	atomic_thread_fence(memorder);
> > +#else
> >  	__atomic_thread_fence(memorder);
> > +#endif
> >  }
> >
> >  #ifdef __cplusplus
> > diff --git a/lib/eal/ppc/include/rte_atomic.h
> > b/lib/eal/ppc/include/rte_atomic.h
> > index 663b4d3..a428a83 100644
> > --- a/lib/eal/ppc/include/rte_atomic.h
> > +++ b/lib/eal/ppc/include/rte_atomic.h
> > @@ -38,9 +38,13 @@
> >  #define rte_io_rmb() rte_rmb()
> >
> >  static __rte_always_inline void
> > -rte_atomic_thread_fence(int memorder)
> > +rte_atomic_thread_fence(rte_memory_order memorder)
> >  {
> > +#ifdef RTE_STDC_ATOMICS
> > +	atomic_thread_fence(memorder);
> > +#else
> >  	__atomic_thread_fence(memorder);
> > +#endif
> >  }
> >
> >  /*------------------------- 16 bit atomic operations
> > -------------------------*/ diff --git
> > a/lib/eal/riscv/include/rte_atomic.h
> > b/lib/eal/riscv/include/rte_atomic.h
> > index 4b4633c..3c203a9 100644
> > --- a/lib/eal/riscv/include/rte_atomic.h
> > +++ b/lib/eal/riscv/include/rte_atomic.h
> > @@ -40,9 +40,13 @@
> >  #define rte_io_rmb()	asm volatile("fence ir, ir" : : : "memory")
> >
> >  static __rte_always_inline void
> > -rte_atomic_thread_fence(int memorder)
> > +rte_atomic_thread_fence(rte_memory_order memorder)
> >  {
> > +#ifdef RTE_STDC_ATOMICS
> > +	atomic_thread_fence(memorder);
> > +#else
> >  	__atomic_thread_fence(memorder);
> > +#endif
> >  }
> >
> >  #ifdef __cplusplus
> > diff --git a/lib/eal/x86/include/rte_atomic.h
> > b/lib/eal/x86/include/rte_atomic.h
> > index f2ee1a9..02d8b12 100644
> > --- a/lib/eal/x86/include/rte_atomic.h
> > +++ b/lib/eal/x86/include/rte_atomic.h
> > @@ -87,12 +87,16 @@
> >   * used instead.
> >   */
> >  static __rte_always_inline void
> > -rte_atomic_thread_fence(int memorder)
> > +rte_atomic_thread_fence(rte_memory_order memorder)
> >  {
> > -	if (memorder == __ATOMIC_SEQ_CST)
> > +	if (memorder == rte_memory_order_seq_cst)
> >  		rte_smp_mb();
> >  	else
> > +#ifdef RTE_STDC_ATOMICS
> > +		atomic_thread_fence(memorder);
> > +#else
> >  		__atomic_thread_fence(memorder);
> > +#endif
> >  }
> >
> >  /*------------------------- 16 bit atomic operations
> > -------------------------*/ diff --git a/meson_options.txt
> > b/meson_options.txt index 0852849..acbcbb8 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -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_stdatomics', type: 'boolean', value: false, description:
> > +       'enable use of standard C11 atomics.')
> >  option('enable_trace_fp', type: 'boolean', value: false, description:
> >         'enable fast path trace points.')  option('tests', type:
> > 'boolean', value: true, description:
> >
> 
> 
> 
>
  

> From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli@arm.com]
> Sent: Wednesday, 1 February 2023 02.08
> 
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Tuesday, January 31, 2023 4:42 PM
> >
> > Honnappa, please could you give your view on the future of atomics in
> DPDK?
> Thanks Thomas, apologies it has taken me a while to get to this
> discussion.
> 
> IMO, we do not need DPDK's own abstractions. APIs from stdatomic.h
> (stdatomics as is called here) already serve the purpose. These APIs
> are well understood and documented.
> 
> For environments where stdatomics are not supported, we could have a
> stdatomic.h in DPDK implementing the same APIs (we have to support only
> _explicit APIs). This allows the code to use stdatomics APIs and when
> we move to minimum supported standard C11, we just need to get rid of
> the file in DPDK repo.

I agree with Honnappa.

DPDK should include a shim to expose the C11 stdatomic.h API for environments not already having the C11 stdatomic.h.

With this, DPDK steps outside its native rte_ prefixed namespace, but it should not collide with anyone else's namespace.

It could collide with someone else's namespace if the application implements a similar shim. We cannot prevent this from happening, but if it does, it should be fixed in the application.

> 
> >
> >
> > 12/01/2023 22:26, Tyler Retzlaff:
> > > Introduce atomics abstraction that permits optional use of standard
> > > C11 atomics when meson is provided the new enable_stdatomics=true
> > option.
> > >
> > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
  

On Wed, Feb 01, 2023 at 01:07:59AM +0000, Honnappa Nagarahalli wrote:
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Tuesday, January 31, 2023 4:42 PM
> > To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > Cc: dev@dpdk.org; bruce.richardson@intel.com; mb@smartsharesystems.com;
> > Tyler Retzlaff <roretzla@linux.microsoft.com>; david.marchand@redhat.com;
> > jerinj@marvell.com; konstantin.ananyev@huawei.com; ferruh.yigit@amd.com
> > Subject: Re: [PATCH] eal: introduce atomics abstraction
> > 
> > Honnappa, please could you give your view on the future of atomics in DPDK?
> Thanks Thomas, apologies it has taken me a while to get to this discussion.
> 
> IMO, we do not need DPDK's own abstractions. APIs from stdatomic.h (stdatomics as is called here) already serve the purpose. These APIs are well understood and documented.

i agree that whatever atomics APIs we advocate for should align with the
standard C atomics for the reasons you state including implied semantics.

> 
> For environments where stdatomics are not supported, we could have a stdatomic.h in DPDK implementing the same APIs (we have to support only _explicit APIs). This allows the code to use stdatomics APIs and when we move to minimum supported standard C11, we just need to get rid of the file in DPDK repo.

my concern with this is that if we provide a stdatomic.h or introduce names
from stdatomic.h it's a violation of the C standard.

references:
 * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
 * GNU libc manual
   https://www.gnu.org/software/libc/manual/html_node/Reserved-Names.html

in effect the header, the names and in some instances namespaces introduced
are reserved by the implementation. there are several reasons in the GNU libc
manual that explain the justification for these reservations and if
if we think about ODR and ABI compatibility we can conceive of others.

i'll also remark that the inter-mingling of names from the POSIX
standard implicitly exposed as a part of the EAL public API has been
problematic for portability.

let's discuss this from here. if there's still overwhelming desire to go
this route then we'll just do our best.

ty
  

> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Wednesday, 1 February 2023 22.41
> 
> On Wed, Feb 01, 2023 at 01:07:59AM +0000, Honnappa Nagarahalli wrote:
> >
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > Sent: Tuesday, January 31, 2023 4:42 PM
> > >
> > > Honnappa, please could you give your view on the future of atomics
> in DPDK?
> > Thanks Thomas, apologies it has taken me a while to get to this
> discussion.
> >
> > IMO, we do not need DPDK's own abstractions. APIs from stdatomic.h
> (stdatomics as is called here) already serve the purpose. These APIs
> are well understood and documented.
> 
> i agree that whatever atomics APIs we advocate for should align with
> the
> standard C atomics for the reasons you state including implied
> semantics.
> 
> >
> > For environments where stdatomics are not supported, we could have a
> stdatomic.h in DPDK implementing the same APIs (we have to support only
> _explicit APIs). This allows the code to use stdatomics APIs and when
> we move to minimum supported standard C11, we just need to get rid of
> the file in DPDK repo.

Perhaps we can use something already existing, such as this:
https://android.googlesource.com/platform/bionic/+/lollipop-release/libc/include/stdatomic.h

> 
> my concern with this is that if we provide a stdatomic.h or introduce
> names
> from stdatomic.h it's a violation of the C standard.
> 
> references:
>  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
>  * GNU libc manual
>    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> Names.html
> 
> in effect the header, the names and in some instances namespaces
> introduced
> are reserved by the implementation. there are several reasons in the
> GNU libc
> manual that explain the justification for these reservations and if
> if we think about ODR and ABI compatibility we can conceive of others.

I we are going to move to C11 soon, I consider the shim interim, and am inclined to ignore these warning factors.

If we are not moving to C11 soon, I would consider these disadvantages more seriously.

> 
> i'll also remark that the inter-mingling of names from the POSIX
> standard implicitly exposed as a part of the EAL public API has been
> problematic for portability.

This is a very important remark, which should be considered carefully! Tyler has firsthand experience with DPDK portability. If he thinks porting to Windows is going to be a headache if we expose the stdatomic.h API, we must listen! So, what is your gut feeling here, Tyler?

> 
> let's discuss this from here. if there's still overwhelming desire to
> go
> this route then we'll just do our best.
> 
> ty

I have a preference for exposing the stdatomic.h API. Tyler listed the disadvantages above. (I also have a preference for moving to C11 soon.)

Exposing a 1:1 similar API with RTE prefixes would also be acceptable for me. The disadvantage is that the names are different than the C11 names, which might lead to some confusion. And from an ABI stability perspective, such an important API should not be marked experimental. This means that years will pass before we can get rid of it again, due to ABI stability policies.

-Morten
  

On Thu, Feb 02, 2023 at 09:43:58AM +0100, Morten Brørup wrote:
> > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > Sent: Wednesday, 1 February 2023 22.41
> > 
> > On Wed, Feb 01, 2023 at 01:07:59AM +0000, Honnappa Nagarahalli wrote:
> > >
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > Sent: Tuesday, January 31, 2023 4:42 PM
> > > >
> > > > Honnappa, please could you give your view on the future of atomics
> > in DPDK?
> > > Thanks Thomas, apologies it has taken me a while to get to this
> > discussion.
> > >
> > > IMO, we do not need DPDK's own abstractions. APIs from stdatomic.h
> > (stdatomics as is called here) already serve the purpose. These APIs
> > are well understood and documented.
> > 
> > i agree that whatever atomics APIs we advocate for should align with
> > the
> > standard C atomics for the reasons you state including implied
> > semantics.
> > 
> > >
> > > For environments where stdatomics are not supported, we could have a
> > stdatomic.h in DPDK implementing the same APIs (we have to support only
> > _explicit APIs). This allows the code to use stdatomics APIs and when
> > we move to minimum supported standard C11, we just need to get rid of
> > the file in DPDK repo.
> 
> Perhaps we can use something already existing, such as this:
> https://android.googlesource.com/platform/bionic/+/lollipop-release/libc/include/stdatomic.h
> 
> > 
> > my concern with this is that if we provide a stdatomic.h or introduce
> > names
> > from stdatomic.h it's a violation of the C standard.
> > 
> > references:
> >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> >  * GNU libc manual
> >    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > Names.html
> > 
> > in effect the header, the names and in some instances namespaces
> > introduced
> > are reserved by the implementation. there are several reasons in the
> > GNU libc
> > manual that explain the justification for these reservations and if
> > if we think about ODR and ABI compatibility we can conceive of others.
> 
> I we are going to move to C11 soon, I consider the shim interim, and am inclined to ignore these warning factors.
> 
> If we are not moving to C11 soon, I would consider these disadvantages more seriously.

I think it's reasonable to assume that we are talking years here.

We've had a few discussions about minimum C standard. I think my first
mailing list exchanges about C99 was almost 2 years ago. Given that we
still aren't on C99 now (though i know Bruce has a series up) indicates
that progression to C11 isn't going to happen any time soon and even if
it was the baseline we still can't just use it (reasons described
later).

Also, i'll point out that we seem to have accepted moving to C99 with
one of the holdback compilers technically being non-conformant but it
isn't blocking us because it provides the subset of C99 features without
being conforming that we happen to be using.

> 
> > 
> > i'll also remark that the inter-mingling of names from the POSIX
> > standard implicitly exposed as a part of the EAL public API has been
> > problematic for portability.
> 
> This is a very important remark, which should be considered carefully! Tyler has firsthand experience with DPDK portability. If he thinks porting to Windows is going to be a headache if we expose the stdatomic.h API, we must listen! So, what is your gut feeling here, Tyler?

I think this is even more of a concern with language standard than it is
with a platform standard. Because the language standard is used across
platforms.

On the surface it looks appealing to just go through all the dpdk code
one last time and #include <stdatomic.h> and directly depend on names
that "look" standard. In practice though we aren't depending on the
toolchain / libc surface we are binding ourselves to the shim and the
implementation it provides.

This is aside from the mechanics of making it work in the different
contexts we now have to care about. Here is a story of how things
become tricky.

When i #include <stdatomic.h> which one gets used if the implementation
provides one? Let's force our stdatomic.h

Now i need to force the build system to prefer my shim header? Keeping
in mind that the presence of a libc stdatomic.h does not mean that the
toolchain in fact supports standard atomics. Okay, that's under our
control by editing some meson.build files maybe it isn't so bad but...

It seems my application also has to do the same in their build system
now because...

The type definitions (size, alignment) and code generated from the
body of inline functions as seen by the application built translation
units may differ from those in the dpdk translation units if they don't
use our header. The potential for ABI compat problems is increasing but
maybe it is managable? it can be worse...

We can't limit our scope to thinking that there is just an
application (a single binary) and dpdk. Complex applications will
invariably depend on other libraries and if the application needs to
interface with those compatibily at the ABI level using standard atomics
then we've made it very difficult since the application has to choose to
use our conflicting named atomic types which may not be compatible or
the real standard atomics.  They can of course produce horrible shims
of their own to interoperate.

We need consistency across the entire binary at runtime and i don't
think it's practical to say that anyone who uses dpdk has to compile
their whole world with our shim. So dealing with all this complexity
for the sake of asthetics "looking" like the standard api seems kind
of not worth it. Sure it saves having to deprecate later and one last
session of shotgun surgery but that's kind of all we get.

Don't think i'm being biased in favor of windows/msvc here. From the
perspective of the windows/msvc combination i intend to use only the
standard C ABI provided by the implementation. I have no intention of
trying to introduce support for the current ABI that doesn't use the
standard atomic types. my discouraging of this approach is about avoiding
subtle to detect but very painful problems on {linux,unix}/compiler<version>
combinations that already have a shipped/stable ABI.

> > 
> > let's discuss this from here. if there's still overwhelming desire to
> > go
> > this route then we'll just do our best.
> > 
> > ty
> 
> I have a preference for exposing the stdatomic.h API. Tyler listed the disadvantages above. (I also have a preference for moving to C11 soon.)

I am eager to see this happen, but as explained in my original proposal
it doesn't eliminate the need for an abstraction. Unless we are willing
to break our compatibility promises and potentially take a performance
hit on some platform/compiler combinations which as i understand is not
acceptable.

> 
> Exposing a 1:1 similar API with RTE prefixes would also be acceptable for me. The disadvantage is that the names are different than the C11 names, which might lead to some confusion. And from an ABI stability perspective, such an important API should not be marked experimental. This means that years will pass before we can get rid of it again, due to ABI stability policies.

I think the key to success with rte_ prefixed names is making absolutely
sure we mirror the semantics and types in the standard.

I will point out one bit of fine print here is that we will not support
atomic operations on struct/union types (something the standard supports).
With the rte_ namespace i think this becomes less ambiguous, if we present
standard C names though what's to avoid the confusion? Aside from it fails
to compile with one compiler vs another.

I agree that this may be around for years. But how many years depends a
lot on how long we have to maintain compatibility for the existing
platform/compiler combinations that can't (and aren't enabled) to use
the standard.

Even if we introduced standard names we still have to undergo some kind
of mutant deprecation process to get the world to recompile everything
against the actual standard, so it doesn't give us forward
compatibility.

Let me know what folks would like to do, i guess i'm firmly leaned
toward no-shim and just rte_ explicit. But as a community i'll pursue
whatever you decide.

Thanks!
  

> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Thursday, 2 February 2023 20.00
> 
> On Thu, Feb 02, 2023 at 09:43:58AM +0100, Morten Brørup wrote:
> > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > Sent: Wednesday, 1 February 2023 22.41
> > >
> > > On Wed, Feb 01, 2023 at 01:07:59AM +0000, Honnappa Nagarahalli
> wrote:
> > > >
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > Sent: Tuesday, January 31, 2023 4:42 PM
> > > > >
> > > > > Honnappa, please could you give your view on the future of
> atomics
> > > in DPDK?
> > > > Thanks Thomas, apologies it has taken me a while to get to this
> > > discussion.
> > > >
> > > > IMO, we do not need DPDK's own abstractions. APIs from
> stdatomic.h
> > > (stdatomics as is called here) already serve the purpose. These
> APIs
> > > are well understood and documented.
> > >
> > > i agree that whatever atomics APIs we advocate for should align
> with
> > > the
> > > standard C atomics for the reasons you state including implied
> > > semantics.
> > >
> > > >
> > > > For environments where stdatomics are not supported, we could
> have a
> > > stdatomic.h in DPDK implementing the same APIs (we have to support
> only
> > > _explicit APIs). This allows the code to use stdatomics APIs and
> when
> > > we move to minimum supported standard C11, we just need to get rid
> of
> > > the file in DPDK repo.
> >
> > Perhaps we can use something already existing, such as this:
> > https://android.googlesource.com/platform/bionic/+/lollipop-
> release/libc/include/stdatomic.h
> >
> > >
> > > my concern with this is that if we provide a stdatomic.h or
> introduce
> > > names
> > > from stdatomic.h it's a violation of the C standard.
> > >
> > > references:
> > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > >  * GNU libc manual
> > >    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > > Names.html
> > >
> > > in effect the header, the names and in some instances namespaces
> > > introduced
> > > are reserved by the implementation. there are several reasons in
> the
> > > GNU libc
> > > manual that explain the justification for these reservations and if
> > > if we think about ODR and ABI compatibility we can conceive of
> others.
> >
> > I we are going to move to C11 soon, I consider the shim interim, and
> am inclined to ignore these warning factors.
> >
> > If we are not moving to C11 soon, I would consider these
> disadvantages more seriously.
> 
> I think it's reasonable to assume that we are talking years here.
> 
> We've had a few discussions about minimum C standard. I think my first
> mailing list exchanges about C99 was almost 2 years ago. Given that we
> still aren't on C99 now (though i know Bruce has a series up) indicates
> that progression to C11 isn't going to happen any time soon and even if
> it was the baseline we still can't just use it (reasons described
> later).
> 
> Also, i'll point out that we seem to have accepted moving to C99 with
> one of the holdback compilers technically being non-conformant but it
> isn't blocking us because it provides the subset of C99 features
> without
> being conforming that we happen to be using.
> 
> >
> > >
> > > i'll also remark that the inter-mingling of names from the POSIX
> > > standard implicitly exposed as a part of the EAL public API has
> been
> > > problematic for portability.
> >
> > This is a very important remark, which should be considered
> carefully! Tyler has firsthand experience with DPDK portability. If he
> thinks porting to Windows is going to be a headache if we expose the
> stdatomic.h API, we must listen! So, what is your gut feeling here,
> Tyler?
> 
> I think this is even more of a concern with language standard than it
> is
> with a platform standard. Because the language standard is used across
> platforms.
> 
> On the surface it looks appealing to just go through all the dpdk code
> one last time and #include <stdatomic.h> and directly depend on names
> that "look" standard. In practice though we aren't depending on the
> toolchain / libc surface we are binding ourselves to the shim and the
> implementation it provides.
> 
> This is aside from the mechanics of making it work in the different
> contexts we now have to care about. Here is a story of how things
> become tricky.
> 
> When i #include <stdatomic.h> which one gets used if the implementation
> provides one? Let's force our stdatomic.h
> 
> Now i need to force the build system to prefer my shim header? Keeping
> in mind that the presence of a libc stdatomic.h does not mean that the
> toolchain in fact supports standard atomics. Okay, that's under our
> control by editing some meson.build files maybe it isn't so bad but...
> 
> It seems my application also has to do the same in their build system
> now because...
> 
> The type definitions (size, alignment) and code generated from the
> body of inline functions as seen by the application built translation
> units may differ from those in the dpdk translation units if they don't
> use our header. The potential for ABI compat problems is increasing but
> maybe it is managable? it can be worse...
> 
> We can't limit our scope to thinking that there is just an
> application (a single binary) and dpdk. Complex applications will
> invariably depend on other libraries and if the application needs to
> interface with those compatibily at the ABI level using standard
> atomics
> then we've made it very difficult since the application has to choose
> to
> use our conflicting named atomic types which may not be compatible or
> the real standard atomics.  They can of course produce horrible shims
> of their own to interoperate.
> 
> We need consistency across the entire binary at runtime and i don't
> think it's practical to say that anyone who uses dpdk has to compile
> their whole world with our shim. So dealing with all this complexity
> for the sake of asthetics "looking" like the standard api seems kind
> of not worth it. Sure it saves having to deprecate later and one last
> session of shotgun surgery but that's kind of all we get.
> 
> Don't think i'm being biased in favor of windows/msvc here. From the
> perspective of the windows/msvc combination i intend to use only the
> standard C ABI provided by the implementation. I have no intention of
> trying to introduce support for the current ABI that doesn't use the
> standard atomic types. my discouraging of this approach is about
> avoiding
> subtle to detect but very painful problems on
> {linux,unix}/compiler<version>
> combinations that already have a shipped/stable ABI.
> 
> > >
> > > let's discuss this from here. if there's still overwhelming desire
> to
> > > go
> > > this route then we'll just do our best.
> > >
> > > ty
> >
> > I have a preference for exposing the stdatomic.h API. Tyler listed
> the disadvantages above. (I also have a preference for moving to C11
> soon.)
> 
> I am eager to see this happen, but as explained in my original proposal
> it doesn't eliminate the need for an abstraction. Unless we are willing
> to break our compatibility promises and potentially take a performance
> hit on some platform/compiler combinations which as i understand is not
> acceptable.
> 
> >
> > Exposing a 1:1 similar API with RTE prefixes would also be acceptable
> for me. The disadvantage is that the names are different than the C11
> names, which might lead to some confusion. And from an ABI stability
> perspective, such an important API should not be marked experimental.
> This means that years will pass before we can get rid of it again, due
> to ABI stability policies.
> 
> I think the key to success with rte_ prefixed names is making
> absolutely
> sure we mirror the semantics and types in the standard.
> 
> I will point out one bit of fine print here is that we will not support
> atomic operations on struct/union types (something the standard
> supports).
> With the rte_ namespace i think this becomes less ambiguous, if we
> present
> standard C names though what's to avoid the confusion? Aside from it
> fails
> to compile with one compiler vs another.
> 
> I agree that this may be around for years. But how many years depends a
> lot on how long we have to maintain compatibility for the existing
> platform/compiler combinations that can't (and aren't enabled) to use
> the standard.
> 
> Even if we introduced standard names we still have to undergo some kind
> of mutant deprecation process to get the world to recompile everything
> against the actual standard, so it doesn't give us forward
> compatibility.
> 
> Let me know what folks would like to do, i guess i'm firmly leaned
> toward no-shim and just rte_ explicit. But as a community i'll pursue
> whatever you decide.
> 
> Thanks!

Tyler is making a very strong case here.

I have changed my mind, and now support Tyler's approach.

-Morten
  

On Thu, Feb 02, 2023 at 11:00:23AM -0800, Tyler Retzlaff wrote:
> On Thu, Feb 02, 2023 at 09:43:58AM +0100, Morten Brørup wrote:
> > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > Sent: Wednesday, 1 February 2023 22.41
> > > 
> > > On Wed, Feb 01, 2023 at 01:07:59AM +0000, Honnappa Nagarahalli wrote:
> > > >
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > Sent: Tuesday, January 31, 2023 4:42 PM
> > > > >
> > > > > Honnappa, please could you give your view on the future of atomics
> > > in DPDK?
> > > > Thanks Thomas, apologies it has taken me a while to get to this
> > > discussion.
> > > >
> > > > IMO, we do not need DPDK's own abstractions. APIs from stdatomic.h
> > > (stdatomics as is called here) already serve the purpose. These APIs
> > > are well understood and documented.
> > > 
> > > i agree that whatever atomics APIs we advocate for should align with
> > > the
> > > standard C atomics for the reasons you state including implied
> > > semantics.
> > > 
> > > >
> > > > For environments where stdatomics are not supported, we could have a
> > > stdatomic.h in DPDK implementing the same APIs (we have to support only
> > > _explicit APIs). This allows the code to use stdatomics APIs and when
> > > we move to minimum supported standard C11, we just need to get rid of
> > > the file in DPDK repo.
> > 
> > Perhaps we can use something already existing, such as this:
> > https://android.googlesource.com/platform/bionic/+/lollipop-release/libc/include/stdatomic.h
> > 
> > > 
> > > my concern with this is that if we provide a stdatomic.h or introduce
> > > names
> > > from stdatomic.h it's a violation of the C standard.
> > > 
> > > references:
> > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > >  * GNU libc manual
> > >    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > > Names.html
> > > 
> > > in effect the header, the names and in some instances namespaces
> > > introduced
> > > are reserved by the implementation. there are several reasons in the
> > > GNU libc
> > > manual that explain the justification for these reservations and if
> > > if we think about ODR and ABI compatibility we can conceive of others.
> > 
> > I we are going to move to C11 soon, I consider the shim interim, and am inclined to ignore these warning factors.
> > 
> > If we are not moving to C11 soon, I would consider these disadvantages more seriously.
> 
> I think it's reasonable to assume that we are talking years here.
> 
> We've had a few discussions about minimum C standard. I think my first
> mailing list exchanges about C99 was almost 2 years ago. Given that we
> still aren't on C99 now (though i know Bruce has a series up) indicates
> that progression to C11 isn't going to happen any time soon and even if
> it was the baseline we still can't just use it (reasons described
> later).
> 
> Also, i'll point out that we seem to have accepted moving to C99 with
> one of the holdback compilers technically being non-conformant but it
> isn't blocking us because it provides the subset of C99 features without
> being conforming that we happen to be using.
> 
What compiler is this? As far as I know, all our currently support
compilers claim to support C99 fully. All should support C11 also,
except for GCC 4.8 on RHEL/CentOS 7. Once we drop support for Centos 7, I
think we can require at minimum a c11 compiler for building DPDK itself.
I'm still a little uncertain about requiring that users build their own
code with -std=c11, though.

/Bruce
  

On Thu, Feb 02, 2023 at 09:44:52PM +0100, Morten Brørup wrote:
> > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > Sent: Thursday, 2 February 2023 20.00
> > 
> > On Thu, Feb 02, 2023 at 09:43:58AM +0100, Morten Brørup wrote:
> > > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > > Sent: Wednesday, 1 February 2023 22.41
> > > >
> > > > On Wed, Feb 01, 2023 at 01:07:59AM +0000, Honnappa Nagarahalli
> > wrote:
> > > > >
> > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > Sent: Tuesday, January 31, 2023 4:42 PM
> > > > > >
> > > > > > Honnappa, please could you give your view on the future of
> > atomics
> > > > in DPDK?
> > > > > Thanks Thomas, apologies it has taken me a while to get to this
> > > > discussion.
> > > > >
> > > > > IMO, we do not need DPDK's own abstractions. APIs from
> > stdatomic.h
> > > > (stdatomics as is called here) already serve the purpose. These
> > APIs
> > > > are well understood and documented.
> > > >
> > > > i agree that whatever atomics APIs we advocate for should align
> > with
> > > > the
> > > > standard C atomics for the reasons you state including implied
> > > > semantics.
> > > >
> > > > >
> > > > > For environments where stdatomics are not supported, we could
> > have a
> > > > stdatomic.h in DPDK implementing the same APIs (we have to support
> > only
> > > > _explicit APIs). This allows the code to use stdatomics APIs and
> > when
> > > > we move to minimum supported standard C11, we just need to get rid
> > of
> > > > the file in DPDK repo.
> > >
> > > Perhaps we can use something already existing, such as this:
> > > https://android.googlesource.com/platform/bionic/+/lollipop-
> > release/libc/include/stdatomic.h
> > >
> > > >
> > > > my concern with this is that if we provide a stdatomic.h or
> > introduce
> > > > names
> > > > from stdatomic.h it's a violation of the C standard.
> > > >
> > > > references:
> > > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > > >  * GNU libc manual
> > > >    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > > > Names.html
> > > >
> > > > in effect the header, the names and in some instances namespaces
> > > > introduced
> > > > are reserved by the implementation. there are several reasons in
> > the
> > > > GNU libc
> > > > manual that explain the justification for these reservations and if
> > > > if we think about ODR and ABI compatibility we can conceive of
> > others.
> > >
> > > I we are going to move to C11 soon, I consider the shim interim, and
> > am inclined to ignore these warning factors.
> > >
> > > If we are not moving to C11 soon, I would consider these
> > disadvantages more seriously.
> > 
> > I think it's reasonable to assume that we are talking years here.
> > 
> > We've had a few discussions about minimum C standard. I think my first
> > mailing list exchanges about C99 was almost 2 years ago. Given that we
> > still aren't on C99 now (though i know Bruce has a series up) indicates
> > that progression to C11 isn't going to happen any time soon and even if
> > it was the baseline we still can't just use it (reasons described
> > later).
> > 
> > Also, i'll point out that we seem to have accepted moving to C99 with
> > one of the holdback compilers technically being non-conformant but it
> > isn't blocking us because it provides the subset of C99 features
> > without
> > being conforming that we happen to be using.
> > 
> > >
> > > >
> > > > i'll also remark that the inter-mingling of names from the POSIX
> > > > standard implicitly exposed as a part of the EAL public API has
> > been
> > > > problematic for portability.
> > >
> > > This is a very important remark, which should be considered
> > carefully! Tyler has firsthand experience with DPDK portability. If he
> > thinks porting to Windows is going to be a headache if we expose the
> > stdatomic.h API, we must listen! So, what is your gut feeling here,
> > Tyler?
> > 
> > I think this is even more of a concern with language standard than it
> > is
> > with a platform standard. Because the language standard is used across
> > platforms.
> > 
> > On the surface it looks appealing to just go through all the dpdk code
> > one last time and #include <stdatomic.h> and directly depend on names
> > that "look" standard. In practice though we aren't depending on the
> > toolchain / libc surface we are binding ourselves to the shim and the
> > implementation it provides.
> > 
> > This is aside from the mechanics of making it work in the different
> > contexts we now have to care about. Here is a story of how things
> > become tricky.
> > 
> > When i #include <stdatomic.h> which one gets used if the implementation
> > provides one? Let's force our stdatomic.h
> > 
> > Now i need to force the build system to prefer my shim header? Keeping
> > in mind that the presence of a libc stdatomic.h does not mean that the
> > toolchain in fact supports standard atomics. Okay, that's under our
> > control by editing some meson.build files maybe it isn't so bad but...
> > 
> > It seems my application also has to do the same in their build system
> > now because...
> > 
> > The type definitions (size, alignment) and code generated from the
> > body of inline functions as seen by the application built translation
> > units may differ from those in the dpdk translation units if they don't
> > use our header. The potential for ABI compat problems is increasing but
> > maybe it is managable? it can be worse...
> > 
> > We can't limit our scope to thinking that there is just an
> > application (a single binary) and dpdk. Complex applications will
> > invariably depend on other libraries and if the application needs to
> > interface with those compatibily at the ABI level using standard
> > atomics
> > then we've made it very difficult since the application has to choose
> > to
> > use our conflicting named atomic types which may not be compatible or
> > the real standard atomics.  They can of course produce horrible shims
> > of their own to interoperate.
> > 
> > We need consistency across the entire binary at runtime and i don't
> > think it's practical to say that anyone who uses dpdk has to compile
> > their whole world with our shim. So dealing with all this complexity
> > for the sake of asthetics "looking" like the standard api seems kind
> > of not worth it. Sure it saves having to deprecate later and one last
> > session of shotgun surgery but that's kind of all we get.
> > 
> > Don't think i'm being biased in favor of windows/msvc here. From the
> > perspective of the windows/msvc combination i intend to use only the
> > standard C ABI provided by the implementation. I have no intention of
> > trying to introduce support for the current ABI that doesn't use the
> > standard atomic types. my discouraging of this approach is about
> > avoiding
> > subtle to detect but very painful problems on
> > {linux,unix}/compiler<version>
> > combinations that already have a shipped/stable ABI.
> > 
> > > >
> > > > let's discuss this from here. if there's still overwhelming desire
> > to
> > > > go
> > > > this route then we'll just do our best.
> > > >
> > > > ty
> > >
> > > I have a preference for exposing the stdatomic.h API. Tyler listed
> > the disadvantages above. (I also have a preference for moving to C11
> > soon.)
> > 
> > I am eager to see this happen, but as explained in my original proposal
> > it doesn't eliminate the need for an abstraction. Unless we are willing
> > to break our compatibility promises and potentially take a performance
> > hit on some platform/compiler combinations which as i understand is not
> > acceptable.
> > 
> > >
> > > Exposing a 1:1 similar API with RTE prefixes would also be acceptable
> > for me. The disadvantage is that the names are different than the C11
> > names, which might lead to some confusion. And from an ABI stability
> > perspective, such an important API should not be marked experimental.
> > This means that years will pass before we can get rid of it again, due
> > to ABI stability policies.
> > 
> > I think the key to success with rte_ prefixed names is making
> > absolutely
> > sure we mirror the semantics and types in the standard.
> > 
> > I will point out one bit of fine print here is that we will not support
> > atomic operations on struct/union types (something the standard
> > supports).
> > With the rte_ namespace i think this becomes less ambiguous, if we
> > present
> > standard C names though what's to avoid the confusion? Aside from it
> > fails
> > to compile with one compiler vs another.
> > 
> > I agree that this may be around for years. But how many years depends a
> > lot on how long we have to maintain compatibility for the existing
> > platform/compiler combinations that can't (and aren't enabled) to use
> > the standard.
> > 
> > Even if we introduced standard names we still have to undergo some kind
> > of mutant deprecation process to get the world to recompile everything
> > against the actual standard, so it doesn't give us forward
> > compatibility.
> > 
> > Let me know what folks would like to do, i guess i'm firmly leaned
> > toward no-shim and just rte_ explicit. But as a community i'll pursue
> > whatever you decide.
> > 
> > Thanks!
> 
> Tyler is making a very strong case here.
> 
> I have changed my mind, and now support Tyler's approach.
> 

Having read through the whole thread a second time, I am starting to
realise how complex a problem this could be. From what I read, I would tend
towards the opinion that we shouldn't provide any atomics in DPDK at all,
and just rely on the C standard ones. The main complication in any solution
I suspect is going to be the use of atomics in static inline functions we
have in our header files.

/Bruce
  

> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Friday, 3 February 2023 14.57
> 
> Having read through the whole thread a second time, I am starting to
> realise how complex a problem this could be. From what I read, I would
> tend
> towards the opinion that we shouldn't provide any atomics in DPDK at
> all,
> and just rely on the C standard ones.

Yes, but atomics were only standardized for C with C11. So that is the problem - and a good reason for pushing for C11.

Before C11, there were only compiler specific intrinsics.

> The main complication in any
> solution
> I suspect is going to be the use of atomics in static inline functions
> we
> have in our header files.

Not only that.

If we consider atomic variables (and the type of such variables) private to DPDK, we would need to provide public functions to manipulate those atomic variables. If not, the application will be unable to interact with them.

And if we expose such functions, why not also make the DPDK atomic type itself public? In other words, expose a DPDK API for atomics, such as Tyler's.

(And as I mentioned on the techboard meeting, I think atomic operations should be available in inline code, for performance reasons.)

-Morten
  

On Fri, Feb 03, 2023 at 12:19:13PM +0000, Bruce Richardson wrote:
> On Thu, Feb 02, 2023 at 11:00:23AM -0800, Tyler Retzlaff wrote:
> > On Thu, Feb 02, 2023 at 09:43:58AM +0100, Morten Brørup wrote:
> > > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > > Sent: Wednesday, 1 February 2023 22.41
> > > > 
> > > > On Wed, Feb 01, 2023 at 01:07:59AM +0000, Honnappa Nagarahalli wrote:
> > > > >
> > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > Sent: Tuesday, January 31, 2023 4:42 PM
> > > > > >
> > > > > > Honnappa, please could you give your view on the future of atomics
> > > > in DPDK?
> > > > > Thanks Thomas, apologies it has taken me a while to get to this
> > > > discussion.
> > > > >
> > > > > IMO, we do not need DPDK's own abstractions. APIs from stdatomic.h
> > > > (stdatomics as is called here) already serve the purpose. These APIs
> > > > are well understood and documented.
> > > > 
> > > > i agree that whatever atomics APIs we advocate for should align with
> > > > the
> > > > standard C atomics for the reasons you state including implied
> > > > semantics.
> > > > 
> > > > >
> > > > > For environments where stdatomics are not supported, we could have a
> > > > stdatomic.h in DPDK implementing the same APIs (we have to support only
> > > > _explicit APIs). This allows the code to use stdatomics APIs and when
> > > > we move to minimum supported standard C11, we just need to get rid of
> > > > the file in DPDK repo.
> > > 
> > > Perhaps we can use something already existing, such as this:
> > > https://android.googlesource.com/platform/bionic/+/lollipop-release/libc/include/stdatomic.h
> > > 
> > > > 
> > > > my concern with this is that if we provide a stdatomic.h or introduce
> > > > names
> > > > from stdatomic.h it's a violation of the C standard.
> > > > 
> > > > references:
> > > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > > >  * GNU libc manual
> > > >    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > > > Names.html
> > > > 
> > > > in effect the header, the names and in some instances namespaces
> > > > introduced
> > > > are reserved by the implementation. there are several reasons in the
> > > > GNU libc
> > > > manual that explain the justification for these reservations and if
> > > > if we think about ODR and ABI compatibility we can conceive of others.
> > > 
> > > I we are going to move to C11 soon, I consider the shim interim, and am inclined to ignore these warning factors.
> > > 
> > > If we are not moving to C11 soon, I would consider these disadvantages more seriously.
> > 
> > I think it's reasonable to assume that we are talking years here.
> > 
> > We've had a few discussions about minimum C standard. I think my first
> > mailing list exchanges about C99 was almost 2 years ago. Given that we
> > still aren't on C99 now (though i know Bruce has a series up) indicates
> > that progression to C11 isn't going to happen any time soon and even if
> > it was the baseline we still can't just use it (reasons described
> > later).
> > 
> > Also, i'll point out that we seem to have accepted moving to C99 with
> > one of the holdback compilers technically being non-conformant but it
> > isn't blocking us because it provides the subset of C99 features without
> > being conforming that we happen to be using.
> > 
> What compiler is this? As far as I know, all our currently support
> compilers claim to support C99 fully. All should support C11 also,
> except for GCC 4.8 on RHEL/CentOS 7. Once we drop support for Centos 7, I
> think we can require at minimum a c11 compiler for building DPDK itself.
> I'm still a little uncertain about requiring that users build their own
> code with -std=c11, though.

perhaps i'm mistaken but it was my understanding that the gcc version on
RHEL 7 did not fully conform to C99? maybe i read C99 when it was actually
C11.

regardless, even if every supported compiler for dpdk was C11 conformant
including stdatomics which are optional we can't just move from
intrinsic/builtins to standard C atomics (because of the compatibility
and performance issues mentioned previously).

so just re-orienting this discussion, the purpose of this abstraction is
to allow the optional use of standard C atomics when a conformant compiler
is available and satisfactory code is generated for the desired target.

> 
> /Bruce
  

> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Friday, 3 February 2023 21.49
> 
> On Fri, Feb 03, 2023 at 12:19:13PM +0000, Bruce Richardson wrote:
> > On Thu, Feb 02, 2023 at 11:00:23AM -0800, Tyler Retzlaff wrote:
> > > On Thu, Feb 02, 2023 at 09:43:58AM +0100, Morten Brørup wrote:
> > > > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > > > Sent: Wednesday, 1 February 2023 22.41
> > > > >
> > > > > On Wed, Feb 01, 2023 at 01:07:59AM +0000, Honnappa Nagarahalli
> wrote:
> > > > > >
> > > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > Sent: Tuesday, January 31, 2023 4:42 PM
> > > > > > >
> > > > > > > Honnappa, please could you give your view on the future of
> atomics
> > > > > in DPDK?
> > > > > > Thanks Thomas, apologies it has taken me a while to get to
> this
> > > > > discussion.
> > > > > >
> > > > > > IMO, we do not need DPDK's own abstractions. APIs from
> stdatomic.h
> > > > > (stdatomics as is called here) already serve the purpose. These
> APIs
> > > > > are well understood and documented.
> > > > >
> > > > > i agree that whatever atomics APIs we advocate for should align
> with
> > > > > the
> > > > > standard C atomics for the reasons you state including implied
> > > > > semantics.
> > > > >
> > > > > >
> > > > > > For environments where stdatomics are not supported, we could
> have a
> > > > > stdatomic.h in DPDK implementing the same APIs (we have to
> support only
> > > > > _explicit APIs). This allows the code to use stdatomics APIs
> and when
> > > > > we move to minimum supported standard C11, we just need to get
> rid of
> > > > > the file in DPDK repo.
> > > >
> > > > Perhaps we can use something already existing, such as this:
> > > > https://android.googlesource.com/platform/bionic/+/lollipop-
> release/libc/include/stdatomic.h
> > > >
> > > > >
> > > > > my concern with this is that if we provide a stdatomic.h or
> introduce
> > > > > names
> > > > > from stdatomic.h it's a violation of the C standard.
> > > > >
> > > > > references:
> > > > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > > > >  * GNU libc manual
> > > > >    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > > > > Names.html
> > > > >
> > > > > in effect the header, the names and in some instances
> namespaces
> > > > > introduced
> > > > > are reserved by the implementation. there are several reasons
> in the
> > > > > GNU libc
> > > > > manual that explain the justification for these reservations
> and if
> > > > > if we think about ODR and ABI compatibility we can conceive of
> others.
> > > >
> > > > I we are going to move to C11 soon, I consider the shim interim,
> and am inclined to ignore these warning factors.
> > > >
> > > > If we are not moving to C11 soon, I would consider these
> disadvantages more seriously.
> > >
> > > I think it's reasonable to assume that we are talking years here.
> > >
> > > We've had a few discussions about minimum C standard. I think my
> first
> > > mailing list exchanges about C99 was almost 2 years ago. Given that
> we
> > > still aren't on C99 now (though i know Bruce has a series up)
> indicates
> > > that progression to C11 isn't going to happen any time soon and
> even if
> > > it was the baseline we still can't just use it (reasons described
> > > later).
> > >
> > > Also, i'll point out that we seem to have accepted moving to C99
> with
> > > one of the holdback compilers technically being non-conformant but
> it
> > > isn't blocking us because it provides the subset of C99 features
> without
> > > being conforming that we happen to be using.
> > >
> > What compiler is this? As far as I know, all our currently support
> > compilers claim to support C99 fully. All should support C11 also,
> > except for GCC 4.8 on RHEL/CentOS 7. Once we drop support for Centos
> 7, I
> > think we can require at minimum a c11 compiler for building DPDK
> itself.
> > I'm still a little uncertain about requiring that users build their
> own
> > code with -std=c11, though.
> 
> perhaps i'm mistaken but it was my understanding that the gcc version
> on
> RHEL 7 did not fully conform to C99? maybe i read C99 when it was
> actually
> C11.

RHEL does supports C99, it's C11 that it doesn't support [1].

[1]: http://inbox.dpdk.org/dev/98CBD80474FA8B44BF855DF32C47DC35D8762F@smartserver.smartshare.dk/

> 
> regardless, even if every supported compiler for dpdk was C11
> conformant
> including stdatomics which are optional we can't just move from
> intrinsic/builtins to standard C atomics (because of the compatibility
> and performance issues mentioned previously).

For example, with C11, you can make structures atomic. And an atomic instance of a type can have a different size than the non-atomic type.

If do we make a shim, it will have some limitations compared to C11 atomics, e.g. it cannot handle atomic structures.

Either we accept these limitations of the shim, or we use our own namespace. If we accept the limitations, we risk that someone with a C11 build environment uses them anyway, and it will not work in non-C11 build environments. So a shim is not a rose without thorns.

> 
> so just re-orienting this discussion, the purpose of this abstraction
> is
> to allow the optional use of standard C atomics when a conformant
> compiler
> is available and satisfactory code is generated for the desired target.

I think it is more important getting this feature into DPDK than using the C11 stdatomic.h API for atomics in DPDK.

I don't feel strongly about this API, and will accept either the proposed patch series (with the C11-like API, but rte_ prefixed namespace), or a C11 stdatomic.h API shim.
  

On Tue, Feb 07, 2023 at 04:16:58PM +0100, Morten Brørup wrote:
> > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > Sent: Friday, 3 February 2023 21.49
> > 
> > On Fri, Feb 03, 2023 at 12:19:13PM +0000, Bruce Richardson wrote:
> > > On Thu, Feb 02, 2023 at 11:00:23AM -0800, Tyler Retzlaff wrote:
> > > > On Thu, Feb 02, 2023 at 09:43:58AM +0100, Morten Brørup wrote:
> > > > > > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > > > > > Sent: Wednesday, 1 February 2023 22.41
> > > > > >
> > > > > > On Wed, Feb 01, 2023 at 01:07:59AM +0000, Honnappa Nagarahalli
> > wrote:
> > > > > > >
> > > > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > > Sent: Tuesday, January 31, 2023 4:42 PM
> > > > > > > >
> > > > > > > > Honnappa, please could you give your view on the future of
> > atomics
> > > > > > in DPDK?
> > > > > > > Thanks Thomas, apologies it has taken me a while to get to
> > this
> > > > > > discussion.
> > > > > > >
> > > > > > > IMO, we do not need DPDK's own abstractions. APIs from
> > stdatomic.h
> > > > > > (stdatomics as is called here) already serve the purpose. These
> > APIs
> > > > > > are well understood and documented.
> > > > > >
> > > > > > i agree that whatever atomics APIs we advocate for should align
> > with
> > > > > > the
> > > > > > standard C atomics for the reasons you state including implied
> > > > > > semantics.
> > > > > >
> > > > > > >
> > > > > > > For environments where stdatomics are not supported, we could
> > have a
> > > > > > stdatomic.h in DPDK implementing the same APIs (we have to
> > support only
> > > > > > _explicit APIs). This allows the code to use stdatomics APIs
> > and when
> > > > > > we move to minimum supported standard C11, we just need to get
> > rid of
> > > > > > the file in DPDK repo.
> > > > >
> > > > > Perhaps we can use something already existing, such as this:
> > > > > https://android.googlesource.com/platform/bionic/+/lollipop-
> > release/libc/include/stdatomic.h
> > > > >
> > > > > >
> > > > > > my concern with this is that if we provide a stdatomic.h or
> > introduce
> > > > > > names
> > > > > > from stdatomic.h it's a violation of the C standard.
> > > > > >
> > > > > > references:
> > > > > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > > > > >  * GNU libc manual
> > > > > >    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > > > > > Names.html
> > > > > >
> > > > > > in effect the header, the names and in some instances
> > namespaces
> > > > > > introduced
> > > > > > are reserved by the implementation. there are several reasons
> > in the
> > > > > > GNU libc
> > > > > > manual that explain the justification for these reservations
> > and if
> > > > > > if we think about ODR and ABI compatibility we can conceive of
> > others.
> > > > >
> > > > > I we are going to move to C11 soon, I consider the shim interim,
> > and am inclined to ignore these warning factors.
> > > > >
> > > > > If we are not moving to C11 soon, I would consider these
> > disadvantages more seriously.
> > > >
> > > > I think it's reasonable to assume that we are talking years here.
> > > >
> > > > We've had a few discussions about minimum C standard. I think my
> > first
> > > > mailing list exchanges about C99 was almost 2 years ago. Given that
> > we
> > > > still aren't on C99 now (though i know Bruce has a series up)
> > indicates
> > > > that progression to C11 isn't going to happen any time soon and
> > even if
> > > > it was the baseline we still can't just use it (reasons described
> > > > later).
> > > >
> > > > Also, i'll point out that we seem to have accepted moving to C99
> > with
> > > > one of the holdback compilers technically being non-conformant but
> > it
> > > > isn't blocking us because it provides the subset of C99 features
> > without
> > > > being conforming that we happen to be using.
> > > >
> > > What compiler is this? As far as I know, all our currently support
> > > compilers claim to support C99 fully. All should support C11 also,
> > > except for GCC 4.8 on RHEL/CentOS 7. Once we drop support for Centos
> > 7, I
> > > think we can require at minimum a c11 compiler for building DPDK
> > itself.
> > > I'm still a little uncertain about requiring that users build their
> > own
> > > code with -std=c11, though.
> > 
> > perhaps i'm mistaken but it was my understanding that the gcc version
> > on
> > RHEL 7 did not fully conform to C99? maybe i read C99 when it was
> > actually
> > C11.
> 
> RHEL does supports C99, it's C11 that it doesn't support [1].
> 
> [1]: http://inbox.dpdk.org/dev/98CBD80474FA8B44BF855DF32C47DC35D8762F@smartserver.smartshare.dk/
> 
> > 
> > regardless, even if every supported compiler for dpdk was C11
> > conformant
> > including stdatomics which are optional we can't just move from
> > intrinsic/builtins to standard C atomics (because of the compatibility
> > and performance issues mentioned previously).
> 
> For example, with C11, you can make structures atomic. And an atomic instance of a type can have a different size than the non-atomic type.
> 
> If do we make a shim, it will have some limitations compared to C11 atomics, e.g. it cannot handle atomic structures.

right, so it "looks" like standard but then doesn't work like standard.

> 
> Either we accept these limitations of the shim, or we use our own namespace. If we accept the limitations, we risk that someone with a C11 build environment uses them anyway, and it will not work in non-C11 build environments. So a shim is not a rose without thorns.

the standard says even integer types can have different alignment and size
so it isn't strictly portable to replace any integer type with the similar
_Atomic type. here is an example of something i would prefer not to have
to navigate which using a shim would sign us up for.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65146

> > 
> > so just re-orienting this discussion, the purpose of this abstraction
> > is
> > to allow the optional use of standard C atomics when a conformant
> > compiler
> > is available and satisfactory code is generated for the desired target.
> 
> I think it is more important getting this feature into DPDK than using the C11 stdatomic.h API for atomics in DPDK.
> 
> I don't feel strongly about this API, and will accept either the proposed patch series (with the C11-like API, but rte_ prefixed namespace), or a C11 stdatomic.h API shim.

let's just stay out of the standard namespace, it doesn't buy us the
forward compatibility we want and being explicit in the namespace makes
it obvious that we aren't.

ty
  

<snip>

> > >
> > > Honnappa, please could you give your view on the future of atomics in
> DPDK?
> > Thanks Thomas, apologies it has taken me a while to get to this discussion.
> >
> > IMO, we do not need DPDK's own abstractions. APIs from stdatomic.h
> (stdatomics as is called here) already serve the purpose. These APIs are well
> understood and documented.
> 
> i agree that whatever atomics APIs we advocate for should align with the
> standard C atomics for the reasons you state including implied semantics.
Another point I want to make is, we need 'xxx_explicit' APIs only, as we want memory ordering explicitly provided at each call site. (This can be discussed later).

> 
> >
> > For environments where stdatomics are not supported, we could have a
> stdatomic.h in DPDK implementing the same APIs (we have to support only
> _explicit APIs). This allows the code to use stdatomics APIs and when we move
> to minimum supported standard C11, we just need to get rid of the file in DPDK
> repo.
> 
> my concern with this is that if we provide a stdatomic.h or introduce names
> from stdatomic.h it's a violation of the C standard.
> 
> references:
>  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
>  * GNU libc manual
>    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> Names.html
> 
> in effect the header, the names and in some instances namespaces introduced
> are reserved by the implementation. there are several reasons in the GNU libc
Wouldn't this apply only after the particular APIs were introduced? i.e. it should not apply if the compiler does not support stdatomics.

> manual that explain the justification for these reservations and if if we think
> about ODR and ABI compatibility we can conceive of others.
> 
> i'll also remark that the inter-mingling of names from the POSIX standard
> implicitly exposed as a part of the EAL public API has been problematic for
> portability.
These should be exposed as EAL APIs only when compiled with a compiler that does not support stdatomics.

> 
> let's discuss this from here. if there's still overwhelming desire to go this route
> then we'll just do our best.
> 
> ty
  

On Tue, Feb 07, 2023 at 11:34:14PM +0000, Honnappa Nagarahalli wrote:
> <snip>
> 
> > > >
> > > > Honnappa, please could you give your view on the future of atomics in
> > DPDK?
> > > Thanks Thomas, apologies it has taken me a while to get to this discussion.
> > >
> > > IMO, we do not need DPDK's own abstractions. APIs from stdatomic.h
> > (stdatomics as is called here) already serve the purpose. These APIs are well
> > understood and documented.
> > 
> > i agree that whatever atomics APIs we advocate for should align with the
> > standard C atomics for the reasons you state including implied semantics.
> Another point I want to make is, we need 'xxx_explicit' APIs only, as we want memory ordering explicitly provided at each call site. (This can be discussed later).

i don't have any issue with removing the non-explicit versions. they're
just just convenience for seq_cst anyway. if people don't want them we
don't have to have them.

> 
> > 
> > >
> > > For environments where stdatomics are not supported, we could have a
> > stdatomic.h in DPDK implementing the same APIs (we have to support only
> > _explicit APIs). This allows the code to use stdatomics APIs and when we move
> > to minimum supported standard C11, we just need to get rid of the file in DPDK
> > repo.
> > 
> > my concern with this is that if we provide a stdatomic.h or introduce names
> > from stdatomic.h it's a violation of the C standard.
> > 
> > references:
> >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> >  * GNU libc manual
> >    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > Names.html
> > 
> > in effect the header, the names and in some instances namespaces introduced
> > are reserved by the implementation. there are several reasons in the GNU libc
> Wouldn't this apply only after the particular APIs were introduced? i.e. it should not apply if the compiler does not support stdatomics.

yeah, i agree they're being a bit wishy washy in the wording, but i'm
not convinced glibc folks are documenting this as permissive guidance
against.

> 
> > manual that explain the justification for these reservations and if if we think
> > about ODR and ABI compatibility we can conceive of others.
> > 
> > i'll also remark that the inter-mingling of names from the POSIX standard
> > implicitly exposed as a part of the EAL public API has been problematic for
> > portability.
> These should be exposed as EAL APIs only when compiled with a compiler that does not support stdatomics.

you don't necessarily compile dpdk, the application or its other
dynamically linked dependencies with the same compiler at the same time.
i.e. basically the model of any dpdk-dev package on any linux distribution.

if dpdk is built without real stdatomic types but the application has to
interoperate with a different kit or library that does they would be forced
to dance around dpdk with their own version of a shim to hide our
faked up stdatomics.

> 
> > 
> > let's discuss this from here. if there's still overwhelming desire to go this route
> > then we'll just do our best.
> > 
> > ty
  

> From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> Sent: Wednesday, 8 February 2023 02.21
> 
> On Tue, Feb 07, 2023 at 11:34:14PM +0000, Honnappa Nagarahalli wrote:
> > <snip>
> >
> > > > >
> > > > > Honnappa, please could you give your view on the future of
> atomics in
> > > DPDK?
> > > > Thanks Thomas, apologies it has taken me a while to get to this
> discussion.
> > > >
> > > > IMO, we do not need DPDK's own abstractions. APIs from
> stdatomic.h
> > > (stdatomics as is called here) already serve the purpose. These
> APIs are well
> > > understood and documented.
> > >
> > > i agree that whatever atomics APIs we advocate for should align
> with the
> > > standard C atomics for the reasons you state including implied
> semantics.
> > Another point I want to make is, we need 'xxx_explicit' APIs only, as
> we want memory ordering explicitly provided at each call site. (This
> can be discussed later).
> 
> i don't have any issue with removing the non-explicit versions. they're
> just just convenience for seq_cst anyway. if people don't want them we
> don't have to have them.

I agree with Honnappa on this point.

The non-explicit versions are for lazy (or not so experienced) developers, and might impact performance if used instead of the correct explicit versions.

I'm working on porting some of our application code from DPDK's rte_atomic32 operations to modern atomics, and I'm temporarily using acq_rel with a FIXME comment on each operation until I have the overview to determine if another memory order is better for each operation. And if I don't get around to fixing the memory order, it is still a step in the right direct direction to get rid of the old __sync based atomics; and the FIXME's remain to be fixed in a later release.

So here's an idea: Alternatively to omitting the non-explicit versions, we could include them for application developers, but document them as placeholders for "memory order to be determined later" and emit a warning when used. It might speed up the transition away from old atomic operations. Alternatively, we risk thoughtless use of seq_cst with the explicit versions, which might be difficult to detect in code reviews.
 
Either way, with or without non-explicit versions, is fine with me.

> 
> >
> > >
> > > >
> > > > For environments where stdatomics are not supported, we could
> have a
> > > stdatomic.h in DPDK implementing the same APIs (we have to support
> only
> > > _explicit APIs). This allows the code to use stdatomics APIs and
> when we move
> > > to minimum supported standard C11, we just need to get rid of the
> file in DPDK
> > > repo.
> > >
> > > my concern with this is that if we provide a stdatomic.h or
> introduce names
> > > from stdatomic.h it's a violation of the C standard.
> > >
> > > references:
> > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > >  * GNU libc manual
> > >    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > > Names.html
> > >
> > > in effect the header, the names and in some instances namespaces
> introduced
> > > are reserved by the implementation. there are several reasons in
> the GNU libc
> > Wouldn't this apply only after the particular APIs were introduced?
> i.e. it should not apply if the compiler does not support stdatomics.
> 
> yeah, i agree they're being a bit wishy washy in the wording, but i'm
> not convinced glibc folks are documenting this as permissive guidance
> against.
> 
> >
> > > manual that explain the justification for these reservations and if
> if we think
> > > about ODR and ABI compatibility we can conceive of others.
> > >
> > > i'll also remark that the inter-mingling of names from the POSIX
> standard
> > > implicitly exposed as a part of the EAL public API has been
> problematic for
> > > portability.
> > These should be exposed as EAL APIs only when compiled with a
> compiler that does not support stdatomics.
> 
> you don't necessarily compile dpdk, the application or its other
> dynamically linked dependencies with the same compiler at the same
> time.
> i.e. basically the model of any dpdk-dev package on any linux
> distribution.
> 
> if dpdk is built without real stdatomic types but the application has
> to
> interoperate with a different kit or library that does they would be
> forced
> to dance around dpdk with their own version of a shim to hide our
> faked up stdatomics.
> 

So basically, if we want a binary DPDK distribution to be compatible with a separate application build environment, they both have to implement atomics the same way, i.e. agree on the ABI for atomics.

Summing up, this leaves us with only two realistic options:

1. Go all in on C11 stdatomics, also requiring the application build environment to support C11 stdatomics.
2. Provide our own DPDK atomics library.

(As mentioned by Tyler, the third option - using C11 stdatomics inside DPDK, and requiring a build environment without C11 stdatomics to implement a shim - is not realistic!)

I strongly want atomics to be available for use across inline and compiled code; i.e. it must be possible for both compiled DPDK functions and inline functions to perform atomic transactions on the same atomic variable.

So either we upgrade the DPDK build requirements to support C11 (including the optional stdatomics), or we provide our own DPDK atomics.

> >
> > >
> > > let's discuss this from here. if there's still overwhelming desire
> to go this route
> > > then we'll just do our best.
> > >
> > > ty
  

On Wed, Feb 08, 2023 at 09:31:32AM +0100, Morten Brørup wrote:
> > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com]
> > Sent: Wednesday, 8 February 2023 02.21
> > 
> > On Tue, Feb 07, 2023 at 11:34:14PM +0000, Honnappa Nagarahalli wrote:
> > > <snip>
> > >
> > > > > >
> > > > > > Honnappa, please could you give your view on the future of
> > atomics in
> > > > DPDK?
> > > > > Thanks Thomas, apologies it has taken me a while to get to this
> > discussion.
> > > > >
> > > > > IMO, we do not need DPDK's own abstractions. APIs from
> > stdatomic.h
> > > > (stdatomics as is called here) already serve the purpose. These
> > APIs are well
> > > > understood and documented.
> > > >
> > > > i agree that whatever atomics APIs we advocate for should align
> > with the
> > > > standard C atomics for the reasons you state including implied
> > semantics.
> > > Another point I want to make is, we need 'xxx_explicit' APIs only, as
> > we want memory ordering explicitly provided at each call site. (This
> > can be discussed later).
> > 
> > i don't have any issue with removing the non-explicit versions. they're
> > just just convenience for seq_cst anyway. if people don't want them we
> > don't have to have them.
> 
> I agree with Honnappa on this point.
> 
> The non-explicit versions are for lazy (or not so experienced) developers, and might impact performance if used instead of the correct explicit versions.
> 
> I'm working on porting some of our application code from DPDK's rte_atomic32 operations to modern atomics, and I'm temporarily using acq_rel with a FIXME comment on each operation until I have the overview to determine if another memory order is better for each operation. And if I don't get around to fixing the memory order, it is still a step in the right direct direction to get rid of the old __sync based atomics; and the FIXME's remain to be fixed in a later release.
> 
> So here's an idea: Alternatively to omitting the non-explicit versions, we could include them for application developers, but document them as placeholders for "memory order to be determined later" and emit a warning when used. It might speed up the transition away from old atomic operations. Alternatively, we risk thoughtless use of seq_cst with the explicit versions, which might be difficult to detect in code reviews.

i think it may be cleaner to ust remove the non-explicit versions. if we
are publishing api in the rte_xxx namespace then there are no
pre-existing expectations that they are present.

it also reduces the api surface that eventually gets retired ~years from
now when all ports and compilers in the matrix are std=C11.

i'll update the patch accordingly just so we have a visual.

>  
> Either way, with or without non-explicit versions, is fine with me.
> 
> > 
> > >
> > > >
> > > > >
> > > > > For environments where stdatomics are not supported, we could
> > have a
> > > > stdatomic.h in DPDK implementing the same APIs (we have to support
> > only
> > > > _explicit APIs). This allows the code to use stdatomics APIs and
> > when we move
> > > > to minimum supported standard C11, we just need to get rid of the
> > file in DPDK
> > > > repo.
> > > >
> > > > my concern with this is that if we provide a stdatomic.h or
> > introduce names
> > > > from stdatomic.h it's a violation of the C standard.
> > > >
> > > > references:
> > > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > > >  * GNU libc manual
> > > >    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > > > Names.html
> > > >
> > > > in effect the header, the names and in some instances namespaces
> > introduced
> > > > are reserved by the implementation. there are several reasons in
> > the GNU libc
> > > Wouldn't this apply only after the particular APIs were introduced?
> > i.e. it should not apply if the compiler does not support stdatomics.
> > 
> > yeah, i agree they're being a bit wishy washy in the wording, but i'm
> > not convinced glibc folks are documenting this as permissive guidance
> > against.
> > 
> > >
> > > > manual that explain the justification for these reservations and if
> > if we think
> > > > about ODR and ABI compatibility we can conceive of others.
> > > >
> > > > i'll also remark that the inter-mingling of names from the POSIX
> > standard
> > > > implicitly exposed as a part of the EAL public API has been
> > problematic for
> > > > portability.
> > > These should be exposed as EAL APIs only when compiled with a
> > compiler that does not support stdatomics.
> > 
> > you don't necessarily compile dpdk, the application or its other
> > dynamically linked dependencies with the same compiler at the same
> > time.
> > i.e. basically the model of any dpdk-dev package on any linux
> > distribution.
> > 
> > if dpdk is built without real stdatomic types but the application has
> > to
> > interoperate with a different kit or library that does they would be
> > forced
> > to dance around dpdk with their own version of a shim to hide our
> > faked up stdatomics.
> > 
> 
> So basically, if we want a binary DPDK distribution to be compatible with a separate application build environment, they both have to implement atomics the same way, i.e. agree on the ABI for atomics.
> 
> Summing up, this leaves us with only two realistic options:
> 
> 1. Go all in on C11 stdatomics, also requiring the application build environment to support C11 stdatomics.
> 2. Provide our own DPDK atomics library.
> 
> (As mentioned by Tyler, the third option - using C11 stdatomics inside DPDK, and requiring a build environment without C11 stdatomics to implement a shim - is not realistic!)
> 
> I strongly want atomics to be available for use across inline and compiled code; i.e. it must be possible for both compiled DPDK functions and inline functions to perform atomic transactions on the same atomic variable.

i consider it a mandatory requirement. i don't see practically how we
could withdraw existing use and even if we had clean way i don't see why
we would want to. so this item is defintely settled if you were
concerned.

> 
> So either we upgrade the DPDK build requirements to support C11 (including the optional stdatomics), or we provide our own DPDK atomics.

i think the issue of requiring a toolchain conformant to a specific
standard is a separate matter because any adoption of C11 standard
atomics is a potential abi break from the current use of intrinsics.

the abstraction (whatever namespace it resides) allows the existing
toolchain/platform combinations to maintain compatibility by defaulting
to current non-standard intrinsics.

once in place it provides an opportunity to introduce new toolchain/platform
combinations and enables an opt-in capability to use stdatomics on
existing toolchain/platform combinations subject to community discussion
on how/if/when.

it would be good to get more participants into the discussion so i'll cc
techboard for some attention. i feel like the only area that isn't
decided is to do or not do this in rte_ namespace.

i'm strongly in favor of rte_ namespace after discussion, mainly due to
to disadvantages of trying to overlap with the standard namespace while not
providing a compatible api/abi and because it provides clear
disambiguation of that difference in semantics and compatibility with
the standard api.

so far i've noted the following

* we will not provide the non-explicit apis.
* we will make no attempt to support operate on struct/union atomics
  with our apis.
* we will mirror the standard api potentially in the rte_ namespace to
  - reference the standard api documentation.
  - assume compatible semantics (sans exceptions from first 2 points).

my vote is to remove 'potentially' from the last point above for reasons
previously discussed in postings to the mail thread.

thanks all for the discussion, i'll send up a patch removing
non-explicit apis for viewing.

ty
  

<snip>

> > > > >
> > > > > >
> > > > > > For environments where stdatomics are not supported, we could
> > > have a
> > > > > stdatomic.h in DPDK implementing the same APIs (we have to
> > > > > support
> > > only
> > > > > _explicit APIs). This allows the code to use stdatomics APIs and
> > > when we move
> > > > > to minimum supported standard C11, we just need to get rid of
> > > > > the
> > > file in DPDK
> > > > > repo.
> > > > >
> > > > > my concern with this is that if we provide a stdatomic.h or
> > > introduce names
> > > > > from stdatomic.h it's a violation of the C standard.
> > > > >
> > > > > references:
> > > > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > > > >  * GNU libc manual
> > > > >    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > > > > Names.html
> > > > >
> > > > > in effect the header, the names and in some instances namespaces
> > > introduced
> > > > > are reserved by the implementation. there are several reasons in
> > > the GNU libc
> > > > Wouldn't this apply only after the particular APIs were introduced?
> > > i.e. it should not apply if the compiler does not support stdatomics.
> > >
> > > yeah, i agree they're being a bit wishy washy in the wording, but
> > > i'm not convinced glibc folks are documenting this as permissive
> > > guidance against.
> > >
> > > >
> > > > > manual that explain the justification for these reservations and
> > > > > if
> > > if we think
> > > > > about ODR and ABI compatibility we can conceive of others.
> > > > >
> > > > > i'll also remark that the inter-mingling of names from the POSIX
> > > standard
> > > > > implicitly exposed as a part of the EAL public API has been
> > > problematic for
> > > > > portability.
> > > > These should be exposed as EAL APIs only when compiled with a
> > > compiler that does not support stdatomics.
> > >
> > > you don't necessarily compile dpdk, the application or its other
> > > dynamically linked dependencies with the same compiler at the same
> > > time.
> > > i.e. basically the model of any dpdk-dev package on any linux
> > > distribution.
> > >
> > > if dpdk is built without real stdatomic types but the application
> > > has to interoperate with a different kit or library that does they
> > > would be forced to dance around dpdk with their own version of a
> > > shim to hide our faked up stdatomics.
> > >
> >
> > So basically, if we want a binary DPDK distribution to be compatible with a
> separate application build environment, they both have to implement atomics
> the same way, i.e. agree on the ABI for atomics.
> >
> > Summing up, this leaves us with only two realistic options:
> >
> > 1. Go all in on C11 stdatomics, also requiring the application build
> environment to support C11 stdatomics.
> > 2. Provide our own DPDK atomics library.
> >
> > (As mentioned by Tyler, the third option - using C11 stdatomics inside
> > DPDK, and requiring a build environment without C11 stdatomics to
> > implement a shim - is not realistic!)
> >
> > I strongly want atomics to be available for use across inline and compiled
> code; i.e. it must be possible for both compiled DPDK functions and inline
> functions to perform atomic transactions on the same atomic variable.
> 
> i consider it a mandatory requirement. i don't see practically how we could
> withdraw existing use and even if we had clean way i don't see why we would
> want to. so this item is defintely settled if you were concerned.
I think I agree here.

> 
> >
> > So either we upgrade the DPDK build requirements to support C11 (including
> the optional stdatomics), or we provide our own DPDK atomics.
> 
> i think the issue of requiring a toolchain conformant to a specific standard is a
> separate matter because any adoption of C11 standard atomics is a potential
> abi break from the current use of intrinsics.
I am not sure why you are calling it as ABI break. Referring to [1], I just see wrappers around intrinsics (though [2] does not use the intrinsics).

[1] https://github.com/gcc-mirror/gcc/blob/master/gcc/ginclude/stdatomic.h
[2] https://github.com/llvm-mirror/clang/blob/master/lib/Headers/stdatomic.h

> 
> the abstraction (whatever namespace it resides) allows the existing
> toolchain/platform combinations to maintain compatibility by defaulting to
> current non-standard intrinsics.
How about using the intrinsics (__atomic_xxx) name space for abstraction? This covers the GCC and Clang compilers.
If there is another platform that uses the same name space for something else, I think DPDK should not be supporting that platform.
What problems do you see?

> 
> once in place it provides an opportunity to introduce new toolchain/platform
> combinations and enables an opt-in capability to use stdatomics on existing
> toolchain/platform combinations subject to community discussion on
> how/if/when.
> 
> it would be good to get more participants into the discussion so i'll cc techboard
> for some attention. i feel like the only area that isn't decided is to do or not do
> this in rte_ namespace.
> 
> i'm strongly in favor of rte_ namespace after discussion, mainly due to to
> disadvantages of trying to overlap with the standard namespace while not
> providing a compatible api/abi and because it provides clear disambiguation of
> that difference in semantics and compatibility with the standard api.
> 
> so far i've noted the following
> 
> * we will not provide the non-explicit apis.
+1

> * we will make no attempt to support operate on struct/union atomics
>   with our apis.
+1

> * we will mirror the standard api potentially in the rte_ namespace to
>   - reference the standard api documentation.
>   - assume compatible semantics (sans exceptions from first 2 points).
> 
> my vote is to remove 'potentially' from the last point above for reasons
> previously discussed in postings to the mail thread.
> 
> thanks all for the discussion, i'll send up a patch removing non-explicit apis for
> viewing.
> 
> ty
  

> From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli@arm.com]
> Sent: Thursday, 9 February 2023 01.17
> 
> <snip>
> 
> > > > > >
> > > > > > >
> > > > > > > For environments where stdatomics are not supported, we
> could
> > > > have a
> > > > > > stdatomic.h in DPDK implementing the same APIs (we have to
> > > > > > support
> > > > only
> > > > > > _explicit APIs). This allows the code to use stdatomics APIs
> and
> > > > when we move
> > > > > > to minimum supported standard C11, we just need to get rid of
> > > > > > the
> > > > file in DPDK
> > > > > > repo.
> > > > > >
> > > > > > my concern with this is that if we provide a stdatomic.h or
> > > > introduce names
> > > > > > from stdatomic.h it's a violation of the C standard.
> > > > > >
> > > > > > references:
> > > > > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > > > > >  * GNU libc manual
> > > > > >
> https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > > > > > Names.html
> > > > > >
> > > > > > in effect the header, the names and in some instances
> namespaces
> > > > introduced
> > > > > > are reserved by the implementation. there are several reasons
> in
> > > > the GNU libc
> > > > > Wouldn't this apply only after the particular APIs were
> introduced?
> > > > i.e. it should not apply if the compiler does not support
> stdatomics.
> > > >
> > > > yeah, i agree they're being a bit wishy washy in the wording, but
> > > > i'm not convinced glibc folks are documenting this as permissive
> > > > guidance against.
> > > >
> > > > >
> > > > > > manual that explain the justification for these reservations
> and
> > > > > > if
> > > > if we think
> > > > > > about ODR and ABI compatibility we can conceive of others.
> > > > > >
> > > > > > i'll also remark that the inter-mingling of names from the
> POSIX
> > > > standard
> > > > > > implicitly exposed as a part of the EAL public API has been
> > > > problematic for
> > > > > > portability.
> > > > > These should be exposed as EAL APIs only when compiled with a
> > > > compiler that does not support stdatomics.
> > > >
> > > > you don't necessarily compile dpdk, the application or its other
> > > > dynamically linked dependencies with the same compiler at the
> same
> > > > time.
> > > > i.e. basically the model of any dpdk-dev package on any linux
> > > > distribution.
> > > >
> > > > if dpdk is built without real stdatomic types but the application
> > > > has to interoperate with a different kit or library that does
> they
> > > > would be forced to dance around dpdk with their own version of a
> > > > shim to hide our faked up stdatomics.
> > > >
> > >
> > > So basically, if we want a binary DPDK distribution to be
> compatible with a
> > separate application build environment, they both have to implement
> atomics
> > the same way, i.e. agree on the ABI for atomics.
> > >
> > > Summing up, this leaves us with only two realistic options:
> > >
> > > 1. Go all in on C11 stdatomics, also requiring the application
> build
> > environment to support C11 stdatomics.
> > > 2. Provide our own DPDK atomics library.
> > >
> > > (As mentioned by Tyler, the third option - using C11 stdatomics
> inside
> > > DPDK, and requiring a build environment without C11 stdatomics to
> > > implement a shim - is not realistic!)
> > >
> > > I strongly want atomics to be available for use across inline and
> compiled
> > code; i.e. it must be possible for both compiled DPDK functions and
> inline
> > functions to perform atomic transactions on the same atomic variable.
> >
> > i consider it a mandatory requirement. i don't see practically how we
> could
> > withdraw existing use and even if we had clean way i don't see why we
> would
> > want to. so this item is defintely settled if you were concerned.
> I think I agree here.
> 
> >
> > >
> > > So either we upgrade the DPDK build requirements to support C11
> (including
> > the optional stdatomics), or we provide our own DPDK atomics.
> >
> > i think the issue of requiring a toolchain conformant to a specific
> standard is a
> > separate matter because any adoption of C11 standard atomics is a
> potential
> > abi break from the current use of intrinsics.
> I am not sure why you are calling it as ABI break. Referring to [1], I
> just see wrappers around intrinsics (though [2] does not use the
> intrinsics).
> 
> [1] https://github.com/gcc-
> mirror/gcc/blob/master/gcc/ginclude/stdatomic.h
> [2] https://github.com/llvm-
> mirror/clang/blob/master/lib/Headers/stdatomic.h

Good input, Honnappa.

This means that the ABI break is purely academic, and there is no ABI breakage in reality.

Since the underlying implementation is the same, it is perfectly OK to mix C11 and intrinsic atomics, even when the DPDK and the application are built in different environments (with and without C11 atomics, or vice versa).

This eliminates my only remaining practical concern about this approach.

> 
> >
> > the abstraction (whatever namespace it resides) allows the existing
> > toolchain/platform combinations to maintain compatibility by
> defaulting to
> > current non-standard intrinsics.
> How about using the intrinsics (__atomic_xxx) name space for
> abstraction? This covers the GCC and Clang compilers.
> If there is another platform that uses the same name space for
> something else, I think DPDK should not be supporting that platform.
> What problems do you see?
> 
> >
> > once in place it provides an opportunity to introduce new
> toolchain/platform
> > combinations and enables an opt-in capability to use stdatomics on
> existing
> > toolchain/platform combinations subject to community discussion on
> > how/if/when.
> >
> > it would be good to get more participants into the discussion so i'll
> cc techboard
> > for some attention. i feel like the only area that isn't decided is
> to do or not do
> > this in rte_ namespace.
> >
> > i'm strongly in favor of rte_ namespace after discussion, mainly due
> to to
> > disadvantages of trying to overlap with the standard namespace while
> not
> > providing a compatible api/abi and because it provides clear
> disambiguation of
> > that difference in semantics and compatibility with the standard api.
> >
> > so far i've noted the following
> >
> > * we will not provide the non-explicit apis.
> +1
> 
> > * we will make no attempt to support operate on struct/union atomics
> >   with our apis.
> +1
> 
> > * we will mirror the standard api potentially in the rte_ namespace
> to
> >   - reference the standard api documentation.
> >   - assume compatible semantics (sans exceptions from first 2
> points).
> >
> > my vote is to remove 'potentially' from the last point above for
> reasons
> > previously discussed in postings to the mail thread.
> >
> > thanks all for the discussion, i'll send up a patch removing non-
> explicit apis for
> > viewing.
> >
> > ty
  

On Thu, Feb 09, 2023 at 12:16:38AM +0000, Honnappa Nagarahalli wrote:
> <snip>
> 
> > > > > >
> > > > > > >
> > > > > > > For environments where stdatomics are not supported, we could
> > > > have a
> > > > > > stdatomic.h in DPDK implementing the same APIs (we have to
> > > > > > support
> > > > only
> > > > > > _explicit APIs). This allows the code to use stdatomics APIs and
> > > > when we move
> > > > > > to minimum supported standard C11, we just need to get rid of
> > > > > > the
> > > > file in DPDK
> > > > > > repo.
> > > > > >
> > > > > > my concern with this is that if we provide a stdatomic.h or
> > > > introduce names
> > > > > > from stdatomic.h it's a violation of the C standard.
> > > > > >
> > > > > > references:
> > > > > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > > > > >  * GNU libc manual
> > > > > >    https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > > > > > Names.html
> > > > > >
> > > > > > in effect the header, the names and in some instances namespaces
> > > > introduced
> > > > > > are reserved by the implementation. there are several reasons in
> > > > the GNU libc
> > > > > Wouldn't this apply only after the particular APIs were introduced?
> > > > i.e. it should not apply if the compiler does not support stdatomics.
> > > >
> > > > yeah, i agree they're being a bit wishy washy in the wording, but
> > > > i'm not convinced glibc folks are documenting this as permissive
> > > > guidance against.
> > > >
> > > > >
> > > > > > manual that explain the justification for these reservations and
> > > > > > if
> > > > if we think
> > > > > > about ODR and ABI compatibility we can conceive of others.
> > > > > >
> > > > > > i'll also remark that the inter-mingling of names from the POSIX
> > > > standard
> > > > > > implicitly exposed as a part of the EAL public API has been
> > > > problematic for
> > > > > > portability.
> > > > > These should be exposed as EAL APIs only when compiled with a
> > > > compiler that does not support stdatomics.
> > > >
> > > > you don't necessarily compile dpdk, the application or its other
> > > > dynamically linked dependencies with the same compiler at the same
> > > > time.
> > > > i.e. basically the model of any dpdk-dev package on any linux
> > > > distribution.
> > > >
> > > > if dpdk is built without real stdatomic types but the application
> > > > has to interoperate with a different kit or library that does they
> > > > would be forced to dance around dpdk with their own version of a
> > > > shim to hide our faked up stdatomics.
> > > >
> > >
> > > So basically, if we want a binary DPDK distribution to be compatible with a
> > separate application build environment, they both have to implement atomics
> > the same way, i.e. agree on the ABI for atomics.
> > >
> > > Summing up, this leaves us with only two realistic options:
> > >
> > > 1. Go all in on C11 stdatomics, also requiring the application build
> > environment to support C11 stdatomics.
> > > 2. Provide our own DPDK atomics library.
> > >
> > > (As mentioned by Tyler, the third option - using C11 stdatomics inside
> > > DPDK, and requiring a build environment without C11 stdatomics to
> > > implement a shim - is not realistic!)
> > >
> > > I strongly want atomics to be available for use across inline and compiled
> > code; i.e. it must be possible for both compiled DPDK functions and inline
> > functions to perform atomic transactions on the same atomic variable.
> > 
> > i consider it a mandatory requirement. i don't see practically how we could
> > withdraw existing use and even if we had clean way i don't see why we would
> > want to. so this item is defintely settled if you were concerned.
> I think I agree here.
> 
> > 
> > >
> > > So either we upgrade the DPDK build requirements to support C11 (including
> > the optional stdatomics), or we provide our own DPDK atomics.
> > 
> > i think the issue of requiring a toolchain conformant to a specific standard is a
> > separate matter because any adoption of C11 standard atomics is a potential
> > abi break from the current use of intrinsics.
> I am not sure why you are calling it as ABI break. Referring to [1], I just see wrappers around intrinsics (though [2] does not use the intrinsics).
> 
> [1] https://github.com/gcc-mirror/gcc/blob/master/gcc/ginclude/stdatomic.h
> [2] https://github.com/llvm-mirror/clang/blob/master/lib/Headers/stdatomic.h

it's a potential abi break because atomic types are not the same types as
their corresponding integer types etc.. (or at least are not guaranteed to
be by all implementations of c as an abstract language).

    ISO/IEC 9899:2011

    6.2.5 (27)
    Further, there is the _Atomic qualifier. The presence of the _Atomic
    qualifier designates an atomic type. The size, representation, and alignment
    of an atomic type need not be the same as those of the corresponding
    unqualified type.

    7.17.6 (3)
    NOTE The representation of atomic integer types need not have the same size
    as their corresponding regular types. They should have the same size whenever
    possible, as it eases effort required to port existing code.

i use the term `potential abi break' with intent because for me to assert
in absolute terms i would have to evaluate the implementation of every
current and potential future compilers atomic vs non-atomic types. this
as i'm sure you understand is not practical, it would also defeat the
purpose of moving to a standard. therefore i rely on the specification
prescribed by the standard not the detail of a specific implementation.


> > the abstraction (whatever namespace it resides) allows the existing
> > toolchain/platform combinations to maintain compatibility by defaulting to
> > current non-standard intrinsics.
> How about using the intrinsics (__atomic_xxx) name space for abstraction? This covers the GCC and Clang compilers.

the namespace starting with `__` is also reserved for the implementation.
this is why compilers gcc/clang/msvc place name their intrinsic and
builtin functions starting with __ to explicitly avoid collision with the
application namespace.

    ISO/IEC 9899:2011

    7.1.3 (1)
    All identifiers that begin with an underscore and either an uppercase
    letter or another underscore are always reserved for any use.

    ...

> If there is another platform that uses the same name space for something else, I think DPDK should not be supporting that platform.

that's effectively a statement excluding windows platform and all
non-gcc compilers from ever supporting dpdk.

> What problems do you see?

i'm fairly certain at least one other compiler uses the __atomic
namespace but it would take me time to check, the most notable potential
issue that comes to mind is if such an intrinsic with the same name is
provided in a different implementation and has either regressive code
generation or different semantics it would be bad because it is
intrinsic you can't just hack around it with #undef __atomic to shim in
a semantically correct version.

how about this, is there another possible namespace you might suggest
that conforms or doesn't conflict with the the rules defined in
ISO/IEC 9899:2011 7.1.3 i think if there were that would satisfy all of
my concerns related to namespaces.

keep in mind the point of moving to a standard is to achieve portability
so if we do things that will regress us back to being dependent on an
implementation we haven't succeeded. that's all i'm trying to guarantee
here.

i feel like we are really close on this discussion, if we can just iron
this issue out we can probably get going on the actual changes.

thanks for the consideration.

> 
> > 
> > once in place it provides an opportunity to introduce new toolchain/platform
> > combinations and enables an opt-in capability to use stdatomics on existing
> > toolchain/platform combinations subject to community discussion on
> > how/if/when.
> > 
> > it would be good to get more participants into the discussion so i'll cc techboard
> > for some attention. i feel like the only area that isn't decided is to do or not do
> > this in rte_ namespace.
> > 
> > i'm strongly in favor of rte_ namespace after discussion, mainly due to to
> > disadvantages of trying to overlap with the standard namespace while not
> > providing a compatible api/abi and because it provides clear disambiguation of
> > that difference in semantics and compatibility with the standard api.
> > 
> > so far i've noted the following
> > 
> > * we will not provide the non-explicit apis.
> +1
> 
> > * we will make no attempt to support operate on struct/union atomics
> >   with our apis.
> +1
> 
> > * we will mirror the standard api potentially in the rte_ namespace to
> >   - reference the standard api documentation.
> >   - assume compatible semantics (sans exceptions from first 2 points).
> > 
> > my vote is to remove 'potentially' from the last point above for reasons
> > previously discussed in postings to the mail thread.
> > 
> > thanks all for the discussion, i'll send up a patch removing non-explicit apis for
> > viewing.
> > 
> > ty
  

<snip>

> On Thu, Feb 09, 2023 at 12:16:38AM +0000, Honnappa Nagarahalli wrote:
> > <snip>
> >
> > > > > > >
> > > > > > > >
> > > > > > > > For environments where stdatomics are not supported, we
> > > > > > > > could
> > > > > have a
> > > > > > > stdatomic.h in DPDK implementing the same APIs (we have to
> > > > > > > support
> > > > > only
> > > > > > > _explicit APIs). This allows the code to use stdatomics APIs
> > > > > > > and
> > > > > when we move
> > > > > > > to minimum supported standard C11, we just need to get rid
> > > > > > > of the
> > > > > file in DPDK
> > > > > > > repo.
> > > > > > >
> > > > > > > my concern with this is that if we provide a stdatomic.h or
> > > > > introduce names
> > > > > > > from stdatomic.h it's a violation of the C standard.
> > > > > > >
> > > > > > > references:
> > > > > > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > > > > > >  * GNU libc manual
> > > > > > >
> > > > > > > https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > > > > > > Names.html
> > > > > > >
> > > > > > > in effect the header, the names and in some instances
> > > > > > > namespaces
> > > > > introduced
> > > > > > > are reserved by the implementation. there are several
> > > > > > > reasons in
> > > > > the GNU libc
> > > > > > Wouldn't this apply only after the particular APIs were introduced?
> > > > > i.e. it should not apply if the compiler does not support stdatomics.
> > > > >
> > > > > yeah, i agree they're being a bit wishy washy in the wording,
> > > > > but i'm not convinced glibc folks are documenting this as
> > > > > permissive guidance against.
> > > > >
> > > > > >
> > > > > > > manual that explain the justification for these reservations
> > > > > > > and if
> > > > > if we think
> > > > > > > about ODR and ABI compatibility we can conceive of others.
> > > > > > >
> > > > > > > i'll also remark that the inter-mingling of names from the
> > > > > > > POSIX
> > > > > standard
> > > > > > > implicitly exposed as a part of the EAL public API has been
> > > > > problematic for
> > > > > > > portability.
> > > > > > These should be exposed as EAL APIs only when compiled with a
> > > > > compiler that does not support stdatomics.
> > > > >
> > > > > you don't necessarily compile dpdk, the application or its other
> > > > > dynamically linked dependencies with the same compiler at the
> > > > > same time.
> > > > > i.e. basically the model of any dpdk-dev package on any linux
> > > > > distribution.
> > > > >
> > > > > if dpdk is built without real stdatomic types but the
> > > > > application has to interoperate with a different kit or library
> > > > > that does they would be forced to dance around dpdk with their
> > > > > own version of a shim to hide our faked up stdatomics.
> > > > >
> > > >
> > > > So basically, if we want a binary DPDK distribution to be
> > > > compatible with a
> > > separate application build environment, they both have to implement
> > > atomics the same way, i.e. agree on the ABI for atomics.
> > > >
> > > > Summing up, this leaves us with only two realistic options:
> > > >
> > > > 1. Go all in on C11 stdatomics, also requiring the application
> > > > build
> > > environment to support C11 stdatomics.
> > > > 2. Provide our own DPDK atomics library.
> > > >
> > > > (As mentioned by Tyler, the third option - using C11 stdatomics
> > > > inside DPDK, and requiring a build environment without C11
> > > > stdatomics to implement a shim - is not realistic!)
> > > >
> > > > I strongly want atomics to be available for use across inline and
> > > > compiled
> > > code; i.e. it must be possible for both compiled DPDK functions and
> > > inline functions to perform atomic transactions on the same atomic variable.
> > >
> > > i consider it a mandatory requirement. i don't see practically how
> > > we could withdraw existing use and even if we had clean way i don't
> > > see why we would want to. so this item is defintely settled if you were
> concerned.
> > I think I agree here.
> >
> > >
> > > >
> > > > So either we upgrade the DPDK build requirements to support C11
> > > > (including
> > > the optional stdatomics), or we provide our own DPDK atomics.
> > >
> > > i think the issue of requiring a toolchain conformant to a specific
> > > standard is a separate matter because any adoption of C11 standard
> > > atomics is a potential abi break from the current use of intrinsics.
> > I am not sure why you are calling it as ABI break. Referring to [1], I just see
> wrappers around intrinsics (though [2] does not use the intrinsics).
> >
> > [1]
> > https://github.com/gcc-mirror/gcc/blob/master/gcc/ginclude/stdatomic.h
> > [2]
> > https://github.com/llvm-mirror/clang/blob/master/lib/Headers/stdatomic
> > .h
> 
> it's a potential abi break because atomic types are not the same types as their
> corresponding integer types etc.. (or at least are not guaranteed to be by all
> implementations of c as an abstract language).
> 
>     ISO/IEC 9899:2011
> 
>     6.2.5 (27)
>     Further, there is the _Atomic qualifier. The presence of the _Atomic
>     qualifier designates an atomic type. The size, representation, and alignment
>     of an atomic type need not be the same as those of the corresponding
>     unqualified type.
> 
>     7.17.6 (3)
>     NOTE The representation of atomic integer types need not have the same size
>     as their corresponding regular types. They should have the same size
> whenever
>     possible, as it eases effort required to port existing code.
> 
> i use the term `potential abi break' with intent because for me to assert in
> absolute terms i would have to evaluate the implementation of every current
> and potential future compilers atomic vs non-atomic types. this as i'm sure you
> understand is not practical, it would also defeat the purpose of moving to a
> standard. therefore i rely on the specification prescribed by the standard not
> the detail of a specific implementation.
Can we say that the platforms 'supported' by DPDK today do not have this problem? Any future platforms that will come to DPDK have to evaluate this.

> 
> 
> > > the abstraction (whatever namespace it resides) allows the existing
> > > toolchain/platform combinations to maintain compatibility by
> > > defaulting to current non-standard intrinsics.
> > How about using the intrinsics (__atomic_xxx) name space for abstraction?
> This covers the GCC and Clang compilers.
> 
> the namespace starting with `__` is also reserved for the implementation.
> this is why compilers gcc/clang/msvc place name their intrinsic and builtin
> functions starting with __ to explicitly avoid collision with the application
> namespace.
Agreed. But, here we are considering '__atomic_' specifically (i.e. not just '__')

> 
>     ISO/IEC 9899:2011
> 
>     7.1.3 (1)
>     All identifiers that begin with an underscore and either an uppercase
>     letter or another underscore are always reserved for any use.
> 
>     ...
> 
> > If there is another platform that uses the same name space for something
> else, I think DPDK should not be supporting that platform.
> 
> that's effectively a statement excluding windows platform and all non-gcc
> compilers from ever supporting dpdk.
Apologies, I did not understand your comment on windows platform. Do you mean to say a compiler for windows platform uses '__atomic_xxx' name space to provide some other functionality (and hence it would get excluded)? 
Clang supports these intrinsics. I am not sure about the merit of supporting other non-gcc compilers. May be a topic Techboard discussion.

> 
> > What problems do you see?
> 
> i'm fairly certain at least one other compiler uses the __atomic namespace but
Do you mean __atomic namespace is used for some other purpose?

> it would take me time to check, the most notable potential issue that comes to
> mind is if such an intrinsic with the same name is provided in a different
> implementation and has either regressive code generation or different
> semantics it would be bad because it is intrinsic you can't just hack around it
> with #undef __atomic to shim in a semantically correct version.
I do not think we should worry about regressive code generation problem. It should be fixed by that compiler.
Different semantics is something we need to worry about. It would be good to find out more about a compiler that does this.

> 
> how about this, is there another possible namespace you might suggest that
> conforms or doesn't conflict with the the rules defined in ISO/IEC 9899:2011
> 7.1.3 i think if there were that would satisfy all of my concerns related to
> namespaces.
> 
> keep in mind the point of moving to a standard is to achieve portability so if we
> do things that will regress us back to being dependent on an implementation
> we haven't succeeded. that's all i'm trying to guarantee here.
Agree. We are trying to solve a problem that is temporary. I am trying to keep the problem scope narrow which might help us push to adopt the standard sooner.

> 
> i feel like we are really close on this discussion, if we can just iron this issue out
> we can probably get going on the actual changes.
> 
> thanks for the consideration.
> 
> >
> > >
> > > once in place it provides an opportunity to introduce new
> > > toolchain/platform combinations and enables an opt-in capability to
> > > use stdatomics on existing toolchain/platform combinations subject
> > > to community discussion on how/if/when.
> > >
> > > it would be good to get more participants into the discussion so
> > > i'll cc techboard for some attention. i feel like the only area that
> > > isn't decided is to do or not do this in rte_ namespace.
> > >
> > > i'm strongly in favor of rte_ namespace after discussion, mainly due
> > > to to disadvantages of trying to overlap with the standard namespace
> > > while not providing a compatible api/abi and because it provides
> > > clear disambiguation of that difference in semantics and compatibility with
> the standard api.
> > >
> > > so far i've noted the following
> > >
> > > * we will not provide the non-explicit apis.
> > +1
> >
> > > * we will make no attempt to support operate on struct/union atomics
> > >   with our apis.
> > +1
> >
> > > * we will mirror the standard api potentially in the rte_ namespace to
> > >   - reference the standard api documentation.
> > >   - assume compatible semantics (sans exceptions from first 2 points).
> > >
> > > my vote is to remove 'potentially' from the last point above for
> > > reasons previously discussed in postings to the mail thread.
> > >
> > > thanks all for the discussion, i'll send up a patch removing
> > > non-explicit apis for viewing.
> > >
> > > ty
  

On Fri, Feb 10, 2023 at 05:30:00AM +0000, Honnappa Nagarahalli wrote:
> <snip>
> 
> > On Thu, Feb 09, 2023 at 12:16:38AM +0000, Honnappa Nagarahalli wrote:
> > > <snip>
> > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > For environments where stdatomics are not supported, we
> > > > > > > > > could
> > > > > > have a
> > > > > > > > stdatomic.h in DPDK implementing the same APIs (we have to
> > > > > > > > support
> > > > > > only
> > > > > > > > _explicit APIs). This allows the code to use stdatomics APIs
> > > > > > > > and
> > > > > > when we move
> > > > > > > > to minimum supported standard C11, we just need to get rid
> > > > > > > > of the
> > > > > > file in DPDK
> > > > > > > > repo.
> > > > > > > >
> > > > > > > > my concern with this is that if we provide a stdatomic.h or
> > > > > > introduce names
> > > > > > > > from stdatomic.h it's a violation of the C standard.
> > > > > > > >
> > > > > > > > references:
> > > > > > > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > > > > > > >  * GNU libc manual
> > > > > > > >
> > > > > > > > https://www.gnu.org/software/libc/manual/html_node/Reserved-
> > > > > > > > Names.html
> > > > > > > >
> > > > > > > > in effect the header, the names and in some instances
> > > > > > > > namespaces
> > > > > > introduced
> > > > > > > > are reserved by the implementation. there are several
> > > > > > > > reasons in
> > > > > > the GNU libc
> > > > > > > Wouldn't this apply only after the particular APIs were introduced?
> > > > > > i.e. it should not apply if the compiler does not support stdatomics.
> > > > > >
> > > > > > yeah, i agree they're being a bit wishy washy in the wording,
> > > > > > but i'm not convinced glibc folks are documenting this as
> > > > > > permissive guidance against.
> > > > > >
> > > > > > >
> > > > > > > > manual that explain the justification for these reservations
> > > > > > > > and if
> > > > > > if we think
> > > > > > > > about ODR and ABI compatibility we can conceive of others.
> > > > > > > >
> > > > > > > > i'll also remark that the inter-mingling of names from the
> > > > > > > > POSIX
> > > > > > standard
> > > > > > > > implicitly exposed as a part of the EAL public API has been
> > > > > > problematic for
> > > > > > > > portability.
> > > > > > > These should be exposed as EAL APIs only when compiled with a
> > > > > > compiler that does not support stdatomics.
> > > > > >
> > > > > > you don't necessarily compile dpdk, the application or its other
> > > > > > dynamically linked dependencies with the same compiler at the
> > > > > > same time.
> > > > > > i.e. basically the model of any dpdk-dev package on any linux
> > > > > > distribution.
> > > > > >
> > > > > > if dpdk is built without real stdatomic types but the
> > > > > > application has to interoperate with a different kit or library
> > > > > > that does they would be forced to dance around dpdk with their
> > > > > > own version of a shim to hide our faked up stdatomics.
> > > > > >
> > > > >
> > > > > So basically, if we want a binary DPDK distribution to be
> > > > > compatible with a
> > > > separate application build environment, they both have to implement
> > > > atomics the same way, i.e. agree on the ABI for atomics.
> > > > >
> > > > > Summing up, this leaves us with only two realistic options:
> > > > >
> > > > > 1. Go all in on C11 stdatomics, also requiring the application
> > > > > build
> > > > environment to support C11 stdatomics.
> > > > > 2. Provide our own DPDK atomics library.
> > > > >
> > > > > (As mentioned by Tyler, the third option - using C11 stdatomics
> > > > > inside DPDK, and requiring a build environment without C11
> > > > > stdatomics to implement a shim - is not realistic!)
> > > > >
> > > > > I strongly want atomics to be available for use across inline and
> > > > > compiled
> > > > code; i.e. it must be possible for both compiled DPDK functions and
> > > > inline functions to perform atomic transactions on the same atomic variable.
> > > >
> > > > i consider it a mandatory requirement. i don't see practically how
> > > > we could withdraw existing use and even if we had clean way i don't
> > > > see why we would want to. so this item is defintely settled if you were
> > concerned.
> > > I think I agree here.
> > >
> > > >
> > > > >
> > > > > So either we upgrade the DPDK build requirements to support C11
> > > > > (including
> > > > the optional stdatomics), or we provide our own DPDK atomics.
> > > >
> > > > i think the issue of requiring a toolchain conformant to a specific
> > > > standard is a separate matter because any adoption of C11 standard
> > > > atomics is a potential abi break from the current use of intrinsics.
> > > I am not sure why you are calling it as ABI break. Referring to [1], I just see
> > wrappers around intrinsics (though [2] does not use the intrinsics).
> > >
> > > [1]
> > > https://github.com/gcc-mirror/gcc/blob/master/gcc/ginclude/stdatomic.h
> > > [2]
> > > https://github.com/llvm-mirror/clang/blob/master/lib/Headers/stdatomic
> > > .h
> > 
> > it's a potential abi break because atomic types are not the same types as their
> > corresponding integer types etc.. (or at least are not guaranteed to be by all
> > implementations of c as an abstract language).
> > 
> >     ISO/IEC 9899:2011
> > 
> >     6.2.5 (27)
> >     Further, there is the _Atomic qualifier. The presence of the _Atomic
> >     qualifier designates an atomic type. The size, representation, and alignment
> >     of an atomic type need not be the same as those of the corresponding
> >     unqualified type.
> > 
> >     7.17.6 (3)
> >     NOTE The representation of atomic integer types need not have the same size
> >     as their corresponding regular types. They should have the same size
> > whenever
> >     possible, as it eases effort required to port existing code.
> > 
> > i use the term `potential abi break' with intent because for me to assert in
> > absolute terms i would have to evaluate the implementation of every current
> > and potential future compilers atomic vs non-atomic types. this as i'm sure you
> > understand is not practical, it would also defeat the purpose of moving to a
> > standard. therefore i rely on the specification prescribed by the standard not
> > the detail of a specific implementation.
> Can we say that the platforms 'supported' by DPDK today do not have this problem? Any future platforms that will come to DPDK have to evaluate this.

sadly i don't think we can. i believe in an earlier post i linked a bug
filed on gcc that shows that clang / gcc were producing different
layout than the equivalent non-atomic type.

> 
> > 
> > 
> > > > the abstraction (whatever namespace it resides) allows the existing
> > > > toolchain/platform combinations to maintain compatibility by
> > > > defaulting to current non-standard intrinsics.
> > > How about using the intrinsics (__atomic_xxx) name space for abstraction?
> > This covers the GCC and Clang compilers.

i haven't investigated fully but there are usages of these intrinsics
that indicate there may be undesirable difference between clang and gcc
versions. the hint is there seems to be conditionally compiled code
under __clang__ when using some __atomic's.

for the purpose of this discussion clang just tries to look like gcc so
i don't regard them as being different compilers for the purpose of this
discussion.

> > 
> > the namespace starting with `__` is also reserved for the implementation.
> > this is why compilers gcc/clang/msvc place name their intrinsic and builtin
> > functions starting with __ to explicitly avoid collision with the application
> > namespace.

> Agreed. But, here we are considering '__atomic_' specifically (i.e. not just '__')

i don't understand the confusion __atomic is within the __ namespace
that is reserved.

let me ask this another way, what benefit do you see to trying to
overlap with the standard namespace? the only benefit i can see is that
at some point in the future it avoids having to perform a mechanical
change to eventually retire the abstraction once all platform/toolchains
support standard atomics. i.e. basically s/rte_atomic/atomic/g

is there another benefit i'm missing?

> 
> > 
> >     ISO/IEC 9899:2011
> > 
> >     7.1.3 (1)
> >     All identifiers that begin with an underscore and either an uppercase
> >     letter or another underscore are always reserved for any use.
> > 
> >     ...
> > 
> > > If there is another platform that uses the same name space for something
> > else, I think DPDK should not be supporting that platform.
> > 
> > that's effectively a statement excluding windows platform and all non-gcc
> > compilers from ever supporting dpdk.
> Apologies, I did not understand your comment on windows platform. Do you mean to say a compiler for windows platform uses '__atomic_xxx' name space to provide some other functionality (and hence it would get excluded)? 

i mean dpdk can never fully be supported without msvc except for
statically linked builds which are niche and limit it too severely for
many consumers to practically use dpdk. there are also many application
developers who would like to integrate dpdk but can't and telling them
their only choice is to re-port their entire application to clang isn't
feasible.

i can see no technical reason why we should be excluding a major
compiler in broad use if it is capable of building dpdk. msvc arguably
has some of the most sophisticated security features in the industry
and the use of those features is mandated by many of the customers who
might deploy dpdk applications on windows.

> Clang supports these intrinsics. I am not sure about the merit of supporting other non-gcc compilers. May be a topic Techboard discussion.
> 
> > 
> > > What problems do you see?
> > 
> > i'm fairly certain at least one other compiler uses the __atomic namespace but
> Do you mean __atomic namespace is used for some other purpose?
> 
> > it would take me time to check, the most notable potential issue that comes to
> > mind is if such an intrinsic with the same name is provided in a different
> > implementation and has either regressive code generation or different
> > semantics it would be bad because it is intrinsic you can't just hack around it
> > with #undef __atomic to shim in a semantically correct version.
> I do not think we should worry about regressive code generation problem. It should be fixed by that compiler.
> Different semantics is something we need to worry about. It would be good to find out more about a compiler that does this.

again, this is about portability it's about potential not that we can
find an example.

> 
> > 
> > how about this, is there another possible namespace you might suggest that
> > conforms or doesn't conflict with the the rules defined in ISO/IEC 9899:2011
> > 7.1.3 i think if there were that would satisfy all of my concerns related to
> > namespaces.
> > 
> > keep in mind the point of moving to a standard is to achieve portability so if we
> > do things that will regress us back to being dependent on an implementation
> > we haven't succeeded. that's all i'm trying to guarantee here.
> Agree. We are trying to solve a problem that is temporary. I am trying to keep the problem scope narrow which might help us push to adopt the standard sooner.

i do wish we could just target the standard but unless we are willing to
draw a line and say no more non std=c11 and also we potentially break
the abi we are talking years. i don't think it is reasonable to block
progress for years, so i'm offering a transitional path. it's an
evolution over time that we have to manage.

> 
> > 
> > i feel like we are really close on this discussion, if we can just iron this issue out
> > we can probably get going on the actual changes.
> > 
> > thanks for the consideration.
> > 
> > >
> > > >
> > > > once in place it provides an opportunity to introduce new
> > > > toolchain/platform combinations and enables an opt-in capability to
> > > > use stdatomics on existing toolchain/platform combinations subject
> > > > to community discussion on how/if/when.
> > > >
> > > > it would be good to get more participants into the discussion so
> > > > i'll cc techboard for some attention. i feel like the only area that
> > > > isn't decided is to do or not do this in rte_ namespace.
> > > >
> > > > i'm strongly in favor of rte_ namespace after discussion, mainly due
> > > > to to disadvantages of trying to overlap with the standard namespace
> > > > while not providing a compatible api/abi and because it provides
> > > > clear disambiguation of that difference in semantics and compatibility with
> > the standard api.
> > > >
> > > > so far i've noted the following
> > > >
> > > > * we will not provide the non-explicit apis.
> > > +1
> > >
> > > > * we will make no attempt to support operate on struct/union atomics
> > > >   with our apis.
> > > +1
> > >
> > > > * we will mirror the standard api potentially in the rte_ namespace to
> > > >   - reference the standard api documentation.
> > > >   - assume compatible semantics (sans exceptions from first 2 points).
> > > >
> > > > my vote is to remove 'potentially' from the last point above for
> > > > reasons previously discussed in postings to the mail thread.
> > > >
> > > > thanks all for the discussion, i'll send up a patch removing
> > > > non-explicit apis for viewing.
> > > >
> > > > ty
  

Hi Tyler,
	Few more comments inline. Let us continue to make progress, I will add this topic for Techboard discussion for 22nd Feb.

> -----Original Message-----
> From: Tyler Retzlaff <roretzla@linux.microsoft.com>
> Sent: Friday, February 10, 2023 2:30 PM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Cc: Morten Brørup <mb@smartsharesystems.com>; thomas@monjalon.net;
> dev@dpdk.org; bruce.richardson@intel.com; david.marchand@redhat.com;
> jerinj@marvell.com; konstantin.ananyev@huawei.com;
> ferruh.yigit@amd.com; nd <nd@arm.com>; techboard@dpdk.org
> Subject: Re: [PATCH] eal: introduce atomics abstraction
> 
> On Fri, Feb 10, 2023 at 05:30:00AM +0000, Honnappa Nagarahalli wrote:
> > <snip>
> >
> > > On Thu, Feb 09, 2023 at 12:16:38AM +0000, Honnappa Nagarahalli wrote:
> > > > <snip>
> > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > For environments where stdatomics are not supported,
> > > > > > > > > > we could
> > > > > > > have a
> > > > > > > > > stdatomic.h in DPDK implementing the same APIs (we have
> > > > > > > > > to support
> > > > > > > only
> > > > > > > > > _explicit APIs). This allows the code to use stdatomics
> > > > > > > > > APIs and
> > > > > > > when we move
> > > > > > > > > to minimum supported standard C11, we just need to get
> > > > > > > > > rid of the
> > > > > > > file in DPDK
> > > > > > > > > repo.
> > > > > > > > >
> > > > > > > > > my concern with this is that if we provide a stdatomic.h
> > > > > > > > > or
> > > > > > > introduce names
> > > > > > > > > from stdatomic.h it's a violation of the C standard.
> > > > > > > > >
> > > > > > > > > references:
> > > > > > > > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > > > > > > > >  * GNU libc manual
> > > > > > > > >
> > > > > > > > > https://www.gnu.org/software/libc/manual/html_node/Reser
> > > > > > > > > ved-
> > > > > > > > > Names.html
> > > > > > > > >
> > > > > > > > > in effect the header, the names and in some instances
> > > > > > > > > namespaces
> > > > > > > introduced
> > > > > > > > > are reserved by the implementation. there are several
> > > > > > > > > reasons in
> > > > > > > the GNU libc
> > > > > > > > Wouldn't this apply only after the particular APIs were
> introduced?
> > > > > > > i.e. it should not apply if the compiler does not support stdatomics.
> > > > > > >
> > > > > > > yeah, i agree they're being a bit wishy washy in the
> > > > > > > wording, but i'm not convinced glibc folks are documenting
> > > > > > > this as permissive guidance against.
> > > > > > >
> > > > > > > >
> > > > > > > > > manual that explain the justification for these
> > > > > > > > > reservations and if
> > > > > > > if we think
> > > > > > > > > about ODR and ABI compatibility we can conceive of others.
> > > > > > > > >
> > > > > > > > > i'll also remark that the inter-mingling of names from
> > > > > > > > > the POSIX
> > > > > > > standard
> > > > > > > > > implicitly exposed as a part of the EAL public API has
> > > > > > > > > been
> > > > > > > problematic for
> > > > > > > > > portability.
> > > > > > > > These should be exposed as EAL APIs only when compiled
> > > > > > > > with a
> > > > > > > compiler that does not support stdatomics.
> > > > > > >
> > > > > > > you don't necessarily compile dpdk, the application or its
> > > > > > > other dynamically linked dependencies with the same compiler
> > > > > > > at the same time.
> > > > > > > i.e. basically the model of any dpdk-dev package on any
> > > > > > > linux distribution.
> > > > > > >
> > > > > > > if dpdk is built without real stdatomic types but the
> > > > > > > application has to interoperate with a different kit or
> > > > > > > library that does they would be forced to dance around dpdk
> > > > > > > with their own version of a shim to hide our faked up stdatomics.
> > > > > > >
> > > > > >
> > > > > > So basically, if we want a binary DPDK distribution to be
> > > > > > compatible with a
> > > > > separate application build environment, they both have to
> > > > > implement atomics the same way, i.e. agree on the ABI for atomics.
> > > > > >
> > > > > > Summing up, this leaves us with only two realistic options:
> > > > > >
> > > > > > 1. Go all in on C11 stdatomics, also requiring the application
> > > > > > build
> > > > > environment to support C11 stdatomics.
> > > > > > 2. Provide our own DPDK atomics library.
> > > > > >
> > > > > > (As mentioned by Tyler, the third option - using C11
> > > > > > stdatomics inside DPDK, and requiring a build environment
> > > > > > without C11 stdatomics to implement a shim - is not
> > > > > > realistic!)
> > > > > >
> > > > > > I strongly want atomics to be available for use across inline
> > > > > > and compiled
> > > > > code; i.e. it must be possible for both compiled DPDK functions
> > > > > and inline functions to perform atomic transactions on the same
> atomic variable.
> > > > >
> > > > > i consider it a mandatory requirement. i don't see practically
> > > > > how we could withdraw existing use and even if we had clean way
> > > > > i don't see why we would want to. so this item is defintely
> > > > > settled if you were
> > > concerned.
> > > > I think I agree here.
> > > >
> > > > >
> > > > > >
> > > > > > So either we upgrade the DPDK build requirements to support
> > > > > > C11 (including
> > > > > the optional stdatomics), or we provide our own DPDK atomics.
> > > > >
> > > > > i think the issue of requiring a toolchain conformant to a
> > > > > specific standard is a separate matter because any adoption of
> > > > > C11 standard atomics is a potential abi break from the current use of
> intrinsics.
> > > > I am not sure why you are calling it as ABI break. Referring to
> > > > [1], I just see
> > > wrappers around intrinsics (though [2] does not use the intrinsics).
> > > >
> > > > [1]
> > > > https://github.com/gcc-mirror/gcc/blob/master/gcc/ginclude/stdatom
> > > > ic.h
> > > > [2]
> > > > https://github.com/llvm-mirror/clang/blob/master/lib/Headers/stdat
> > > > omic
> > > > .h
> > >
> > > it's a potential abi break because atomic types are not the same
> > > types as their corresponding integer types etc.. (or at least are
> > > not guaranteed to be by all implementations of c as an abstract language).
> > >
> > >     ISO/IEC 9899:2011
> > >
> > >     6.2.5 (27)
> > >     Further, there is the _Atomic qualifier. The presence of the _Atomic
> > >     qualifier designates an atomic type. The size, representation, and
> alignment
> > >     of an atomic type need not be the same as those of the corresponding
> > >     unqualified type.
> > >
> > >     7.17.6 (3)
> > >     NOTE The representation of atomic integer types need not have the
> same size
> > >     as their corresponding regular types. They should have the same
> > > size whenever
> > >     possible, as it eases effort required to port existing code.
> > >
> > > i use the term `potential abi break' with intent because for me to
> > > assert in absolute terms i would have to evaluate the implementation
> > > of every current and potential future compilers atomic vs non-atomic
> > > types. this as i'm sure you understand is not practical, it would
> > > also defeat the purpose of moving to a standard. therefore i rely on
> > > the specification prescribed by the standard not the detail of a specific
> implementation.
> > Can we say that the platforms 'supported' by DPDK today do not have this
> problem? Any future platforms that will come to DPDK have to evaluate this.
> 
> sadly i don't think we can. i believe in an earlier post i linked a bug filed on
> gcc that shows that clang / gcc were producing different layout than the
> equivalent non-atomic type.
I looked at that bug again, it is to do with structure.

> 
> >
> > >
> > >
> > > > > the abstraction (whatever namespace it resides) allows the
> > > > > existing toolchain/platform combinations to maintain
> > > > > compatibility by defaulting to current non-standard intrinsics.
> > > > How about using the intrinsics (__atomic_xxx) name space for
> abstraction?
> > > This covers the GCC and Clang compilers.
> 
> i haven't investigated fully but there are usages of these intrinsics that
> indicate there may be undesirable difference between clang and gcc versions.
> the hint is there seems to be conditionally compiled code under __clang__
> when using some __atomic's.
I sent an RFC to address this [1]. I think the size specific intrinsics are not necessary.

[1] http://patches.dpdk.org/project/dpdk/patch/20230211015622.408487-1-honnappa.nagarahalli@arm.com/

> 
> for the purpose of this discussion clang just tries to look like gcc so i don't
> regard them as being different compilers for the purpose of this discussion.
> 
> > >
> > > the namespace starting with `__` is also reserved for the implementation.
> > > this is why compilers gcc/clang/msvc place name their intrinsic and
> > > builtin functions starting with __ to explicitly avoid collision
> > > with the application namespace.
> 
> > Agreed. But, here we are considering '__atomic_' specifically (i.e.
> > not just '__')
> 
> i don't understand the confusion __atomic is within the __ namespace that is
> reserved.
What I mean is, we are not formulating a policy/rule to allow for any name space that starts with '__'.

> 
> let me ask this another way, what benefit do you see to trying to overlap with
> the standard namespace? the only benefit i can see is that at some point in
> the future it avoids having to perform a mechanical change to eventually
> retire the abstraction once all platform/toolchains support standard atomics.
> i.e. basically s/rte_atomic/atomic/g
> 
> is there another benefit i'm missing?
The abstraction you have proposed solves the problem for the long term. The proposed abstraction stops us from thinking about moving to stdatomics.
IMO, the problem is short term. Using the __atomic_ name space does not have any practical issues with the platforms DPDK supports (unless msvc has a problem with this, more questions below).

> 
> >
> > >
> > >     ISO/IEC 9899:2011
> > >
> > >     7.1.3 (1)
> > >     All identifiers that begin with an underscore and either an uppercase
> > >     letter or another underscore are always reserved for any use.
> > >
> > >     ...
> > >
> > > > If there is another platform that uses the same name space for
> > > > something
> > > else, I think DPDK should not be supporting that platform.
> > >
> > > that's effectively a statement excluding windows platform and all
> > > non-gcc compilers from ever supporting dpdk.
> > Apologies, I did not understand your comment on windows platform. Do
> you mean to say a compiler for windows platform uses '__atomic_xxx' name
> space to provide some other functionality (and hence it would get excluded)?
> 
> i mean dpdk can never fully be supported without msvc except for statically
> linked builds which are niche and limit it too severely for many consumers to
> practically use dpdk. there are also many application developers who would
> like to integrate dpdk but can't and telling them their only choice is to re-port
> their entire application to clang isn't feasible.
> 
> i can see no technical reason why we should be excluding a major compiler in
> broad use if it is capable of building dpdk. msvc arguably has some of the
> most sophisticated security features in the industry and the use of those
> features is mandated by many of the customers who might deploy dpdk
> applications on windows.
I did not mean DPDK should not support msvc (may be my sentence below was misunderstood).
Does msvc provide '__atomic_xxx' intrinsics?

> 
> > Clang supports these intrinsics. I am not sure about the merit of supporting
> other non-gcc compilers. May be a topic Techboard discussion.
> >
> > >
> > > > What problems do you see?
> > >
> > > i'm fairly certain at least one other compiler uses the __atomic
> > > namespace but
> > Do you mean __atomic namespace is used for some other purpose?
> >
> > > it would take me time to check, the most notable potential issue
> > > that comes to mind is if such an intrinsic with the same name is
> > > provided in a different implementation and has either regressive
> > > code generation or different semantics it would be bad because it is
> > > intrinsic you can't just hack around it with #undef __atomic to shim in a
> semantically correct version.
> > I do not think we should worry about regressive code generation problem. It
> should be fixed by that compiler.
> > Different semantics is something we need to worry about. It would be good
> to find out more about a compiler that does this.
> 
> again, this is about portability it's about potential not that we can find an
> example.
> 
> >
> > >
> > > how about this, is there another possible namespace you might
> > > suggest that conforms or doesn't conflict with the the rules defined
> > > in ISO/IEC 9899:2011
> > > 7.1.3 i think if there were that would satisfy all of my concerns
> > > related to namespaces.
> > >
> > > keep in mind the point of moving to a standard is to achieve
> > > portability so if we do things that will regress us back to being
> > > dependent on an implementation we haven't succeeded. that's all i'm
> trying to guarantee here.
> > Agree. We are trying to solve a problem that is temporary. I am trying to
> keep the problem scope narrow which might help us push to adopt the
> standard sooner.
> 
> i do wish we could just target the standard but unless we are willing to draw a
> line and say no more non std=c11 and also we potentially break the abi we
> are talking years. i don't think it is reasonable to block progress for years, so
> i'm offering a transitional path. it's an evolution over time that we have to
> manage.
Apologies if I am sounding like I am blocking progress. Rest assured, we will find a way. It is just about which solution we are going to pick.
Also, is there are any information on how long before we move to C11?

> 
> >
> > >
> > > i feel like we are really close on this discussion, if we can just
> > > iron this issue out we can probably get going on the actual changes.
> > >
> > > thanks for the consideration.
> > >
> > > >
> > > > >
> > > > > once in place it provides an opportunity to introduce new
> > > > > toolchain/platform combinations and enables an opt-in capability
> > > > > to use stdatomics on existing toolchain/platform combinations
> > > > > subject to community discussion on how/if/when.
> > > > >
> > > > > it would be good to get more participants into the discussion so
> > > > > i'll cc techboard for some attention. i feel like the only area
> > > > > that isn't decided is to do or not do this in rte_ namespace.
> > > > >
> > > > > i'm strongly in favor of rte_ namespace after discussion, mainly
> > > > > due to to disadvantages of trying to overlap with the standard
> > > > > namespace while not providing a compatible api/abi and because
> > > > > it provides clear disambiguation of that difference in semantics
> > > > > and compatibility with
> > > the standard api.
> > > > >
> > > > > so far i've noted the following
> > > > >
> > > > > * we will not provide the non-explicit apis.
> > > > +1
> > > >
> > > > > * we will make no attempt to support operate on struct/union atomics
> > > > >   with our apis.
> > > > +1
> > > >
> > > > > * we will mirror the standard api potentially in the rte_ namespace to
> > > > >   - reference the standard api documentation.
> > > > >   - assume compatible semantics (sans exceptions from first 2 points).
> > > > >
> > > > > my vote is to remove 'potentially' from the last point above for
> > > > > reasons previously discussed in postings to the mail thread.
> > > > >
> > > > > thanks all for the discussion, i'll send up a patch removing
> > > > > non-explicit apis for viewing.
> > > > >
> > > > > ty
  

There is a thread discussing a change to the standard [1] but I have not
seen anything explicit yet about moving to C11.  I am personally in favor
of making the jump to C11 now as part of the 23.x branch and provided my
thoughts in the linked thread (what other projects using DPDK have as
minimum compiler requirements, CentOS 7 EOL dates).

Is the long term plan to backport this change set to the existing LTS
release or is this meant to be something introduced for use in 23.x and
going forward?  I think I was (probably naively) assuming this would be a
new feature in the 23.x going forward only.

[1] http://mails.dpdk.org/archives/dev/2023-February/262188.html

On Mon, Feb 13, 2023 at 12:05 AM Honnappa Nagarahalli <
Honnappa.Nagarahalli@arm.com> wrote:

> Hi Tyler,
>         Few more comments inline. Let us continue to make progress, I will
> add this topic for Techboard discussion for 22nd Feb.
>
> > -----Original Message-----
> > From: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > Sent: Friday, February 10, 2023 2:30 PM
> > To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > Cc: Morten Brørup <mb@smartsharesystems.com>; thomas@monjalon.net;
> > dev@dpdk.org; bruce.richardson@intel.com; david.marchand@redhat.com;
> > jerinj@marvell.com; konstantin.ananyev@huawei.com;
> > ferruh.yigit@amd.com; nd <nd@arm.com>; techboard@dpdk.org
> > Subject: Re: [PATCH] eal: introduce atomics abstraction
> >
> > On Fri, Feb 10, 2023 at 05:30:00AM +0000, Honnappa Nagarahalli wrote:
> > > <snip>
> > >
> > > > On Thu, Feb 09, 2023 at 12:16:38AM +0000, Honnappa Nagarahalli wrote:
> > > > > <snip>
> > > > >
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > For environments where stdatomics are not supported,
> > > > > > > > > > > we could
> > > > > > > > have a
> > > > > > > > > > stdatomic.h in DPDK implementing the same APIs (we have
> > > > > > > > > > to support
> > > > > > > > only
> > > > > > > > > > _explicit APIs). This allows the code to use stdatomics
> > > > > > > > > > APIs and
> > > > > > > > when we move
> > > > > > > > > > to minimum supported standard C11, we just need to get
> > > > > > > > > > rid of the
> > > > > > > > file in DPDK
> > > > > > > > > > repo.
> > > > > > > > > >
> > > > > > > > > > my concern with this is that if we provide a stdatomic.h
> > > > > > > > > > or
> > > > > > > > introduce names
> > > > > > > > > > from stdatomic.h it's a violation of the C standard.
> > > > > > > > > >
> > > > > > > > > > references:
> > > > > > > > > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > > > > > > > > >  * GNU libc manual
> > > > > > > > > >
> > > > > > > > > > https://www.gnu.org/software/libc/manual/html_node/Reser
> > > > > > > > > > ved-
> > > > > > > > > > Names.html
> > > > > > > > > >
> > > > > > > > > > in effect the header, the names and in some instances
> > > > > > > > > > namespaces
> > > > > > > > introduced
> > > > > > > > > > are reserved by the implementation. there are several
> > > > > > > > > > reasons in
> > > > > > > > the GNU libc
> > > > > > > > > Wouldn't this apply only after the particular APIs were
> > introduced?
> > > > > > > > i.e. it should not apply if the compiler does not support
> stdatomics.
> > > > > > > >
> > > > > > > > yeah, i agree they're being a bit wishy washy in the
> > > > > > > > wording, but i'm not convinced glibc folks are documenting
> > > > > > > > this as permissive guidance against.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > manual that explain the justification for these
> > > > > > > > > > reservations and if
> > > > > > > > if we think
> > > > > > > > > > about ODR and ABI compatibility we can conceive of
> others.
> > > > > > > > > >
> > > > > > > > > > i'll also remark that the inter-mingling of names from
> > > > > > > > > > the POSIX
> > > > > > > > standard
> > > > > > > > > > implicitly exposed as a part of the EAL public API has
> > > > > > > > > > been
> > > > > > > > problematic for
> > > > > > > > > > portability.
> > > > > > > > > These should be exposed as EAL APIs only when compiled
> > > > > > > > > with a
> > > > > > > > compiler that does not support stdatomics.
> > > > > > > >
> > > > > > > > you don't necessarily compile dpdk, the application or its
> > > > > > > > other dynamically linked dependencies with the same compiler
> > > > > > > > at the same time.
> > > > > > > > i.e. basically the model of any dpdk-dev package on any
> > > > > > > > linux distribution.
> > > > > > > >
> > > > > > > > if dpdk is built without real stdatomic types but the
> > > > > > > > application has to interoperate with a different kit or
> > > > > > > > library that does they would be forced to dance around dpdk
> > > > > > > > with their own version of a shim to hide our faked up
> stdatomics.
> > > > > > > >
> > > > > > >
> > > > > > > So basically, if we want a binary DPDK distribution to be
> > > > > > > compatible with a
> > > > > > separate application build environment, they both have to
> > > > > > implement atomics the same way, i.e. agree on the ABI for
> atomics.
> > > > > > >
> > > > > > > Summing up, this leaves us with only two realistic options:
> > > > > > >
> > > > > > > 1. Go all in on C11 stdatomics, also requiring the application
> > > > > > > build
> > > > > > environment to support C11 stdatomics.
> > > > > > > 2. Provide our own DPDK atomics library.
> > > > > > >
> > > > > > > (As mentioned by Tyler, the third option - using C11
> > > > > > > stdatomics inside DPDK, and requiring a build environment
> > > > > > > without C11 stdatomics to implement a shim - is not
> > > > > > > realistic!)
> > > > > > >
> > > > > > > I strongly want atomics to be available for use across inline
> > > > > > > and compiled
> > > > > > code; i.e. it must be possible for both compiled DPDK functions
> > > > > > and inline functions to perform atomic transactions on the same
> > atomic variable.
> > > > > >
> > > > > > i consider it a mandatory requirement. i don't see practically
> > > > > > how we could withdraw existing use and even if we had clean way
> > > > > > i don't see why we would want to. so this item is defintely
> > > > > > settled if you were
> > > > concerned.
> > > > > I think I agree here.
> > > > >
> > > > > >
> > > > > > >
> > > > > > > So either we upgrade the DPDK build requirements to support
> > > > > > > C11 (including
> > > > > > the optional stdatomics), or we provide our own DPDK atomics.
> > > > > >
> > > > > > i think the issue of requiring a toolchain conformant to a
> > > > > > specific standard is a separate matter because any adoption of
> > > > > > C11 standard atomics is a potential abi break from the current
> use of
> > intrinsics.
> > > > > I am not sure why you are calling it as ABI break. Referring to
> > > > > [1], I just see
> > > > wrappers around intrinsics (though [2] does not use the intrinsics).
> > > > >
> > > > > [1]
> > > > > https://github.com/gcc-mirror/gcc/blob/master/gcc/ginclude/stdatom
> > > > > ic.h
> > > > > [2]
> > > > > https://github.com/llvm-mirror/clang/blob/master/lib/Headers/stdat
> > > > > omic
> > > > > .h
> > > >
> > > > it's a potential abi break because atomic types are not the same
> > > > types as their corresponding integer types etc.. (or at least are
> > > > not guaranteed to be by all implementations of c as an abstract
> language).
> > > >
> > > >     ISO/IEC 9899:2011
> > > >
> > > >     6.2.5 (27)
> > > >     Further, there is the _Atomic qualifier. The presence of the
> _Atomic
> > > >     qualifier designates an atomic type. The size, representation,
> and
> > alignment
> > > >     of an atomic type need not be the same as those of the
> corresponding
> > > >     unqualified type.
> > > >
> > > >     7.17.6 (3)
> > > >     NOTE The representation of atomic integer types need not have the
> > same size
> > > >     as their corresponding regular types. They should have the same
> > > > size whenever
> > > >     possible, as it eases effort required to port existing code.
> > > >
> > > > i use the term `potential abi break' with intent because for me to
> > > > assert in absolute terms i would have to evaluate the implementation
> > > > of every current and potential future compilers atomic vs non-atomic
> > > > types. this as i'm sure you understand is not practical, it would
> > > > also defeat the purpose of moving to a standard. therefore i rely on
> > > > the specification prescribed by the standard not the detail of a
> specific
> > implementation.
> > > Can we say that the platforms 'supported' by DPDK today do not have
> this
> > problem? Any future platforms that will come to DPDK have to evaluate
> this.
> >
> > sadly i don't think we can. i believe in an earlier post i linked a bug
> filed on
> > gcc that shows that clang / gcc were producing different layout than the
> > equivalent non-atomic type.
> I looked at that bug again, it is to do with structure.
>
> >
> > >
> > > >
> > > >
> > > > > > the abstraction (whatever namespace it resides) allows the
> > > > > > existing toolchain/platform combinations to maintain
> > > > > > compatibility by defaulting to current non-standard intrinsics.
> > > > > How about using the intrinsics (__atomic_xxx) name space for
> > abstraction?
> > > > This covers the GCC and Clang compilers.
> >
> > i haven't investigated fully but there are usages of these intrinsics
> that
> > indicate there may be undesirable difference between clang and gcc
> versions.
> > the hint is there seems to be conditionally compiled code under __clang__
> > when using some __atomic's.
> I sent an RFC to address this [1]. I think the size specific intrinsics
> are not necessary.
>
> [1]
> http://patches.dpdk.org/project/dpdk/patch/20230211015622.408487-1-honnappa.nagarahalli@arm.com/
>
> >
> > for the purpose of this discussion clang just tries to look like gcc so
> i don't
> > regard them as being different compilers for the purpose of this
> discussion.
> >
> > > >
> > > > the namespace starting with `__` is also reserved for the
> implementation.
> > > > this is why compilers gcc/clang/msvc place name their intrinsic and
> > > > builtin functions starting with __ to explicitly avoid collision
> > > > with the application namespace.
> >
> > > Agreed. But, here we are considering '__atomic_' specifically (i.e.
> > > not just '__')
> >
> > i don't understand the confusion __atomic is within the __ namespace
> that is
> > reserved.
> What I mean is, we are not formulating a policy/rule to allow for any name
> space that starts with '__'.
>
> >
> > let me ask this another way, what benefit do you see to trying to
> overlap with
> > the standard namespace? the only benefit i can see is that at some point
> in
> > the future it avoids having to perform a mechanical change to eventually
> > retire the abstraction once all platform/toolchains support standard
> atomics.
> > i.e. basically s/rte_atomic/atomic/g
> >
> > is there another benefit i'm missing?
> The abstraction you have proposed solves the problem for the long term.
> The proposed abstraction stops us from thinking about moving to stdatomics.
> IMO, the problem is short term. Using the __atomic_ name space does not
> have any practical issues with the platforms DPDK supports (unless msvc has
> a problem with this, more questions below).
>
> >
> > >
> > > >
> > > >     ISO/IEC 9899:2011
> > > >
> > > >     7.1.3 (1)
> > > >     All identifiers that begin with an underscore and either an
> uppercase
> > > >     letter or another underscore are always reserved for any use.
> > > >
> > > >     ...
> > > >
> > > > > If there is another platform that uses the same name space for
> > > > > something
> > > > else, I think DPDK should not be supporting that platform.
> > > >
> > > > that's effectively a statement excluding windows platform and all
> > > > non-gcc compilers from ever supporting dpdk.
> > > Apologies, I did not understand your comment on windows platform. Do
> > you mean to say a compiler for windows platform uses '__atomic_xxx' name
> > space to provide some other functionality (and hence it would get
> excluded)?
> >
> > i mean dpdk can never fully be supported without msvc except for
> statically
> > linked builds which are niche and limit it too severely for many
> consumers to
> > practically use dpdk. there are also many application developers who
> would
> > like to integrate dpdk but can't and telling them their only choice is
> to re-port
> > their entire application to clang isn't feasible.
> >
> > i can see no technical reason why we should be excluding a major
> compiler in
> > broad use if it is capable of building dpdk. msvc arguably has some of
> the
> > most sophisticated security features in the industry and the use of those
> > features is mandated by many of the customers who might deploy dpdk
> > applications on windows.
> I did not mean DPDK should not support msvc (may be my sentence below was
> misunderstood).
> Does msvc provide '__atomic_xxx' intrinsics?
>
> >
> > > Clang supports these intrinsics. I am not sure about the merit of
> supporting
> > other non-gcc compilers. May be a topic Techboard discussion.
> > >
> > > >
> > > > > What problems do you see?
> > > >
> > > > i'm fairly certain at least one other compiler uses the __atomic
> > > > namespace but
> > > Do you mean __atomic namespace is used for some other purpose?
> > >
> > > > it would take me time to check, the most notable potential issue
> > > > that comes to mind is if such an intrinsic with the same name is
> > > > provided in a different implementation and has either regressive
> > > > code generation or different semantics it would be bad because it is
> > > > intrinsic you can't just hack around it with #undef __atomic to shim
> in a
> > semantically correct version.
> > > I do not think we should worry about regressive code generation
> problem. It
> > should be fixed by that compiler.
> > > Different semantics is something we need to worry about. It would be
> good
> > to find out more about a compiler that does this.
> >
> > again, this is about portability it's about potential not that we can
> find an
> > example.
> >
> > >
> > > >
> > > > how about this, is there another possible namespace you might
> > > > suggest that conforms or doesn't conflict with the the rules defined
> > > > in ISO/IEC 9899:2011
> > > > 7.1.3 i think if there were that would satisfy all of my concerns
> > > > related to namespaces.
> > > >
> > > > keep in mind the point of moving to a standard is to achieve
> > > > portability so if we do things that will regress us back to being
> > > > dependent on an implementation we haven't succeeded. that's all i'm
> > trying to guarantee here.
> > > Agree. We are trying to solve a problem that is temporary. I am trying
> to
> > keep the problem scope narrow which might help us push to adopt the
> > standard sooner.
> >
> > i do wish we could just target the standard but unless we are willing to
> draw a
> > line and say no more non std=c11 and also we potentially break the abi we
> > are talking years. i don't think it is reasonable to block progress for
> years, so
> > i'm offering a transitional path. it's an evolution over time that we
> have to
> > manage.
> Apologies if I am sounding like I am blocking progress. Rest assured, we
> will find a way. It is just about which solution we are going to pick.
> Also, is there are any information on how long before we move to C11?
>
> >
> > >
> > > >
> > > > i feel like we are really close on this discussion, if we can just
> > > > iron this issue out we can probably get going on the actual changes.
> > > >
> > > > thanks for the consideration.
> > > >
> > > > >
> > > > > >
> > > > > > once in place it provides an opportunity to introduce new
> > > > > > toolchain/platform combinations and enables an opt-in capability
> > > > > > to use stdatomics on existing toolchain/platform combinations
> > > > > > subject to community discussion on how/if/when.
> > > > > >
> > > > > > it would be good to get more participants into the discussion so
> > > > > > i'll cc techboard for some attention. i feel like the only area
> > > > > > that isn't decided is to do or not do this in rte_ namespace.
> > > > > >
> > > > > > i'm strongly in favor of rte_ namespace after discussion, mainly
> > > > > > due to to disadvantages of trying to overlap with the standard
> > > > > > namespace while not providing a compatible api/abi and because
> > > > > > it provides clear disambiguation of that difference in semantics
> > > > > > and compatibility with
> > > > the standard api.
> > > > > >
> > > > > > so far i've noted the following
> > > > > >
> > > > > > * we will not provide the non-explicit apis.
> > > > > +1
> > > > >
> > > > > > * we will make no attempt to support operate on struct/union
> atomics
> > > > > >   with our apis.
> > > > > +1
> > > > >
> > > > > > * we will mirror the standard api potentially in the rte_
> namespace to
> > > > > >   - reference the standard api documentation.
> > > > > >   - assume compatible semantics (sans exceptions from first 2
> points).
> > > > > >
> > > > > > my vote is to remove 'potentially' from the last point above for
> > > > > > reasons previously discussed in postings to the mail thread.
> > > > > >
> > > > > > thanks all for the discussion, i'll send up a patch removing
> > > > > > non-explicit apis for viewing.
> > > > > >
> > > > > > ty
>
  

On Mon, Feb 13, 2023 at 10:28:40AM -0500, Ben Magistro wrote:
>    There is a thread discussing a change to the standard [1] but I have
>    not seen anything explicit yet about moving to C11.  I am personally in
>    favor of making the jump to C11 now as part of the 23.x branch and
>    provided my thoughts in the linked thread (what other projects using
>    DPDK have as minimum compiler requirements, CentOS 7 EOL dates).
>    Is the long term plan to backport this change set to the existing LTS
>    release or is this meant to be something introduced for use in 23.x and
>    going forward?  I think I was (probably naively) assuming this would be
>    a new feature in the 23.x going forward only.
>    [1] [1]http://mails.dpdk.org/archives/dev/2023-February/262188.html
> 
We don't bump requirements for older LTS releases, so any change to minimum
required versions would only be for the 23.x series releases.

/Bruce
  

On Mon, Feb 13, 2023 at 10:55 AM Bruce Richardson <
bruce.richardson@intel.com> wrote:

> On Mon, Feb 13, 2023 at 10:28:40AM -0500, Ben Magistro wrote:
> >    There is a thread discussing a change to the standard [1] but I have
> >    not seen anything explicit yet about moving to C11.  I am personally
> in
> >    favor of making the jump to C11 now as part of the 23.x branch and
> >    provided my thoughts in the linked thread (what other projects using
> >    DPDK have as minimum compiler requirements, CentOS 7 EOL dates).
> >    Is the long term plan to backport this change set to the existing LTS
> >    release or is this meant to be something introduced for use in 23.x
> and
> >    going forward?  I think I was (probably naively) assuming this would
> be
> >    a new feature in the 23.x going forward only.
> >    [1] [1]http://mails.dpdk.org/archives/dev/2023-February/262188.html
> >
> We don't bump requirements for older LTS releases, so any change to minimum
> required versions would only be for the 23.x series releases


I meant the atomics change set, I should have been clearer in that
question, my apologies.  If the atomic work is planned to be backported,
the question of if/when the C11 standard would be adopted seems less
relevant since it would need to be supported for DPDK versions that do not
have the C11 standard requirement too.
  

Hi Ben,

 

Only bug fixes are backported to LTS. Not even new NIC drivers are backported.

 

Reference: https://doc.dpdk.org/guides/contributing/stable.html#what-changes-should-be-backported <https://doc.dpdk.org/guides/contributing/stable.html#what-changes-should-be-backported> 

 

-Morten

 

From: Ben Magistro [mailto:koncept1@gmail.com] 
Sent: Monday, 13 February 2023 17.47



 

On Mon, Feb 13, 2023 at 10:55 AM Bruce Richardson <bruce.richardson@intel.com> wrote:

	On Mon, Feb 13, 2023 at 10:28:40AM -0500, Ben Magistro wrote:
	>    There is a thread discussing a change to the standard [1] but I have
	>    not seen anything explicit yet about moving to C11.  I am personally in
	>    favor of making the jump to C11 now as part of the 23.x branch and
	>    provided my thoughts in the linked thread (what other projects using
	>    DPDK have as minimum compiler requirements, CentOS 7 EOL dates).
	>    Is the long term plan to backport this change set to the existing LTS
	>    release or is this meant to be something introduced for use in 23.x and
	>    going forward?  I think I was (probably naively) assuming this would be
	>    a new feature in the 23.x going forward only.
	>    [1] [1]http://mails.dpdk.org/archives/dev/2023-February/262188.html
	> 
	We don't bump requirements for older LTS releases, so any change to minimum
	required versions would only be for the 23.x series releases

 

I meant the atomics change set, I should have been clearer in that question, my apologies.  If the atomic work is planned to be backported, the question of if/when the C11 standard would be adopted seems less relevant since it would need to be supported for DPDK versions that do not have the C11 standard requirement too.
  

On Mon, Feb 13, 2023 at 05:04:49AM +0000, Honnappa Nagarahalli wrote:
> Hi Tyler,
> 	Few more comments inline. Let us continue to make progress, I will add this topic for Techboard discussion for 22nd Feb.
> 
> > -----Original Message-----
> > From: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > Sent: Friday, February 10, 2023 2:30 PM
> > To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > Cc: Morten Brørup <mb@smartsharesystems.com>; thomas@monjalon.net;
> > dev@dpdk.org; bruce.richardson@intel.com; david.marchand@redhat.com;
> > jerinj@marvell.com; konstantin.ananyev@huawei.com;
> > ferruh.yigit@amd.com; nd <nd@arm.com>; techboard@dpdk.org
> > Subject: Re: [PATCH] eal: introduce atomics abstraction
> > 
> > On Fri, Feb 10, 2023 at 05:30:00AM +0000, Honnappa Nagarahalli wrote:
> > > <snip>
> > >
> > > > On Thu, Feb 09, 2023 at 12:16:38AM +0000, Honnappa Nagarahalli wrote:
> > > > > <snip>
> > > > >
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > For environments where stdatomics are not supported,
> > > > > > > > > > > we could
> > > > > > > > have a
> > > > > > > > > > stdatomic.h in DPDK implementing the same APIs (we have
> > > > > > > > > > to support
> > > > > > > > only
> > > > > > > > > > _explicit APIs). This allows the code to use stdatomics
> > > > > > > > > > APIs and
> > > > > > > > when we move
> > > > > > > > > > to minimum supported standard C11, we just need to get
> > > > > > > > > > rid of the
> > > > > > > > file in DPDK
> > > > > > > > > > repo.
> > > > > > > > > >
> > > > > > > > > > my concern with this is that if we provide a stdatomic.h
> > > > > > > > > > or
> > > > > > > > introduce names
> > > > > > > > > > from stdatomic.h it's a violation of the C standard.
> > > > > > > > > >
> > > > > > > > > > references:
> > > > > > > > > >  * ISO/IEC 9899:2011 sections 7.1.2, 7.1.3.
> > > > > > > > > >  * GNU libc manual
> > > > > > > > > >
> > > > > > > > > > https://www.gnu.org/software/libc/manual/html_node/Reser
> > > > > > > > > > ved-
> > > > > > > > > > Names.html
> > > > > > > > > >
> > > > > > > > > > in effect the header, the names and in some instances
> > > > > > > > > > namespaces
> > > > > > > > introduced
> > > > > > > > > > are reserved by the implementation. there are several
> > > > > > > > > > reasons in
> > > > > > > > the GNU libc
> > > > > > > > > Wouldn't this apply only after the particular APIs were
> > introduced?
> > > > > > > > i.e. it should not apply if the compiler does not support stdatomics.
> > > > > > > >
> > > > > > > > yeah, i agree they're being a bit wishy washy in the
> > > > > > > > wording, but i'm not convinced glibc folks are documenting
> > > > > > > > this as permissive guidance against.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > manual that explain the justification for these
> > > > > > > > > > reservations and if
> > > > > > > > if we think
> > > > > > > > > > about ODR and ABI compatibility we can conceive of others.
> > > > > > > > > >
> > > > > > > > > > i'll also remark that the inter-mingling of names from
> > > > > > > > > > the POSIX
> > > > > > > > standard
> > > > > > > > > > implicitly exposed as a part of the EAL public API has
> > > > > > > > > > been
> > > > > > > > problematic for
> > > > > > > > > > portability.
> > > > > > > > > These should be exposed as EAL APIs only when compiled
> > > > > > > > > with a
> > > > > > > > compiler that does not support stdatomics.
> > > > > > > >
> > > > > > > > you don't necessarily compile dpdk, the application or its
> > > > > > > > other dynamically linked dependencies with the same compiler
> > > > > > > > at the same time.
> > > > > > > > i.e. basically the model of any dpdk-dev package on any
> > > > > > > > linux distribution.
> > > > > > > >
> > > > > > > > if dpdk is built without real stdatomic types but the
> > > > > > > > application has to interoperate with a different kit or
> > > > > > > > library that does they would be forced to dance around dpdk
> > > > > > > > with their own version of a shim to hide our faked up stdatomics.
> > > > > > > >
> > > > > > >
> > > > > > > So basically, if we want a binary DPDK distribution to be
> > > > > > > compatible with a
> > > > > > separate application build environment, they both have to
> > > > > > implement atomics the same way, i.e. agree on the ABI for atomics.
> > > > > > >
> > > > > > > Summing up, this leaves us with only two realistic options:
> > > > > > >
> > > > > > > 1. Go all in on C11 stdatomics, also requiring the application
> > > > > > > build
> > > > > > environment to support C11 stdatomics.
> > > > > > > 2. Provide our own DPDK atomics library.
> > > > > > >
> > > > > > > (As mentioned by Tyler, the third option - using C11
> > > > > > > stdatomics inside DPDK, and requiring a build environment
> > > > > > > without C11 stdatomics to implement a shim - is not
> > > > > > > realistic!)
> > > > > > >
> > > > > > > I strongly want atomics to be available for use across inline
> > > > > > > and compiled
> > > > > > code; i.e. it must be possible for both compiled DPDK functions
> > > > > > and inline functions to perform atomic transactions on the same
> > atomic variable.
> > > > > >
> > > > > > i consider it a mandatory requirement. i don't see practically
> > > > > > how we could withdraw existing use and even if we had clean way
> > > > > > i don't see why we would want to. so this item is defintely
> > > > > > settled if you were
> > > > concerned.
> > > > > I think I agree here.
> > > > >
> > > > > >
> > > > > > >
> > > > > > > So either we upgrade the DPDK build requirements to support
> > > > > > > C11 (including
> > > > > > the optional stdatomics), or we provide our own DPDK atomics.
> > > > > >
> > > > > > i think the issue of requiring a toolchain conformant to a
> > > > > > specific standard is a separate matter because any adoption of
> > > > > > C11 standard atomics is a potential abi break from the current use of
> > intrinsics.
> > > > > I am not sure why you are calling it as ABI break. Referring to
> > > > > [1], I just see
> > > > wrappers around intrinsics (though [2] does not use the intrinsics).
> > > > >
> > > > > [1]
> > > > > https://github.com/gcc-mirror/gcc/blob/master/gcc/ginclude/stdatom
> > > > > ic.h
> > > > > [2]
> > > > > https://github.com/llvm-mirror/clang/blob/master/lib/Headers/stdat
> > > > > omic
> > > > > .h
> > > >
> > > > it's a potential abi break because atomic types are not the same
> > > > types as their corresponding integer types etc.. (or at least are
> > > > not guaranteed to be by all implementations of c as an abstract language).
> > > >
> > > >     ISO/IEC 9899:2011
> > > >
> > > >     6.2.5 (27)
> > > >     Further, there is the _Atomic qualifier. The presence of the _Atomic
> > > >     qualifier designates an atomic type. The size, representation, and
> > alignment
> > > >     of an atomic type need not be the same as those of the corresponding
> > > >     unqualified type.
> > > >
> > > >     7.17.6 (3)
> > > >     NOTE The representation of atomic integer types need not have the
> > same size
> > > >     as their corresponding regular types. They should have the same
> > > > size whenever
> > > >     possible, as it eases effort required to port existing code.
> > > >
> > > > i use the term `potential abi break' with intent because for me to
> > > > assert in absolute terms i would have to evaluate the implementation
> > > > of every current and potential future compilers atomic vs non-atomic
> > > > types. this as i'm sure you understand is not practical, it would
> > > > also defeat the purpose of moving to a standard. therefore i rely on
> > > > the specification prescribed by the standard not the detail of a specific
> > implementation.
> > > Can we say that the platforms 'supported' by DPDK today do not have this
> > problem? Any future platforms that will come to DPDK have to evaluate this.
> > 
> > sadly i don't think we can. i believe in an earlier post i linked a bug filed on
> > gcc that shows that clang / gcc were producing different layout than the
> > equivalent non-atomic type.
> I looked at that bug again, it is to do with structure.

just to be clear, you're saying you aren't concerned because we don't
have in our public api struct objects to which we apply atomic
operations?

if that guarantee is absolute and stays true in our public api then i am
satisfied and we can drop the issue.

hypothetically if we make this assumption are you proposing that all
platform/toolchain combinations that support std=c11 and optional
stdatomic should adopt them as default on?

there are other implications to doing this, let's dig into the details
at the next technical board meeting.

> 
> > 
> > >
> > > >
> > > >
> > > > > > the abstraction (whatever namespace it resides) allows the
> > > > > > existing toolchain/platform combinations to maintain
> > > > > > compatibility by defaulting to current non-standard intrinsics.
> > > > > How about using the intrinsics (__atomic_xxx) name space for
> > abstraction?
> > > > This covers the GCC and Clang compilers.
> > 
> > i haven't investigated fully but there are usages of these intrinsics that
> > indicate there may be undesirable difference between clang and gcc versions.
> > the hint is there seems to be conditionally compiled code under __clang__
> > when using some __atomic's.
> I sent an RFC to address this [1]. I think the size specific intrinsics are not necessary.
> 
> [1] http://patches.dpdk.org/project/dpdk/patch/20230211015622.408487-1-honnappa.nagarahalli@arm.com/

yep, looks good to me. i acked the change.

thank you.

> 
> > 
> > for the purpose of this discussion clang just tries to look like gcc so i don't
> > regard them as being different compilers for the purpose of this discussion.
> > 
> > > >
> > > > the namespace starting with `__` is also reserved for the implementation.
> > > > this is why compilers gcc/clang/msvc place name their intrinsic and
> > > > builtin functions starting with __ to explicitly avoid collision
> > > > with the application namespace.
> > 
> > > Agreed. But, here we are considering '__atomic_' specifically (i.e.
> > > not just '__')
> > 
> > i don't understand the confusion __atomic is within the __ namespace that is
> > reserved.
> What I mean is, we are not formulating a policy/rule to allow for any name space that starts with '__'.

understood, but we appear to be trying to formulate a policy allowing a name
within that space which is reserved by the standard for and claimed by gcc.

anyway, let's discuss further at the meeting.

> 
> > 
> > let me ask this another way, what benefit do you see to trying to overlap with
> > the standard namespace? the only benefit i can see is that at some point in
> > the future it avoids having to perform a mechanical change to eventually
> > retire the abstraction once all platform/toolchains support standard atomics.
> > i.e. basically s/rte_atomic/atomic/g
> > 
> > is there another benefit i'm missing?
> The abstraction you have proposed solves the problem for the long term. The proposed abstraction stops us from thinking about moving to stdatomics.

i think this is where you've got me a bit confused. i'd like to
understand how it stops us thinking about moving to stdatomics.

> IMO, the problem is short term. Using the __atomic_ name space does not have any practical issues with the platforms DPDK supports (unless msvc has a problem with this, more questions below).

oh, sorry for not answering this previously. msvc (and as it happens
clang) both use __c11_atomic_xxx as a namespace. i'm only aware of gcc
and potentially compilers that try to look like gcc using __atomic_xxxx.

so if you're asking if that selection would interfere with msvc, it
wouldn't. i'm only concerned with mingling in a namespace that gcc has
claimed.

> 
> > 
> > >
> > > >
> > > >     ISO/IEC 9899:2011
> > > >
> > > >     7.1.3 (1)
> > > >     All identifiers that begin with an underscore and either an uppercase
> > > >     letter or another underscore are always reserved for any use.
> > > >
> > > >     ...
> > > >
> > > > > If there is another platform that uses the same name space for
> > > > > something
> > > > else, I think DPDK should not be supporting that platform.
> > > >
> > > > that's effectively a statement excluding windows platform and all
> > > > non-gcc compilers from ever supporting dpdk.
> > > Apologies, I did not understand your comment on windows platform. Do
> > you mean to say a compiler for windows platform uses '__atomic_xxx' name
> > space to provide some other functionality (and hence it would get excluded)?
> > 
> > i mean dpdk can never fully be supported without msvc except for statically
> > linked builds which are niche and limit it too severely for many consumers to
> > practically use dpdk. there are also many application developers who would
> > like to integrate dpdk but can't and telling them their only choice is to re-port
> > their entire application to clang isn't feasible.
> > 
> > i can see no technical reason why we should be excluding a major compiler in
> > broad use if it is capable of building dpdk. msvc arguably has some of the
> > most sophisticated security features in the industry and the use of those
> > features is mandated by many of the customers who might deploy dpdk
> > applications on windows.
> I did not mean DPDK should not support msvc (may be my sentence below was misunderstood).
> Does msvc provide '__atomic_xxx' intrinsics?

msvc provides stdatomic (behind stdatomic there are intrinsics)

> 
> > 
> > > Clang supports these intrinsics. I am not sure about the merit of supporting
> > other non-gcc compilers. May be a topic Techboard discussion.
> > >
> > > >
> > > > > What problems do you see?
> > > >
> > > > i'm fairly certain at least one other compiler uses the __atomic
> > > > namespace but
> > > Do you mean __atomic namespace is used for some other purpose?
> > >
> > > > it would take me time to check, the most notable potential issue
> > > > that comes to mind is if such an intrinsic with the same name is
> > > > provided in a different implementation and has either regressive
> > > > code generation or different semantics it would be bad because it is
> > > > intrinsic you can't just hack around it with #undef __atomic to shim in a
> > semantically correct version.
> > > I do not think we should worry about regressive code generation problem. It
> > should be fixed by that compiler.
> > > Different semantics is something we need to worry about. It would be good
> > to find out more about a compiler that does this.
> > 
> > again, this is about portability it's about potential not that we can find an
> > example.
> > 
> > >
> > > >
> > > > how about this, is there another possible namespace you might
> > > > suggest that conforms or doesn't conflict with the the rules defined
> > > > in ISO/IEC 9899:2011
> > > > 7.1.3 i think if there were that would satisfy all of my concerns
> > > > related to namespaces.
> > > >
> > > > keep in mind the point of moving to a standard is to achieve
> > > > portability so if we do things that will regress us back to being
> > > > dependent on an implementation we haven't succeeded. that's all i'm
> > trying to guarantee here.
> > > Agree. We are trying to solve a problem that is temporary. I am trying to
> > keep the problem scope narrow which might help us push to adopt the
> > standard sooner.
> > 
> > i do wish we could just target the standard but unless we are willing to draw a
> > line and say no more non std=c11 and also we potentially break the abi we
> > are talking years. i don't think it is reasonable to block progress for years, so
> > i'm offering a transitional path. it's an evolution over time that we have to
> > manage.
> Apologies if I am sounding like I am blocking progress. Rest assured, we will find a way. It is just about which solution we are going to pick.

no problems, i really appreciate any help.

> Also, is there are any information on how long before we move to C11?

we need to clear all long term compatibility promises for gcc/linux
platforms that don't support -std=c11 and implement stdatomic option. the
last discussion was that it was years i believe.

Bruce has a patch series and another thread going talking about moving
to -std=c99 which is good but of course doesn't get us to std=c99.

> 
> > 
> > >
> > > >
> > > > i feel like we are really close on this discussion, if we can just
> > > > iron this issue out we can probably get going on the actual changes.
> > > >
> > > > thanks for the consideration.
> > > >
> > > > >
> > > > > >
> > > > > > once in place it provides an opportunity to introduce new
> > > > > > toolchain/platform combinations and enables an opt-in capability
> > > > > > to use stdatomics on existing toolchain/platform combinations
> > > > > > subject to community discussion on how/if/when.
> > > > > >
> > > > > > it would be good to get more participants into the discussion so
> > > > > > i'll cc techboard for some attention. i feel like the only area
> > > > > > that isn't decided is to do or not do this in rte_ namespace.
> > > > > >
> > > > > > i'm strongly in favor of rte_ namespace after discussion, mainly
> > > > > > due to to disadvantages of trying to overlap with the standard
> > > > > > namespace while not providing a compatible api/abi and because
> > > > > > it provides clear disambiguation of that difference in semantics
> > > > > > and compatibility with
> > > > the standard api.
> > > > > >
> > > > > > so far i've noted the following
> > > > > >
> > > > > > * we will not provide the non-explicit apis.
> > > > > +1
> > > > >
> > > > > > * we will make no attempt to support operate on struct/union atomics
> > > > > >   with our apis.
> > > > > +1
> > > > >
> > > > > > * we will mirror the standard api potentially in the rte_ namespace to
> > > > > >   - reference the standard api documentation.
> > > > > >   - assume compatible semantics (sans exceptions from first 2 points).
> > > > > >
> > > > > > my vote is to remove 'potentially' from the last point above for
> > > > > > reasons previously discussed in postings to the mail thread.
> > > > > >
> > > > > > thanks all for the discussion, i'll send up a patch removing
> > > > > > non-explicit apis for viewing.
> > > > > >
> > > > > > ty