use reference counter for pipeline groups (v3)

Track pipeline groups using a shared reference counter
instead of a linked list.

(cherry picked from commit 31f392db20207a1b05d6286c3c56f76c8d69e574)
(cherry picked from commit 2211120222)

lib/ns/client.c

@@ -316,6 +316,75 @@ read_settimeout(ns_client_t *client, bool newconn) {
 	}
 }
 
+/*%
+ * Allocate a reference counter that will track the number of client structures
+ * using the TCP connection that 'client' called accept() for.  This counter
+ * will be shared between all client structures associated with this TCP
+ * connection.
+ */
+static void
+pipeline_init(ns_client_t *client) {
+	isc_refcount_t *refs;
+
+	REQUIRE(client->pipeline_refs == NULL);
+
+	/*
+	 * A global memory context is used for the allocation as different
+	 * client structures may have different memory contexts assigned and a
+	 * reference counter allocated here might need to be freed by a
+	 * different client.  The performance impact caused by memory context
+	 * contention here is expected to be negligible, given that this code
+	 * is only executed for TCP connections.
+	 */
+	refs = isc_mem_allocate(client->sctx->mctx, sizeof(*refs));
+	isc_refcount_init(refs, 1);
+	client->pipeline_refs = refs;
+}
+
+/*%
+ * Increase the count of client structures using the TCP connection that
+ * 'source' is associated with and put a pointer to that count in 'target',
+ * thus associating it with the same TCP connection.
+ */
+static void
+pipeline_attach(ns_client_t *source, ns_client_t *target) {
+	int old_refs;
+
+	REQUIRE(source->pipeline_refs != NULL);
+	REQUIRE(target->pipeline_refs == NULL);
+
+	old_refs = isc_refcount_increment(source->pipeline_refs);
+	INSIST(old_refs > 0);
+	target->pipeline_refs = source->pipeline_refs;
+}
+
+/*%
+ * Decrease the count of client structures using the TCP connection that
+ * 'client' is associated with.  If this is the last client using this TCP
+ * connection, free the reference counter and return true; otherwise, return
+ * false.
+ */
+static bool
+pipeline_detach(ns_client_t *client) {
+	isc_refcount_t *refs;
+	int old_refs;
+
+	REQUIRE(client->pipeline_refs != NULL);
+
+	refs = client->pipeline_refs;
+	client->pipeline_refs = NULL;
+
+	old_refs = isc_refcount_decrement(refs);
+	INSIST(old_refs > 0);
+
+	if (old_refs == 1) {
+		isc_mem_free(client->sctx->mctx, refs);
+		return (true);
+	}
+
+	return (false);
+}
+
 /*%
  * Check for a deactivation or shutdown request and take appropriate
  * action.  Returns true if either is in progress; in this case
@@ -438,6 +507,40 @@ exit_check(ns_client_t *client) {
 			client->tcpmsg_valid = false;
 		}
 
+		if (client->tcpquota != NULL) {
+			if (client->pipeline_refs == NULL ||
+			    pipeline_detach(client))
+			{
+				/*
+				 * Only detach from the TCP client quota if
+				 * there are no more client structures using
+				 * this TCP connection.
+				 *
+				 * Note that we check 'pipeline_refs' and not
+				 * 'pipelined' because in some cases (e.g.
+				 * after receiving a request with an opcode
+				 * different than QUERY) 'pipelined' is set to
+				 * false after the reference counter gets
+				 * allocated in pipeline_init() and we must
+				 * still drop our reference as failing to do so
+				 * would prevent the reference counter itself
+				 * from being freed.
+				 */
+				isc_quota_detach(&client->tcpquota);
+			} else {
+				/*
+				 * There are other client structures using this
+				 * TCP connection, so we cannot detach from the
+				 * TCP client quota to prevent excess TCP
+				 * connections from being accepted.  However,
+				 * this client structure might later be reused
+				 * for accepting new connections and thus must
+				 * have its 'tcpquota' field set to NULL.
+				 */
+				client->tcpquota = NULL;
+			}
+		}
+
 		if (client->tcpsocket != NULL) {
 			CTRACE("closetcp");
 			isc_socket_detach(&client->tcpsocket);
@@ -451,44 +554,6 @@ exit_check(ns_client_t *client) {
 			}
 		}
 
