[v6,0/6] rte atomics API for optional stdatomic

  This series introduces API additions prefixed in the rte namespace that allow
the optional use of stdatomics.h from C11 using enable_stdatomics=true for
targets where enable_stdatomics=false no functional change is intended.

Be aware this does not contain all changes to use stdatomics across the DPDK
tree it only introduces the minimum to allow the option to be used which is
a pre-requisite for a clean CI (probably using clang) that can be run
with enable_stdatomics=true enabled.

It is planned that subsequent series will be introduced per lib/driver as
appropriate to further enable stdatomics use when enable_stdatomics=true.

Notes:

* Additional libraries beyond EAL make visible atomics use across the
  API/ABI surface they will be converted in the subsequent series.

* The eal: add rte atomic qualifier with casts patch needs some discussion
  as to whether or not the legacy rte_atomic APIs should be converted to
  work with enable_stdatomic=true right now some implementation dependent
  casts are used to prevent cascading / having to convert too much in
  the intial series.

* Windows will obviously need complete conversion of libraries including
  atomics that are not crossing API/ABI boundaries. those conversions will
  introduced in separate series as new along side the existing msvc series.

Please keep in mind we would like to prioritize the review / acceptance of
this patch since it needs to be completed in the 23.11 merge window.

Thank you all for the discussion that lead to the formation of this series.

v6:
  * Adjust checkpatches to warn about use of __rte_atomic_thread_fence
    and suggest use of rte_atomic_thread_fence. Use the existing check
    more generic check for __atomic_xxx to catch use of __atomic_thread_fence
    and recommend rte_atomic_xxx.

v5:
  * Add RTE_ATOMIC to doxygen configuration PREDEFINED macros list to
    fix documentation generation failure
  * Fix two typos in expansion of C11 atomics macros strong -> weak and
    add missing _explicit
  * Adjust devtools/checkpatches messages based on feedback. i have chosen
    not to try and catch use of C11 atomics or _Atomic since using those
    directly will be picked up by existing CI passes where by compilation
    error where enable_stdatomic=false (the default for most platforms)

v4:
  * Move the definition of #define RTE_ATOMIC(type) to patch 1 where it
    belongs (a mistake in v3)
  * Provide comments for both RTE_ATOMIC and __rte_atomic macros indicating
    their use as specified or qualified contexts.

v3:
  * Remove comments from APIs mentioning the mapping to C++ memory model
    memory orders
  * Introduce and use new macro RTE_ATOMIC(type) to be used in contexts
    where _Atomic is used as a type specifier to declare variables. The
    macro allows more clarity about what the atomic type being specified
    is. e.g. _Atomic(T *) vs _Atomic(T) it is easier to understand that
    the former is an atomic pointer type and the latter is an atomic
    type. it also has the benefit of (in the future) being interoperable
    with c++23 syntactically
    note: Morten i have retained your 'reviewed-by' tags if you disagree
    given the changes in the above version please indicate as such but
    i believe the changes are in the spirit of the feedback you provided

v2:
  * Wrap meson_options.txt option description to newline and indent to
    be consistent with other options.
  * Provide separate typedef of rte_memory_order for enable_stdatomic=true
    VS enable_stdatomic=false instead of a single typedef to int
    note: slight tweak to reviewers feedback i've chosen to use a typedef
          for both enable_stdatomic={true,false} (just seemed more consistent)
  * Bring in assert.h and use static_assert macro instead of _Static_assert
    keyword to better interoperate with c/c++
  * Directly include rte_stdatomic.h where into other places it is consumed
    instead of hacking it globally into rte_config.h
  * Provide and use __rte_atomic_thread_fence to allow conditional expansion
    within the body of existing rte_atomic_thread_fence inline function to
    maintain per-arch optimizations when enable_stdatomic=false

