From 9624faec8653121386fdf986540f33676b00f5fc Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
Date: Wed, 25 Jul 2018 08:04:20 +0200
Subject: [PATCH] MINOR: queue: centralize dequeuing code a bit better

For now the pendconns may be dequeued at two places :
  - pendconn_unlink(), which operates on a locked queue
  - pendconn_free(), which operates on an unlocked queue and frees
    everything.

Some changes are coming to the queue and we'll need to be able to be a
bit stricter regarding the places where we dequeue to keep the accounting
accurate. This first step renames the locked function __pendconn_unlink()
as it's for use by those aware of it, and introduces a new general purpose
pendconn_unlink() function which automatically grabs the necessary locks
before calling the former, and pendconn_cond_unlink() which additionally
checks the pointer and the presence in the queue.
---
 include/proto/queue.h | 12 ++++++++++++
 src/queue.c           | 40 ++++++++++++++++++++++++++++++++++++----
 2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/include/proto/queue.h b/include/proto/queue.h
index 2d4773a09..98e1269e4 100644
--- a/include/proto/queue.h
+++ b/include/proto/queue.h
@@ -44,6 +44,18 @@ void process_srv_queue(struct server *s);
 unsigned int srv_dynamic_maxconn(const struct server *s);
 int pendconn_redistribute(struct server *s);
 int pendconn_grab_from_px(struct server *s);
+void pendconn_unlink(struct pendconn *p);
+
+/* Removes the pendconn from the server/proxy queue. It supports being called
+ * 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.
+ */
+static inline void pendconn_cond_unlink(struct pendconn *p)
+{
+	if (p && !LIST_ISEMPTY(&p->list))
+		pendconn_unlink(p);
+}
 
 /* Returns 0 if all slots are full on a server, or 1 if there are slots available. */
 static inline int server_has_room(const struct server *s) {
diff --git a/src/queue.c b/src/queue.c
index 1c730c75c..b0815959c 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -69,7 +69,7 @@ unsigned int srv_dynamic_maxconn(const struct server *s)
  * The caller must own the lock on the pendconn _AND_ the queue containing the
  * pendconn. The pendconn must still be queued.
  */
-static void pendconn_unlink(struct pendconn *p)
+static void __pendconn_unlink(struct pendconn *p)
 {
 	if (p->srv)
 		p->srv->nbpend--;
@@ -80,6 +80,38 @@ static void pendconn_unlink(struct pendconn *p)
 	LIST_INIT(&p->list);
 }
 
+/* Removes the pendconn from the server/proxy queue. At this stage, the
+ * connection is not really dequeued. It will be done during process_stream().
+ * This function takes all the required locks for the operation. The caller is
+ * responsible for ensuring that <p> is valid and still in the queue. Use
+ * pendconn_cond_unlink() if unsure. When the locks are already held, please
+ * use __pendconn_unlink() instead.
+ */
+void pendconn_unlink(struct pendconn *p)
+{
+	struct server __maybe_unused *sv;
+	struct proxy __maybe_unused *px;
+
+	HA_SPIN_LOCK(PENDCONN_LOCK, &p->lock);
+
+	px = p->px;
+	sv = p->srv;
+
+	if (sv)
+		HA_SPIN_LOCK(SERVER_LOCK, &sv->lock);
+	else
+		HA_SPIN_LOCK(PROXY_LOCK, &px->lock);
+
+	__pendconn_unlink(p);
+
+	if (sv)
+		HA_SPIN_UNLOCK(SERVER_LOCK, &sv->lock);
+	else
+		HA_SPIN_UNLOCK(PROXY_LOCK, &px->lock);
+
+	HA_SPIN_UNLOCK(PENDCONN_LOCK, &p->lock);
+}
+
 /* Process the next pending connection from either a server or a proxy, and
  * returns a strictly positive value on success (see below). If no pending
  * connection is found, 0 is returned.  Note that neither <srv> nor <px> may be
@@ -142,7 +174,7 @@ static int pendconn_process_next_strm(struct server *srv, struct proxy *px)
 		return 0;
 
   pendconn_found:
-	pendconn_unlink(p);
+	__pendconn_unlink(p);
 	p->strm_flags |= SF_ASSIGNED;
 	p->srv = srv;
 
@@ -259,7 +291,7 @@ int pendconn_redistribute(struct server *s)
 			continue;
 
 		/* it's left to the dispatcher to choose a server */
-		pendconn_unlink(p);
+		__pendconn_unlink(p);
 		p->strm_flags &= ~(SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET);
 
 		remote |= !(p->strm->task->thread_mask & tid_bit);
@@ -296,7 +328,7 @@ int pendconn_grab_from_px(struct server *s)
 		if (HA_SPIN_TRYLOCK(PENDCONN_LOCK, &p->lock))
 			continue;
 
-		pendconn_unlink(p);
+		__pendconn_unlink(p);
 		p->srv = s;
 
 		remote |= !(p->strm->task->thread_mask & tid_bit);