From: Willy Tarreau <w@1wt.eu>
Date: Wed, 25 Jul 2018 09:13:53 +0000 (+0200)
Subject: MEDIUM: queue: make pendconn_free() work on the stream instead
X-Git-Tag: v1.9-dev1~25
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d0ad4a87f03f5dafa9588f711788c15ba9eb85ff;p=thirdparty%2Fhaproxy.git

MEDIUM: queue: make pendconn_free() work on the stream instead

Now pendconn_free() takes a stream, checks that pend_pos is set, clears
it, and uses pendconn_unlink() to complete the job. It's cleaner and
centralizes all the bookkeeping work in pendconn_unlink() only and
ensures that there's a single place where the stream's position in the
queue is manipulated.
---

diff --git a/include/proto/queue.h b/include/proto/queue.h
index 98e1269e46..11696dbc47 100644
--- a/include/proto/queue.h
+++ b/include/proto/queue.h
@@ -39,7 +39,6 @@ extern struct pool_head *pool_head_pendconn;
 int init_pendconn();
 struct pendconn *pendconn_add(struct stream *strm);
 int pendconn_dequeue(struct stream *strm);
-void pendconn_free(struct pendconn *p);
 void process_srv_queue(struct server *s);
 unsigned int srv_dynamic_maxconn(const struct server *s);
 int pendconn_redistribute(struct server *s);
@@ -57,6 +56,26 @@ static inline void pendconn_cond_unlink(struct pendconn *p)
 		pendconn_unlink(p);
 }
 
+/* Releases the pendconn associated to stream <s> if it has any, and decreases
+ * the pending count if needed. The connection might have been queued to a
+ * specific server as well as to the proxy. The stream also gets marked
+ * unqueued.
+ *
+ * This function must be called by the stream itself, so in the context of
+ * process_stream, without any lock held among the pendconn, the server's queue
+ * nor the proxy's queue.
+ */
+static inline void pendconn_free(struct stream *s)
+{
+	struct pendconn *p = s->pend_pos;
+
+	if (p) {
+		pendconn_cond_unlink(p);
+		s->pend_pos = NULL;
+		pool_free(pool_head_pendconn, 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) {
 	return !s->maxconn || s->cur_sess < srv_dynamic_maxconn(s);
diff --git a/src/proto_http.c b/src/proto_http.c
index 6fa38dd232..42c9d1a622 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -4429,8 +4429,7 @@ void http_end_txn_clean_session(struct stream *s)
 	s->logs.bytes_in = s->req.total = ci_data(&s->req);
 	s->logs.bytes_out = s->res.total = ci_data(&s->res);
 
-	if (s->pend_pos)
-		pendconn_free(s->pend_pos);
+	pendconn_free(s);
 
 	if (objt_server(s->target)) {
 		if (s->flags & SF_CURR_SESS) {
diff --git a/src/queue.c b/src/queue.c
index b0815959c1..c4f7366412 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -386,44 +386,6 @@ int pendconn_dequeue(struct stream *strm)
 	return 0;
 }
 
-/* Release the pending connection <p>, and decreases the pending count if
- * needed. The connection might have been queued to a specific server as well as
- * to the proxy. The stream also gets marked unqueued. <p> must always be
- * defined here. So it is the caller responsibility to check its existance.
- *
- * This function must be called by the stream itself, so in the context of
- * process_stream.
- */
-void pendconn_free(struct pendconn *p)
-{
-	struct stream *strm = p->strm;
-
-	HA_SPIN_LOCK(PENDCONN_LOCK, &p->lock);
-
-	/* The pendconn was already unlinked, just release it. */
-	if (LIST_ISEMPTY(&p->list))
-		goto release;
-
-	if (p->srv) {
-		HA_SPIN_LOCK(SERVER_LOCK, &p->srv->lock);
-		p->srv->nbpend--;
-		LIST_DEL(&p->list);
-		HA_SPIN_UNLOCK(SERVER_LOCK, &p->srv->lock);
-	}
-	else {
-		HA_SPIN_LOCK(PROXY_LOCK, &p->px->lock);
-		p->px->nbpend--;
-		LIST_DEL(&p->list);
-		HA_SPIN_UNLOCK(PROXY_LOCK, &p->px->lock);
-	}
-	HA_ATOMIC_SUB(&p->px->totpend, 1);
-
-  release:
-	strm->pend_pos = NULL;
-	HA_SPIN_UNLOCK(PENDCONN_LOCK, &p->lock);
-	pool_free(pool_head_pendconn, p);
-}
-
 /*
  * Local variables:
  *  c-indent-level: 8
diff --git a/src/stream.c b/src/stream.c
index d07a92830f..d794c28a94 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -304,8 +304,7 @@ static void stream_free(struct stream *s)
 	int must_free_sess;
 	int i;
 
-	if (s->pend_pos)
-		pendconn_free(s->pend_pos);
+	pendconn_free(s);
 
 	if (objt_server(s->target)) { /* there may be requests left pending in queue */
 		if (s->flags & SF_CURR_SESS) {