From 78fa54863dcd6c2f26e467c4cd9dfcc96c0b5f7d Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 17 Aug 2023 08:59:15 +0200 Subject: [PATCH] MINOR: atomic: make sure to always relax after a failed CAS There were a few places left where we forgot to call __ha_cpu_relax() after a failed CAS, in the HA_ATOMIC_UPDATE_{MIN,MAX} macros, and in a few sync_* API macros (the same as above plus HA_ATOMIC_CAS and HA_ATOMIC_XCHG). Let's add them now. This could have been a cause of contention, particularly with process_stream() calling stream_update_time_stats() which uses 8 of them in a call (4 for the server, 4 for the proxy). This may be a possible explanation for the high CPU consumption reported in GH issue #2251. This should be backported at least to 2.6 as it's harmless. --- include/haproxy/atomic.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/include/haproxy/atomic.h b/include/haproxy/atomic.h index 7e3c826ed..d64e19289 100644 --- a/include/haproxy/atomic.h +++ b/include/haproxy/atomic.h @@ -210,7 +210,7 @@ typeof(*(val)) __old_store; \ typeof((new)) __new_store = (new); \ do { __old_store = *__val_store; \ - } while (!__sync_bool_compare_and_swap(__val_store, __old_store, __new_store)); \ + } while (!__sync_bool_compare_and_swap(__val_store, __old_store, __new_store) && __ha_cpu_relax()); \ }) #define HA_ATOMIC_XCHG(val, new) \ @@ -219,7 +219,7 @@ typeof(*(val)) __old_xchg; \ typeof((new)) __new_xchg = (new); \ do { __old_xchg = *__val_xchg; \ - } while (!__sync_bool_compare_and_swap(__val_xchg, __old_xchg, __new_xchg)); \ + } while (!__sync_bool_compare_and_swap(__val_xchg, __old_xchg, __new_xchg) && __ha_cpu_relax()); \ __old_xchg; \ }) @@ -270,7 +270,7 @@ do { \ __oldv_cas = *__val_cas; \ __ret_cas = __sync_bool_compare_and_swap(__val_cas, *__oldp_cas, __new_cas); \ - } while (!__ret_cas && *__oldp_cas == __oldv_cas); \ + } while (!__ret_cas && *__oldp_cas == __oldv_cas && __ha_cpu_relax()); \ if (!__ret_cas) \ *__oldp_cas = __oldv_cas; \ __ret_cas; \ @@ -286,7 +286,7 @@ typeof(*(val)) __new_max = (new); \ \ while (__old_max < __new_max && \ - !HA_ATOMIC_CAS(__val, &__old_max, __new_max)); \ + !HA_ATOMIC_CAS(__val, &__old_max, __new_max) && __ha_cpu_relax()); \ *__val; \ }) @@ -297,7 +297,7 @@ typeof(*(val)) __new_min = (new); \ \ while (__old_min > __new_min && \ - !HA_ATOMIC_CAS(__val, &__old_min, __new_min)); \ + !HA_ATOMIC_CAS(__val, &__old_min, __new_min) && __ha_cpu_relax()); \ *__val; \ }) @@ -405,7 +405,7 @@ typeof(*(val)) __new_max = (new); \ \ while (__old_max < __new_max && \ - !HA_ATOMIC_CAS(__val, &__old_max, __new_max)); \ + !HA_ATOMIC_CAS(__val, &__old_max, __new_max) && __ha_cpu_relax()); \ *__val; \ }) @@ -416,7 +416,7 @@ typeof(*(val)) __new_min = (new); \ \ while (__old_min > __new_min && \ - !HA_ATOMIC_CAS(__val, &__old_min, __new_min)); \ + !HA_ATOMIC_CAS(__val, &__old_min, __new_min) && __ha_cpu_relax()); \ *__val; \ })