CLEANUP: atomic: add a fetch-and-xxx variant for common operations

The fetch_and_xxx variant is often missing for add/sub/and/or. In fact
it was only provided for ADD under the name XADD which corresponds to
the x86 instruction name. But for destructive operations like AND and
OR it's missing even more as it's not possible to know the value before
modifying it.

This patch explicitly adds HA_ATOMIC_FETCH_{OR,AND,ADD,SUB} which
cover these standard operations, and renames XADD to FETCH_ADD (there
were only 6 call places).

In the future, backport of fixes involving such operations could simply
remap FETCH_ADD(x) to XADD(x), FETCH_SUB(x) to XADD(-x), and for the
OR/AND if needed, these could possibly be done using BTS/BTR.

It's worth noting that xchg could have been renamed to fetch_and_store()
but xchg already has well understood semantics and it wasn't needed to
go further.

diff --git a/include/haproxy/atomic.h b/include/haproxy/atomic.h
index edf4cf58610180ecbb06e70d7b52b45a2fe380da..e31c87462fcb592af93e52574c09ae8cacb8e596 100644 (file)
--- a/include/haproxy/atomic.h
+++ b/include/haproxy/atomic.h
@@ -83,12 +83,36 @@
 #define HA_ATOMIC_ADD_FETCH(val, i)     ({ *(val) += (i); })
 #define HA_ATOMIC_SUB_FETCH(val, i)     ({ *(val) -= (i); })
 
-#define HA_ATOMIC_XADD(val, i)                                         \
+#define HA_ATOMIC_FETCH_AND(val, i)                                    \
        ({                                                              \
-               typeof((val)) __p_xadd = (val);                         \
-               typeof(*(val)) __old_xadd = *__p_xadd;                  \
-               *__p_xadd += i;                                         \
-               __old_xadd;                                             \
+               typeof((val)) __p_val = (val);                          \
+               typeof(*(val)) __old_val = *__p_val;                    \
+               *__p_val &= (i);                                        \
+               __old_val;                                              \
+       })
+
+#define HA_ATOMIC_FETCH_OR(val, i)                                     \
+       ({                                                              \
+               typeof((val)) __p_val = (val);                          \
+               typeof(*(val)) __old_val = *__p_val;                    \
+               *__p_val |= (i);                                        \
+               __old_val;                                              \
+       })
+
+#define HA_ATOMIC_FETCH_ADD(val, i)                                    \
+       ({                                                              \
+               typeof((val)) __p_val = (val);                          \
+               typeof(*(val)) __old_val = *__p_val;                    \
+               *__p_val += (i);                                        \
+               __old_val;                                              \
+       })
+
+#define HA_ATOMIC_FETCH_SUB(val, i)                                    \
+       ({                                                              \
+               typeof((val)) __p_val = (val);                          \
+               typeof(*(val)) __old_val = *__p_val;                    \
+               *__p_val -= (i);                                        \
+               __old_val;                                              \
        })
 
 #define HA_ATOMIC_BTS(val, bit)                                                \
@@ -208,7 +232,10 @@
 #define HA_ATOMIC_ADD_FETCH(val, i)     __sync_add_and_fetch(val, i)
 #define HA_ATOMIC_SUB_FETCH(val, i)     __sync_sub_and_fetch(val, i)
 
