use reference counter for pipeline groups (v3)

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

(cherry picked from commit 72eb9275ab8f97364c18abbc79671795f9cc1f23)
(cherry picked from commit 890dbb8200765c8e9a517750a29be29b993bf7fe)

diff --git a/lib/ns/client.c b/lib/ns/client.c
index f0193b64aafc446d5e00f668223696921b09e5bb..7bcb8928fc3f984a66777058fa8df9f03f7e88c1 100644 (file)
--- a/lib/ns/client.c
+++ b/lib/ns/client.c
@@ -325,6 +325,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
@@ -447,6 +516,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);
@@ -460,44 +563,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,
@@ -3109,6 +3174,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;
@@ -3131,7 +3197,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;
@@ -3320,6 +3385,7 @@ client_newconn(isc_task_t *task, isc_event_t *event) {
                     !dns_acl_allowed(&netaddr, NULL, NULL, 0, NULL,
                                     client->sctx->keepresporder, env)))
                {
+                       pipeline_init(client);
                        client->pipelined = true;
                }
 
@@ -3801,36 +3867,17 @@ 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;
-
-       /*
-        * 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->tcpquota = &client->sctx->tcpquota;
 
        client->dscp = ifp->dscp;
 
        client->attributes |= NS_CLIENTATTR_TCP;
-       client->pipelined = true;
        client->mortal = true;
        client->sendcb = NULL;
 
+       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);

diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h
index 318df717f9ea1ae0406a87482025c34803776215..c0e773ad049758bd0bfae5458a34e22fc27c95f0 100644 (file)
--- a/lib/ns/include/ns/client.h
+++ b/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;
@@ -167,7 +168,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;