From b7ba1d901174cb1193033f7d967987ef74e89856 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 21 Oct 2020 11:04:08 +0200 Subject: [PATCH] OPTIM: queue: don't call pendconn_unlink() when the pendconn is not queued On connection error processing, we can see massive storms of calls to pendconn_cond_unlink() to release a possible place in the queue. For example, in issue #908, on average half of the threads are caught in this function via back_try_conn_req() consecutive to a synchronous error. However we wait until grabbing the lock to know if the pendconn is effectively in a queue, which is expensive for many cases. We know the transition may only happen from in-queue to out-of-queue so it's safe to first run a preliminary check to see if it's worth going further. This will allow to avoid the cost of locking for most requests. This should not change anything for those completing correctly as they're already run through pendconn_free() which doesn't call pendconn_cond_unlink() unless deemed necessary. --- include/haproxy/queue.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/include/haproxy/queue.h b/include/haproxy/queue.h index 5cc571d..dcc3888 100644 --- a/include/haproxy/queue.h +++ b/include/haproxy/queue.h @@ -44,10 +44,15 @@ void pendconn_unlink(struct pendconn *p); * with NULL for pendconn and with a pendconn not in the list. It is the * function to be used by default when unsure. Do not call it with server * or proxy locks held however. + * + * The unlocked check on leaf_p is safe because one must own the pendconn to + * add it, thus it may only switch from non-null to null, which allows to first + * test if it's worth taking the lock to go further. This test is performed + * again under a lock in pendconn_unlink() to get the final verdict. */ static inline void pendconn_cond_unlink(struct pendconn *p) { - if (p) + if (p && p->node.node.leaf_p) pendconn_unlink(p); } -- 1.7.10.4