-#define HA_ATOMIC_XADD(val, i)       __sync_fetch_and_add(val, i)
+#define HA_ATOMIC_FETCH_AND(val, flags) __sync_fetch_and_and(val, flags)
+#define HA_ATOMIC_FETCH_OR(val, flags)  __sync_fetch_and_or(val,  flags)
+#define HA_ATOMIC_FETCH_ADD(val, i)     __sync_fetch_and_add(val, i)
+#define HA_ATOMIC_FETCH_SUB(val, i)     __sync_fetch_and_sub(val, i)
 
 #define HA_ATOMIC_BTS(val, bit)                                                \
        ({                                                              \
@@ -289,7 +316,10 @@
 #define HA_ATOMIC_ADD_FETCH(val, i)     __atomic_add_fetch(val, i, __ATOMIC_SEQ_CST)
 #define HA_ATOMIC_SUB_FETCH(val, i)     __atomic_sub_fetch(val, i, __ATOMIC_SEQ_CST)
 
-#define HA_ATOMIC_XADD(val, i)       __atomic_fetch_add(val, i, __ATOMIC_SEQ_CST)
+#define HA_ATOMIC_FETCH_AND(val, flags) __atomic_fetch_and(val, flags, __ATOMIC_SEQ_CST)
+#define HA_ATOMIC_FETCH_OR(val, flags)  __atomic_fetch_or(val,  flags, __ATOMIC_SEQ_CST)
+#define HA_ATOMIC_FETCH_ADD(val, i)     __atomic_fetch_add(val, i, __ATOMIC_SEQ_CST)
+#define HA_ATOMIC_FETCH_SUB(val, i)     __atomic_fetch_sub(val, i, __ATOMIC_SEQ_CST)
 
 #define HA_ATOMIC_BTS(val, bit)                                                \
        ({                                                              \
@@ -351,7 +381,11 @@
 #define _HA_ATOMIC_ADD_FETCH(val, i)     __atomic_add_fetch(val, i, __ATOMIC_RELAXED)
 #define _HA_ATOMIC_SUB_FETCH(val, i)     __atomic_sub_fetch(val, i, __ATOMIC_RELAXED)
 
-#define _HA_ATOMIC_XADD(val, i)       __atomic_fetch_add(val, i, __ATOMIC_RELAXED)
+#define _HA_ATOMIC_FETCH_AND(val, flags) __atomic_fetch_and(val, flags, __ATOMIC_RELAXED)
+#define _HA_ATOMIC_FETCH_OR(val, flags)  __atomic_fetch_or(val,  flags, __ATOMIC_RELAXED)
+#define _HA_ATOMIC_FETCH_ADD(val, i)     __atomic_fetch_add(val, i, __ATOMIC_RELAXED)
+#define _HA_ATOMIC_FETCH_SUB(val, i)     __atomic_fetch_sub(val, i, __ATOMIC_RELAXED)
+
 #define _HA_ATOMIC_CAS(val, old, new) __atomic_compare_exchange_n(val, old, new, 0, __ATOMIC_RELAXED, __ATOMIC_RELAXED)
 /* warning, n is a pointer to the double value for dwcas */
 #define _HA_ATOMIC_DWCAS(val, o, n)   __ha_cas_dw(val, o, n)
@@ -663,9 +697,9 @@ static inline void __ha_compiler_barrier(void)
 #define _HA_ATOMIC_ADD_FETCH HA_ATOMIC_ADD_FETCH
 #endif /* !_HA_ATOMIC_ADD_FETCH */
 
-#ifndef _HA_ATOMIC_XADD
-#define _HA_ATOMIC_XADD HA_ATOMIC_XADD
-#endif /* !_HA_ATOMIC_SUB */
+#ifndef _HA_ATOMIC_FETCH_ADD
+#define _HA_ATOMIC_FETCH_ADD HA_ATOMIC_FETCH_ADD
+#endif /* !_HA_ATOMIC_FETCH_ADD */
 
 #ifndef _HA_ATOMIC_SUB
 #define _HA_ATOMIC_SUB HA_ATOMIC_SUB
@@ -675,6 +709,10 @@ static inline void __ha_compiler_barrier(void)
 #define _HA_ATOMIC_SUB_FETCH HA_ATOMIC_SUB_FETCH
 #endif /* !_HA_ATOMIC_SUB_FETCH */
 
+#ifndef _HA_ATOMIC_FETCH_SUB
+#define _HA_ATOMIC_FETCH_SUB HA_ATOMIC_FETCH_SUB
+#endif /* !_HA_ATOMIC_FETCH_SUB */
+
 #ifndef _HA_ATOMIC_AND
 #define _HA_ATOMIC_AND HA_ATOMIC_AND
 #endif /* !_HA_ATOMIC_AND */
@@ -683,6 +721,10 @@ static inline void __ha_compiler_barrier(void)
 #define _HA_ATOMIC_AND_FETCH HA_ATOMIC_AND_FETCH
 #endif /* !_HA_ATOMIC_AND_FETCH */
 
+#ifndef _HA_ATOMIC_FETCH_AND
+#define _HA_ATOMIC_FETCH_AND HA_ATOMIC_FETCH_AND
+#endif /* !_HA_ATOMIC_FETCH_AND */
+
 #ifndef _HA_ATOMIC_OR
 #define _HA_ATOMIC_OR HA_ATOMIC_OR
 #endif /* !_HA_ATOMIC_OR */
@@ -691,6 +733,10 @@ static inline void __ha_compiler_barrier(void)
 #define _HA_ATOMIC_OR_FETCH HA_ATOMIC_OR_FETCH
 #endif /* !_HA_ATOMIC_OR_FETCH */
 
+#ifndef _HA_ATOMIC_FETCH_OR
+#define _HA_ATOMIC_FETCH_OR HA_ATOMIC_FETCH_OR
+#endif /* !_HA_ATOMIC_FETCH_OR */
+
 #ifndef _HA_ATOMIC_XCHG
 #define _HA_ATOMIC_XCHG HA_ATOMIC_XCHG
 #endif /* !_HA_ATOMIC_XCHG */

diff --git a/src/haproxy.c b/src/haproxy.c
index a8f2274b26921863e3c49e9b3dca10ff7813c624..407ce424d20566a29359ee0e1406932d9ff41d6c 100644 (file)
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2501,7 +2501,7 @@ static void *run_thread_poll_loop(void *data)
         */
        if (!(global.tune.options & GTUNE_INSECURE_SETUID) && !master) {
                static int warn_fail;
-               if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) == -1 && !_HA_ATOMIC_XADD(&warn_fail, 1)) {
+               if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) == -1 && !_HA_ATOMIC_FETCH_ADD(&warn_fail, 1)) {
                        ha_warning("Failed to disable setuid, please report to developers with detailed "
                                   "information about your operating system. You can silence this warning "
                                   "by adding 'insecure-setuid-wanted' in the 'global' section.\n");
@@ -2519,7 +2519,7 @@ static void *run_thread_poll_loop(void *data)
                struct rlimit limit = { .rlim_cur = 0, .rlim_max = 0 };
                static int warn_fail;
 
-               if (setrlimit(RLIMIT_NPROC, &limit) == -1 && !_HA_ATOMIC_XADD(&warn_fail, 1)) {
+               if (setrlimit(RLIMIT_NPROC, &limit) == -1 && !_HA_ATOMIC_FETCH_ADD(&warn_fail, 1)) {
                        ha_warning("Failed to disable forks, please report to developers with detailed "
                                   "information about your operating system. You can silence this warning "
                                   "by adding 'insecure-fork-wanted' in the 'global' section.\n");

diff --git a/src/log.c b/src/log.c
index 21a75a75029c5d6e09f513fc90ac07dfc5e62ed0..3db78bdeb769fd4c10772a40f329c01f2e239ae6 100644 (file)
--- a/src/log.c
+++ b/src/log.c
@@ -2194,7 +2194,7 @@ int sess_build_logline(struct session *sess, struct stream *s, char *dst, size_t
                be_conn = NULL;
                status = 0;
                s_flags = SF_ERR_PRXCOND | SF_FINST_R;
-               uniq_id = _HA_ATOMIC_XADD(&global.req_count, 1);
+               uniq_id = _HA_ATOMIC_FETCH_ADD(&global.req_count, 1);
 
                /* prepare a valid log structure */
                tmp_strm_log.tv_accept = sess->tv_accept;

diff --git a/src/proxy.c b/src/proxy.c
index 1b2cd2e477e8430b6f6c16984cfb1bc98b7b157e..80eba92ae96a2ed5da0e3db1bdb4a6ff44e00e9d 100644 (file)
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -2220,7 +2220,7 @@ void proxy_capture_error(struct proxy *proxy, int is_back,
        int len1, len2;
        unsigned int ev_id;
 
-       ev_id = HA_ATOMIC_XADD(&error_snapshot_id, 1);
+       ev_id = HA_ATOMIC_FETCH_ADD(&error_snapshot_id, 1);
 
        buf_len = b_data(buf) - buf_out;
 

diff --git a/src/stream.c b/src/stream.c
index 702082bdaaa986c3147845db673aa05b71a7e844..5320105c5ca5082950afb514ce83214a83cae0cb 100644 (file)
--- a/src/stream.c
+++ b/src/stream.c
@@ -415,7 +415,7 @@ struct stream *stream_new(struct session *sess, enum obj_type *origin, struct bu
        s->si[1].flags = SI_FL_ISBACK;
 
        s->stream_epoch = _HA_ATOMIC_LOAD(&stream_epoch);
-       s->uniq_id = _HA_ATOMIC_XADD(&global.req_count, 1);
+       s->uniq_id = _HA_ATOMIC_FETCH_ADD(&global.req_count, 1);
 
        /* OK, we're keeping the stream, so let's properly initialize the stream */
        LIST_INIT(&s->back_refs);
@@ -3376,7 +3376,7 @@ static int cli_parse_show_sess(char **args, char *payload, struct appctx *appctx
        /* let's set our own stream's epoch to the current one and increment
         * it so that we know which streams were already there before us.
         */
-       si_strm(appctx->owner)->stream_epoch = _HA_ATOMIC_XADD(&stream_epoch, 1);
+       si_strm(appctx->owner)->stream_epoch = _HA_ATOMIC_FETCH_ADD(&stream_epoch, 1);
        return 0;
 }