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.

diff --git a/include/haproxy/server.h b/include/haproxy/server.h
index 72387448485a3ceefabdadf1d424b12e33ac9647..85cf98f4ef95ce0785392c2e59058c3e04101f54 100644 (file)
--- 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 3381c8ecad124dedb7f8fc86b4f80b58df46aa42..699113e889c9dcdf24df6b19d05197796184cbee 100644 (file)
--- 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;