From a94678ff77b64e86013e66b5381d5e2cac84d67c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 17 Dec 2021 11:34:57 +0100 Subject: [PATCH 1/6] Create per-thread task and memory context for zonemgr Previously, the zonemgr created 1 task per 100 zones and 1 memory context per 1000 zones (with minimum 10 tasks and 2 memory contexts) to reduce the contention between threads. Instead of reducing the contention by having many resources, create a per-nm_thread memory context, loadtask and zonetask and spread the zones between just per-thread resources. Note: this commit alone does decrease performance when loading the zone by couple seconds (in case of 1M zone) and thus there's more work in this whole MR fixing the performance. --- bin/named/server.c | 12 +- lib/dns/include/dns/zone.h | 17 +-- lib/dns/tests/dnstest.c | 8 +- lib/dns/tests/zonemgr_test.c | 27 +---- lib/dns/update.c | 1 - lib/dns/zone.c | 160 +++++++++++++++----------- lib/isc/Makefile.am | 2 - lib/isc/include/isc/pool.h | 40 +------ lib/isc/include/isc/taskpool.h | 135 ---------------------- lib/isc/pool.c | 91 ++------------- lib/isc/taskpool.c | 157 ------------------------- lib/isc/tests/Makefile.am | 1 - lib/isc/tests/pool_test.c | 48 -------- lib/isc/tests/taskpool_test.c | 204 --------------------------------- lib/ns/tests/nstest.c | 8 +- lib/ns/update.c | 1 - 16 files changed, 116 insertions(+), 796 deletions(-) delete mode 100644 lib/isc/include/isc/taskpool.h delete mode 100644 lib/isc/taskpool.c delete mode 100644 lib/isc/tests/taskpool_test.c diff --git a/bin/named/server.c b/bin/named/server.c index 536dcdac72..5311e91a60 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -9221,14 +9221,6 @@ load_configuration(const char *filename, named_server_t *server, dns_view_detach(&view); } - /* - * Zones have been counted; set the zone manager task pool size. - */ - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, - "sizing zone task pool based on %d zones", num_zones); - CHECK(dns_zonemgr_setsize(named_g_server->zonemgr, num_zones)); - /* * Configure and freeze all explicit views. Explicit * views that have zones were already created at parsing @@ -10192,10 +10184,8 @@ named_server_create(isc_mem_t *mctx, named_server_t **serverp) { CHECKFATAL(dns_zonemgr_create(named_g_mctx, named_g_taskmgr, named_g_timermgr, named_g_netmgr, - &server->zonemgr), + named_g_cpus, &server->zonemgr), "dns_zonemgr_create"); - CHECKFATAL(dns_zonemgr_setsize(server->zonemgr, 1000), "dns_zonemgr_" - "setsize"); server->statsfile = isc_mem_strdup(server->mctx, "named.stats"); CHECKFATAL(server->statsfile == NULL ? ISC_R_NOMEMORY : ISC_R_SUCCESS, diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index 8859262fdd..52bc941da0 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -1783,10 +1783,9 @@ dns_zone_getdnsseckeys(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, isc_result_t dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, isc_timermgr_t *timermgr, isc_nm_t *netmgr, - dns_zonemgr_t **zmgrp); + unsigned int workers, dns_zonemgr_t **zmgrp); /*%< - * Create a zone manager. Note: the zone manager will not be able to - * manage any zones until dns_zonemgr_setsize() has been run. + * Create a zone manager. * * Requires: *\li 'mctx' to be a valid memory context. @@ -1795,18 +1794,6 @@ dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, *\li 'zmgrp' to point to a NULL pointer. */ -isc_result_t -dns_zonemgr_setsize(dns_zonemgr_t *zmgr, int num_zones); -/*%< - * Set the size of the zone manager task pool. This must be run - * before zmgr can be used for managing zones. Currently, it can only - * be run once; the task pool cannot be resized. - * - * Requires: - *\li zmgr is a valid zone manager. - *\li zmgr->zonetasks has been initialized. - */ - isc_result_t dns_zonemgr_createzone(dns_zonemgr_t *zmgr, dns_zone_t **zonep); /*%< diff --git a/lib/dns/tests/dnstest.c b/lib/dns/tests/dnstest.c index 30b967dacb..cf60c428d1 100644 --- a/lib/dns/tests/dnstest.c +++ b/lib/dns/tests/dnstest.c @@ -292,7 +292,8 @@ dns_test_setupzonemgr(void) { isc_result_t result; REQUIRE(zonemgr == NULL); - result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, NULL, &zonemgr); + result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, NULL, ncpus, + &zonemgr); return (result); } @@ -301,11 +302,6 @@ dns_test_managezone(dns_zone_t *zone) { isc_result_t result; REQUIRE(zonemgr != NULL); - result = dns_zonemgr_setsize(zonemgr, 1); - if (result != ISC_R_SUCCESS) { - return (result); - } - result = dns_zonemgr_managezone(zonemgr, zone); return (result); } diff --git a/lib/dns/tests/zonemgr_test.c b/lib/dns/tests/zonemgr_test.c index cadf413fc9..2a76eaf5e8 100644 --- a/lib/dns/tests/zonemgr_test.c +++ b/lib/dns/tests/zonemgr_test.c @@ -64,7 +64,7 @@ zonemgr_create(void **state) { UNUSED(state); - result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, NULL, + result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, NULL, 1, &myzonemgr); assert_int_equal(result, ISC_R_SUCCESS); @@ -82,22 +82,15 @@ zonemgr_managezone(void **state) { UNUSED(state); - result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, NULL, + result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, NULL, 1, &myzonemgr); assert_int_equal(result, ISC_R_SUCCESS); result = dns_test_makezone("foo", &zone, NULL, false); assert_int_equal(result, ISC_R_SUCCESS); - /* This should not succeed until the dns_zonemgr_setsize() is run */ - result = dns_zonemgr_managezone(myzonemgr, zone); - assert_int_equal(result, ISC_R_FAILURE); - assert_int_equal(dns_zonemgr_getcount(myzonemgr, DNS_ZONESTATE_ANY), 0); - result = dns_zonemgr_setsize(myzonemgr, 1); - assert_int_equal(result, ISC_R_SUCCESS); - /* Now it should succeed */ result = dns_zonemgr_managezone(myzonemgr, zone); assert_int_equal(result, ISC_R_SUCCESS); @@ -123,18 +116,10 @@ zonemgr_createzone(void **state) { UNUSED(state); - result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, NULL, + result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, NULL, 1, &myzonemgr); assert_int_equal(result, ISC_R_SUCCESS); - /* This should not succeed until the dns_zonemgr_setsize() is run */ - result = dns_zonemgr_createzone(myzonemgr, &zone); - assert_int_equal(result, ISC_R_FAILURE); - - result = dns_zonemgr_setsize(myzonemgr, 1); - assert_int_equal(result, ISC_R_SUCCESS); - - /* Now it should succeed */ result = dns_zonemgr_createzone(myzonemgr, &zone); assert_int_equal(result, ISC_R_SUCCESS); assert_non_null(zone); @@ -162,16 +147,13 @@ zonemgr_unreachable(void **state) { TIME_NOW(&now); - result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, NULL, + result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, NULL, 1, &myzonemgr); assert_int_equal(result, ISC_R_SUCCESS); result = dns_test_makezone("foo", &zone, NULL, false); assert_int_equal(result, ISC_R_SUCCESS); - result = dns_zonemgr_setsize(myzonemgr, 1); - assert_int_equal(result, ISC_R_SUCCESS); - result = dns_zonemgr_managezone(myzonemgr, zone); assert_int_equal(result, ISC_R_SUCCESS); @@ -224,7 +206,6 @@ zonemgr_unreachable(void **state) { * - dns_zonemgr_forcemaint * - dns_zonemgr_resumexfrs * - dns_zonemgr_shutdown - * - dns_zonemgr_setsize * - dns_zonemgr_settransfersin * - dns_zonemgr_getttransfersin * - dns_zonemgr_settransfersperns diff --git a/lib/dns/update.c b/lib/dns/update.c index 9241b1cad6..60009cc570 100644 --- a/lib/dns/update.c +++ b/lib/dns/update.c @@ -26,7 +26,6 @@ #include #include #include -#include #include #include diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 2b3189cc55..971f6eee10 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -35,7 +35,7 @@ #include #include #include -#include +#include #include #include #include @@ -595,8 +595,10 @@ struct dns_zonemgr { isc_taskmgr_t *taskmgr; isc_timermgr_t *timermgr; isc_nm_t *netmgr; - isc_taskpool_t *zonetasks; - isc_taskpool_t *loadtasks; + atomic_uint_fast32_t nzonetasks; + isc_pool_t *zonetasks; + atomic_uint_fast32_t nloadtasks; + isc_pool_t *loadtasks; isc_task_t *task; isc_pool_t *mctxpool; isc_ratelimiter_t *checkdsrl; @@ -970,6 +972,8 @@ static void zonemgr_putio(dns_io_t **iop); static void zonemgr_cancelio(dns_io_t *io); +static isc_result_t +zonemgr_setsize(dns_zonemgr_t *zmgr, unsigned int workers); static void rss_post(dns_zone_t *, isc_event_t *); @@ -18798,10 +18802,12 @@ zonemgr_keymgmt_find(dns_zonemgr_t *zmgr, dns_zone_t *zone, isc_result_t dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, isc_timermgr_t *timermgr, isc_nm_t *netmgr, - dns_zonemgr_t **zmgrp) { + unsigned int workers, dns_zonemgr_t **zmgrp) { dns_zonemgr_t *zmgr; isc_result_t result; + REQUIRE(workers >= 1); + zmgr = isc_mem_get(mctx, sizeof(*zmgr)); zmgr->mctx = NULL; isc_refcount_init(&zmgr->refs, 1); @@ -18810,7 +18816,9 @@ dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, zmgr->timermgr = timermgr; zmgr->netmgr = netmgr; zmgr->zonetasks = NULL; + atomic_init(&zmgr->nzonetasks, 0); zmgr->loadtasks = NULL; + atomic_init(&zmgr->nloadtasks, 0); zmgr->mctxpool = NULL; zmgr->task = NULL; zmgr->checkdsrl = NULL; @@ -18870,6 +18878,11 @@ dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, goto free_startupnotifyrl; } + result = zonemgr_setsize(zmgr, workers); + if (result != ISC_R_SUCCESS) { + goto free_startuprefreshrl; + } + /* Key file I/O locks. */ zonemgr_keymgmt_init(zmgr); @@ -18900,6 +18913,8 @@ dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, free_iolock: isc_mutex_destroy(&zmgr->iolock); #endif /* if 0 */ +free_startuprefreshrl: + isc_ratelimiter_detach(&zmgr->startuprefreshrl); free_startupnotifyrl: isc_ratelimiter_detach(&zmgr->startupnotifyrl); free_refreshrl: @@ -18963,8 +18978,8 @@ dns_zonemgr_managezone(dns_zonemgr_t *zmgr, dns_zone_t *zone) { REQUIRE(zone->timer == NULL); REQUIRE(zone->zmgr == NULL); - isc_taskpool_gettask(zmgr->zonetasks, &zone->task); - isc_taskpool_gettask(zmgr->loadtasks, &zone->loadtask); + isc_task_attach(isc_pool_get(zmgr->zonetasks), &zone->task); + isc_task_attach(isc_pool_get(zmgr->loadtasks), &zone->loadtask); /* * Set the task name. The tag will arbitrarily point to one @@ -19097,10 +19112,10 @@ dns_zonemgr_shutdown(dns_zonemgr_t *zmgr) { isc_task_destroy(&zmgr->task); } if (zmgr->zonetasks != NULL) { - isc_taskpool_destroy(&zmgr->zonetasks); + isc_pool_destroy(&zmgr->zonetasks); } if (zmgr->loadtasks != NULL) { - isc_taskpool_destroy(&zmgr->loadtasks); + isc_pool_destroy(&zmgr->loadtasks); } if (zmgr->mctxpool != NULL) { isc_pool_destroy(&zmgr->mctxpool); @@ -19117,6 +19132,49 @@ dns_zonemgr_shutdown(dns_zonemgr_t *zmgr) { RWUNLOCK(&zmgr->rwlock, isc_rwlocktype_read); } +static isc_result_t +taskinit(void **target, void *arg, uint32_t bindto, unsigned int quantum, + bool priv) { + dns_zonemgr_t *zmgr = (dns_zonemgr_t *)arg; + isc_task_t *task = NULL; + isc_result_t result = isc_task_create_bound(zmgr->taskmgr, quantum, + &task, bindto); + if (result != ISC_R_SUCCESS) { + return (result); + } + isc_task_setprivilege(task, priv); + isc_task_setname(task, "zonemgr-taskpool", NULL); + *target = task; + + return (ISC_R_SUCCESS); +} + +static void +taskfree(void **target) { + isc_task_t *task = *target; + isc_task_detach(&task); +} + +static isc_result_t +zonetaskinit(void **target, void *arg) { + dns_zonemgr_t *zmgr = (dns_zonemgr_t *)arg; + uint32_t bindto = atomic_fetch_add(&zmgr->nzonetasks, 1); + return (taskinit(target, arg, bindto, 2, false)); +} + +static isc_result_t +loadtaskinit(void **target, void *arg) { + /* + * We always set all tasks in the zone-load task pool to + * privileged. This prevents other tasks in the system from + * running while the server task manager is in privileged + * mode. + */ + dns_zonemgr_t *zmgr = (dns_zonemgr_t *)arg; + uint32_t bindto = atomic_fetch_add(&zmgr->nloadtasks, 1); + return (taskinit(target, arg, bindto, UINT_MAX, true)); +} + static isc_result_t mctxinit(void **target, void *arg) { isc_mem_t *mctx = NULL; @@ -19139,74 +19197,44 @@ mctxfree(void **target) { *target = NULL; } -#define ZONES_PER_TASK 100 -#define ZONES_PER_MCTX 1000 - -isc_result_t -dns_zonemgr_setsize(dns_zonemgr_t *zmgr, int num_zones) { +static isc_result_t +zonemgr_setsize(dns_zonemgr_t *zmgr, unsigned int workers) { isc_result_t result; - int ntasks = num_zones / ZONES_PER_TASK; - int nmctx = num_zones / ZONES_PER_MCTX; - isc_taskpool_t *pool = NULL; - isc_pool_t *mctxpool = NULL; - REQUIRE(DNS_ZONEMGR_VALID(zmgr)); - - /* - * For anything fewer than 1000 zones we use 10 tasks in - * the task pools. More than that, and we'll scale at one - * task per 100 zones. Similarly, for anything smaller than - * 2000 zones we use 2 memory contexts, then scale at 1:1000. - */ - if (ntasks < 10) { - ntasks = 10; - } - if (nmctx < 2) { - nmctx = 2; + /* Create the zone tasks pool. */ + REQUIRE(zmgr->zonetasks == NULL); + result = isc_pool_create(zmgr->mctx, workers, taskfree, zonetaskinit, + zmgr, &zmgr->zonetasks); + if (result != ISC_R_SUCCESS) { + goto fail; } - /* Create or resize the zone task pools. */ - if (zmgr->zonetasks == NULL) { - result = isc_taskpool_create(zmgr->taskmgr, zmgr->mctx, ntasks, - 2, false, &pool); - } else { - result = isc_taskpool_expand(&zmgr->zonetasks, ntasks, false, - &pool); + /* Create the zone load tasks pool. */ + REQUIRE(zmgr->loadtasks == NULL); + result = isc_pool_create(zmgr->mctx, workers, taskfree, loadtaskinit, + zmgr, &zmgr->loadtasks); + if (result != ISC_R_SUCCESS) { + goto fail; } - if (result == ISC_R_SUCCESS) { - zmgr->zonetasks = pool; + /* Create the zone memory context pool. */ + REQUIRE(zmgr->mctxpool == NULL); + result = isc_pool_create(zmgr->mctx, workers, mctxfree, mctxinit, NULL, + &zmgr->mctxpool); + if (result != ISC_R_SUCCESS) { + goto fail; } - /* - * We always set all tasks in the zone-load task pool to - * privileged. This prevents other tasks in the system from - * running while the server task manager is in privileged - * mode. - */ - pool = NULL; - if (zmgr->loadtasks == NULL) { - result = isc_taskpool_create(zmgr->taskmgr, zmgr->mctx, ntasks, - UINT_MAX, true, &pool); - } else { - result = isc_taskpool_expand(&zmgr->loadtasks, ntasks, true, - &pool); + return (ISC_R_SUCCESS); +fail: + if (zmgr->mctxpool != NULL) { + isc_pool_destroy(&zmgr->mctxpool); } - - if (result == ISC_R_SUCCESS) { - zmgr->loadtasks = pool; + if (zmgr->loadtasks != NULL) { + isc_pool_destroy(&zmgr->loadtasks); } - - /* Create or resize the zone memory context pool. */ - if (zmgr->mctxpool == NULL) { - result = isc_pool_create(zmgr->mctx, nmctx, mctxfree, mctxinit, - NULL, &mctxpool); - } else { - result = isc_pool_expand(&zmgr->mctxpool, nmctx, &mctxpool); - } - - if (result == ISC_R_SUCCESS) { - zmgr->mctxpool = mctxpool; + if (zmgr->zonetasks != NULL) { + isc_pool_destroy(&zmgr->zonetasks); } return (result); diff --git a/lib/isc/Makefile.am b/lib/isc/Makefile.am index eea0bb5f77..e2760034f1 100644 --- a/lib/isc/Makefile.am +++ b/lib/isc/Makefile.am @@ -90,7 +90,6 @@ libisc_la_HEADERS = \ include/isc/symtab.h \ include/isc/syslog.h \ include/isc/task.h \ - include/isc/taskpool.h \ include/isc/thread.h \ include/isc/time.h \ include/isc/timer.h \ @@ -191,7 +190,6 @@ libisc_la_SOURCES = \ syslog.c \ task.c \ task_p.h \ - taskpool.c \ thread.c \ time.c \ timer.c \ diff --git a/lib/isc/include/isc/pool.h b/lib/isc/include/isc/pool.h index 90b8934a0e..1333cb7430 100644 --- a/lib/isc/include/isc/pool.h +++ b/lib/isc/include/isc/pool.h @@ -84,45 +84,7 @@ void * isc_pool_get(isc_pool_t *pool); /*%< * Returns a pointer to an object from the pool. Currently the object - * is chosen from the pool at random. (This may be changed in the future - * to something that guaratees balance.) - */ - -int -isc_pool_count(isc_pool_t *pool); -/*%< - * Returns the number of objcts in the pool 'pool'. - */ - -isc_result_t -isc_pool_expand(isc_pool_t **sourcep, unsigned int count, isc_pool_t **targetp); - -/*%< - * If 'size' is larger than the number of objects in the pool pointed to by - * 'sourcep', then a new pool of size 'count' is allocated, the existing - * objects are copied into it, additional ones created to bring the - * total number up to 'count', and the resulting pool is attached to - * 'targetp'. - * - * If 'count' is less than or equal to the number of objects in 'source', then - * 'sourcep' is attached to 'targetp' without any other action being taken. - * - * In either case, 'sourcep' is detached. - * - * Requires: - * - * \li 'sourcep' is not NULL and '*source' is not NULL - * \li 'targetp' is not NULL and '*source' is NULL - * - * Ensures: - * - * \li On success, '*targetp' points to a valid task pool. - * \li On success, '*sourcep' points to NULL. - * - * Returns: - * - * \li #ISC_R_SUCCESS - * \li #ISC_R_NOMEMORY + * is chosen from the pool at random. */ void diff --git a/lib/isc/include/isc/taskpool.h b/lib/isc/include/isc/taskpool.h deleted file mode 100644 index cde2287fc1..0000000000 --- a/lib/isc/include/isc/taskpool.h +++ /dev/null @@ -1,135 +0,0 @@ -/* - * Copyright (C) Internet Systems Consortium, Inc. ("ISC") - * - * SPDX-License-Identifier: MPL-2.0 - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * - * See the COPYRIGHT file distributed with this work for additional - * information regarding copyright ownership. - */ - -#pragma once - -/***** -***** Module Info -*****/ - -/*! \file isc/taskpool.h - * \brief A task pool is a mechanism for sharing a small number of tasks - * among a large number of objects such that each object is - * assigned a unique task, but each task may be shared by several - * objects. - * - * Task pools are used to let objects that can exist in large - * numbers (e.g., zones) use tasks for synchronization without - * the memory overhead and unfair scheduling competition that - * could result from creating a separate task for each object. - */ - -/*** - *** Imports. - ***/ - -#include - -#include -#include - -ISC_LANG_BEGINDECLS - -/***** -***** Types. -*****/ - -typedef struct isc_taskpool isc_taskpool_t; - -/***** -***** Functions. -*****/ - -isc_result_t -isc_taskpool_create(isc_taskmgr_t *tmgr, isc_mem_t *mctx, unsigned int ntasks, - unsigned int quantum, bool priv, isc_taskpool_t **poolp); -/*%< - * Create a task pool of "ntasks" tasks, each with quantum - * "quantum". - * - * Requires: - * - *\li 'tmgr' is a valid task manager. - * - *\li 'mctx' is a valid memory context. - * - *\li poolp != NULL && *poolp == NULL - * - * Ensures: - * - *\li On success, '*taskp' points to the new task pool. - * - * Returns: - * - *\li #ISC_R_SUCCESS - *\li #ISC_R_NOMEMORY - *\li #ISC_R_UNEXPECTED - */ - -void -isc_taskpool_gettask(isc_taskpool_t *pool, isc_task_t **targetp); -/*%< - * Attach to a task from the pool. Currently the next task is chosen - * from the pool at random. (This may be changed in the future to - * something that guaratees balance.) - */ - -int -isc_taskpool_size(isc_taskpool_t *pool); -/*%< - * Returns the number of tasks in the task pool 'pool'. - */ - -isc_result_t -isc_taskpool_expand(isc_taskpool_t **sourcep, unsigned int size, bool priv, - isc_taskpool_t **targetp); - -/*%< - * If 'size' is larger than the number of tasks in the pool pointed to by - * 'sourcep', then a new taskpool of size 'size' is allocated, the existing - * tasks from are moved into it, additional tasks are created to bring the - * total number up to 'size', and the resulting pool is attached to - * 'targetp'. - * - * If 'size' is less than or equal to the tasks in pool 'source', then - * 'sourcep' is attached to 'targetp' without any other action being taken. - * - * In either case, 'sourcep' is detached. - * - * Requires: - * - * \li 'sourcep' is not NULL and '*source' is not NULL - * \li 'targetp' is not NULL and '*source' is NULL - * - * Ensures: - * - * \li On success, '*targetp' points to a valid task pool. - * \li On success, '*sourcep' points to NULL. - * - * Returns: - * - * \li #ISC_R_SUCCESS - * \li #ISC_R_NOMEMORY - */ - -void -isc_taskpool_destroy(isc_taskpool_t **poolp); -/*%< - * Destroy a task pool. The tasks in the pool are detached but not - * shut down. - * - * Requires: - * \li '*poolp' is a valid task pool. - */ - -ISC_LANG_ENDDECLS diff --git a/lib/isc/pool.c b/lib/isc/pool.c index 5a5fb12d2b..251e353321 100644 --- a/lib/isc/pool.c +++ b/lib/isc/pool.c @@ -37,24 +37,6 @@ struct isc_pool { *** Functions. ***/ -static isc_result_t -alloc_pool(isc_mem_t *mctx, unsigned int count, isc_pool_t **poolp) { - isc_pool_t *pool; - - pool = isc_mem_get(mctx, sizeof(*pool)); - pool->count = count; - pool->free = NULL; - pool->init = NULL; - pool->initarg = NULL; - pool->mctx = NULL; - isc_mem_attach(mctx, &pool->mctx); - pool->pool = isc_mem_get(mctx, count * sizeof(void *)); - memset(pool->pool, 0, count * sizeof(void *)); - - *poolp = pool; - return (ISC_R_SUCCESS); -} - isc_result_t isc_pool_create(isc_mem_t *mctx, unsigned int count, isc_pooldeallocator_t release, isc_poolinitializer_t init, @@ -66,14 +48,16 @@ isc_pool_create(isc_mem_t *mctx, unsigned int count, INSIST(count > 0); /* Allocate the pool structure */ - result = alloc_pool(mctx, count, &pool); - if (result != ISC_R_SUCCESS) { - return (result); - } - - pool->free = release; - pool->init = init; - pool->initarg = initarg; + pool = isc_mem_get(mctx, sizeof(*pool)); + *pool = (isc_pool_t){ + .count = count, + .free = release, + .init = init, + .initarg = initarg, + }; + isc_mem_attach(mctx, &pool->mctx); + pool->pool = isc_mem_get(mctx, count * sizeof(void *)); + memset(pool->pool, 0, count * sizeof(void *)); /* Populate the pool */ for (i = 0; i < count; i++) { @@ -93,61 +77,6 @@ isc_pool_get(isc_pool_t *pool) { return (pool->pool[isc_random_uniform(pool->count)]); } -int -isc_pool_count(isc_pool_t *pool) { - REQUIRE(pool != NULL); - return (pool->count); -} - -isc_result_t -isc_pool_expand(isc_pool_t **sourcep, unsigned int count, - isc_pool_t **targetp) { - isc_result_t result; - isc_pool_t *pool; - - REQUIRE(sourcep != NULL && *sourcep != NULL); - REQUIRE(targetp != NULL && *targetp == NULL); - - pool = *sourcep; - *sourcep = NULL; - if (count > pool->count) { - isc_pool_t *newpool = NULL; - unsigned int i; - - /* Allocate a new pool structure */ - result = alloc_pool(pool->mctx, count, &newpool); - if (result != ISC_R_SUCCESS) { - return (result); - } - - newpool->free = pool->free; - newpool->init = pool->init; - newpool->initarg = pool->initarg; - - /* Populate the new entries */ - for (i = pool->count; i < count; i++) { - result = newpool->init(&newpool->pool[i], - newpool->initarg); - if (result != ISC_R_SUCCESS) { - isc_pool_destroy(&newpool); - return (result); - } - } - - /* Copy over the objects from the old pool */ - for (i = 0; i < pool->count; i++) { - newpool->pool[i] = pool->pool[i]; - pool->pool[i] = NULL; - } - - isc_pool_destroy(&pool); - pool = newpool; - } - - *targetp = pool; - return (ISC_R_SUCCESS); -} - void isc_pool_destroy(isc_pool_t **poolp) { unsigned int i; diff --git a/lib/isc/taskpool.c b/lib/isc/taskpool.c deleted file mode 100644 index 3099532ca9..0000000000 --- a/lib/isc/taskpool.c +++ /dev/null @@ -1,157 +0,0 @@ -/* - * Copyright (C) Internet Systems Consortium, Inc. ("ISC") - * - * SPDX-License-Identifier: MPL-2.0 - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * - * See the COPYRIGHT file distributed with this work for additional - * information regarding copyright ownership. - */ - -/*! \file */ - -#include - -#include -#include -#include -#include - -/*** - *** Types. - ***/ - -struct isc_taskpool { - isc_mem_t *mctx; - isc_taskmgr_t *tmgr; - unsigned int ntasks; - unsigned int quantum; - isc_task_t **tasks; -}; - -/*** - *** Functions. - ***/ - -static void -alloc_pool(isc_taskmgr_t *tmgr, isc_mem_t *mctx, unsigned int ntasks, - unsigned int quantum, isc_taskpool_t **poolp) { - isc_taskpool_t *pool; - unsigned int i; - - pool = isc_mem_get(mctx, sizeof(*pool)); - - pool->mctx = NULL; - isc_mem_attach(mctx, &pool->mctx); - pool->ntasks = ntasks; - pool->quantum = quantum; - pool->tmgr = tmgr; - pool->tasks = isc_mem_get(mctx, ntasks * sizeof(isc_task_t *)); - for (i = 0; i < ntasks; i++) { - pool->tasks[i] = NULL; - } - - *poolp = pool; -} - -isc_result_t -isc_taskpool_create(isc_taskmgr_t *tmgr, isc_mem_t *mctx, unsigned int ntasks, - unsigned int quantum, bool priv, isc_taskpool_t **poolp) { - unsigned int i; - isc_taskpool_t *pool = NULL; - - INSIST(ntasks > 0); - - /* Allocate the pool structure */ - alloc_pool(tmgr, mctx, ntasks, quantum, &pool); - - /* Create the tasks */ - for (i = 0; i < ntasks; i++) { - isc_result_t result = isc_task_create_bound(tmgr, quantum, - &pool->tasks[i], i); - if (result != ISC_R_SUCCESS) { - isc_taskpool_destroy(&pool); - return (result); - } - isc_task_setprivilege(pool->tasks[i], priv); - isc_task_setname(pool->tasks[i], "taskpool", NULL); - } - - *poolp = pool; - return (ISC_R_SUCCESS); -} - -void -isc_taskpool_gettask(isc_taskpool_t *pool, isc_task_t **targetp) { - isc_task_attach(pool->tasks[isc_random_uniform(pool->ntasks)], targetp); -} - -int -isc_taskpool_size(isc_taskpool_t *pool) { - REQUIRE(pool != NULL); - return (pool->ntasks); -} - -isc_result_t -isc_taskpool_expand(isc_taskpool_t **sourcep, unsigned int size, bool priv, - isc_taskpool_t **targetp) { - isc_taskpool_t *pool; - - REQUIRE(sourcep != NULL && *sourcep != NULL); - REQUIRE(targetp != NULL && *targetp == NULL); - - pool = *sourcep; - *sourcep = NULL; - if (size > pool->ntasks) { - isc_taskpool_t *newpool = NULL; - unsigned int i; - - /* Allocate a new pool structure */ - alloc_pool(pool->tmgr, pool->mctx, size, pool->quantum, - &newpool); - - /* Copy over the tasks from the old pool */ - for (i = 0; i < pool->ntasks; i++) { - newpool->tasks[i] = pool->tasks[i]; - pool->tasks[i] = NULL; - } - - /* Create new tasks */ - for (i = pool->ntasks; i < size; i++) { - isc_result_t result = - isc_task_create_bound(pool->tmgr, pool->quantum, - &newpool->tasks[i], i); - if (result != ISC_R_SUCCESS) { - *sourcep = pool; - isc_taskpool_destroy(&newpool); - return (result); - } - isc_task_setprivilege(newpool->tasks[i], priv); - isc_task_setname(newpool->tasks[i], "taskpool", NULL); - } - - isc_taskpool_destroy(&pool); - pool = newpool; - } - - *targetp = pool; - return (ISC_R_SUCCESS); -} - -void -isc_taskpool_destroy(isc_taskpool_t **poolp) { - unsigned int i; - isc_taskpool_t *pool = *poolp; - *poolp = NULL; - for (i = 0; i < pool->ntasks; i++) { - if (pool->tasks[i] != NULL) { - isc_task_detach(&pool->tasks[i]); - } - } - isc_mem_put(pool->mctx, pool->tasks, - pool->ntasks * sizeof(isc_task_t *)); - isc_mem_putanddetach(&pool->mctx, pool, sizeof(*pool)); -} diff --git a/lib/isc/tests/Makefile.am b/lib/isc/tests/Makefile.am index d84e167e1b..1aba558ddc 100644 --- a/lib/isc/tests/Makefile.am +++ b/lib/isc/tests/Makefile.am @@ -42,7 +42,6 @@ check_PROGRAMS = \ stats_test \ symtab_test \ task_test \ - taskpool_test \ time_test \ timer_test diff --git a/lib/isc/tests/pool_test.c b/lib/isc/tests/pool_test.c index 1ce51f65f1..462d3138db 100644 --- a/lib/isc/tests/pool_test.c +++ b/lib/isc/tests/pool_test.c @@ -84,57 +84,11 @@ create_pool(void **state) { result = isc_pool_create(test_mctx, 8, poolfree, poolinit, taskmgr, &pool); assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(isc_pool_count(pool), 8); isc_pool_destroy(&pool); assert_null(pool); } -/* Resize a pool */ -static void -expand_pool(void **state) { - isc_result_t result; - isc_pool_t *pool1 = NULL, *pool2 = NULL, *hold = NULL; - - UNUSED(state); - - result = isc_pool_create(test_mctx, 10, poolfree, poolinit, taskmgr, - &pool1); - assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(isc_pool_count(pool1), 10); - - /* resizing to a smaller size should have no effect */ - hold = pool1; - result = isc_pool_expand(&pool1, 5, &pool2); - assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(isc_pool_count(pool2), 10); - assert_ptr_equal(pool2, hold); - assert_null(pool1); - pool1 = pool2; - pool2 = NULL; - - /* resizing to the same size should have no effect */ - hold = pool1; - result = isc_pool_expand(&pool1, 10, &pool2); - assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(isc_pool_count(pool2), 10); - assert_ptr_equal(pool2, hold); - assert_null(pool1); - pool1 = pool2; - pool2 = NULL; - - /* resizing to larger size should make a new pool */ - hold = pool1; - result = isc_pool_expand(&pool1, 20, &pool2); - assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(isc_pool_count(pool2), 20); - assert_ptr_not_equal(pool2, hold); - assert_null(pool1); - - isc_pool_destroy(&pool2); - assert_null(pool2); -} - /* Get objects */ static void get_objects(void **state) { @@ -148,7 +102,6 @@ get_objects(void **state) { result = isc_pool_create(test_mctx, 2, poolfree, poolinit, taskmgr, &pool); assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(isc_pool_count(pool), 2); item = isc_pool_get(pool); assert_non_null(item); @@ -174,7 +127,6 @@ int main(void) { const struct CMUnitTest tests[] = { cmocka_unit_test_setup_teardown(create_pool, _setup, _teardown), - cmocka_unit_test_setup_teardown(expand_pool, _setup, _teardown), cmocka_unit_test_setup_teardown(get_objects, _setup, _teardown), }; diff --git a/lib/isc/tests/taskpool_test.c b/lib/isc/tests/taskpool_test.c deleted file mode 100644 index 64a46410de..0000000000 --- a/lib/isc/tests/taskpool_test.c +++ /dev/null @@ -1,204 +0,0 @@ -/* - * Copyright (C) Internet Systems Consortium, Inc. ("ISC") - * - * SPDX-License-Identifier: MPL-2.0 - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * - * See the COPYRIGHT file distributed with this work for additional - * information regarding copyright ownership. - */ - -#if HAVE_CMOCKA - -#include /* IWYU pragma: keep */ -#include -#include -#include -#include -#include -#include - -#define UNIT_TESTING -#include - -#include -#include -#include - -#include "isctest.h" - -#define TASK_MAGIC ISC_MAGIC('T', 'A', 'S', 'K') -#define VALID_TASK(t) ISC_MAGIC_VALID(t, TASK_MAGIC) - -static int -_setup(void **state) { - isc_result_t result; - - UNUSED(state); - - result = isc_test_begin(NULL, true, 0); - assert_int_equal(result, ISC_R_SUCCESS); - - return (0); -} - -static int -_teardown(void **state) { - UNUSED(state); - - isc_test_end(); - - return (0); -} - -/* Create a taskpool */ -static void -create_pool(void **state) { - isc_result_t result; - isc_taskpool_t *pool = NULL; - - UNUSED(state); - - result = isc_taskpool_create(taskmgr, test_mctx, 8, 2, false, &pool); - assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(isc_taskpool_size(pool), 8); - - isc_taskpool_destroy(&pool); - assert_null(pool); -} - -/* Resize a taskpool */ -static void -expand_pool(void **state) { - isc_result_t result; - isc_taskpool_t *pool1 = NULL, *pool2 = NULL, *hold = NULL; - - UNUSED(state); - - result = isc_taskpool_create(taskmgr, test_mctx, 10, 2, false, &pool1); - assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(isc_taskpool_size(pool1), 10); - - /* resizing to a smaller size should have no effect */ - hold = pool1; - result = isc_taskpool_expand(&pool1, 5, false, &pool2); - assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(isc_taskpool_size(pool2), 10); - assert_ptr_equal(pool2, hold); - assert_null(pool1); - pool1 = pool2; - pool2 = NULL; - - /* resizing to the same size should have no effect */ - hold = pool1; - result = isc_taskpool_expand(&pool1, 10, false, &pool2); - assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(isc_taskpool_size(pool2), 10); - assert_ptr_equal(pool2, hold); - assert_null(pool1); - pool1 = pool2; - pool2 = NULL; - - /* resizing to larger size should make a new pool */ - hold = pool1; - result = isc_taskpool_expand(&pool1, 20, false, &pool2); - assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(isc_taskpool_size(pool2), 20); - assert_ptr_not_equal(pool2, hold); - assert_null(pool1); - - isc_taskpool_destroy(&pool2); - assert_null(pool2); -} - -/* Get tasks */ -static void -get_tasks(void **state) { - isc_result_t result; - isc_taskpool_t *pool = NULL; - isc_task_t *task1 = NULL, *task2 = NULL, *task3 = NULL; - - UNUSED(state); - - result = isc_taskpool_create(taskmgr, test_mctx, 2, 2, false, &pool); - assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(isc_taskpool_size(pool), 2); - - /* two tasks in pool; make sure we can access them more than twice */ - isc_taskpool_gettask(pool, &task1); - assert_true(VALID_TASK(task1)); - - isc_taskpool_gettask(pool, &task2); - assert_true(VALID_TASK(task2)); - - isc_taskpool_gettask(pool, &task3); - assert_true(VALID_TASK(task3)); - - isc_task_destroy(&task1); - isc_task_destroy(&task2); - isc_task_destroy(&task3); - - isc_taskpool_destroy(&pool); - assert_null(pool); -} - -/* Set privileges */ -static void -set_privilege(void **state) { - isc_result_t result; - isc_taskpool_t *pool = NULL; - isc_task_t *task1 = NULL, *task2 = NULL, *task3 = NULL; - - UNUSED(state); - - result = isc_taskpool_create(taskmgr, test_mctx, 2, 2, true, &pool); - assert_int_equal(result, ISC_R_SUCCESS); - assert_int_equal(isc_taskpool_size(pool), 2); - - isc_taskpool_gettask(pool, &task1); - isc_taskpool_gettask(pool, &task2); - isc_taskpool_gettask(pool, &task3); - - assert_true(VALID_TASK(task1)); - assert_true(VALID_TASK(task2)); - assert_true(VALID_TASK(task3)); - - assert_true(isc_task_getprivilege(task1)); - assert_true(isc_task_getprivilege(task2)); - assert_true(isc_task_getprivilege(task3)); - - isc_task_destroy(&task1); - isc_task_destroy(&task2); - isc_task_destroy(&task3); - - isc_taskpool_destroy(&pool); - assert_null(pool); -} - -int -main(void) { - const struct CMUnitTest tests[] = { - cmocka_unit_test_setup_teardown(create_pool, _setup, _teardown), - cmocka_unit_test_setup_teardown(expand_pool, _setup, _teardown), - cmocka_unit_test_setup_teardown(get_tasks, _setup, _teardown), - cmocka_unit_test_setup_teardown(set_privilege, _setup, - _teardown), - }; - - return (cmocka_run_group_tests(tests, NULL, NULL)); -} - -#else /* HAVE_CMOCKA */ - -#include - -int -main(void) { - printf("1..0 # Skipped: cmocka not available\n"); - return (SKIPPED_TEST_EXIT_CODE); -} - -#endif /* if HAVE_CMOCKA */ diff --git a/lib/ns/tests/nstest.c b/lib/ns/tests/nstest.c index 122db814f3..2e5d85cf29 100644 --- a/lib/ns/tests/nstest.c +++ b/lib/ns/tests/nstest.c @@ -448,7 +448,8 @@ ns_test_setupzonemgr(void) { isc_result_t result; REQUIRE(zonemgr == NULL); - result = dns_zonemgr_create(mctx, taskmgr, timermgr, NULL, &zonemgr); + result = dns_zonemgr_create(mctx, taskmgr, timermgr, NULL, ncpus, + &zonemgr); return (result); } @@ -457,11 +458,6 @@ ns_test_managezone(dns_zone_t *zone) { isc_result_t result; REQUIRE(zonemgr != NULL); - result = dns_zonemgr_setsize(zonemgr, 1); - if (result != ISC_R_SUCCESS) { - return (result); - } - result = dns_zonemgr_managezone(zonemgr, zone); return (result); } diff --git a/lib/ns/update.c b/lib/ns/update.c index 5659a8c916..0e5abf0d6d 100644 --- a/lib/ns/update.c +++ b/lib/ns/update.c @@ -19,7 +19,6 @@ #include #include #include -#include #include #include From 8b4ba366dd1050b152e4b6b81444d7ac8d1163ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 10 Jan 2022 22:02:20 +0100 Subject: [PATCH 2/6] Remove the zone counting in the named The zone counting in the named was used to properly size the zonemgr resources (memory contexts, zonetasks and loadtasks). Since this is no longer the case, remove the whole zone counting from named. --- bin/named/server.c | 229 +++++++++------------------------------------ 1 file changed, 46 insertions(+), 183 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 5311e91a60..a23f2d5929 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -137,11 +137,9 @@ #ifdef HAVE_LMDB #include -#define count_newzones count_newzones_db #define configure_newzones configure_newzones_db #define dumpzone dumpzone_db #else /* HAVE_LMDB */ -#define count_newzones count_newzones_file #define configure_newzones configure_newzones_file #define dumpzone dumpzone_file #endif /* HAVE_LMDB */ @@ -467,13 +465,7 @@ putuint8(isc_buffer_t **b, uint8_t val); static isc_result_t putnull(isc_buffer_t **b); -static int -count_zones(const cfg_obj_t *conf); - #ifdef HAVE_LMDB -static isc_result_t -migrate_nzf(dns_view_t *view); - static isc_result_t nzd_writable(dns_view_t *view); @@ -488,14 +480,14 @@ nzd_env_close(dns_view_t *view); static isc_result_t nzd_close(MDB_txn **txnp, bool commit); - -static isc_result_t -nzd_count(dns_view_t *view, int *countp); #else /* ifdef HAVE_LMDB */ static isc_result_t nzf_append(dns_view_t *view, const cfg_obj_t *zconfig); #endif /* ifdef HAVE_LMDB */ +static isc_result_t +load_nzf(dns_view_t *view, ns_cfgctx_t *nzcfg); + /*% * Configure a single view ACL at '*aclp'. Get its configuration from * 'vconfig' (for per-view configuration) and maybe from 'config' @@ -7714,94 +7706,9 @@ cleanup: return (result); } -#ifndef HAVE_LMDB -static isc_result_t -count_newzones(dns_view_t *view, ns_cfgctx_t *nzcfg, int *num_zonesp) { - isc_result_t result; - - /* The new zone file may not exist. That is OK. */ - if (!isc_file_exists(view->new_zone_file)) { - *num_zonesp = 0; - return (ISC_R_SUCCESS); - } - - /* - * In the case of NZF files, we also parse the configuration in - * the file at this stage. - * - * This may be called in multiple views, so we reset - * the parser each time. - */ - cfg_parser_reset(named_g_addparser); - result = cfg_parse_file(named_g_addparser, view->new_zone_file, - &cfg_type_addzoneconf, &nzcfg->nzf_config); - if (result == ISC_R_SUCCESS) { - int num_zones; - - num_zones = count_zones(nzcfg->nzf_config); - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, - "NZF file '%s' contains %d zones", - view->new_zone_file, num_zones); - if (num_zonesp != NULL) { - *num_zonesp = num_zones; - } - } else { - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, - "Error parsing NZF file '%s': %s", - view->new_zone_file, isc_result_totext(result)); - } - - return (result); -} - -#else /* HAVE_LMDB */ - -static isc_result_t -count_newzones(dns_view_t *view, ns_cfgctx_t *nzcfg, int *num_zonesp) { - isc_result_t result; - int n; - - UNUSED(nzcfg); - - REQUIRE(num_zonesp != NULL); - - LOCK(&view->new_zone_lock); - - CHECK(migrate_nzf(view)); - - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, - "loading NZD zone count from '%s' " - "for view '%s'", - view->new_zone_db, view->name); - - CHECK(nzd_count(view, &n)); - - *num_zonesp = n; - - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, - "NZD database '%s' contains %d zones", view->new_zone_db, - n); - -cleanup: - if (result != ISC_R_SUCCESS) { - *num_zonesp = 0; - } - - UNLOCK(&view->new_zone_lock); - - return (ISC_R_SUCCESS); -} - -#endif /* HAVE_LMDB */ - static isc_result_t setup_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, - cfg_parser_t *conf_parser, cfg_aclconfctx_t *actx, - int *num_zones) { + cfg_parser_t *conf_parser, cfg_aclconfctx_t *actx) { isc_result_t result = ISC_R_SUCCESS; bool allow = false; ns_cfgctx_t *nzcfg = NULL; @@ -7899,28 +7806,29 @@ setup_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, if (!allow) { dns_view_setnewzones(view, false, NULL, NULL, 0ULL); - if (num_zones != NULL) { - *num_zones = 0; - } return (ISC_R_SUCCESS); } nzcfg = isc_mem_get(view->mctx, sizeof(*nzcfg)); + *nzcfg = (ns_cfgctx_t){ 0 }; /* * We attach the parser that was used for config as well * as the one that will be used for added zones, to avoid * a shutdown race later. */ - memset(nzcfg, 0, sizeof(*nzcfg)); + isc_mem_attach(view->mctx, &nzcfg->mctx); cfg_parser_attach(conf_parser, &nzcfg->conf_parser); cfg_parser_attach(named_g_addparser, &nzcfg->add_parser); - isc_mem_attach(view->mctx, &nzcfg->mctx); cfg_aclconfctx_attach(actx, &nzcfg->actx); result = dns_view_setnewzones(view, true, nzcfg, newzone_cfgctx_destroy, mapsize); if (result != ISC_R_SUCCESS) { + cfg_aclconfctx_detach(&nzcfg->actx); + cfg_parser_destroy(&nzcfg->add_parser); + cfg_parser_destroy(&nzcfg->conf_parser); + isc_mem_putanddetach(&nzcfg->mctx, nzcfg, sizeof(*nzcfg)); dns_view_setnewzones(view, false, NULL, NULL, 0ULL); return (result); } @@ -7930,7 +7838,7 @@ setup_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, cfg_obj_attach(vconfig, &nzcfg->vconfig); } - result = count_newzones(view, nzcfg, num_zones); + result = load_nzf(view, nzcfg); return (result); } @@ -8320,24 +8228,6 @@ cleanup: #endif /* HAVE_LMDB */ -static int -count_zones(const cfg_obj_t *conf) { - const cfg_obj_t *zonelist = NULL; - const cfg_listelt_t *element; - int n = 0; - - REQUIRE(conf != NULL); - - cfg_map_get(conf, "zone", &zonelist); - for (element = cfg_list_first(zonelist); element != NULL; - element = cfg_list_next(element)) - { - n++; - } - - return (n); -} - static isc_result_t check_lockfile(named_server_t *server, const cfg_obj_t *config, bool first_time) { @@ -8443,7 +8333,6 @@ load_configuration(const char *filename, named_server_t *server, dns_viewlist_t viewlist, builtin_viewlist; in_port_t listen_port, udpport_low, udpport_high; int i, backlog; - int num_zones = 0; bool exclusive = false; isc_interval_t interval; isc_logconfig_t *logc = NULL; @@ -9185,19 +9074,14 @@ load_configuration(const char *filename, named_server_t *server, element = cfg_list_next(element)) { cfg_obj_t *vconfig = cfg_listelt_value(element); - const cfg_obj_t *voptions = cfg_tuple_get(vconfig, "options"); - int nzf_num_zones; view = NULL; CHECK(create_view(vconfig, &viewlist, &view)); INSIST(view != NULL); - num_zones += count_zones(voptions); - CHECK(setup_newzones(view, config, vconfig, conf_parser, - named_g_aclconfctx, &nzf_num_zones)); - num_zones += nzf_num_zones; + named_g_aclconfctx)); dns_view_detach(&view); } @@ -9207,16 +9091,11 @@ load_configuration(const char *filename, named_server_t *server, * view here. */ if (views == NULL) { - int nzf_num_zones; - CHECK(create_view(NULL, &viewlist, &view)); INSIST(view != NULL); - num_zones = count_zones(config); - CHECK(setup_newzones(view, config, NULL, conf_parser, - named_g_aclconfctx, &nzf_num_zones)); - num_zones += nzf_num_zones; + named_g_aclconfctx)); dns_view_detach(&view); } @@ -13089,7 +12968,32 @@ cleanup: return (result); } -#else /* HAVE_LMDB */ +static isc_result_t +load_nzf(dns_view_t *view, ns_cfgctx_t *nzcfg) { + isc_result_t result; + + /* The new zone file may not exist. That is OK. */ + if (!isc_file_exists(view->new_zone_file)) { + return (ISC_R_SUCCESS); + } + + /* + * Parse the configuration in the NZF file. This may be called in + * multiple views, so we reset the parser each time. + */ + cfg_parser_reset(named_g_addparser); + result = cfg_parse_file(named_g_addparser, view->new_zone_file, + &cfg_type_addzoneconf, &nzcfg->nzf_config); + if (result != ISC_R_SUCCESS) { + isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, + NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, + "Error parsing NZF file '%s': %s", + view->new_zone_file, isc_result_totext(result)); + } + + return (result); +} +#else /* HAVE_LMDB */ static void nzd_setkey(MDB_val *key, dns_name_t *name, char *namebuf, size_t buflen) { @@ -13414,52 +13318,16 @@ nzd_close(MDB_txn **txnp, bool commit) { } /* - * Count the zones configured in the new zone database for 'view' and store the - * result in 'countp'. + * If there's an existing NZF file, load it and migrate its data + * to the NZD. * - * Caller must hold 'view->new_zone_lock'. - */ -static isc_result_t -nzd_count(dns_view_t *view, int *countp) { - isc_result_t result; - int status; - MDB_txn *txn = NULL; - MDB_dbi dbi; - MDB_stat statbuf; - - REQUIRE(countp != NULL); - - result = nzd_open(view, MDB_RDONLY, &txn, &dbi); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } - - status = mdb_stat(txn, dbi, &statbuf); - if (status != MDB_SUCCESS) { - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, - NAMED_LOGMODULE_SERVER, ISC_LOG_WARNING, - "mdb_stat: %s", mdb_strerror(status)); - result = ISC_R_FAILURE; - goto cleanup; - } - - *countp = statbuf.ms_entries; - -cleanup: - (void)nzd_close(&txn, false); - - return (result); -} - -/* - * Migrate zone configuration from an NZF file to an NZD database. * Caller must hold view->new_zone_lock. */ static isc_result_t -migrate_nzf(dns_view_t *view) { +load_nzf(dns_view_t *view, ns_cfgctx_t *nzcfg) { isc_result_t result; cfg_obj_t *nzf_config = NULL; - int status, n; + int status; isc_buffer_t *text = NULL; bool commit = false; const cfg_obj_t *zonelist; @@ -13470,6 +13338,8 @@ migrate_nzf(dns_view_t *view) { MDB_val key, data; ns_dzarg_t dzarg; + UNUSED(nzcfg); + /* * If NZF file doesn't exist, or NZD DB exists and already * has data, return without attempting migration. @@ -13479,12 +13349,6 @@ migrate_nzf(dns_view_t *view) { goto cleanup; } - result = nzd_count(view, &n); - if (result == ISC_R_SUCCESS && n > 0) { - result = ISC_R_SUCCESS; - goto cleanup; - } - isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, "Migrating zones from NZF file '%s' to " @@ -13562,7 +13426,7 @@ migrate_nzf(dns_view_t *view) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR, "Error writing zone config to " - "buffer in migrate_nzf(): %s", + "buffer in load_nzf(): %s", isc_result_totext(result)); result = dzarg.result; goto cleanup; @@ -13615,7 +13479,6 @@ cleanup: return (result); } - #endif /* HAVE_LMDB */ static isc_result_t From 2707d0eeb72937ae370e289ea19f034e62199246 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 17 Mar 2022 23:37:26 +0100 Subject: [PATCH 3/6] Set hard thread affinity for each zone After switching to per-thread resources in the zonemgr, the performance was decreased because the memory context, zonetask and loadtask was picked from the pool at random. Pin the zone to single threadid (.tid) and align the memory context, zonetask and loadtask to be the same, this sets the hard affinity of the zone to the netmgr thread. --- bin/check/check-tool.c | 2 +- bin/named/server.c | 67 ++++++++-------- bin/tests/system/dyndb/driver/zone.c | 2 +- lib/dns/dlz.c | 4 +- lib/dns/include/dns/zone.h | 22 +++++- lib/dns/zone.c | 114 ++++++++++++++++----------- lib/isc/include/isc/pool.h | 2 +- lib/isc/pool.c | 6 +- lib/ns/tests/nstest.c | 2 +- 9 files changed, 131 insertions(+), 90 deletions(-) diff --git a/bin/check/check-tool.c b/bin/check/check-tool.c index ae100966c5..1fb6e7c7c1 100644 --- a/bin/check/check-tool.c +++ b/bin/check/check-tool.c @@ -593,7 +593,7 @@ load_zone(isc_mem_t *mctx, const char *zonename, const char *filename, zonename, filename, classname); } - CHECK(dns_zone_create(&zone, mctx)); + CHECK(dns_zone_create(&zone, mctx, 0)); dns_zone_settype(zone, dns_zone_primary); diff --git a/bin/named/server.c b/bin/named/server.c index a23f2d5929..21315de139 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -431,7 +431,7 @@ configure_alternates(const cfg_obj_t *config, dns_view_t *view, static isc_result_t configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig, - const cfg_obj_t *vconfig, isc_mem_t *mctx, dns_view_t *view, + const cfg_obj_t *vconfig, dns_view_t *view, dns_viewlist_t *viewlist, dns_kasplist_t *kasplist, cfg_aclconfctx_t *aclconf, bool added, bool old_rpz_ok, bool modify); @@ -442,7 +442,7 @@ configure_zone_setviewcommit(isc_result_t result, const cfg_obj_t *zconfig, static isc_result_t configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, - isc_mem_t *mctx, cfg_aclconfctx_t *actx); + cfg_aclconfctx_t *actx); static isc_result_t add_keydata_zone(dns_view_t *view, const char *directory, isc_mem_t *mctx); @@ -1951,7 +1951,7 @@ dns64_reverse(dns_view_t *view, isc_mem_t *mctx, isc_netaddr_t *na, isc_buffer_constinit(&b, reverse, strlen(reverse)); isc_buffer_add(&b, strlen(reverse)); CHECK(dns_name_fromtext(name, &b, dns_rootname, 0, NULL)); - CHECK(dns_zone_create(&zone, mctx)); + CHECK(dns_zone_create(&zone, mctx, 0)); CHECK(dns_zone_setorigin(zone, name)); dns_zone_setview(zone, view); CHECK(dns_zonemgr_managezone(named_g_server->zonemgr, zone)); @@ -2773,10 +2773,10 @@ catz_addmodzone_taskaction(isc_task_t *task, isc_event_t *event0) { result = isc_task_beginexclusive(task); RUNTIME_CHECK(result == ISC_R_SUCCESS); dns_view_thaw(ev->view); - result = configure_zone( - cfg->config, zoneobj, cfg->vconfig, ev->cbd->server->mctx, - ev->view, &ev->cbd->server->viewlist, - &ev->cbd->server->kasplist, cfg->actx, true, false, ev->mod); + result = configure_zone(cfg->config, zoneobj, cfg->vconfig, ev->view, + &ev->cbd->server->viewlist, + &ev->cbd->server->kasplist, cfg->actx, true, + false, ev->mod); dns_view_freeze(ev->view); isc_task_endexclusive(task); @@ -3620,7 +3620,7 @@ create_ipv4only_zone(dns_zone_t *pzone, dns_view_t *view, /* * Create the actual zone. */ - CHECK(dns_zone_create(&zone, mctx)); + CHECK(dns_zone_create(&zone, mctx, 0)); CHECK(dns_zone_setorigin(zone, name)); CHECK(dns_zonemgr_managezone(named_g_server->zonemgr, zone)); dns_zone_setclass(zone, view->rdclass); @@ -4139,9 +4139,8 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, element = cfg_list_next(element)) { const cfg_obj_t *zconfig = cfg_listelt_value(element); - CHECK(configure_zone(config, zconfig, vconfig, mctx, view, - viewlist, kasplist, actx, false, - old_rpz_ok, false)); + CHECK(configure_zone(config, zconfig, vconfig, view, viewlist, + kasplist, actx, false, old_rpz_ok, false)); } zones_configured = true; @@ -4177,7 +4176,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config, * from the newzone file for zones that were added during previous * runs. */ - CHECK(configure_newzones(view, config, vconfig, mctx, actx)); + CHECK(configure_newzones(view, config, vconfig, actx)); /* * Create Dynamically Loadable Zone driver. @@ -6412,7 +6411,7 @@ create_view(const cfg_obj_t *vconfig, dns_viewlist_t *viewlist, */ static isc_result_t configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig, - const cfg_obj_t *vconfig, isc_mem_t *mctx, dns_view_t *view, + const cfg_obj_t *vconfig, dns_view_t *view, dns_viewlist_t *viewlist, dns_kasplist_t *kasplist, cfg_aclconfctx_t *aclconf, bool added, bool old_rpz_ok, bool modify) { @@ -6796,7 +6795,8 @@ configure_zone(const cfg_obj_t *config, const cfg_obj_t *zconfig, if (inline_signing) { dns_zone_getraw(zone, &raw); if (raw == NULL) { - CHECK(dns_zone_create(&raw, mctx)); + CHECK(dns_zone_create(&raw, dns_zone_getmem(zone), + dns_zone_gettid(zone))); CHECK(dns_zone_setorigin(raw, origin)); dns_zone_setview(raw, view); dns_zone_setstats(raw, named_g_server->zonestats); @@ -7886,7 +7886,7 @@ configure_zone_setviewcommit(isc_result_t result, const cfg_obj_t *zconfig, static isc_result_t configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, - isc_mem_t *mctx, cfg_aclconfctx_t *actx) { + cfg_aclconfctx_t *actx) { isc_result_t result; ns_cfgctx_t *nzctx; const cfg_obj_t *zonelist; @@ -7908,7 +7908,7 @@ configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, element = cfg_list_next(element)) { const cfg_obj_t *zconfig = cfg_listelt_value(element); - CHECK(configure_zone(config, zconfig, vconfig, mctx, view, + CHECK(configure_zone(config, zconfig, vconfig, view, &named_g_server->viewlist, &named_g_server->kasplist, actx, true, false, false)); @@ -8005,7 +8005,7 @@ cleanup: */ typedef isc_result_t (*newzone_cfg_cb_t)(const cfg_obj_t *zconfig, cfg_obj_t *config, cfg_obj_t *vconfig, - isc_mem_t *mctx, dns_view_t *view, + dns_view_t *view, cfg_aclconfctx_t *actx); /*% @@ -8021,7 +8021,7 @@ typedef isc_result_t (*newzone_cfg_cb_t)(const cfg_obj_t *zconfig, */ static isc_result_t for_all_newzone_cfgs(newzone_cfg_cb_t callback, cfg_obj_t *config, - cfg_obj_t *vconfig, isc_mem_t *mctx, dns_view_t *view, + cfg_obj_t *vconfig, dns_view_t *view, cfg_aclconfctx_t *actx, MDB_txn *txn, MDB_dbi dbi) { const cfg_obj_t *zconfig, *zlist; isc_result_t result = ISC_R_SUCCESS; @@ -8064,7 +8064,7 @@ for_all_newzone_cfgs(newzone_cfg_cb_t callback, cfg_obj_t *config, /* * Invoke callback. */ - result = callback(zconfig, config, vconfig, mctx, view, actx); + result = callback(zconfig, config, vconfig, view, actx); if (result != ISC_R_SUCCESS) { break; } @@ -8091,10 +8091,10 @@ for_all_newzone_cfgs(newzone_cfg_cb_t callback, cfg_obj_t *config, */ static isc_result_t configure_newzone(const cfg_obj_t *zconfig, cfg_obj_t *config, - cfg_obj_t *vconfig, isc_mem_t *mctx, dns_view_t *view, + cfg_obj_t *vconfig, dns_view_t *view, cfg_aclconfctx_t *actx) { return (configure_zone( - config, zconfig, vconfig, mctx, view, &named_g_server->viewlist, + config, zconfig, vconfig, view, &named_g_server->viewlist, &named_g_server->kasplist, actx, true, false, false)); } @@ -8103,11 +8103,10 @@ configure_newzone(const cfg_obj_t *zconfig, cfg_obj_t *config, */ static isc_result_t configure_newzone_revert(const cfg_obj_t *zconfig, cfg_obj_t *config, - cfg_obj_t *vconfig, isc_mem_t *mctx, dns_view_t *view, + cfg_obj_t *vconfig, dns_view_t *view, cfg_aclconfctx_t *actx) { UNUSED(config); UNUSED(vconfig); - UNUSED(mctx); UNUSED(actx); configure_zone_setviewcommit(ISC_R_FAILURE, zconfig, view); @@ -8117,7 +8116,7 @@ configure_newzone_revert(const cfg_obj_t *zconfig, cfg_obj_t *config, static isc_result_t configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, - isc_mem_t *mctx, cfg_aclconfctx_t *actx) { + cfg_aclconfctx_t *actx) { isc_result_t result; MDB_txn *txn = NULL; MDB_dbi dbi; @@ -8140,8 +8139,8 @@ configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, "for view '%s'", view->new_zone_db, view->name); - result = for_all_newzone_cfgs(configure_newzone, config, vconfig, mctx, - view, actx, txn, dbi); + result = for_all_newzone_cfgs(configure_newzone, config, vconfig, view, + actx, txn, dbi); if (result != ISC_R_SUCCESS) { /* * An error was encountered while attempting to configure zones @@ -8152,7 +8151,7 @@ configure_newzones(dns_view_t *view, cfg_obj_t *config, cfg_obj_t *vconfig, * terms of trying to make things right. */ (void)for_all_newzone_cfgs(configure_newzone_revert, config, - vconfig, mctx, view, actx, txn, dbi); + vconfig, view, actx, txn, dbi); } (void)nzd_close(&txn, false); @@ -13733,10 +13732,9 @@ do_addzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, /* Mark view unfrozen and configure zone */ dns_view_thaw(view); - result = configure_zone(cfg->config, zoneobj, cfg->vconfig, - server->mctx, view, &server->viewlist, - &server->kasplist, cfg->actx, true, false, - false); + result = configure_zone(cfg->config, zoneobj, cfg->vconfig, view, + &server->viewlist, &server->kasplist, cfg->actx, + true, false, false); dns_view_freeze(view); isc_task_endexclusive(server->task); @@ -13921,10 +13919,9 @@ do_modzone(named_server_t *server, ns_cfgctx_t *cfg, dns_view_t *view, /* Reconfigure the zone */ dns_view_thaw(view); - result = configure_zone(cfg->config, zoneobj, cfg->vconfig, - server->mctx, view, &server->viewlist, - &server->kasplist, cfg->actx, true, false, - true); + result = configure_zone(cfg->config, zoneobj, cfg->vconfig, view, + &server->viewlist, &server->kasplist, cfg->actx, + true, false, true); dns_view_freeze(view); exclusive = false; diff --git a/bin/tests/system/dyndb/driver/zone.c b/bin/tests/system/dyndb/driver/zone.c index 59f87a61cf..bae2d60ef7 100644 --- a/bin/tests/system/dyndb/driver/zone.c +++ b/bin/tests/system/dyndb/driver/zone.c @@ -67,7 +67,7 @@ create_zone(sample_instance_t *const inst, dns_name_t *const name, zone_argv[0] = inst->db_name; - result = dns_zone_create(&raw, inst->mctx); + result = dns_zone_create(&raw, inst->mctx, 0); /* FIXME */ if (result != ISC_R_SUCCESS) { log_write(ISC_LOG_ERROR, "create_zone: dns_zone_create -> %s\n", isc_result_totext(result)); diff --git a/lib/dns/dlz.c b/lib/dns/dlz.c index 4462a7b001..bf76190249 100644 --- a/lib/dns/dlz.c +++ b/lib/dns/dlz.c @@ -58,7 +58,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -454,7 +456,7 @@ dns_dlz_writeablezone(dns_view_t *view, dns_dlzdb_t *dlzdb, INSIST(dupzone == NULL); /* Create it */ - result = dns_zone_create(&zone, view->mctx); + result = dns_zone_create(&zone, view->mctx, 0); if (result != ISC_R_SUCCESS) { goto cleanup; } diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index 52bc941da0..0564d5bc96 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -147,7 +147,7 @@ ISC_LANG_BEGINDECLS ***/ isc_result_t -dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx); +dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx, unsigned int tid); /*%< * Creates a new empty zone and attach '*zonep' to it. * @@ -2751,3 +2751,23 @@ dns_zonetype_name(dns_zonetype_t type); /*%< * Return the name of the zone type 'type'. */ + +isc_mem_t * +dns_zone_getmem(dns_zone_t *zone); +/**< + * \brief Return memory context associated with the zone. + * + * @param zone valid dns_zone_t object. + * + * @return memory context associated with the zone + */ + +unsigned int +dns_zone_gettid(dns_zone_t *zone); +/**< + * \brief Return thread-id associated with the zone. + * + * \param valid dns_zone_t object + * + * \return thread id associated with the zone + */ diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 971f6eee10..90c079a723 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -234,6 +234,8 @@ struct dns_zone { isc_rwlock_t dblock; dns_db_t *db; /* Locked by dblock */ + unsigned int tid; + /* Locked */ dns_zonemgr_t *zmgr; ISC_LINK(dns_zone_t) link; /* Used by zmgr. */ @@ -595,6 +597,7 @@ struct dns_zonemgr { isc_taskmgr_t *taskmgr; isc_timermgr_t *timermgr; isc_nm_t *netmgr; + unsigned int workers; atomic_uint_fast32_t nzonetasks; isc_pool_t *zonetasks; atomic_uint_fast32_t nloadtasks; @@ -1103,48 +1106,50 @@ inc_stats(dns_zone_t *zone, isc_statscounter_t counter) { ***/ isc_result_t -dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx) { +dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx, unsigned int tid) { isc_result_t result; isc_time_t now; dns_zone_t *zone = NULL; - dns_zone_t z = { .masterformat = dns_masterformat_none, - .journalsize = -1, - .rdclass = dns_rdataclass_none, - .type = dns_zone_none, - .refresh = DNS_ZONE_DEFAULTREFRESH, - .retry = DNS_ZONE_DEFAULTRETRY, - .maxrefresh = DNS_ZONE_MAXREFRESH, - .minrefresh = DNS_ZONE_MINREFRESH, - .maxretry = DNS_ZONE_MAXRETRY, - .minretry = DNS_ZONE_MINRETRY, - .notifytype = dns_notifytype_yes, - .zero_no_soa_ttl = true, - .check_names = dns_severity_ignore, - .idlein = DNS_DEFAULT_IDLEIN, - .idleout = DNS_DEFAULT_IDLEOUT, - .notifysrc4dscp = -1, - .notifysrc6dscp = -1, - .parentalsrc4dscp = -1, - .parentalsrc6dscp = -1, - .xfrsource4dscp = -1, - .xfrsource6dscp = -1, - .altxfrsource4dscp = -1, - .altxfrsource6dscp = -1, - .maxxfrin = MAX_XFER_TIME, - .maxxfrout = MAX_XFER_TIME, - .sigvalidityinterval = 30 * 24 * 3600, - .sigresigninginterval = 7 * 24 * 3600, - .statlevel = dns_zonestat_none, - .notifydelay = 5, - .signatures = 10, - .nodes = 100, - .privatetype = (dns_rdatatype_t)0xffffU, - .rpz_num = DNS_RPZ_INVALID_NUM, - .requestixfr = true, - .ixfr_ratio = 100, - .requestexpire = true, - .updatemethod = dns_updatemethod_increment, - .magic = ZONE_MAGIC }; + dns_zone_t z = { + .masterformat = dns_masterformat_none, + .journalsize = -1, + .rdclass = dns_rdataclass_none, + .type = dns_zone_none, + .refresh = DNS_ZONE_DEFAULTREFRESH, + .retry = DNS_ZONE_DEFAULTRETRY, + .maxrefresh = DNS_ZONE_MAXREFRESH, + .minrefresh = DNS_ZONE_MINREFRESH, + .maxretry = DNS_ZONE_MAXRETRY, + .minretry = DNS_ZONE_MINRETRY, + .notifytype = dns_notifytype_yes, + .zero_no_soa_ttl = true, + .check_names = dns_severity_ignore, + .idlein = DNS_DEFAULT_IDLEIN, + .idleout = DNS_DEFAULT_IDLEOUT, + .notifysrc4dscp = -1, + .notifysrc6dscp = -1, + .parentalsrc4dscp = -1, + .parentalsrc6dscp = -1, + .xfrsource4dscp = -1, + .xfrsource6dscp = -1, + .altxfrsource4dscp = -1, + .altxfrsource6dscp = -1, + .maxxfrin = MAX_XFER_TIME, + .maxxfrout = MAX_XFER_TIME, + .sigvalidityinterval = 30 * 24 * 3600, + .sigresigninginterval = 7 * 24 * 3600, + .statlevel = dns_zonestat_none, + .notifydelay = 5, + .signatures = 10, + .nodes = 100, + .privatetype = (dns_rdatatype_t)0xffffU, + .rpz_num = DNS_RPZ_INVALID_NUM, + .requestixfr = true, + .ixfr_ratio = 100, + .requestexpire = true, + .updatemethod = dns_updatemethod_increment, + .tid = tid, + }; REQUIRE(zonep != NULL && *zonep == NULL); REQUIRE(mctx != NULL); @@ -1206,6 +1211,8 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx) { goto free_refs; } + zone->magic = ZONE_MAGIC; + /* Must be after magic is set. */ dns_zone_setdbtype(zone, dbargc_default, dbargv_default); @@ -18938,7 +18945,7 @@ dns_zonemgr_createzone(dns_zonemgr_t *zmgr, dns_zone_t **zonep) { isc_result_t result; isc_mem_t *mctx = NULL; dns_zone_t *zone = NULL; - void *item; + unsigned int tid; REQUIRE(DNS_ZONEMGR_VALID(zmgr)); REQUIRE(zonep != NULL && *zonep == NULL); @@ -18947,14 +18954,14 @@ dns_zonemgr_createzone(dns_zonemgr_t *zmgr, dns_zone_t **zonep) { return (ISC_R_FAILURE); } - item = isc_pool_get(zmgr->mctxpool); - if (item == NULL) { + tid = isc_random_uniform(zmgr->workers); + + mctx = isc_pool_get(zmgr->mctxpool, tid); + if (mctx == NULL) { return (ISC_R_FAILURE); } - isc_mem_attach((isc_mem_t *)item, &mctx); - result = dns_zone_create(&zone, mctx); - isc_mem_detach(&mctx); + result = dns_zone_create(&zone, mctx, tid); if (result == ISC_R_SUCCESS) { *zonep = zone; @@ -18978,8 +18985,9 @@ dns_zonemgr_managezone(dns_zonemgr_t *zmgr, dns_zone_t *zone) { REQUIRE(zone->timer == NULL); REQUIRE(zone->zmgr == NULL); - isc_task_attach(isc_pool_get(zmgr->zonetasks), &zone->task); - isc_task_attach(isc_pool_get(zmgr->loadtasks), &zone->loadtask); + isc_task_attach(isc_pool_get(zmgr->zonetasks, zone->tid), &zone->task); + isc_task_attach(isc_pool_get(zmgr->loadtasks, zone->tid), + &zone->loadtask); /* * Set the task name. The tag will arbitrarily point to one @@ -19201,6 +19209,8 @@ static isc_result_t zonemgr_setsize(dns_zonemgr_t *zmgr, unsigned int workers) { isc_result_t result; + zmgr->workers = workers; + /* Create the zone tasks pool. */ REQUIRE(zmgr->zonetasks == NULL); result = isc_pool_create(zmgr->mctx, workers, taskfree, zonetaskinit, @@ -23713,3 +23723,13 @@ dns_zonemgr_set_tlsctx_cache(dns_zonemgr_t *zmgr, RWUNLOCK(&zmgr->rwlock, isc_rwlocktype_write); } + +isc_mem_t * +dns_zone_getmem(dns_zone_t *zone) { + return (zone->mctx); +} + +unsigned int +dns_zone_gettid(dns_zone_t *zone) { + return (zone->tid); +} diff --git a/lib/isc/include/isc/pool.h b/lib/isc/include/isc/pool.h index 1333cb7430..a08e1f21a4 100644 --- a/lib/isc/include/isc/pool.h +++ b/lib/isc/include/isc/pool.h @@ -81,7 +81,7 @@ isc_pool_create(isc_mem_t *mctx, unsigned int count, isc_pooldeallocator_t free, */ void * -isc_pool_get(isc_pool_t *pool); +isc_pool_get(isc_pool_t *pool, unsigned int tid); /*%< * Returns a pointer to an object from the pool. Currently the object * is chosen from the pool at random. diff --git a/lib/isc/pool.c b/lib/isc/pool.c index 251e353321..66fa09c1fa 100644 --- a/lib/isc/pool.c +++ b/lib/isc/pool.c @@ -73,8 +73,10 @@ isc_pool_create(isc_mem_t *mctx, unsigned int count, } void * -isc_pool_get(isc_pool_t *pool) { - return (pool->pool[isc_random_uniform(pool->count)]); +isc_pool_get(isc_pool_t *pool, unsigned int tid) { + REQUIRE(tid < pool->count); + + return (pool->pool[tid]); } void diff --git a/lib/ns/tests/nstest.c b/lib/ns/tests/nstest.c index 2e5d85cf29..d4e126b848 100644 --- a/lib/ns/tests/nstest.c +++ b/lib/ns/tests/nstest.c @@ -412,7 +412,7 @@ ns_test_makezone(const char *name, dns_zone_t **zonep, dns_view_t *view, zone = *zonep; if (zone == NULL) { - CHECK(dns_zone_create(&zone, mctx)); + CHECK(dns_zone_create(&zone, mctx, 0)); } isc_buffer_constinit(&buffer, name, strlen(name)); From 2bc7303af20363178bf28a4146889a7a3e8c111c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 18 Mar 2022 10:14:17 +0100 Subject: [PATCH 4/6] Use isc_nm_getnworkers to manage zone resources Instead of passing the number of worker to the dns_zonemgr manually, get the number of nm threads using the new isc_nm_getnworkers() call. Additionally, remove the isc_pool API and manage the array of memory context, zonetasks and loadtasks directly in the zonemgr. --- bin/named/server.c | 2 +- lib/dns/include/dns/zone.h | 16 +-- lib/dns/tests/dnstest.c | 5 +- lib/dns/tests/zonemgr_test.c | 8 +- lib/dns/zone.c | 260 ++++++++++++----------------------- lib/isc/tests/pool_test.c | 6 +- lib/ns/tests/nstest.c | 3 +- 7 files changed, 106 insertions(+), 194 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index 21315de139..ba197e4711 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -10062,7 +10062,7 @@ named_server_create(isc_mem_t *mctx, named_server_t **serverp) { CHECKFATAL(dns_zonemgr_create(named_g_mctx, named_g_taskmgr, named_g_timermgr, named_g_netmgr, - named_g_cpus, &server->zonemgr), + &server->zonemgr), "dns_zonemgr_create"); server->statsfile = isc_mem_strdup(server->mctx, "named.stats"); diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index 0564d5bc96..cda0aa1bf8 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -1479,16 +1479,6 @@ dns_zone_getredirecttype(dns_zone_t *zone); *\li 'dns_zone_secondary' */ -void -dns_zone_settask(dns_zone_t *zone, isc_task_t *task); -/*%< - * Give a zone a task to work with. Any current task will be detached. - * - * Requires: - *\li 'zone' to be valid. - *\li 'task' to be valid. - */ - void dns_zone_gettask(dns_zone_t *zone, isc_task_t **target); /*%< @@ -1783,7 +1773,7 @@ dns_zone_getdnsseckeys(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, isc_result_t dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, isc_timermgr_t *timermgr, isc_nm_t *netmgr, - unsigned int workers, dns_zonemgr_t **zmgrp); + dns_zonemgr_t **zmgrp); /*%< * Create a zone manager. * @@ -2757,9 +2747,9 @@ dns_zone_getmem(dns_zone_t *zone); /**< * \brief Return memory context associated with the zone. * - * @param zone valid dns_zone_t object. + * \param zone valid dns_zone_t object. * - * @return memory context associated with the zone + * \return memory context associated with the zone */ unsigned int diff --git a/lib/dns/tests/dnstest.c b/lib/dns/tests/dnstest.c index cf60c428d1..7da8c3cc83 100644 --- a/lib/dns/tests/dnstest.c +++ b/lib/dns/tests/dnstest.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -236,7 +237,7 @@ dns_test_makezone(const char *name, dns_zone_t **zonep, dns_view_t *view, /* * Create the zone structure. */ - result = dns_zone_create(&zone, dt_mctx); + result = dns_zone_create(&zone, dt_mctx, 0); if (result != ISC_R_SUCCESS) { return (result); } @@ -292,7 +293,7 @@ dns_test_setupzonemgr(void) { isc_result_t result; REQUIRE(zonemgr == NULL); - result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, NULL, ncpus, + result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, netmgr, &zonemgr); return (result); } diff --git a/lib/dns/tests/zonemgr_test.c b/lib/dns/tests/zonemgr_test.c index 2a76eaf5e8..e2f48e62a4 100644 --- a/lib/dns/tests/zonemgr_test.c +++ b/lib/dns/tests/zonemgr_test.c @@ -64,7 +64,7 @@ zonemgr_create(void **state) { UNUSED(state); - result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, NULL, 1, + result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, netmgr, &myzonemgr); assert_int_equal(result, ISC_R_SUCCESS); @@ -82,7 +82,7 @@ zonemgr_managezone(void **state) { UNUSED(state); - result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, NULL, 1, + result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, netmgr, &myzonemgr); assert_int_equal(result, ISC_R_SUCCESS); @@ -116,7 +116,7 @@ zonemgr_createzone(void **state) { UNUSED(state); - result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, NULL, 1, + result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, netmgr, &myzonemgr); assert_int_equal(result, ISC_R_SUCCESS); @@ -147,7 +147,7 @@ zonemgr_unreachable(void **state) { TIME_NOW(&now); - result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, NULL, 1, + result = dns_zonemgr_create(dt_mctx, taskmgr, timermgr, netmgr, &myzonemgr); assert_int_equal(result, ISC_R_SUCCESS); diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 90c079a723..e8f5092cbb 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -597,13 +597,11 @@ struct dns_zonemgr { isc_taskmgr_t *taskmgr; isc_timermgr_t *timermgr; isc_nm_t *netmgr; - unsigned int workers; - atomic_uint_fast32_t nzonetasks; - isc_pool_t *zonetasks; - atomic_uint_fast32_t nloadtasks; - isc_pool_t *loadtasks; + uint32_t workers; isc_task_t *task; - isc_pool_t *mctxpool; + isc_task_t **zonetasks; + isc_task_t **loadtasks; + isc_mem_t **mctxpool; isc_ratelimiter_t *checkdsrl; isc_ratelimiter_t *notifyrl; isc_ratelimiter_t *refreshrl; @@ -975,8 +973,6 @@ static void zonemgr_putio(dns_io_t **iop); static void zonemgr_cancelio(dns_io_t *io); -static isc_result_t -zonemgr_setsize(dns_zonemgr_t *zmgr, unsigned int workers); static void rss_post(dns_zone_t *, isc_event_t *); @@ -16249,23 +16245,6 @@ dns_zone_getorigin(dns_zone_t *zone) { return (&zone->origin); } -void -dns_zone_settask(dns_zone_t *zone, isc_task_t *task) { - REQUIRE(DNS_ZONE_VALID(zone)); - - LOCK_ZONE(zone); - if (zone->task != NULL) { - isc_task_detach(&zone->task); - } - isc_task_attach(task, &zone->task); - ZONEDB_LOCK(&zone->dblock, isc_rwlocktype_read); - if (zone->db != NULL) { - dns_db_settask(zone->db, zone->task); - } - ZONEDB_UNLOCK(&zone->dblock, isc_rwlocktype_read); - UNLOCK_ZONE(zone); -} - void dns_zone_gettask(dns_zone_t *zone, isc_task_t **target) { REQUIRE(DNS_ZONE_VALID(zone)); @@ -18809,30 +18788,29 @@ zonemgr_keymgmt_find(dns_zonemgr_t *zmgr, dns_zone_t *zone, isc_result_t dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, isc_timermgr_t *timermgr, isc_nm_t *netmgr, - unsigned int workers, dns_zonemgr_t **zmgrp) { + dns_zonemgr_t **zmgrp) { dns_zonemgr_t *zmgr; isc_result_t result; - REQUIRE(workers >= 1); + REQUIRE(mctx != NULL); + REQUIRE(taskmgr != NULL); + REQUIRE(timermgr != NULL); + REQUIRE(netmgr != NULL); zmgr = isc_mem_get(mctx, sizeof(*zmgr)); - zmgr->mctx = NULL; + + *zmgr = (dns_zonemgr_t){ + .taskmgr = taskmgr, + .timermgr = timermgr, + .netmgr = netmgr, + .workers = isc_nm_getnworkers(netmgr), + .transfersin = 10, + .transfersperns = 2, + }; + isc_refcount_init(&zmgr->refs, 1); isc_mem_attach(mctx, &zmgr->mctx); - zmgr->taskmgr = taskmgr; - zmgr->timermgr = timermgr; - zmgr->netmgr = netmgr; - zmgr->zonetasks = NULL; - atomic_init(&zmgr->nzonetasks, 0); - zmgr->loadtasks = NULL; - atomic_init(&zmgr->nloadtasks, 0); - zmgr->mctxpool = NULL; - zmgr->task = NULL; - zmgr->checkdsrl = NULL; - zmgr->notifyrl = NULL; - zmgr->refreshrl = NULL; - zmgr->startupnotifyrl = NULL; - zmgr->startuprefreshrl = NULL; + ISC_LIST_INIT(zmgr->zones); ISC_LIST_INIT(zmgr->waiting_for_xfrin); ISC_LIST_INIT(zmgr->xfrin_in_progress); @@ -18842,9 +18820,6 @@ dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, } isc_rwlock_init(&zmgr->rwlock, 0, 0); - zmgr->transfersin = 10; - zmgr->transfersperns = 2; - /* Unreachable lock. */ isc_rwlock_init(&zmgr->urlock, 0, 0); @@ -18885,9 +18860,37 @@ dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, goto free_startupnotifyrl; } - result = zonemgr_setsize(zmgr, workers); - if (result != ISC_R_SUCCESS) { - goto free_startuprefreshrl; + zmgr->zonetasks = isc_mem_get( + zmgr->mctx, zmgr->workers * sizeof(zmgr->zonetasks[0])); + memset(zmgr->zonetasks, 0, zmgr->workers * sizeof(zmgr->zonetasks[0])); + for (size_t i = 0; i < zmgr->workers; i++) { + result = isc_task_create_bound(zmgr->taskmgr, 2, + &zmgr->zonetasks[i], i); + if (result != ISC_R_SUCCESS) { + goto free_zonetasks; + } + isc_task_setname(zmgr->zonetasks[i], "zonemgr-zonetasks", NULL); + } + + zmgr->loadtasks = isc_mem_get( + zmgr->mctx, zmgr->workers * sizeof(zmgr->loadtasks[0])); + memset(zmgr->loadtasks, 0, zmgr->workers * sizeof(zmgr->loadtasks[0])); + for (size_t i = 0; i < zmgr->workers; i++) { + result = isc_task_create_bound(zmgr->taskmgr, UINT_MAX, + &zmgr->loadtasks[i], i); + if (result != ISC_R_SUCCESS) { + goto free_loadtasks; + } + isc_task_setname(zmgr->loadtasks[i], "zonemgr-loadtasks", NULL); + isc_task_setprivilege(zmgr->loadtasks[i], true); + } + + zmgr->mctxpool = isc_mem_get(zmgr->mctx, + zmgr->workers * sizeof(zmgr->mctxpool[0])); + memset(zmgr->mctxpool, 0, zmgr->workers * sizeof(zmgr->mctxpool[0])); + for (size_t i = 0; i < zmgr->workers; i++) { + isc_mem_create(&zmgr->mctxpool[i]); + isc_mem_setname(zmgr->mctxpool[i], "zonemgr-mctxpool"); } /* Key file I/O locks. */ @@ -18920,7 +18923,24 @@ dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, free_iolock: isc_mutex_destroy(&zmgr->iolock); #endif /* if 0 */ -free_startuprefreshrl: +free_loadtasks: + for (size_t i = 0; i < zmgr->workers; i++) { + if (zmgr->loadtasks[i] != NULL) { + isc_task_detach(&zmgr->loadtasks[i]); + } + } + isc_mem_put(zmgr->mctx, zmgr->loadtasks, + zmgr->workers * sizeof(zmgr->loadtasks[0])); + +free_zonetasks: + for (size_t i = 0; i < zmgr->workers; i++) { + if (zmgr->zonetasks[i] != NULL) { + isc_task_detach(&zmgr->zonetasks[i]); + } + } + isc_mem_put(zmgr->mctx, zmgr->zonetasks, + zmgr->workers * sizeof(zmgr->zonetasks[0])); + isc_ratelimiter_detach(&zmgr->startuprefreshrl); free_startupnotifyrl: isc_ratelimiter_detach(&zmgr->startupnotifyrl); @@ -18956,7 +18976,7 @@ dns_zonemgr_createzone(dns_zonemgr_t *zmgr, dns_zone_t **zonep) { tid = isc_random_uniform(zmgr->workers); - mctx = isc_pool_get(zmgr->mctxpool, tid); + mctx = zmgr->mctxpool[tid]; if (mctx == NULL) { return (ISC_R_FAILURE); } @@ -18985,9 +19005,8 @@ dns_zonemgr_managezone(dns_zonemgr_t *zmgr, dns_zone_t *zone) { REQUIRE(zone->timer == NULL); REQUIRE(zone->zmgr == NULL); - isc_task_attach(isc_pool_get(zmgr->zonetasks, zone->tid), &zone->task); - isc_task_attach(isc_pool_get(zmgr->loadtasks, zone->tid), - &zone->loadtask); + isc_task_attach(zmgr->zonetasks[zone->tid], &zone->task); + isc_task_attach(zmgr->loadtasks[zone->tid], &zone->loadtask); /* * Set the task name. The tag will arbitrarily point to one @@ -19119,15 +19138,28 @@ dns_zonemgr_shutdown(dns_zonemgr_t *zmgr) { if (zmgr->task != NULL) { isc_task_destroy(&zmgr->task); } - if (zmgr->zonetasks != NULL) { - isc_pool_destroy(&zmgr->zonetasks); + + for (size_t i = 0; i < zmgr->workers; i++) { + isc_mem_detach(&zmgr->mctxpool[i]); } - if (zmgr->loadtasks != NULL) { - isc_pool_destroy(&zmgr->loadtasks); + isc_mem_put(zmgr->mctx, zmgr->mctxpool, + zmgr->workers * sizeof(zmgr->mctxpool[0])); + + for (size_t i = 0; i < zmgr->workers; i++) { + if (zmgr->loadtasks[i] != NULL) { + isc_task_detach(&zmgr->loadtasks[i]); + } } - if (zmgr->mctxpool != NULL) { - isc_pool_destroy(&zmgr->mctxpool); + isc_mem_put(zmgr->mctx, zmgr->loadtasks, + zmgr->workers * sizeof(zmgr->loadtasks[0])); + + for (size_t i = 0; i < zmgr->workers; i++) { + if (zmgr->zonetasks[i] != NULL) { + isc_task_detach(&zmgr->zonetasks[i]); + } } + isc_mem_put(zmgr->mctx, zmgr->zonetasks, + zmgr->workers * sizeof(zmgr->zonetasks[0])); RWLOCK(&zmgr->rwlock, isc_rwlocktype_read); for (zone = ISC_LIST_HEAD(zmgr->zones); zone != NULL; @@ -19140,116 +19172,6 @@ dns_zonemgr_shutdown(dns_zonemgr_t *zmgr) { RWUNLOCK(&zmgr->rwlock, isc_rwlocktype_read); } -static isc_result_t -taskinit(void **target, void *arg, uint32_t bindto, unsigned int quantum, - bool priv) { - dns_zonemgr_t *zmgr = (dns_zonemgr_t *)arg; - isc_task_t *task = NULL; - isc_result_t result = isc_task_create_bound(zmgr->taskmgr, quantum, - &task, bindto); - if (result != ISC_R_SUCCESS) { - return (result); - } - isc_task_setprivilege(task, priv); - isc_task_setname(task, "zonemgr-taskpool", NULL); - *target = task; - - return (ISC_R_SUCCESS); -} - -static void -taskfree(void **target) { - isc_task_t *task = *target; - isc_task_detach(&task); -} - -static isc_result_t -zonetaskinit(void **target, void *arg) { - dns_zonemgr_t *zmgr = (dns_zonemgr_t *)arg; - uint32_t bindto = atomic_fetch_add(&zmgr->nzonetasks, 1); - return (taskinit(target, arg, bindto, 2, false)); -} - -static isc_result_t -loadtaskinit(void **target, void *arg) { - /* - * We always set all tasks in the zone-load task pool to - * privileged. This prevents other tasks in the system from - * running while the server task manager is in privileged - * mode. - */ - dns_zonemgr_t *zmgr = (dns_zonemgr_t *)arg; - uint32_t bindto = atomic_fetch_add(&zmgr->nloadtasks, 1); - return (taskinit(target, arg, bindto, UINT_MAX, true)); -} - -static isc_result_t -mctxinit(void **target, void *arg) { - isc_mem_t *mctx = NULL; - - UNUSED(arg); - - REQUIRE(target != NULL && *target == NULL); - - isc_mem_create(&mctx); - isc_mem_setname(mctx, "zonemgr-pool"); - - *target = mctx; - return (ISC_R_SUCCESS); -} - -static void -mctxfree(void **target) { - isc_mem_t *mctx = *(isc_mem_t **)target; - isc_mem_detach(&mctx); - *target = NULL; -} - -static isc_result_t -zonemgr_setsize(dns_zonemgr_t *zmgr, unsigned int workers) { - isc_result_t result; - - zmgr->workers = workers; - - /* Create the zone tasks pool. */ - REQUIRE(zmgr->zonetasks == NULL); - result = isc_pool_create(zmgr->mctx, workers, taskfree, zonetaskinit, - zmgr, &zmgr->zonetasks); - if (result != ISC_R_SUCCESS) { - goto fail; - } - - /* Create the zone load tasks pool. */ - REQUIRE(zmgr->loadtasks == NULL); - result = isc_pool_create(zmgr->mctx, workers, taskfree, loadtaskinit, - zmgr, &zmgr->loadtasks); - if (result != ISC_R_SUCCESS) { - goto fail; - } - - /* Create the zone memory context pool. */ - REQUIRE(zmgr->mctxpool == NULL); - result = isc_pool_create(zmgr->mctx, workers, mctxfree, mctxinit, NULL, - &zmgr->mctxpool); - if (result != ISC_R_SUCCESS) { - goto fail; - } - - return (ISC_R_SUCCESS); -fail: - if (zmgr->mctxpool != NULL) { - isc_pool_destroy(&zmgr->mctxpool); - } - if (zmgr->loadtasks != NULL) { - isc_pool_destroy(&zmgr->loadtasks); - } - if (zmgr->zonetasks != NULL) { - isc_pool_destroy(&zmgr->zonetasks); - } - - return (result); -} - static void zonemgr_free(dns_zonemgr_t *zmgr) { isc_mem_t *mctx; diff --git a/lib/isc/tests/pool_test.c b/lib/isc/tests/pool_test.c index 462d3138db..3ced1a2932 100644 --- a/lib/isc/tests/pool_test.c +++ b/lib/isc/tests/pool_test.c @@ -103,15 +103,15 @@ get_objects(void **state) { &pool); assert_int_equal(result, ISC_R_SUCCESS); - item = isc_pool_get(pool); + item = isc_pool_get(pool, 0); assert_non_null(item); isc_task_attach((isc_task_t *)item, &task1); - item = isc_pool_get(pool); + item = isc_pool_get(pool, 1); assert_non_null(item); isc_task_attach((isc_task_t *)item, &task2); - item = isc_pool_get(pool); + item = isc_pool_get(pool, 0); assert_non_null(item); isc_task_attach((isc_task_t *)item, &task3); diff --git a/lib/ns/tests/nstest.c b/lib/ns/tests/nstest.c index d4e126b848..1de321b0af 100644 --- a/lib/ns/tests/nstest.c +++ b/lib/ns/tests/nstest.c @@ -448,8 +448,7 @@ ns_test_setupzonemgr(void) { isc_result_t result; REQUIRE(zonemgr == NULL); - result = dns_zonemgr_create(mctx, taskmgr, timermgr, NULL, ncpus, - &zonemgr); + result = dns_zonemgr_create(mctx, taskmgr, timermgr, netmgr, &zonemgr); return (result); } From 62a72211aa0adf5078365de0b54e336efa472a2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 18 Mar 2022 12:50:18 +0100 Subject: [PATCH 5/6] Remove isc_pool API Since the last user of the isc_pool API is gone, remove the whole isc_pool API. --- lib/dns/zone.c | 1 - lib/isc/Makefile.am | 2 - lib/isc/include/isc/pool.h | 100 ------------------------- lib/isc/pool.c | 94 ------------------------ lib/isc/tests/Makefile.am | 1 - lib/isc/tests/pool_test.c | 146 ------------------------------------- 6 files changed, 344 deletions(-) delete mode 100644 lib/isc/include/isc/pool.h delete mode 100644 lib/isc/pool.c delete mode 100644 lib/isc/tests/pool_test.c diff --git a/lib/dns/zone.c b/lib/dns/zone.c index e8f5092cbb..a7d3610bb6 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -23,7 +23,6 @@ #include #include #include -#include #include #include #include diff --git a/lib/isc/Makefile.am b/lib/isc/Makefile.am index e2760034f1..c0bb69fa4a 100644 --- a/lib/isc/Makefile.am +++ b/lib/isc/Makefile.am @@ -63,7 +63,6 @@ libisc_la_HEADERS = \ include/isc/once.h \ include/isc/os.h \ include/isc/parseint.h \ - include/isc/pool.h \ include/isc/portset.h \ include/isc/print.h \ include/isc/quota.h \ @@ -167,7 +166,6 @@ libisc_la_SOURCES = \ os.c \ os_p.h \ parseint.c \ - pool.c \ portset.c \ quota.c \ radix.c \ diff --git a/lib/isc/include/isc/pool.h b/lib/isc/include/isc/pool.h deleted file mode 100644 index a08e1f21a4..0000000000 --- a/lib/isc/include/isc/pool.h +++ /dev/null @@ -1,100 +0,0 @@ -/* - * Copyright (C) Internet Systems Consortium, Inc. ("ISC") - * - * SPDX-License-Identifier: MPL-2.0 - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * - * See the COPYRIGHT file distributed with this work for additional - * information regarding copyright ownership. - */ - -#pragma once - -/***** -***** Module Info -*****/ - -/*! \file isc/pool.h - * \brief An object pool is a mechanism for sharing a small pool of - * fungible objects among a large number of objects that depend on them. - * - * This is useful, for example, when it causes performance problems for - * large number of zones to share a single memory context or task object, - * but it would create a different set of problems for them each to have an - * independent task or memory context. - */ - -/*** - *** Imports. - ***/ - -#include -#include -#include - -ISC_LANG_BEGINDECLS - -/***** -***** Types. -*****/ - -typedef void (*isc_pooldeallocator_t)(void **object); - -typedef isc_result_t (*isc_poolinitializer_t)(void **target, void *arg); - -typedef struct isc_pool isc_pool_t; - -/***** -***** Functions. -*****/ - -isc_result_t -isc_pool_create(isc_mem_t *mctx, unsigned int count, isc_pooldeallocator_t free, - isc_poolinitializer_t init, void *initarg, isc_pool_t **poolp); -/*%< - * Create a pool of "count" object pointers. If 'free' is not NULL, - * it points to a function that will detach the objects. 'init' - * points to a function that will initialize the arguments, and - * 'arg' to an argument to be passed into that function (for example, - * a relevant manager or context object). - * - * Requires: - * - *\li 'mctx' is a valid memory context. - * - *\li init != NULL - * - *\li poolp != NULL && *poolp == NULL - * - * Ensures: - * - *\li On success, '*poolp' points to the new object pool. - * - * Returns: - * - *\li #ISC_R_SUCCESS - *\li #ISC_R_NOMEMORY - *\li #ISC_R_UNEXPECTED - */ - -void * -isc_pool_get(isc_pool_t *pool, unsigned int tid); -/*%< - * Returns a pointer to an object from the pool. Currently the object - * is chosen from the pool at random. - */ - -void -isc_pool_destroy(isc_pool_t **poolp); -/*%< - * Destroy a task pool. The tasks in the pool are detached but not - * shut down. - * - * Requires: - * \li '*poolp' is a valid task pool. - */ - -ISC_LANG_ENDDECLS diff --git a/lib/isc/pool.c b/lib/isc/pool.c deleted file mode 100644 index 66fa09c1fa..0000000000 --- a/lib/isc/pool.c +++ /dev/null @@ -1,94 +0,0 @@ -/* - * Copyright (C) Internet Systems Consortium, Inc. ("ISC") - * - * SPDX-License-Identifier: MPL-2.0 - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * - * See the COPYRIGHT file distributed with this work for additional - * information regarding copyright ownership. - */ - -/*! \file */ - -#include - -#include -#include -#include -#include - -/*** - *** Types. - ***/ - -struct isc_pool { - isc_mem_t *mctx; - unsigned int count; - isc_pooldeallocator_t free; - isc_poolinitializer_t init; - void *initarg; - void **pool; -}; - -/*** - *** Functions. - ***/ - -isc_result_t -isc_pool_create(isc_mem_t *mctx, unsigned int count, - isc_pooldeallocator_t release, isc_poolinitializer_t init, - void *initarg, isc_pool_t **poolp) { - isc_pool_t *pool = NULL; - isc_result_t result; - unsigned int i; - - INSIST(count > 0); - - /* Allocate the pool structure */ - pool = isc_mem_get(mctx, sizeof(*pool)); - *pool = (isc_pool_t){ - .count = count, - .free = release, - .init = init, - .initarg = initarg, - }; - isc_mem_attach(mctx, &pool->mctx); - pool->pool = isc_mem_get(mctx, count * sizeof(void *)); - memset(pool->pool, 0, count * sizeof(void *)); - - /* Populate the pool */ - for (i = 0; i < count; i++) { - result = init(&pool->pool[i], initarg); - if (result != ISC_R_SUCCESS) { - isc_pool_destroy(&pool); - return (result); - } - } - - *poolp = pool; - return (ISC_R_SUCCESS); -} - -void * -isc_pool_get(isc_pool_t *pool, unsigned int tid) { - REQUIRE(tid < pool->count); - - return (pool->pool[tid]); -} - -void -isc_pool_destroy(isc_pool_t **poolp) { - unsigned int i; - isc_pool_t *pool = *poolp; - *poolp = NULL; - for (i = 0; i < pool->count; i++) { - if (pool->free != NULL && pool->pool[i] != NULL) { - pool->free(&pool->pool[i]); - } - } - isc_mem_put(pool->mctx, pool->pool, pool->count * sizeof(void *)); - isc_mem_putanddetach(&pool->mctx, pool, sizeof(*pool)); -} diff --git a/lib/isc/tests/Makefile.am b/lib/isc/tests/Makefile.am index 1aba558ddc..4e93cfa3b5 100644 --- a/lib/isc/tests/Makefile.am +++ b/lib/isc/tests/Makefile.am @@ -30,7 +30,6 @@ check_PROGRAMS = \ netaddr_test \ netmgr_test \ parse_test \ - pool_test \ quota_test \ radix_test \ random_test \ diff --git a/lib/isc/tests/pool_test.c b/lib/isc/tests/pool_test.c deleted file mode 100644 index 3ced1a2932..0000000000 --- a/lib/isc/tests/pool_test.c +++ /dev/null @@ -1,146 +0,0 @@ -/* - * Copyright (C) Internet Systems Consortium, Inc. ("ISC") - * - * SPDX-License-Identifier: MPL-2.0 - * - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, you can obtain one at https://mozilla.org/MPL/2.0/. - * - * See the COPYRIGHT file distributed with this work for additional - * information regarding copyright ownership. - */ - -#if HAVE_CMOCKA - -#include /* IWYU pragma: keep */ -#include -#include -#include -#include -#include -#include - -#define UNIT_TESTING -#include - -#include -#include -#include - -#include "isctest.h" - -static int -_setup(void **state) { - isc_result_t result; - - UNUSED(state); - - result = isc_test_begin(NULL, true, 0); - assert_int_equal(result, ISC_R_SUCCESS); - - return (0); -} - -static int -_teardown(void **state) { - UNUSED(state); - - isc_test_end(); - - return (0); -} - -static isc_result_t -poolinit(void **target, void *arg) { - isc_result_t result; - - isc_taskmgr_t *mgr = (isc_taskmgr_t *)arg; - isc_task_t *task = NULL; - result = isc_task_create(mgr, 0, &task); - if (result != ISC_R_SUCCESS) { - return (result); - } - - *target = (void *)task; - return (ISC_R_SUCCESS); -} - -static void -poolfree(void **target) { - isc_task_t *task = *(isc_task_t **)target; - isc_task_destroy(&task); - *target = NULL; -} - -/* Create a pool */ -static void -create_pool(void **state) { - isc_result_t result; - isc_pool_t *pool = NULL; - - UNUSED(state); - - result = isc_pool_create(test_mctx, 8, poolfree, poolinit, taskmgr, - &pool); - assert_int_equal(result, ISC_R_SUCCESS); - - isc_pool_destroy(&pool); - assert_null(pool); -} - -/* Get objects */ -static void -get_objects(void **state) { - isc_result_t result; - isc_pool_t *pool = NULL; - void *item; - isc_task_t *task1 = NULL, *task2 = NULL, *task3 = NULL; - - UNUSED(state); - - result = isc_pool_create(test_mctx, 2, poolfree, poolinit, taskmgr, - &pool); - assert_int_equal(result, ISC_R_SUCCESS); - - item = isc_pool_get(pool, 0); - assert_non_null(item); - isc_task_attach((isc_task_t *)item, &task1); - - item = isc_pool_get(pool, 1); - assert_non_null(item); - isc_task_attach((isc_task_t *)item, &task2); - - item = isc_pool_get(pool, 0); - assert_non_null(item); - isc_task_attach((isc_task_t *)item, &task3); - - isc_task_detach(&task1); - isc_task_detach(&task2); - isc_task_detach(&task3); - - isc_pool_destroy(&pool); - assert_null(pool); -} - -int -main(void) { - const struct CMUnitTest tests[] = { - cmocka_unit_test_setup_teardown(create_pool, _setup, _teardown), - cmocka_unit_test_setup_teardown(get_objects, _setup, _teardown), - }; - - return (cmocka_run_group_tests(tests, NULL, NULL)); -} - -#else /* HAVE_CMOCKA */ - -#include - -int -main(void) { - printf("1..0 # Skipped: cmocka not available\n"); - return (SKIPPED_TEST_EXIT_CODE); -} - -#endif /* if HAVE_CMOCKA */ From ae0898e328429bb103803be164ee5ca1b51449e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 23 Mar 2022 13:53:54 +0100 Subject: [PATCH 6/6] Add CHANGES note for [GL #3226] --- CHANGES | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGES b/CHANGES index 3b8025e8c7..1a3210e1ad 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,10 @@ +5846. [func] In dns_zonemgr, create per-thread task, zonetask, and + loadtask and pin the zones to individual threads, + instead of having "many", spreading the zones among + them and hoping for the best. This also removes any + need to dynamically reallocate the pools with memory + contexts and tasks. [GL #3226] + 5845. [bug] Refactor the timer to keep track of posted events as to use isc_task_purgeevent() instead of using isc_task_purgerange(). The isc_task_purgeevent()