From: Evan Hunt Date: Sun, 8 May 2022 03:58:19 +0000 (-0700) Subject: Always use the number of CPUS for resolver->ntasks X-Git-Tag: v9.19.2~26^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6936db2f59a1e63ebec572e7b7ae7776993e4cfe;p=thirdparty%2Fbind9.git 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 8bdcc27aa2f..7d6840f4ca2 100644 --- 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 22145a827c6..8f4d13939bf 100644 --- 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 7264ff1ab64..ec393062571 100644 --- 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 b09230ba0a2..a98be32af28 100644 --- 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 e8aa64658ec..be1f9200bdd 100644 --- 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 6be23ae199c..6cd1c80fac5 100644 --- 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 75516bae28b..4bb96aa5a6a 100644 --- 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 43e5501b67d..d836251015e 100644 --- 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 bf6c3039ff5..162c3b06e6a 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -38,8 +38,6 @@ #include #include -#define ISC_NETMGR_TID_UNKNOWN -1 - /* Must be different from ISC_NETMGR_TID_UNKNOWN */ #define ISC_NETMGR_NON_INTERLOCKED -2