From a4a9bbadc613dd21228fb776fe70e098c7c9b965 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 24 Jun 2021 07:22:12 +0200 Subject: [PATCH] Revert "MINOR: queue: use atomic-ops to update the queue's index" This reverts commit 1335eb9867b76c8e4570ad4242dc728287af3d56. The recent changes since 5304669e1 MEDIUM: queue: make pendconn_process_next_strm() only return the pendconn opened a tiny race condition between stream_free() and process_srv_queue(), as the pendconn is accessed outside of the lock, possibly while it's being freed. A different approach is required. --- src/queue.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/queue.c b/src/queue.c index 20880a239..920891122 100644 --- a/src/queue.c +++ b/src/queue.c @@ -133,7 +133,7 @@ unsigned int srv_dynamic_maxconn(const struct server *s) */ static void __pendconn_unlink_srv(struct pendconn *p) { - p->strm->logs.srv_queue_pos += _HA_ATOMIC_LOAD(&p->srv->queue.idx) - p->queue_idx; + p->strm->logs.srv_queue_pos += p->srv->queue.idx - p->queue_idx; eb32_delete(&p->node); } @@ -146,7 +146,7 @@ static void __pendconn_unlink_srv(struct pendconn *p) */ static void __pendconn_unlink_prx(struct pendconn *p) { - p->strm->logs.prx_queue_pos += _HA_ATOMIC_LOAD(&p->px->queue.idx) - p->queue_idx; + p->strm->logs.prx_queue_pos += p->px->queue.idx - p->queue_idx; eb32_delete(&p->node); } @@ -313,16 +313,16 @@ static struct pendconn *pendconn_process_next_strm(struct server *srv, struct pr use_pp: /* Let's switch from the server pendconn to the proxy pendconn */ __pendconn_unlink_prx(pp); - _HA_ATOMIC_INC(&px->queue.idx); _HA_ATOMIC_DEC(&px->queue.length); _HA_ATOMIC_DEC(&px->totpend); + px->queue.idx++; p = pp; goto unlinked; use_p: __pendconn_unlink_srv(p); - _HA_ATOMIC_INC(&srv->queue.idx); _HA_ATOMIC_DEC(&srv->queue.length); _HA_ATOMIC_DEC(&px->totpend); + srv->queue.idx++; unlinked: p->strm_flags |= SF_ASSIGNED; p->target = srv; @@ -422,7 +422,6 @@ struct pendconn *pendconn_add(struct stream *strm) max_ptr = &px->be_counters.nbpend_max; } - p->queue_idx = _HA_ATOMIC_LOAD(&q->idx) - 1; // for logging only new_max = _HA_ATOMIC_ADD_FETCH(&q->length, 1); old_max = _HA_ATOMIC_LOAD(max_ptr); while (new_max > old_max) { @@ -432,6 +431,7 @@ struct pendconn *pendconn_add(struct stream *strm) __ha_barrier_atomic_store(); HA_SPIN_LOCK(QUEUE_LOCK, &q->lock); + p->queue_idx = q->idx - 1; // for increment eb32_insert(&q->head, &p->node); HA_SPIN_UNLOCK(QUEUE_LOCK, &q->lock);