Tyler Retzlaff (6):
  eal: provide rte stdatomics optional atomics API
  eal: adapt EAL to present rte optional atomics API
  eal: add rte atomic qualifier with casts
  distributor: adapt for EAL optional atomics API changes
  bpf: adapt for EAL optional atomics API changes
  devtools: forbid new direct use of GCC atomic builtins

 app/test/test_mcslock.c                  |   6 +-
 config/meson.build                       |   1 +
 devtools/checkpatches.sh                 |  12 +-
 doc/api/doxy-api.conf.in                 |   1 +
 lib/bpf/bpf_pkt.c                        |   6 +-
 lib/distributor/distributor_private.h    |   2 +-
 lib/distributor/rte_distributor_single.c |  44 +++----
 lib/eal/arm/include/rte_atomic_32.h      |   4 +-
 lib/eal/arm/include/rte_atomic_64.h      |  36 +++---
 lib/eal/arm/include/rte_pause_64.h       |  26 ++--
 lib/eal/arm/rte_power_intrinsics.c       |   8 +-
 lib/eal/common/eal_common_trace.c        |  16 +--
 lib/eal/include/generic/rte_atomic.h     |  67 +++++++----
 lib/eal/include/generic/rte_pause.h      |  50 ++++----
 lib/eal/include/generic/rte_rwlock.h     |  48 ++++----
 lib/eal/include/generic/rte_spinlock.h   |  20 ++--
 lib/eal/include/meson.build              |   1 +
 lib/eal/include/rte_mcslock.h            |  51 ++++----
 lib/eal/include/rte_pflock.h             |  25 ++--
 lib/eal/include/rte_seqcount.h           |  19 +--
 lib/eal/include/rte_stdatomic.h          | 198 +++++++++++++++++++++++++++++++
 lib/eal/include/rte_ticketlock.h         |  43 +++----
 lib/eal/include/rte_trace_point.h        |   5 +-
 lib/eal/loongarch/include/rte_atomic.h   |   4 +-
 lib/eal/ppc/include/rte_atomic.h         |  54 ++++-----
 lib/eal/riscv/include/rte_atomic.h       |   4 +-
 lib/eal/x86/include/rte_atomic.h         |   8 +-
 lib/eal/x86/include/rte_spinlock.h       |   2 +-
 lib/eal/x86/rte_power_intrinsics.c       |   7 +-
 meson_options.txt                        |   2 +
 30 files changed, 501 insertions(+), 269 deletions(-)
 create mode 100644 lib/eal/include/rte_stdatomic.h
  

ping for additional reviewers.

thanks!

On Tue, Aug 22, 2023 at 02:00:39PM -0700, Tyler Retzlaff wrote:
> This series introduces API additions prefixed in the rte namespace that allow
> the optional use of stdatomics.h from C11 using enable_stdatomics=true for
> targets where enable_stdatomics=false no functional change is intended.
> 
> Be aware this does not contain all changes to use stdatomics across the DPDK
> tree it only introduces the minimum to allow the option to be used which is
> a pre-requisite for a clean CI (probably using clang) that can be run
> with enable_stdatomics=true enabled.
> 
> It is planned that subsequent series will be introduced per lib/driver as
> appropriate to further enable stdatomics use when enable_stdatomics=true.
> 
> Notes:
> 
> * Additional libraries beyond EAL make visible atomics use across the
>   API/ABI surface they will be converted in the subsequent series.
> 
> * The eal: add rte atomic qualifier with casts patch needs some discussion
>   as to whether or not the legacy rte_atomic APIs should be converted to
>   work with enable_stdatomic=true right now some implementation dependent
>   casts are used to prevent cascading / having to convert too much in
>   the intial series.
> 
> * Windows will obviously need complete conversion of libraries including
>   atomics that are not crossing API/ABI boundaries. those conversions will
>   introduced in separate series as new along side the existing msvc series.
> 
> Please keep in mind we would like to prioritize the review / acceptance of
> this patch since it needs to be completed in the 23.11 merge window.
> 
> Thank you all for the discussion that lead to the formation of this series.
> 
> v6:
>   * Adjust checkpatches to warn about use of __rte_atomic_thread_fence
>     and suggest use of rte_atomic_thread_fence. Use the existing check
>     more generic check for __atomic_xxx to catch use of __atomic_thread_fence
>     and recommend rte_atomic_xxx.
> 
> v5:
>   * Add RTE_ATOMIC to doxygen configuration PREDEFINED macros list to
>     fix documentation generation failure
>   * Fix two typos in expansion of C11 atomics macros strong -> weak and
>     add missing _explicit
>   * Adjust devtools/checkpatches messages based on feedback. i have chosen
>     not to try and catch use of C11 atomics or _Atomic since using those
>     directly will be picked up by existing CI passes where by compilation
>     error where enable_stdatomic=false (the default for most platforms)
> 
> v4:
>   * Move the definition of #define RTE_ATOMIC(type) to patch 1 where it
>     belongs (a mistake in v3)
>   * Provide comments for both RTE_ATOMIC and __rte_atomic macros indicating
>     their use as specified or qualified contexts.
> 
> v3:
>   * Remove comments from APIs mentioning the mapping to C++ memory model
>     memory orders
>   * Introduce and use new macro RTE_ATOMIC(type) to be used in contexts
>     where _Atomic is used as a type specifier to declare variables. The
>     macro allows more clarity about what the atomic type being specified
>     is. e.g. _Atomic(T *) vs _Atomic(T) it is easier to understand that
>     the former is an atomic pointer type and the latter is an atomic
>     type. it also has the benefit of (in the future) being interoperable
>     with c++23 syntactically
>     note: Morten i have retained your 'reviewed-by' tags if you disagree
>     given the changes in the above version please indicate as such but
>     i believe the changes are in the spirit of the feedback you provided
> 
> v2:
>   * Wrap meson_options.txt option description to newline and indent to
>     be consistent with other options.
>   * Provide separate typedef of rte_memory_order for enable_stdatomic=true
>     VS enable_stdatomic=false instead of a single typedef to int
>     note: slight tweak to reviewers feedback i've chosen to use a typedef
>           for both enable_stdatomic={true,false} (just seemed more consistent)
>   * Bring in assert.h and use static_assert macro instead of _Static_assert
>     keyword to better interoperate with c/c++
>   * Directly include rte_stdatomic.h where into other places it is consumed
>     instead of hacking it globally into rte_config.h
>   * Provide and use __rte_atomic_thread_fence to allow conditional expansion
>     within the body of existing rte_atomic_thread_fence inline function to
>     maintain per-arch optimizations when enable_stdatomic=false
> 
> Tyler Retzlaff (6):
>   eal: provide rte stdatomics optional atomics API
>   eal: adapt EAL to present rte optional atomics API
>   eal: add rte atomic qualifier with casts
>   distributor: adapt for EAL optional atomics API changes
>   bpf: adapt for EAL optional atomics API changes
>   devtools: forbid new direct use of GCC atomic builtins
> 
>  app/test/test_mcslock.c                  |   6 +-
>  config/meson.build                       |   1 +
>  devtools/checkpatches.sh                 |  12 +-
>  doc/api/doxy-api.conf.in                 |   1 +
>  lib/bpf/bpf_pkt.c                        |   6 +-
>  lib/distributor/distributor_private.h    |   2 +-
>  lib/distributor/rte_distributor_single.c |  44 +++----
>  lib/eal/arm/include/rte_atomic_32.h      |   4 +-
>  lib/eal/arm/include/rte_atomic_64.h      |  36 +++---
>  lib/eal/arm/include/rte_pause_64.h       |  26 ++--
>  lib/eal/arm/rte_power_intrinsics.c       |   8 +-
>  lib/eal/common/eal_common_trace.c        |  16 +--
>  lib/eal/include/generic/rte_atomic.h     |  67 +++++++----
>  lib/eal/include/generic/rte_pause.h      |  50 ++++----
>  lib/eal/include/generic/rte_rwlock.h     |  48 ++++----
>  lib/eal/include/generic/rte_spinlock.h   |  20 ++--
>  lib/eal/include/meson.build              |   1 +
>  lib/eal/include/rte_mcslock.h            |  51 ++++----
>  lib/eal/include/rte_pflock.h             |  25 ++--
>  lib/eal/include/rte_seqcount.h           |  19 +--
>  lib/eal/include/rte_stdatomic.h          | 198 +++++++++++++++++++++++++++++++
>  lib/eal/include/rte_ticketlock.h         |  43 +++----
>  lib/eal/include/rte_trace_point.h        |   5 +-
>  lib/eal/loongarch/include/rte_atomic.h   |   4 +-
>  lib/eal/ppc/include/rte_atomic.h         |  54 ++++-----
>  lib/eal/riscv/include/rte_atomic.h       |   4 +-
>  lib/eal/x86/include/rte_atomic.h         |   8 +-
>  lib/eal/x86/include/rte_spinlock.h       |   2 +-
>  lib/eal/x86/rte_power_intrinsics.c       |   7 +-
>  meson_options.txt                        |   2 +
>  30 files changed, 501 insertions(+), 269 deletions(-)
>  create mode 100644 lib/eal/include/rte_stdatomic.h
> 
> -- 
> 1.8.3.1
  

