From 9dc231a6b23fc7d5cf3c233b46e00b9e251325b4 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Mon, 21 Nov 2022 14:32:33 +0100 Subject: [PATCH] BUG/MINOR: server/idle: at least use atomic stores when updating max_used_conns In 2.2, some idle conns usage metrics were added by commit cf612a045 ("MINOR: servers: Add a counter for the number of currently used connections."), which mentioned that the operation doesn't need to be atomic since we're not seeking exact values. This is true but at least we should use atomic stores to make sure not to cause invalid values to appear on archs that wouldn't guarantee atomicity when writing an int, such as writing two 16-bit words. This is pretty unlikely on our targets but better keep the code safe against this. This may be backported as far as 2.2. --- include/haproxy/server.h | 13 ++++++++----- src/server.c | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/include/haproxy/server.h b/include/haproxy/server.h index 723874484..85cf98f4e 100644 --- a/include/haproxy/server.h +++ b/include/haproxy/server.h @@ -264,18 +264,21 @@ static inline enum srv_initaddr srv_get_next_initaddr(unsigned int *list) static inline void srv_use_conn(struct server *srv, struct connection *conn) { - unsigned int curr; + unsigned int curr, prev; curr = _HA_ATOMIC_ADD_FETCH(&srv->curr_used_conns, 1); + /* It's ok not to do that atomically, we don't need an * exact max. */ - if (srv->max_used_conns < curr) - srv->max_used_conns = curr; + prev = HA_ATOMIC_LOAD(&srv->max_used_conns); + if (prev < curr) + HA_ATOMIC_STORE(&srv->max_used_conns, curr); - if (srv->est_need_conns < curr) - srv->est_need_conns = curr; + prev = HA_ATOMIC_LOAD(&srv->est_need_conns); + if (prev < curr) + HA_ATOMIC_STORE(&srv->est_need_conns, curr); } #endif /* _HAPROXY_SERVER_H */ diff --git a/src/server.c b/src/server.c index 3381c8eca..699113e88 100644 --- a/src/server.c +++ b/src/server.c @@ -5933,7 +5933,7 @@ struct task *srv_cleanup_idle_conns(struct task *task, void *context, unsigned i if (srv->est_need_conns < srv->max_used_conns) srv->est_need_conns = srv->max_used_conns; - srv->max_used_conns = srv->curr_used_conns; + HA_ATOMIC_STORE(&srv->max_used_conns, srv->curr_used_conns); if (exceed_conns <= 0) goto remove;