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.
This commit is contained in:
Willy Tarreau 2021-04-06 11:57:41 +02:00
parent a477150fd7
commit 185157201c
5 changed files with 63 additions and 17 deletions

View File

@ -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 */

View File

@ -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");

View File

@ -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;

View File

@ -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;

View File

@ -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;
}