]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Use thread-friendly mctxpool and taskpool in ns_client.
authorWitold Kręcicki <wpk@isc.org>
Tue, 28 Jan 2020 10:05:07 +0000 (11:05 +0100)
committerWitold Kręcicki <wpk@isc.org>
Tue, 18 Feb 2020 09:31:13 +0000 (10:31 +0100)
Make ns_client mctxpool more thread-friendly by sharding it by
netmgr threadid, use task pool also sharded by thread id to avoid
lock contention.

bin/named/server.c
lib/ns/client.c
lib/ns/include/ns/client.h
lib/ns/include/ns/interfacemgr.h
lib/ns/interfacemgr.c
lib/ns/tests/nstest.c

index 5e6cfa50f61a059021eff6f0e3df0971e2c965e3..01d57a725d023f7ae455d6f9ad276689ff6420bc 100644 (file)
@@ -9621,7 +9621,7 @@ run_server(isc_task_t *task, isc_event_t *event) {
                           named_g_mctx, server->sctx, named_g_taskmgr,
                           named_g_timermgr, named_g_socketmgr, named_g_nm,
                           named_g_dispatchmgr, server->task, named_g_udpdisp,
-                          geoip, &server->interfacemgr),
+                          geoip, named_g_cpus, &server->interfacemgr),
                   "creating interface manager");
 
        CHECKFATAL(isc_timer_create(named_g_timermgr, isc_timertype_inactive,
index 62005f90b693e582b4795d2ddf775c31c68e8ea1..63a6441ee5ca8918780ae83367c02625f83dafd8 100644 (file)
 #define NS_CLIENT_DROPPORT 1
 #endif /* ifndef NS_CLIENT_DROPPORT */
 
+#define CLIENT_NMCTXS_PERCPU 8
+/*%<
+ * Number of 'mctx pools' for clients. (Should this be configurable?)
+ * When enabling threads, we use a pool of memory contexts shared by
+ * client objects, since concurrent access to a shared context would cause
+ * heavy contentions.  The above constant is expected to be enough for
+ * completely avoiding contentions among threads for an authoritative-only
+ * server.
+ */
+
+#define CLIENT_NTASKS_PERCPU 32
+/*%<
+ * Number of tasks to be used by clients - those are used only when recursing
+ */
+
 #if defined(_WIN32) && !defined(_WIN64)
 LIBNS_EXTERNAL_DATA atomic_uint_fast32_t ns_client_requests;
 #else  /* if defined(_WIN32) && !defined(_WIN64) */
@@ -133,6 +148,8 @@ compute_cookie(ns_client_t *client, uint32_t when, uint32_t nonce,
               const unsigned char *secret, isc_buffer_t *buf);
 static void
 get_clientmctx(ns_clientmgr_t *manager, isc_mem_t **mctxp);
+static void
+get_clienttask(ns_clientmgr_t *manager, isc_task_t **taskp);
 
 void
 ns_client_recursing(ns_client_t *client) {
@@ -2231,39 +2248,33 @@ ns__client_tcpconn(isc_nmhandle_t *handle, isc_result_t result, void *arg) {
 static void
 get_clientmctx(ns_clientmgr_t *manager, isc_mem_t **mctxp) {
        isc_mem_t *clientmctx;
-#if CLIENT_NMCTXS > 0
-       unsigned int nextmctx;
-#endif /* if CLIENT_NMCTXS > 0 */
-
        MTRACE("clientmctx");
 
-#if CLIENT_NMCTXS > 0
-       LOCK(&manager->lock);
-       if (isc_nm_tid() >= 0) {
-               nextmctx = isc_nm_tid();
-       } else {
-               nextmctx = manager->nextmctx++;
-               if (manager->nextmctx == CLIENT_NMCTXS) {
-                       manager->nextmctx = 0;
-               }
-
-               INSIST(nextmctx < CLIENT_NMCTXS);
+       int tid = isc_nm_tid();
+       if (tid < 0) {
+               tid = isc_random_uniform(manager->ncpus);
        }
-
+       int rand = isc_random_uniform(CLIENT_NMCTXS_PERCPU);
+       int nextmctx = (rand * manager->ncpus) + tid;
        clientmctx = manager->mctxpool[nextmctx];
-       if (clientmctx == NULL) {
-               isc_mem_create(&clientmctx);
-               isc_mem_setname(clientmctx, "client", NULL);
-               manager->mctxpool[nextmctx] = clientmctx;
-       }
-       UNLOCK(&manager->lock);
-#else  /* if CLIENT_NMCTXS > 0 */
-       clientmctx = manager->mctx;
-#endif /* if CLIENT_NMCTXS > 0 */
 
        isc_mem_attach(clientmctx, mctxp);
 }
 
+static void
+get_clienttask(ns_clientmgr_t *manager, isc_task_t **taskp) {
+       MTRACE("clienttask");
+
+       int tid = isc_nm_tid();
+       if (tid < 0) {
+               tid = isc_random_uniform(manager->ncpus);
+       }
+
+       int rand = isc_random_uniform(CLIENT_NTASKS_PERCPU);
+       int nexttask = (rand * manager->ncpus) + tid;
+       isc_task_attach(manager->taskpool[nexttask], taskp);
+}
+
 isc_result_t
 ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) {
        isc_result_t result;
@@ -2285,10 +2296,8 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) {
                get_clientmctx(mgr, &client->mctx);
                clientmgr_attach(mgr, &client->manager);
                ns_server_attach(mgr->sctx, &client->sctx);
-               result = isc_task_create(mgr->taskmgr, 20, &client->task);
-               if (result != ISC_R_SUCCESS) {
-                       goto cleanup;
-               }
+               get_clienttask(mgr, &client->task);
+
                result = dns_message_create(client->mctx,
                                            DNS_MESSAGE_INTENTPARSE,
                                            &client->message);
@@ -2412,22 +2421,19 @@ clientmgr_detach(ns_clientmgr_t **mp) {
 
 static void
 clientmgr_destroy(ns_clientmgr_t *manager) {
-#if CLIENT_NMCTXS > 0
        int i;
-#endif /* if CLIENT_NMCTXS > 0 */
 
        MTRACE("clientmgr_destroy");
 
        isc_refcount_destroy(&manager->references);
        manager->magic = 0;
 
-#if CLIENT_NMCTXS > 0
-       for (i = 0; i < CLIENT_NMCTXS; i++) {
-               if (manager->mctxpool[i] != NULL) {
-                       isc_mem_detach(&manager->mctxpool[i]);
-               }
+       for (i = 0; i < manager->ncpus * CLIENT_NMCTXS_PERCPU; i++) {
+               isc_mem_detach(&manager->mctxpool[i]);
        }
-#endif /* if CLIENT_NMCTXS > 0 */
+       isc_mem_put(manager->mctx, manager->mctxpool,
+                   manager->ncpus * CLIENT_NMCTXS_PERCPU *
+                           sizeof(isc_mem_t *));
 
        if (manager->interface != NULL) {
                ns_interface_detach(&manager->interface);
@@ -2440,13 +2446,14 @@ clientmgr_destroy(ns_clientmgr_t *manager) {
                isc_task_detach(&manager->excl);
        }
 
-       for (i = 0; i < CLIENT_NTASKS; i++) {
+       for (i = 0; i < manager->ncpus * CLIENT_NTASKS_PERCPU; i++) {
                if (manager->taskpool[i] != NULL) {
                        isc_task_detach(&manager->taskpool[i]);
                }
        }
        isc_mem_put(manager->mctx, manager->taskpool,
-                   CLIENT_NTASKS * sizeof(isc_task_t *));
+                   manager->ncpus * CLIENT_NTASKS_PERCPU *
+                           sizeof(isc_task_t *));
        ns_server_detach(&manager->sctx);
 
        isc_mem_put(manager->mctx, manager, sizeof(*manager));
@@ -2455,12 +2462,11 @@ clientmgr_destroy(ns_clientmgr_t *manager) {
 isc_result_t
 ns_clientmgr_create(isc_mem_t *mctx, ns_server_t *sctx, isc_taskmgr_t *taskmgr,
                    isc_timermgr_t *timermgr, ns_interface_t *interface,
-                   ns_clientmgr_t **managerp) {
+                   int ncpus, ns_clientmgr_t **managerp) {
        ns_clientmgr_t *manager;
        isc_result_t result;
-#if CLIENT_NMCTXS > 0
        int i;
-#endif /* if CLIENT_NMCTXS > 0 */
+       int npools;
 
        manager = isc_mem_get(mctx, sizeof(*manager));
        *manager = (ns_clientmgr_t){ .magic = 0 };
@@ -2477,27 +2483,35 @@ ns_clientmgr_create(isc_mem_t *mctx, ns_server_t *sctx, isc_taskmgr_t *taskmgr,
        manager->mctx = mctx;
        manager->taskmgr = taskmgr;
        manager->timermgr = timermgr;
+       manager->ncpus = ncpus;
 
        ns_interface_attach(interface, &manager->interface);
 
        manager->exiting = false;
-       manager->taskpool = isc_mem_get(mctx,
-                                       CLIENT_NTASKS * sizeof(isc_task_t *));
-       for (i = 0; i < CLIENT_NTASKS; i++) {
+       int ntasks = CLIENT_NTASKS_PERCPU * manager->ncpus;
+       manager->taskpool = isc_mem_get(mctx, ntasks * sizeof(isc_task_t *));
+       for (i = 0; i < ntasks; i++) {
                manager->taskpool[i] = NULL;
-               isc_task_create(manager->taskmgr, 20, &manager->taskpool[i]);
+               result = isc_task_create_bound(manager->taskmgr, 20,
+                                              &manager->taskpool[i],
+                                              i % CLIENT_NTASKS_PERCPU);
+               RUNTIME_CHECK(result == ISC_R_SUCCESS);
        }
        isc_refcount_init(&manager->references, 1);
        manager->sctx = NULL;
        ns_server_attach(sctx, &manager->sctx);
 
        ISC_LIST_INIT(manager->recursing);
-#if CLIENT_NMCTXS > 0
-       manager->nextmctx = 0;
-       for (i = 0; i < CLIENT_NMCTXS; i++) {
-               manager->mctxpool[i] = NULL; /* will be created on-demand */
+
+       npools = CLIENT_NMCTXS_PERCPU * manager->ncpus;
+       manager->mctxpool = isc_mem_get(manager->mctx,
+                                       npools * sizeof(isc_mem_t *));
+       for (i = 0; i < npools; i++) {
+               manager->mctxpool[i] = NULL;
+               isc_mem_create(&manager->mctxpool[i]);
+               isc_mem_setname(manager->mctxpool[i], "client", NULL);
        }
-#endif /* if CLIENT_NMCTXS > 0 */
+
        manager->magic = MANAGER_MAGIC;
 
        MTRACE("create");
index dce787a9f2d6c05ca69febc35d7933ae98b4f1bf..12f159b4fac9b5a724985bc7ab3bd82ffd39eae7 100644 (file)
 #define NS_CLIENT_SEND_BUFFER_SIZE 4096
 #define NS_CLIENT_RECV_BUFFER_SIZE 4096
 
-#define CLIENT_NMCTXS 100
-/*%<
- * Number of 'mctx pools' for clients. (Should this be configurable?)
- * When enabling threads, we use a pool of memory contexts shared by
- * client objects, since concurrent access to a shared context would cause
- * heavy contentions.  The above constant is expected to be enough for
- * completely avoiding contentions among threads for an authoritative-only
- * server.
- */
-
-#define CLIENT_NTASKS 100
-/*%<
- * Number of tasks to be used by clients - those are used only when recursing
- */
-
 /*!
  * Client object states.  Ordering is significant: higher-numbered
  * states are generally "more active", meaning that the client can
@@ -166,6 +151,7 @@ struct ns_clientmgr {
        isc_timermgr_t *timermgr;
        isc_task_t *    excl;
        isc_refcount_t  references;
+       int             ncpus;
 
        /* Attached by clients, needed for e.g. recursion */
        isc_task_t **taskpool;
@@ -180,11 +166,8 @@ struct ns_clientmgr {
        isc_mutex_t   reclock;
        client_list_t recursing; /*%< Recursing clients */
 
-#if CLIENT_NMCTXS > 0
        /*%< mctx pool for clients. */
-       unsigned int nextmctx;
-       isc_mem_t *  mctxpool[CLIENT_NMCTXS];
-#endif /* if CLIENT_NMCTXS > 0 */
+       isc_mem_t **mctxpool;
 };
 
 /*% nameserver client structure */
@@ -364,7 +347,7 @@ ns_client_settimeout(ns_client_t *client, unsigned int seconds);
 
 isc_result_t
 ns_clientmgr_create(isc_mem_t *mctx, ns_server_t *sctx, isc_taskmgr_t *taskmgr,
-                   isc_timermgr_t *timermgr, ns_interface_t *ifp,
+                   isc_timermgr_t *timermgr, ns_interface_t *ifp, int ncpus,
                    ns_clientmgr_t **managerp);
 /*%<
  * Create a client manager.
index 424972e8fcde6eac79b4eaca0cf2878c0ddfbf85..e60dc9d412b525967183ad269a77d3051f8a4c34 100644 (file)
@@ -104,7 +104,7 @@ ns_interfacemgr_create(isc_mem_t *mctx, ns_server_t *sctx,
                       isc_socketmgr_t *socketmgr, isc_nm_t *nm,
                       dns_dispatchmgr_t *dispatchmgr, isc_task_t *task,
                       unsigned int udpdisp, dns_geoip_databases_t *geoip,
-                      ns_interfacemgr_t **mgrp);
+                      int ncpus, ns_interfacemgr_t **mgrp);
 /*%<
  * Create a new interface manager.
  *
index 2dcccff7b5182a73cbc4a0a5ecac4d6bc10d3e82..ebf1b0421dd2d80aa9eaf1b2be14e01338667da5 100644 (file)
@@ -77,6 +77,7 @@ struct ns_interfacemgr {
        isc_timermgr_t *timermgr;   /*%< Timer manager. */
        isc_socketmgr_t *socketmgr; /*%< Socket manager. */
        isc_nm_t *nm;               /*%< Net manager. */
+       int ncpus;                  /*%< Number of workers . */
        dns_dispatchmgr_t *dispatchmgr;
        unsigned int generation; /*%< Current generation no. */
        ns_listenlist_t *listenon4;
@@ -183,7 +184,7 @@ ns_interfacemgr_create(isc_mem_t *mctx, ns_server_t *sctx,
                       isc_socketmgr_t *socketmgr, isc_nm_t *nm,
                       dns_dispatchmgr_t *dispatchmgr, isc_task_t *task,
                       unsigned int udpdisp, dns_geoip_databases_t *geoip,
-                      ns_interfacemgr_t **mgrp) {
+                      int ncpus, ns_interfacemgr_t **mgrp) {
        isc_result_t result;
        ns_interfacemgr_t *mgr;
 
@@ -220,6 +221,7 @@ ns_interfacemgr_create(isc_mem_t *mctx, ns_server_t *sctx,
        mgr->listenon4 = NULL;
        mgr->listenon6 = NULL;
        mgr->udpdisp = udpdisp;
+       mgr->ncpus = ncpus;
        atomic_init(&mgr->shuttingdown, false);
 
        ISC_LIST_INIT(mgr->interfaces);
@@ -426,7 +428,8 @@ ns_interface_create(ns_interfacemgr_t *mgr, isc_sockaddr_t *addr,
        ifp->magic = IFACE_MAGIC;
 
        result = ns_clientmgr_create(mgr->mctx, mgr->sctx, mgr->taskmgr,
-                                    mgr->timermgr, ifp, &ifp->clientmgr);
+                                    mgr->timermgr, ifp, mgr->ncpus,
+                                    &ifp->clientmgr);
        if (result != ISC_R_SUCCESS) {
                isc_log_write(IFMGR_COMMON_LOGARGS, ISC_LOG_ERROR,
                              "ns_clientmgr_create() failed: %s",
index 9ea820475370f112ae80bcf57d225041fa8d4c36..2e09d9662fe06e02f84455f1dd027ab9d16a701e 100644 (file)
@@ -234,7 +234,7 @@ create_managers(void) {
 
        CHECK(ns_interfacemgr_create(mctx, sctx, taskmgr, timermgr, socketmgr,
                                     nm, dispatchmgr, maintask, ncpus, NULL,
-                                    &interfacemgr));
+                                    ncpus, &interfacemgr));
 
        CHECK(ns_listenlist_default(mctx, port, -1, true, &listenon));
        ns_interfacemgr_setlistenon4(interfacemgr, listenon);