From: Willy Tarreau Date: Tue, 6 Apr 2021 09:57:41 +0000 (+0200) Subject: CLEANUP: atomic: add a fetch-and-xxx variant for common operations X-Git-Tag: v2.4-dev16~15 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=185157201c3cf1becc95a195b7e4bf2a1eb7ff58;p=thirdparty%2Fhaproxy.git 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 edf4cf5861..e31c87462f 100644 --- 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 a8f2274b26..407ce424d2 100644 --- 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 21a75a7502..3db78bdeb7 100644 --- 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 1b2cd2e477..80eba92ae9 100644 --- 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 702082bdaa..5320105c5c 100644 --- 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; }