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] 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