From: Michał Kępień Date: Thu, 17 Jan 2019 14:53:38 +0000 (+0100) Subject: use reference counter for pipeline groups (v3) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=42f65ae3416c7e4b6ea7974ff05c857170ba3505;p=thirdparty%2Fbind9.git 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 f0193b64aaf..7bcb8928fc3 100644 --- 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 318df717f9e..c0e773ad049 100644 --- 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;