[ovs-dev] [PATCH] ovs-atomic: Add C++ compatible implementation.

Yi-Hung Wei yihung.wei at gmail.com
Tue Oct 17 00:08:34 UTC 2017


Thanks for the patch. I can verify that it works great with C++14 support.

One minor problem is that it may run into some compiler errors if the
C++ compiler does not support C++14 or the support is not enabled.
There was a patch that hit similar issue:
https://patchwork.ozlabs.org/patch/813112/

-Yi-Hung


On Mon, Oct 16, 2017 at 1:57 PM, Ben Pfaff <blp at ovn.org> wrote:
> G++ 5 does not implement the _Atomic keyword, which is part of C11 but not
> C++14, so the existing <stdatomic.h> based atomic implementation doesn't
> work.  This commit adds a new implementation based on the C++14 <atomic>
> header.
>
> In this area, C++ is pickier about types than C, so a few of the
> definitions in ovs-atomic.h have to be updated to use more precise types
> for integer constants.
>
> This updates the code that generates cxxtest.cc to #include <config.h>
> (so that HAVE_ATOMIC is defined) and to automatically regenerate when the
> program is reconfigured (because otherwise the #include <config.h>) won't
> get added without a "make clean" step).
>
> "ovs-atomic.h" is not a public header, but apparently some code was
> using it anyway.
>
> Reported-by: Yi-Hung Wei <yihung.wei at gmail.com>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
>  include/openvswitch/automake.mk | 10 ++++---
>  lib/automake.mk                 |  1 +
>  lib/ovs-atomic-c++.h            | 64 +++++++++++++++++++++++++++++++++++++++++
>  lib/ovs-atomic.h                | 14 +++++----
>  m4/openvswitch.m4               |  3 ++
>  5 files changed, 82 insertions(+), 10 deletions(-)
>  create mode 100644 lib/ovs-atomic-c++.h
>
> diff --git a/include/openvswitch/automake.mk b/include/openvswitch/automake.mk
> index eabeb1e971f3..6cb19b84e220 100644
> --- a/include/openvswitch/automake.mk
> +++ b/include/openvswitch/automake.mk
> @@ -41,10 +41,12 @@ if HAVE_CXX
>  # are acceptable as C++.
>  noinst_LTLIBRARIES += include/openvswitch/libcxxtest.la
>  nodist_include_openvswitch_libcxxtest_la_SOURCES = include/openvswitch/cxxtest.cc
> -include/openvswitch/cxxtest.cc: include/openvswitch/automake.mk
> -       $(AM_V_GEN)for header in $(openvswitchinclude_HEADERS); do      \
> -         echo $$header;                                                \
> -       done | sed 's,^include/\(.*\)$$,#include <\1>,' > $@
> +include/openvswitch/cxxtest.cc: \
> +       include/openvswitch/automake.mk $(top_builddir)/config.status
> +       $(AM_V_GEN){ echo "#include <config.h>"; \
> +       for header in $(openvswitchinclude_HEADERS); do \
> +         echo $$header; \
> +       done | sed 's,^include/\(.*\)$$,#include <\1>,'; } > $@
>  endif
>
>  # OVS does not use C++ itself, but it provides public header files
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 2415f4cd6c25..ca1cf5dd2524 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -159,6 +159,7 @@ lib_libopenvswitch_la_SOURCES = \
>         lib/ofp-version-opt.h \
>         lib/ofp-version-opt.c \
>         lib/ofpbuf.c \
> +       lib/ovs-atomic-c++.h \
>         lib/ovs-atomic-c11.h \
>         lib/ovs-atomic-clang.h \
>         lib/ovs-atomic-flag-gcc4.7+.h \
> diff --git a/lib/ovs-atomic-c++.h b/lib/ovs-atomic-c++.h
> new file mode 100644
> index 000000000000..5bb88536d799
> --- /dev/null
> +++ b/lib/ovs-atomic-c++.h
> @@ -0,0 +1,64 @@
> +/* This header implements atomic operation primitives on compilers that
> + * have built-in support for C11 <stdatomic.h>  */
> +#ifndef IN_OVS_ATOMIC_H
> +#error "This header should only be included indirectly via ovs-atomic.h."
> +#endif
> +
> +#include <atomic>
> +
> +#define ATOMIC(TYPE) std::atomic<TYPE>
> +
> +using std::atomic_init;
> +
> +using std::memory_order_relaxed;
> +using std::memory_order_consume;
> +using std::memory_order_acquire;
> +using std::memory_order_release;
> +using std::memory_order_acq_rel;
> +using std::memory_order_seq_cst;
> +
> +using std::atomic_thread_fence;
> +using std::atomic_signal_fence;
> +using std::atomic_is_lock_free;
> +
> +using std::atomic_store;
> +using std::atomic_store_explicit;
> +
> +using std::atomic_compare_exchange_strong;
> +using std::atomic_compare_exchange_strong_explicit;
> +using std::atomic_compare_exchange_weak;
> +using std::atomic_compare_exchange_weak_explicit;
> +
> +#define atomic_read(SRC, DST) \
> +    atomic_read_explicit(SRC, DST, memory_order_seq_cst)
> +#define atomic_read_explicit(SRC, DST, ORDER)   \
> +    (*(DST) = std::atomic_load_explicit(SRC, ORDER),    \
> +     (void) 0)
> +
> +#define atomic_add(RMW, ARG, ORIG) \
> +    atomic_add_explicit(RMW, ARG, ORIG, memory_order_seq_cst)
> +#define atomic_sub(RMW, ARG, ORIG) \
> +    atomic_sub_explicit(RMW, ARG, ORIG, memory_order_seq_cst)
> +#define atomic_or(RMW, ARG, ORIG) \
> +    atomic_or_explicit(RMW, ARG, ORIG, memory_order_seq_cst)
> +#define atomic_xor(RMW, ARG, ORIG) \
> +    atomic_xor_explicit(RMW, ARG, ORIG, memory_order_seq_cst)
> +#define atomic_and(RMW, ARG, ORIG) \
> +    atomic_and_explicit(RMW, ARG, ORIG, memory_order_seq_cst)
> +
> +#define atomic_add_explicit(RMW, ARG, ORIG, ORDER) \
> +    (*(ORIG) = std::atomic_fetch_add_explicit(RMW, ARG, ORDER), (void) 0)
> +#define atomic_sub_explicit(RMW, ARG, ORIG, ORDER) \
> +    (*(ORIG) = std::atomic_fetch_sub_explicit(RMW, ARG, ORDER), (void) 0)
> +#define atomic_or_explicit(RMW, ARG, ORIG, ORDER) \
> +    (*(ORIG) = std::atomic_fetch_or_explicit(RMW, ARG, ORDER), (void) 0)
> +#define atomic_xor_explicit(RMW, ARG, ORIG, ORDER) \
> +    (*(ORIG) = std::atomic_fetch_xor_explicit(RMW, ARG, ORDER), (void) 0)
> +#define atomic_and_explicit(RMW, ARG, ORIG, ORDER) \
> +    (*(ORIG) = std::atomic_fetch_and_explicit(RMW, ARG, ORDER), (void) 0)
> +
> +using std::atomic_flag;
> +using std::atomic_flag_test_and_set_explicit;
> +using std::atomic_flag_test_and_set;
> +using std::atomic_flag_clear_explicit;
> +using std::atomic_flag_clear;
> diff --git a/lib/ovs-atomic.h b/lib/ovs-atomic.h
> index d1b4e09e70d4..5e4a9c1775b3 100644
> --- a/lib/ovs-atomic.h
> +++ b/lib/ovs-atomic.h
> @@ -325,6 +325,8 @@
>          #include "ovs-atomic-pthreads.h"
>      #elif __has_extension(c_atomic)
>          #include "ovs-atomic-clang.h"
> +    #elif defined(__cplusplus) && HAVE_ATOMIC
> +        #include "ovs-atomic-c++.h"
>      #elif HAVE_STDATOMIC_H
>          #include "ovs-atomic-c11.h"
>      #elif __GNUC__ >= 5
> @@ -446,7 +448,7 @@ atomic_count_inc(atomic_count *count)
>  {
>      unsigned int old;
>
> -    atomic_add_relaxed(&count->count, 1, &old);
> +    atomic_add_relaxed(&count->count, 1u, &old);
>
>      return old;
>  }
> @@ -456,7 +458,7 @@ atomic_count_dec(atomic_count *count)
>  {
>      unsigned int old;
>
> -    atomic_sub_relaxed(&count->count, 1, &old);
> +    atomic_sub_relaxed(&count->count, 1u, &old);
>
>      return old;
>  }
> @@ -486,7 +488,7 @@ struct ovs_refcount {
>  static inline void
>  ovs_refcount_init(struct ovs_refcount *refcount)
>  {
> -    atomic_init(&refcount->count, 1);
> +    atomic_init(&refcount->count, 1u);
>  }
>
>  /* Increments 'refcount'.
> @@ -498,7 +500,7 @@ ovs_refcount_ref(struct ovs_refcount *refcount)
>  {
>      unsigned int old_refcount;
>
> -    atomic_add_explicit(&refcount->count, 1, &old_refcount,
> +    atomic_add_explicit(&refcount->count, 1u, &old_refcount,
>                          memory_order_relaxed);
>      ovs_assert(old_refcount > 0);
>  }
> @@ -521,7 +523,7 @@ ovs_refcount_unref(struct ovs_refcount *refcount)
>  {
>      unsigned int old_refcount;
>
> -    atomic_sub_explicit(&refcount->count, 1, &old_refcount,
> +    atomic_sub_explicit(&refcount->count, 1u, &old_refcount,
>                          memory_order_release);
>      ovs_assert(old_refcount > 0);
>      if (old_refcount == 1) {
> @@ -619,7 +621,7 @@ ovs_refcount_unref_relaxed(struct ovs_refcount *refcount)
>  {
>      unsigned int old_refcount;
>
> -    atomic_sub_explicit(&refcount->count, 1, &old_refcount,
> +    atomic_sub_explicit(&refcount->count, 1u, &old_refcount,
>                          memory_order_relaxed);
>      ovs_assert(old_refcount > 0);
>      return old_refcount;
> diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
> index 00ffad35f6b0..59e1352e3cf5 100644
> --- a/m4/openvswitch.m4
> +++ b/m4/openvswitch.m4
> @@ -632,6 +632,9 @@ AC_DEFUN([OVS_CHECK_CXX],
>     AX_CXX_COMPILE_STDCXX([11], [], [optional])
>     if test $enable_Werror = yes && test $HAVE_CXX11 = 1; then
>       enable_cxx=:
> +     AC_LANG_PUSH([C++])
> +     AC_CHECK_HEADERS([atomic])
> +     AC_LANG_POP([C++])
>     else
>       enable_cxx=false
>     fi
> --
> 2.10.2
>


More information about the dev mailing list