-		if (client->tcpquota != NULL) {
-			/*
-			 * If we are not in a pipeline group, or
-			 * we are the last client in the group, detach from
-			 * tcpquota; otherwise, transfer the quota to
-			 * another client in the same group.
-			 */
-			if (!ISC_LINK_LINKED(client, glink) ||
-			    (client->glink.next == NULL &&
-			     client->glink.prev == NULL))
-			{
-				isc_quota_detach(&client->tcpquota);
-			} else if (client->glink.next != NULL) {
-				INSIST(client->glink.next->tcpquota == NULL);
-				client->glink.next->tcpquota = client->tcpquota;
-				client->tcpquota = NULL;
-			} else {
-				INSIST(client->glink.prev->tcpquota == NULL);
-				client->glink.prev->tcpquota = client->tcpquota;
-				client->tcpquota = NULL;
-			}
-		}
-
-		/*
-		 * Unlink from pipeline group.
-		 */
-		if (ISC_LINK_LINKED(client, glink)) {
-			if (client->glink.next != NULL) {
-				client->glink.next->glink.prev =
-					client->glink.prev;
-			}
-			if (client->glink.prev != NULL) {
-				client->glink.prev->glink.next =
-					client->glink.next;
-			}
-			ISC_LINK_INIT(client, glink);
-		}
-
 		if (client->timerset) {
 			(void)isc_timer_reset(client->timer,
 					      isc_timertype_inactive,
@@ -3106,6 +3171,7 @@ client_create(ns_clientmgr_t *manager, ns_client_t **clientp) {
 	client->mortal = false;
 	client->sendcb = NULL;
 	client->pipelined = false;
+	client->pipeline_refs = NULL;
 	client->tcpquota = NULL;
 	client->recursionquota = NULL;
 	client->interface = NULL;
@@ -3127,7 +3193,6 @@ client_create(ns_clientmgr_t *manager, ns_client_t **clientp) {
 	client->formerrcache.id = 0;
 	ISC_LINK_INIT(client, link);
 	ISC_LINK_INIT(client, rlink);
-	ISC_LINK_INIT(client, glink);
 	ISC_QLINK_INIT(client, ilink);
 	client->keytag = NULL;
 	client->keytag_len = 0;
@@ -3316,6 +3381,7 @@ client_newconn(isc_task_t *task, isc_event_t *event) {
 		     !dns_acl_allowed(&netaddr, NULL,
 				     client->sctx->keepresporder, env)))
 		{
+			pipeline_init(client);
 			client->pipelined = true;
 		}
 
@@ -3786,37 +3852,18 @@ get_worker(ns_clientmgr_t *manager, ns_interface_t *ifp, isc_socket_t *sock,
 	client->newstate = client->state = NS_CLIENTSTATE_WORKING;
 	INSIST(client->recursionquota == NULL);
 	client->sctx = manager->sctx;
-
+	client->tcpquota = &client->sctx->tcpquota;
-	/*
-	 * Transfer TCP quota to the new client.
-	 */
-	INSIST(client->tcpquota == NULL);
-	INSIST(oldclient->tcpquota != NULL);
-	client->tcpquota = oldclient->tcpquota;
-	oldclient->tcpquota = NULL;
-
-	/*
-	 * Link to a pipeline group, creating it if needed.
-	 */
-	if (!ISC_LINK_LINKED(oldclient, glink)) {
-		oldclient->glink.next = NULL;
-		oldclient->glink.prev = NULL;
-	}
-	client->glink.next = oldclient->glink.next;
-	client->glink.prev = oldclient;
-	if (oldclient->glink.next != NULL) {
-		oldclient->glink.next->glink.prev = client;
-	}
-	oldclient->glink.next = client;
 
 	client->dscp = ifp->dscp;
 
 	client->attributes |= NS_CLIENTATTR_TCP;
-	client->pipelined = true;
 	client->mortal = true;
 	client->sendcb = NULL;
 	client->rcode_override = -1;	/* not set */
 
+	pipeline_attach(oldclient, client);
+	client->pipelined = true;
+
 	isc_socket_attach(ifp->tcpsocket, &client->tcplistener);
 	isc_socket_attach(sock, &client->tcpsocket);
 	isc_socket_setname(client->tcpsocket, "worker-tcp", NULL);

lib/ns/include/ns/client.h

@@ -135,6 +135,7 @@ struct ns_client {
 	dns_name_t		*signer;      /*%< NULL if not valid sig */
 	bool			mortal;	      /*%< Die after handling request */
 	bool			pipelined;   /*%< TCP queries not in sequence */
+	isc_refcount_t		*pipeline_refs;
 	isc_quota_t		*tcpquota;
 	isc_quota_t		*recursionquota;
 	ns_interface_t		*interface;
@@ -166,7 +167,6 @@ struct ns_client {
 
 	ISC_LINK(ns_client_t)	link;
 	ISC_LINK(ns_client_t)	rlink;
-	ISC_LINK(ns_client_t)	glink;
 	ISC_QLINK(ns_client_t)	ilink;
 	unsigned char		cookie[8];
 	uint32_t		expire;