Hello,

On Tue, Aug 22, 2023 at 11:00 PM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
>
> This series introduces API additions prefixed in the rte namespace that allow
> the optional use of stdatomics.h from C11 using enable_stdatomics=true for
> targets where enable_stdatomics=false no functional change is intended.
>
> Be aware this does not contain all changes to use stdatomics across the DPDK
> tree it only introduces the minimum to allow the option to be used which is
> a pre-requisite for a clean CI (probably using clang) that can be run
> with enable_stdatomics=true enabled.
>
> It is planned that subsequent series will be introduced per lib/driver as
> appropriate to further enable stdatomics use when enable_stdatomics=true.
>
> Notes:
>
> * Additional libraries beyond EAL make visible atomics use across the
>   API/ABI surface they will be converted in the subsequent series.
>
> * The eal: add rte atomic qualifier with casts patch needs some discussion
>   as to whether or not the legacy rte_atomic APIs should be converted to
>   work with enable_stdatomic=true right now some implementation dependent
>   casts are used to prevent cascading / having to convert too much in
>   the intial series.
>
> * Windows will obviously need complete conversion of libraries including
>   atomics that are not crossing API/ABI boundaries. those conversions will
>   introduced in separate series as new along side the existing msvc series.
>
> Please keep in mind we would like to prioritize the review / acceptance of
> this patch since it needs to be completed in the 23.11 merge window.
>
> Thank you all for the discussion that lead to the formation of this series.

I did a number of updates on this v6:
- moved rte_stdatomic.h from patch 1 to later patches where needed,
- added a RN entry,
- tried to consistently/adjusted indent,
- fixed mentions of stdatomic*s* to simple atomic, like in the build
option name,
- removed unneded comments (Thomas review on patch 1),

Series applied, thanks Tyler.


Two things are missing:
- add doxygen tags in the new API (this can be fixed later in this
release, can you look at it?),
- add compilation tests for enable_stdatomic (I'll send a patch soon
for devtools and GHA),