Always use the number of CPUS for resolver->ntasks

Since the fctx hash table is now self-resizing, and resolver tasks are
selected to match the thread that created the fetch context, there
shouldn't be any significant advantage to having multiple tasks per CPU;
a single task per thread should be sufficient.

Additionally, the fetch context is always pinned to the calling netmgr
thread to minimize the contention just to coalesced fetches - if two
threads starts the same fetch, it will be pinned to the first one to get
the bucket.

diff --git a/bin/named/server.c b/bin/named/server.c
index 8bdcc27aa2f06c29acc233be9bb0ad923766eca5..7d6840f4ca2d3b09f0ccdb2d1e1442cc9754cf10 100644 (file)
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -153,12 +153,6 @@
 #define SIZE_AS_PERCENT ((size_t)-2)
 #endif /* ifndef SIZE_AS_PERCENT */
 
-#ifdef TUNE_LARGE
-#define RESOLVER_NTASKS_PERCPU 32
-#else
-#define RESOLVER_NTASKS_PERCPU 8
-#endif /* TUNE_LARGE */
-
 /* RFC7828 defines timeout as 16-bit value specified in units of 100
  * milliseconds, so the maximum and minimum advertised and keepalive
  * timeouts are capped by the data type (it's ~109 minutes)
@@ -4734,9 +4728,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
 
        ndisp = 4 * ISC_MIN(named_g_udpdisp, MAX_UDP_DISPATCH);
        CHECK(dns_view_createresolver(
-               view, named_g_taskmgr, RESOLVER_NTASKS_PERCPU * named_g_cpus,
-               ndisp, named_g_netmgr, named_g_timermgr, resopts,
-               named_g_dispatchmgr, dispatch4, dispatch6));
+               view, named_g_taskmgr, ndisp, named_g_netmgr, named_g_timermgr,
+               resopts, named_g_dispatchmgr, dispatch4, dispatch6));
 
        if (resstats == NULL) {
                CHECK(isc_stats_create(mctx, &resstats,

diff --git a/lib/dns/client.c b/lib/dns/client.c
index 22145a827c686eff4f78bcca99dc8e6e1c72abbe..8f4d13939bf6964dff24673471d2d603df838993 100644 (file)
--- a/lib/dns/client.c
+++ b/lib/dns/client.c
@@ -62,12 +62,6 @@
 
 #define MAX_RESTARTS 16
 
-#ifdef TUNE_LARGE
-#define RESOLVER_NTASKS 523
-#else /* ifdef TUNE_LARGE */
-#define RESOLVER_NTASKS 31
-#endif /* TUNE_LARGE */
-
 #define CHECK(r)                             \
        do {                                 \
                result = (r);                \
@@ -225,7 +219,7 @@ getudpdispatch(int family, dns_dispatchmgr_t *dispatchmgr,
 
 static isc_result_t
 createview(isc_mem_t *mctx, dns_rdataclass_t rdclass, isc_taskmgr_t *taskmgr,
-          unsigned int ntasks, isc_nm_t *nm, isc_timermgr_t *timermgr,
+          isc_nm_t *nm, isc_timermgr_t *timermgr,
           dns_dispatchmgr_t *dispatchmgr, dns_dispatch_t *dispatchv4,
           dns_dispatch_t *dispatchv6, dns_view_t **viewp) {
        isc_result_t result;
@@ -243,9 +237,8 @@ createview(isc_mem_t *mctx, dns_rdataclass_t rdclass, isc_taskmgr_t *taskmgr,
                return (result);
        }
 
-       result = dns_view_createresolver(view, taskmgr, ntasks, 1, nm, timermgr,
-                                        0, dispatchmgr, dispatchv4,
-                                        dispatchv6);
+       result = dns_view_createresolver(view, taskmgr, 1, nm, timermgr, 0,
+                                        dispatchmgr, dispatchv4, dispatchv6);
        if (result != ISC_R_SUCCESS) {
                dns_view_detach(&view);
                return (result);
@@ -335,9 +328,8 @@ dns_client_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, isc_nm_t *nm,
        isc_refcount_init(&client->references, 1);
 
        /* Create the default view for class IN */
-       result = createview(mctx, dns_rdataclass_in, taskmgr, RESOLVER_NTASKS,
-                           nm, timermgr, client->dispatchmgr, dispatchv4,
-                           dispatchv6, &view);
+       result = createview(mctx, dns_rdataclass_in, taskmgr, nm, timermgr,
+                           client->dispatchmgr, dispatchv4, dispatchv6, &view);
        if (result != ISC_R_SUCCESS) {
                goto cleanup_references;
        }

diff --git a/lib/dns/include/dns/resolver.h b/lib/dns/include/dns/resolver.h
index 7264ff1ab6480209d64746f8fc71e4166873ad00..ec393062571a16b5666d07aa1188197fb278ceae 100644 (file)
--- a/lib/dns/include/dns/resolver.h
+++ b/lib/dns/include/dns/resolver.h
@@ -165,10 +165,10 @@ typedef enum { dns_quotatype_zone = 0, dns_quotatype_server } dns_quotatype_t;
 
 isc_result_t
 dns_resolver_create(dns_view_t *view, isc_taskmgr_t *taskmgr,
-                   unsigned int ntasks, unsigned int ndisp, isc_nm_t *nm,
-                   isc_timermgr_t *timermgr, unsigned int options,
-                   dns_dispatchmgr_t *dispatchmgr, dns_dispatch_t *dispatchv4,
-                   dns_dispatch_t *dispatchv6, dns_resolver_t **resp);
+                   unsigned int ndisp, isc_nm_t *nm, isc_timermgr_t *timermgr,
+                   unsigned int options, dns_dispatchmgr_t *dispatchmgr,
+                   dns_dispatch_t *dispatchv4, dns_dispatch_t *dispatchv6,
+                   dns_resolver_t **resp);
 
 /*%<
  * Create a resolver.

diff --git a/lib/dns/include/dns/view.h b/lib/dns/include/dns/view.h
index b09230ba0a2b8a45993eed195e35ae57201dc995..a98be32af28dfe83de267e65bcf04e8540019a28 100644 (file)
--- a/lib/dns/include/dns/view.h
+++ b/lib/dns/include/dns/view.h
@@ -385,7 +385,7 @@ dns_view_createzonetable(dns_view_t *view);
 
 isc_result_t
 dns_view_createresolver(dns_view_t *view, isc_taskmgr_t *taskmgr,
-                       unsigned int ntasks, unsigned int ndisp, isc_nm_t *nm,
+                       unsigned int ndisp, isc_nm_t *nm,
                        isc_timermgr_t *timermgr, unsigned int options,
                        dns_dispatchmgr_t *dispatchmgr,
                        dns_dispatch_t *dispatchv4, dns_dispatch_t *dispatchv6);

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index e8aa64658ec6328430cc09eb6bc8a93c432cd6ce..be1f9200bdd3c2150dec773e924dae924ea16988 100644 (file)
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -330,7 +330,9 @@ struct fetchctx {
        char *info;
        isc_mem_t *mctx;
        isc_stdtime_t now;
-       isc_task_t *task;
+
+       isc_task_t *restask;
+       unsigned int tid;
 
        /* Atomic */
        isc_refcount_t references;
@@ -3365,10 +3367,10 @@ findname(fetchctx_t *fctx, const dns_name_t *name, in_port_t port,
         * See what we know about this address.
         */
        fctx_addref(fctx);
-       result = dns_adb_createfind(fctx->adb, fctx->task, fctx_finddone, fctx,
-                                   name, fctx->name, fctx->type, options, now,
-                                   NULL, res->view->dstport, fctx->depth + 1,
-                                   fctx->qc, &find);
+       result = dns_adb_createfind(fctx->adb, fctx->restask, fctx_finddone,
+                                   fctx, name, fctx->name, fctx->type, options,
+                                   now, NULL, res->view->dstport,
+                                   fctx->depth + 1, fctx->qc, &find);
 
        isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER,
                      DNS_LOGMODULE_RESOLVER, ISC_LOG_DEBUG(3),
@@ -4172,8 +4174,8 @@ fctx_try(fetchctx_t *fctx, bool retrying, bool badcache) {
                result = dns_resolver_createfetch(
                        fctx->res, fctx->qminname, fctx->qmintype, fctx->domain,
                        &fctx->nameservers, NULL, NULL, 0, options, 0, fctx->qc,
-                       fctx->task, resume_qmin, fctx, &fctx->qminrrset, NULL,
-                       &fctx->qminfetch);
+                       fctx->restask, resume_qmin, fctx, &fctx->qminrrset,
+                       NULL, &fctx->qminfetch);
                if (result != ISC_R_SUCCESS) {
                        fctx_unref(fctx);
                        fctx_done_detach(&fctx, DNS_R_SERVFAIL);
@@ -4433,7 +4435,7 @@ fctx_shutdown(fetchctx_t *fctx) {
        if (fctx->state != fetchstate_init) {
                FCTXTRACE("posting control event");
                cevent = &fctx->control_event;
-               isc_task_send(fctx->task, &cevent);
+               isc_task_send(fctx->restask, &cevent);
        }
 }
 
@@ -4666,22 +4668,26 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type,
        isc_interval_t interval;
        unsigned int findoptions = 0;
        char buf[DNS_NAME_FORMATSIZE + DNS_RDATATYPE_FORMATSIZE + 1];
-       unsigned int tid;
        size_t p;
+       int tid = isc_nm_tid();
+
+       if (tid == ISC_NETMGR_TID_UNKNOWN) {
+               tid = 0;
+       }
 
        /*
         * Caller must be holding the lock for 'bucket'
         */
        REQUIRE(fctxp != NULL && *fctxp == NULL);
 
-       tid = isc_random_uniform(res->ntasks);
        fctx = isc_mem_get(res->mctx, sizeof(*fctx));
        *fctx = (fetchctx_t){
                .type = type,
                .qmintype = type,
                .options = options,
                .bucket = bucket,
-               .task = res->tasks[tid],
+               .tid = tid,
+               .restask = res->tasks[tid],
                .state = fetchstate_init,
                .depth = depth,
                .qmin_labels = 1,
@@ -4869,7 +4875,7 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type,
         * lifetime. It will be made active when the fetch is
         * started.
         */
-       isc_timer_create(res->timermgr, fctx->task, fctx_expired, fctx,
+       isc_timer_create(res->timermgr, fctx->restask, fctx_expired, fctx,
                         &fctx->timer);
 
        /*
@@ -6329,7 +6335,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message,
                                                fctx, message, addrinfo, name,
                                                rdataset->type, rdataset,
                                                sigrdataset, valoptions,
-                                               fctx->task);
+                                               fctx->restask);
                                }
                        } else if (CHAINING(rdataset)) {
                                if (rdataset->type == dns_rdatatype_cname) {
@@ -6436,7 +6442,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message,
 
                result = valcreate(fctx, message, addrinfo, name, vtype,
                                   valrdataset, valsigrdataset, valoptions,
-                                  fctx->task);
+                                  fctx->restask);
        }
 
        if (result == ISC_R_SUCCESS && have_answer) {
@@ -6656,7 +6662,7 @@ ncache_message(fetchctx_t *fctx, dns_message_t *message,
                 * Do negative response validation.
                 */
                result = valcreate(fctx, message, addrinfo, name, fctx->type,
-                                  NULL, NULL, valoptions, fctx->task);
+                                  NULL, NULL, valoptions, fctx->restask);
                /*
                 * If validation is necessary, return now.  Otherwise
                 * continue to process the message, letting the
@@ -9694,7 +9700,7 @@ rctx_chaseds(respctx_t *rctx, dns_message_t *message,
        fctx_addref(fctx);
        result = dns_resolver_createfetch(
                fctx->res, fctx->nsname, dns_rdatatype_ns, NULL, NULL, NULL,
-               NULL, 0, fctx->options, 0, NULL, fctx->task, resume_dslookup,
+               NULL, 0, fctx->options, 0, NULL, fctx->restask, resume_dslookup,
                fctx, &fctx->nsrrset, NULL, &fctx->nsfetch);
        if (result != ISC_R_SUCCESS) {
                if (result == DNS_R_DUPLICATE) {
@@ -10162,10 +10168,10 @@ spillattimer_countdown(isc_task_t *task, isc_event_t *event) {
 
 isc_result_t
 dns_resolver_create(dns_view_t *view, isc_taskmgr_t *taskmgr,
-                   unsigned int ntasks, unsigned int ndisp, isc_nm_t *nm,
-                   isc_timermgr_t *timermgr, unsigned int options,
-                   dns_dispatchmgr_t *dispatchmgr, dns_dispatch_t *dispatchv4,
-                   dns_dispatch_t *dispatchv6, dns_resolver_t **resp) {
+                   unsigned int ndisp, isc_nm_t *nm, isc_timermgr_t *timermgr,
+                   unsigned int options, dns_dispatchmgr_t *dispatchmgr,
+                   dns_dispatch_t *dispatchv4, dns_dispatch_t *dispatchv6,
+                   dns_resolver_t **resp) {
        isc_result_t result = ISC_R_SUCCESS;
        char name[sizeof("res4294967295")];
        dns_resolver_t *res = NULL;
@@ -10176,7 +10182,6 @@ dns_resolver_create(dns_view_t *view, isc_taskmgr_t *taskmgr,
         */
 
        REQUIRE(DNS_VIEW_VALID(view));
-       REQUIRE(ntasks > 0);
        REQUIRE(ndisp > 0);
        REQUIRE(resp != NULL && *resp == NULL);
        REQUIRE(dispatchmgr != NULL);
@@ -10190,7 +10195,7 @@ dns_resolver_create(dns_view_t *view, isc_taskmgr_t *taskmgr,
                                 .taskmgr = taskmgr,
                                 .dispatchmgr = dispatchmgr,
                                 .options = options,
-                                .ntasks = ntasks,
+                                .ntasks = isc_nm_getnworkers(nm),
                                 .udpsize = DEFAULT_EDNS_BUFSIZE,
                                 .spillatmin = 10,
                                 .spillat = 10,
@@ -10217,13 +10222,14 @@ dns_resolver_create(dns_view_t *view, isc_taskmgr_t *taskmgr,
                goto cleanup_res;
        }
 
-       res->tasks = isc_mem_get(view->mctx, ntasks * sizeof(res->tasks[0]));
-       for (uint32_t i = 0; i < ntasks; i++) {
+       res->tasks = isc_mem_get(view->mctx,
+                                res->ntasks * sizeof(res->tasks[0]));
+       memset(res->tasks, 0, res->ntasks * sizeof(res->tasks[0]));
+       for (uint32_t i = 0; i < res->ntasks; i++) {
                /*
                 * Since we have a pool of tasks we bind them to task
                 * queues to spread the load evenly
                 */
-               res->tasks[i] = NULL;
                result = isc_task_create_bound(taskmgr, 0, &res->tasks[i], i);
                if (result != ISC_R_SUCCESS) {
                        goto cleanup_tasks;
@@ -10288,8 +10294,10 @@ cleanup_primelock:
        isc_ht_destroy(&res->buckets);
 
 cleanup_tasks:
-       for (size_t i = 0; i < ntasks; i++) {
-               isc_task_detach(&res->tasks[i]);
+       for (size_t i = 0; i < res->ntasks; i++) {
+               if (res->tasks[i] != NULL) {
+                       isc_task_detach(&res->tasks[i]);
+               }
        }
        isc_mem_put(view->mctx, res->tasks,
                    res->ntasks * sizeof(res->tasks[0]));
@@ -10620,11 +10628,13 @@ get_fctxbucket(dns_resolver_t *res, const dns_name_t *domain) {
        result = isc_ht_find(res->buckets, domain->ndata, domain->length,
                             (void **)&bucket);
        if (result != ISC_R_SUCCESS) {
+               RWUNLOCK(&res->hash_lock, ltype);
                bucket = isc_mem_get(res->mctx, sizeof(*bucket));
-               *bucket = (fctxbucket_t){ .fctxs = { 0 } };
+               *bucket = (fctxbucket_t){
+                       .fctxs = { 0 },
+               };
                ISC_LIST_INIT(bucket->fctxs);
                isc_mutex_init(&bucket->lock);
-               RWUNLOCK(&res->hash_lock, ltype);
 
                ltype = isc_rwlocktype_write;
                RWLOCK(&res->hash_lock, ltype);
@@ -10773,7 +10783,7 @@ dns_resolver_createfetch(dns_resolver_t *res, const dns_name_t *name,
                        ISC_EVENT_INIT(event, sizeof(*event), 0, NULL,
                                       DNS_EVENT_FETCHCONTROL, fctx_start, fctx,
                                       NULL, NULL, NULL);
-                       isc_task_send(fctx->task, &event);
+                       isc_task_send(fctx->restask, &event);
                } else {
                        dodestroy = true;
                }

diff --git a/lib/dns/tests/resolver_test.c b/lib/dns/tests/resolver_test.c
index 6be23ae199ccea4022e83549a9ffb03860cccf42..6cd1c80fac58e8c1dc3e9d4df35d2dba392460a2 100644 (file)
--- a/lib/dns/tests/resolver_test.c
+++ b/lib/dns/tests/resolver_test.c
@@ -81,7 +81,7 @@ static void
 mkres(dns_resolver_t **resolverp) {
        isc_result_t result;
 
-       result = dns_resolver_create(view, taskmgr, 1, 1, netmgr, timermgr, 0,
+       result = dns_resolver_create(view, taskmgr, 1, netmgr, timermgr, 0,
                                     dispatchmgr, dispatch, NULL, resolverp);
        assert_int_equal(result, ISC_R_SUCCESS);
 }

diff --git a/lib/dns/view.c b/lib/dns/view.c
index 75516bae28bc5268fc44be285538a06d0fc79bf8..4bb96aa5a6ac8f60a54d69b3ce87c5ad23da77fc 100644 (file)
--- a/lib/dns/view.c
+++ b/lib/dns/view.c
@@ -617,7 +617,7 @@ dns_view_createzonetable(dns_view_t *view) {
 
 isc_result_t
 dns_view_createresolver(dns_view_t *view, isc_taskmgr_t *taskmgr,
-                       unsigned int ntasks, unsigned int ndisp, isc_nm_t *nm,
+                       unsigned int ndisp, isc_nm_t *nm,
                        isc_timermgr_t *timermgr, unsigned int options,
                        dns_dispatchmgr_t *dispatchmgr,
                        dns_dispatch_t *dispatchv4,
@@ -635,7 +635,7 @@ dns_view_createresolver(dns_view_t *view, isc_taskmgr_t *taskmgr,
        }
        isc_task_setname(view->task, "view", view);
 
-       result = dns_resolver_create(view, taskmgr, ntasks, ndisp, nm, timermgr,
+       result = dns_resolver_create(view, taskmgr, ndisp, nm, timermgr,
                                     options, dispatchmgr, dispatchv4,
                                     dispatchv6, &view->resolver);
        if (result != ISC_R_SUCCESS) {

diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h
index 43e5501b67d790409c83d9357a07d6e9f574c6f9..d836251015eeec412cdcf132b20cae2ae1be8c46 100644 (file)
--- a/lib/isc/include/isc/netmgr.h
+++ b/lib/isc/include/isc/netmgr.h
@@ -102,6 +102,8 @@ isc_nm_detach(isc_nm_t **mgr0);
  * for all other references to be gone.
  */
 
+#define ISC_NETMGR_TID_UNKNOWN -1
+
 /* Return thread ID of current thread, or ISC_NETMGR_TID_UNKNOWN */
 int
 isc_nm_tid(void);

diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index bf6c3039ff5e6d43adf3554059f1e140c31d4c22..162c3b06e6a9acba55a0c97f105e051af7d5176e 100644 (file)
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -38,8 +38,6 @@
 #include <isc/util.h>
 #include <isc/uv.h>
 
-#define ISC_NETMGR_TID_UNKNOWN -1
-
 /* Must be different from ISC_NETMGR_TID_UNKNOWN */
 #define ISC_NETMGR_NON_INTERLOCKED -2