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 edf4cf5..e31c874 100644
--- a/include/haproxy/atomic.h
+++ b/include/haproxy/atomic.h
@@ -83,14 +83,38 @@
#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) \
({ \
typeof((val)) __p_bts = (val); \
@@ -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 @@
#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 @@
#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 @@
#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 @@
#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 a8f2274..407ce42 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2501,7 +2501,7 @@
*/
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 @@
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 21a75a7..3db78bd 100644
--- a/src/log.c
+++ b/src/log.c
@@ -2194,7 +2194,7 @@
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 1b2cd2e..80eba92 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -2220,7 +2220,7 @@
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 702082b..5320105 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -415,7 +415,7 @@
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 @@
/* 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;
}