diff --git a/bin/named/server.c b/bin/named/server.c index ba197e4711..f720cb0016 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -9697,9 +9697,8 @@ view_loaded(void *arg) { } static isc_result_t -load_zones(named_server_t *server, bool init, bool reconfig) { +load_zones(named_server_t *server, bool reconfig) { isc_result_t result; - isc_taskmgr_t *taskmgr = dns_zonemgr_gettaskmgr(server->zonemgr); ns_zoneload_t *zl = NULL; dns_view_t *view = NULL; @@ -9755,25 +9754,7 @@ cleanup: isc_mem_put(server->mctx, zl, sizeof(*zl)); } - if (init) { - /* - * If we're setting up the server for the first time, set - * the task manager into privileged mode; this ensures - * that no other tasks will begin to run until after zone - * loading is complete. We won't return from exclusive mode - * until the loading is finished; we can then drop out of - * privileged mode. - * - * We do *not* want to do this in the case of reload or - * reconfig, as loading a large zone could cause the server - * to be inactive for too long a time. - */ - isc_taskmgr_setmode(taskmgr, isc_taskmgrmode_privileged); - isc_task_endexclusive(server->task); - isc_taskmgr_setmode(taskmgr, isc_taskmgrmode_normal); - } else { - isc_task_endexclusive(server->task); - } + isc_task_endexclusive(server->task); return (result); } @@ -9830,7 +9811,7 @@ run_server(isc_task_t *task, isc_event_t *event) { CHECKFATAL(load_configuration(named_g_conffile, server, true), "loading configuration"); - CHECKFATAL(load_zones(server, true, false), "loading zones"); + CHECKFATAL(load_zones(server, false), "loading zones"); #ifdef ENABLE_AFL named_g_run_done = true; #endif /* ifdef ENABLE_AFL */ @@ -10330,7 +10311,7 @@ reload(named_server_t *server) { CHECK(loadconfig(server)); - result = load_zones(server, false, false); + result = load_zones(server, false); if (result == ISC_R_SUCCESS) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, @@ -10704,7 +10685,7 @@ named_server_reconfigcommand(named_server_t *server) { CHECK(loadconfig(server)); - result = load_zones(server, false, true); + result = load_zones(server, true); if (result == ISC_R_SUCCESS) { isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER, ISC_LOG_INFO, diff --git a/lib/dns/zone.c b/lib/dns/zone.c index a7d3610bb6..ec6c78030f 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -18881,7 +18881,6 @@ dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, 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, diff --git a/lib/isc/include/isc/task.h b/lib/isc/include/isc/task.h index 923d39e18f..01028008b4 100644 --- a/lib/isc/include/isc/task.h +++ b/lib/isc/include/isc/task.h @@ -81,11 +81,6 @@ ISC_LANG_BEGINDECLS *** Types ***/ -typedef enum { - isc_taskmgrmode_normal = 0, - isc_taskmgrmode_privileged -} isc_taskmgrmode_t; - isc_result_t isc_task_create(isc_taskmgr_t *manager, unsigned int quantum, isc_task_t **taskp); @@ -450,42 +445,6 @@ isc_task_exiting(isc_task_t *t); *\li 'task' is a valid task. */ -void -isc_task_setprivilege(isc_task_t *task, bool priv); -/*%< - * Set or unset the task's "privileged" flag depending on the value of - * 'priv'. - * - * Under normal circumstances this flag has no effect on the task behavior, - * but when the task manager has been set to privileged execution mode via - * isc_taskmgr_setmode(), only tasks with the flag set will be executed, - * and all other tasks will wait until they're done. Once all privileged - * tasks have finished executing, the task manager resumes running - * non-privileged tasks. - * - * Requires: - *\li 'task' is a valid task. - */ - -bool -isc_task_getprivilege(isc_task_t *task); -/*%< - * Returns the current value of the task's privilege flag. - * - * Requires: - *\li 'task' is a valid task. - */ - -bool -isc_task_privileged(isc_task_t *task); -/*%< - * Returns true if the task's privilege flag is set *and* the task - * manager is currently in privileged mode. - * - * Requires: - *\li 'task' is a valid task. - */ - /***** ***** Task Manager. *****/ @@ -498,27 +457,6 @@ isc_taskmgr_detach(isc_taskmgr_t **); * Attach/detach the task manager. */ -void -isc_taskmgr_setmode(isc_taskmgr_t *manager, isc_taskmgrmode_t mode); -isc_taskmgrmode_t -isc_taskmgr_mode(isc_taskmgr_t *manager); -/*%< - * Set/get the operating mode of the task manager. Valid modes are: - * - *\li isc_taskmgrmode_normal - *\li isc_taskmgrmode_privileged - * - * In privileged execution mode, only tasks that have had the "privilege" - * flag set via isc_task_setprivilege() can be executed. When all such - * tasks are complete, non-privileged tasks resume running. The task calling - * this function should be in task-exclusive mode; the privileged tasks - * will be run after isc_task_endexclusive() is called. - * - * Requires: - * - *\li 'manager' is a valid task manager. - */ - void isc_taskmgr_setexcltask(isc_taskmgr_t *mgr, isc_task_t *task); /*%< diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index d1965d12fd..1e90f00051 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -202,10 +202,9 @@ isc__nm_dump_active(isc_nm_t *nm); */ typedef enum { NETIEVENT_PRIORITY = 0, - NETIEVENT_PRIVILEGED = 1, - NETIEVENT_TASK = 2, - NETIEVENT_NORMAL = 3, - NETIEVENT_MAX = 4, + NETIEVENT_TASK = 1, + NETIEVENT_NORMAL = 2, + NETIEVENT_MAX = 3, } netievent_type_t; typedef struct isc__nm_uvreq isc__nm_uvreq_t; @@ -332,7 +331,6 @@ typedef enum isc__netievent_type { netievent_sendcb, netievent_task, - netievent_privilegedtask, /* * event type values higher than this will be treated @@ -1961,7 +1959,6 @@ NETIEVENT_TYPE(shutdown); NETIEVENT_TYPE(stop); NETIEVENT_TASK_TYPE(task); -NETIEVENT_TASK_TYPE(privilegedtask); /* Now declared the helper functions */ @@ -2029,7 +2026,6 @@ NETIEVENT_DECL(shutdown); NETIEVENT_DECL(stop); NETIEVENT_TASK_DECL(task); -NETIEVENT_TASK_DECL(privilegedtask); void isc__nm_udp_failed_read_cb(isc_nmsocket_t *sock, isc_result_t result); diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 17d2004685..5f6c04a680 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -442,8 +442,6 @@ isc_nm_resume(isc_nm_t *mgr) { } if (isc__nm_in_netthread()) { - drain_queue(&mgr->workers[isc_nm_tid()], NETIEVENT_PRIVILEGED); - atomic_fetch_sub(&mgr->workers_paused, 1); isc_barrier_wait(&mgr->resuming); } @@ -608,19 +606,11 @@ isc_nm_gettimeouts(isc_nm_t *mgr, uint32_t *initial, uint32_t *idle, * is needed to properly start listening on the interfaces, free * resources on shutdown, or resume from a pause. * - * 2. privileged task queue - only privileged tasks are queued here and - * this is the first queue that gets processed when network manager - * is unpaused using isc_nm_resume(). All netmgr workers need to - * clean the privileged task queue before they all proceed to normal - * operation. Both task queues are processed when the workers are - * shutting down. + * 2. task queue - only (traditional) tasks are scheduled here, and this queue + * is processed when the netmgr workers are finishing. This is needed to + * process the task shutdown events. * - * 3. task queue - only (traditional) tasks are scheduled here, and this - * queue and the privileged task queue are both processed when the - * netmgr workers are finishing. This is needed to process the task - * shutdown events. - * - * 4. normal queue - this is the queue with netmgr events, e.g. reading, + * 3. normal queue - this is the queue with netmgr events, e.g. reading, * sending, callbacks, etc. */ @@ -635,8 +625,8 @@ nm_thread(isc_threadarg_t worker0) { /* * uv_run() runs async_cb() in a loop, which processes * all four event queues until a "pause" or "stop" event - * is encountered. On pause, we process only priority and - * privileged events until resuming. + * is encountered. On pause, we process only priority + * events until resuming. */ int r = uv_run(&worker->loop, UV_RUN_DEFAULT); INSIST(r > 0 || worker->finished); @@ -655,12 +645,6 @@ nm_thread(isc_threadarg_t worker0) { wait_for_priority_queue(worker); } - /* - * All workers must drain the privileged event - * queue before we resume from pause. - */ - drain_queue(worker, NETIEVENT_PRIVILEGED); - atomic_fetch_sub(&mgr->workers_paused, 1); if (isc_barrier_wait(&mgr->resuming) != 0) { LOCK(&mgr->lock); @@ -680,7 +664,6 @@ nm_thread(isc_threadarg_t worker0) { /* * We are shutting down. Drain the queues. */ - drain_queue(worker, NETIEVENT_PRIVILEGED); drain_queue(worker, NETIEVENT_TASK); for (size_t type = 0; type < NETIEVENT_MAX; type++) { @@ -761,20 +744,11 @@ isc_nm_task_enqueue(isc_nm_t *nm, isc_task_t *task, int tid) { worker = &nm->workers[tid]; - if (isc_task_privileged(task)) { - event = (isc__netievent_t *) - isc__nm_get_netievent_privilegedtask(nm, task); - } else { - event = (isc__netievent_t *)isc__nm_get_netievent_task(nm, - task); - } + event = (isc__netievent_t *)isc__nm_get_netievent_task(nm, task); isc__nm_enqueue_ievent(worker, event); } -#define isc__nm_async_privilegedtask(worker, ev0) \ - isc__nm_async_task(worker, ev0) - static void isc__nm_async_task(isc__networker_t *worker, isc__netievent_t *ev0) { isc__netievent_task_t *ievent = (isc__netievent_task_t *)ev0; @@ -853,7 +827,6 @@ process_netievent(isc__networker_t *worker, isc__netievent_t *ievent) { /* Don't process more ievents when we are stopping */ NETIEVENT_CASE_NOMORE(stop); - NETIEVENT_CASE(privilegedtask); NETIEVENT_CASE(task); NETIEVENT_CASE(udpconnect); @@ -1044,7 +1017,6 @@ NETIEVENT_DEF(shutdown); NETIEVENT_DEF(stop); NETIEVENT_TASK_DEF(task); -NETIEVENT_TASK_DEF(privilegedtask); void isc__nm_maybe_enqueue_ievent(isc__networker_t *worker, @@ -1072,9 +1044,6 @@ isc__nm_enqueue_ievent(isc__networker_t *worker, isc__netievent_t *event) { case netievent_prio: UNREACHABLE(); break; - case netievent_privilegedtask: - type = NETIEVENT_PRIVILEGED; - break; case netievent_task: type = NETIEVENT_TASK; break; diff --git a/lib/isc/task.c b/lib/isc/task.c index f2c60b9b8a..c79fec431e 100644 --- a/lib/isc/task.c +++ b/lib/isc/task.c @@ -117,13 +117,11 @@ struct isc_task { bool bound; /* Protected by atomics */ atomic_bool shuttingdown; - atomic_bool privileged; /* Locked by task manager lock. */ LINK(isc_task_t) link; }; #define TASK_SHUTTINGDOWN(t) (atomic_load_acquire(&(t)->shuttingdown)) -#define TASK_PRIVILEGED(t) (atomic_load_acquire(&(t)->privileged)) #define TASK_MANAGER_MAGIC ISC_MAGIC('T', 'S', 'K', 'M') #define VALID_MANAGER(m) ISC_MAGIC_VALID(m, TASK_MANAGER_MAGIC) @@ -240,7 +238,6 @@ isc_task_create_bound(isc_taskmgr_t *manager, unsigned int quantum, task->nevents = 0; task->quantum = (quantum > 0) ? quantum : manager->default_quantum; atomic_init(&task->shuttingdown, false); - atomic_init(&task->privileged, false); task->now = 0; isc_time_settoepoch(&task->tnow); memset(task->name, 0, sizeof(task->name)); @@ -854,7 +851,6 @@ isc__taskmgr_create(isc_mem_t *mctx, unsigned int default_quantum, isc_nm_t *nm, } INIT_LIST(manager->tasks); - atomic_init(&manager->mode, isc_taskmgrmode_normal); atomic_init(&manager->exclusive_req, false); atomic_init(&manager->tasks_count, 0); @@ -1044,37 +1040,6 @@ isc_task_endexclusive(isc_task_t *task) { &(bool){ true }, false)); } -void -isc_taskmgr_setmode(isc_taskmgr_t *manager, isc_taskmgrmode_t mode) { - atomic_store(&manager->mode, mode); -} - -isc_taskmgrmode_t -isc_taskmgr_mode(isc_taskmgr_t *manager) { - return (atomic_load(&manager->mode)); -} - -void -isc_task_setprivilege(isc_task_t *task, bool priv) { - REQUIRE(VALID_TASK(task)); - - atomic_store_release(&task->privileged, priv); -} - -bool -isc_task_getprivilege(isc_task_t *task) { - REQUIRE(VALID_TASK(task)); - - return (TASK_PRIVILEGED(task)); -} - -bool -isc_task_privileged(isc_task_t *task) { - REQUIRE(VALID_TASK(task)); - - return (isc_taskmgr_mode(task->manager) && TASK_PRIVILEGED(task)); -} - bool isc_task_exiting(isc_task_t *task) { REQUIRE(VALID_TASK(task)); diff --git a/lib/isc/tests/task_test.c b/lib/isc/tests/task_test.c index 57a1bec477..f71a8ee9c1 100644 --- a/lib/isc/tests/task_test.c +++ b/lib/isc/tests/task_test.c @@ -123,18 +123,6 @@ set(isc_task_t *task, isc_event_t *event) { #include -static void -set_and_drop(isc_task_t *task, isc_event_t *event) { - atomic_int_fast32_t *value = (atomic_int_fast32_t *)event->ev_arg; - - UNUSED(task); - - isc_event_free(&event); - LOCK(&lock); - atomic_store(value, atomic_fetch_add(&counter, 1)); - UNLOCK(&lock); -} - /* Create a task */ static void create_task(void **state) { @@ -194,230 +182,6 @@ all_events(void **state) { assert_null(task); } -/* Privileged events */ -static void -privileged_events(void **state) { - isc_result_t result; - isc_task_t *task1 = NULL, *task2 = NULL; - isc_event_t *event = NULL; - atomic_int_fast32_t a, b, c, d, e; - int i = 0; - - UNUSED(state); - - atomic_init(&counter, 1); - atomic_init(&a, -1); - atomic_init(&b, -1); - atomic_init(&c, -1); - atomic_init(&d, -1); - atomic_init(&e, -1); - - /* - * Pause the net/task manager so we can fill up the work - * queue without things happening while we do it. - */ - isc_nm_pause(netmgr); - isc_taskmgr_setmode(taskmgr, isc_taskmgrmode_privileged); - - result = isc_task_create(taskmgr, 0, &task1); - assert_int_equal(result, ISC_R_SUCCESS); - isc_task_setname(task1, "privileged", NULL); - assert_false(isc_task_getprivilege(task1)); - isc_task_setprivilege(task1, true); - assert_true(isc_task_getprivilege(task1)); - - result = isc_task_create(taskmgr, 0, &task2); - assert_int_equal(result, ISC_R_SUCCESS); - isc_task_setname(task2, "normal", NULL); - assert_false(isc_task_getprivilege(task2)); - - /* First event: privileged */ - event = isc_event_allocate(test_mctx, task1, ISC_TASKEVENT_TEST, set, - &a, sizeof(isc_event_t)); - assert_non_null(event); - - assert_int_equal(atomic_load(&a), -1); - isc_task_send(task1, &event); - - /* Second event: not privileged */ - event = isc_event_allocate(test_mctx, task2, ISC_TASKEVENT_TEST, set, - &b, sizeof(isc_event_t)); - assert_non_null(event); - - assert_int_equal(atomic_load(&b), -1); - isc_task_send(task2, &event); - - /* Third event: privileged */ - event = isc_event_allocate(test_mctx, task1, ISC_TASKEVENT_TEST, set, - &c, sizeof(isc_event_t)); - assert_non_null(event); - - assert_int_equal(atomic_load(&c), -1); - isc_task_send(task1, &event); - - /* Fourth event: privileged */ - event = isc_event_allocate(test_mctx, task1, ISC_TASKEVENT_TEST, set, - &d, sizeof(isc_event_t)); - assert_non_null(event); - - assert_int_equal(atomic_load(&d), -1); - isc_task_send(task1, &event); - - /* Fifth event: not privileged */ - event = isc_event_allocate(test_mctx, task2, ISC_TASKEVENT_TEST, set, - &e, sizeof(isc_event_t)); - assert_non_null(event); - - assert_int_equal(atomic_load(&e), -1); - isc_task_send(task2, &event); - - isc_nm_resume(netmgr); - - /* We're waiting for *all* variables to be set */ - while ((atomic_load(&a) < 0 || atomic_load(&b) < 0 || - atomic_load(&c) < 0 || atomic_load(&d) < 0 || - atomic_load(&e) < 0) && - i++ < 5000) - { - isc_test_nap(1000); - } - - /* - * We can't guarantee what order the events fire, but - * we do know the privileged tasks that set a, c, and d - * would have fired first. - */ - assert_true(atomic_load(&a) <= 3); - assert_true(atomic_load(&c) <= 3); - assert_true(atomic_load(&d) <= 3); - - /* ...and the non-privileged tasks that set b and e, last */ - assert_true(atomic_load(&b) > 3); - assert_true(atomic_load(&e) > 3); - - assert_int_equal(atomic_load(&counter), 6); - - isc_task_setprivilege(task1, false); - assert_false(isc_task_getprivilege(task1)); - - isc_task_destroy(&task1); - assert_null(task1); - isc_task_destroy(&task2); - assert_null(task2); -} - -/* - * Edge case: this tests that the task manager behaves as expected when - * we explicitly set it into normal mode *while* running privileged. - */ -static void -privilege_drop(void **state) { - isc_result_t result; - isc_task_t *task1 = NULL, *task2 = NULL; - isc_event_t *event = NULL; - atomic_int_fast32_t a, b, c, d, e; /* non valid states */ - int i = 0; - - UNUSED(state); - - atomic_init(&counter, 1); - atomic_init(&a, -1); - atomic_init(&b, -1); - atomic_init(&c, -1); - atomic_init(&d, -1); - atomic_init(&e, -1); - - /* - * Pause the net/task manager so we can fill up the work queue - * without things happening while we do it. - */ - isc_nm_pause(netmgr); - isc_taskmgr_setmode(taskmgr, isc_taskmgrmode_privileged); - - result = isc_task_create(taskmgr, 0, &task1); - assert_int_equal(result, ISC_R_SUCCESS); - isc_task_setname(task1, "privileged", NULL); - assert_false(isc_task_getprivilege(task1)); - isc_task_setprivilege(task1, true); - assert_true(isc_task_getprivilege(task1)); - - result = isc_task_create(taskmgr, 0, &task2); - assert_int_equal(result, ISC_R_SUCCESS); - isc_task_setname(task2, "normal", NULL); - assert_false(isc_task_getprivilege(task2)); - - /* First event: privileged */ - event = isc_event_allocate(test_mctx, task1, ISC_TASKEVENT_TEST, - set_and_drop, &a, sizeof(isc_event_t)); - assert_non_null(event); - - assert_int_equal(atomic_load(&a), -1); - isc_task_send(task1, &event); - - /* Second event: not privileged */ - event = isc_event_allocate(test_mctx, task2, ISC_TASKEVENT_TEST, - set_and_drop, &b, sizeof(isc_event_t)); - assert_non_null(event); - - assert_int_equal(atomic_load(&b), -1); - isc_task_send(task2, &event); - - /* Third event: privileged */ - event = isc_event_allocate(test_mctx, task1, ISC_TASKEVENT_TEST, - set_and_drop, &c, sizeof(isc_event_t)); - assert_non_null(event); - - assert_int_equal(atomic_load(&c), -1); - isc_task_send(task1, &event); - - /* Fourth event: privileged */ - event = isc_event_allocate(test_mctx, task1, ISC_TASKEVENT_TEST, - set_and_drop, &d, sizeof(isc_event_t)); - assert_non_null(event); - - assert_int_equal(atomic_load(&d), -1); - isc_task_send(task1, &event); - - /* Fifth event: not privileged */ - event = isc_event_allocate(test_mctx, task2, ISC_TASKEVENT_TEST, - set_and_drop, &e, sizeof(isc_event_t)); - assert_non_null(event); - - assert_int_equal(atomic_load(&e), -1); - isc_task_send(task2, &event); - - isc_nm_resume(netmgr); - - /* We're waiting for all variables to be set. */ - while ((atomic_load(&a) == -1 || atomic_load(&b) == -1 || - atomic_load(&c) == -1 || atomic_load(&d) == -1 || - atomic_load(&e) == -1) && - i++ < 5000) - { - isc_test_nap(1000); - } - - /* - * We need to check that all privilege mode events were fired - * in privileged mode, and non privileged in non-privileged. - */ - assert_true(atomic_load(&a) <= 3); - assert_true(atomic_load(&c) <= 3); - assert_true(atomic_load(&d) <= 3); - - /* ...and neither of the non-privileged tasks did... */ - assert_true(atomic_load(&b) > 3); - assert_true(atomic_load(&e) > 3); - - /* ...but all five of them did run. */ - assert_int_equal(atomic_load(&counter), 6); - - isc_task_destroy(&task1); - assert_null(task1); - isc_task_destroy(&task2); - assert_null(task2); -} - /* * Basic task functions: */ @@ -1089,10 +853,6 @@ main(int argc, char **argv) { cmocka_unit_test_setup_teardown(create_task, _setup, _teardown), cmocka_unit_test_setup_teardown(post_shutdown, _setup2, _teardown), - cmocka_unit_test_setup_teardown(privilege_drop, _setup, - _teardown), - cmocka_unit_test_setup_teardown(privileged_events, _setup, - _teardown), cmocka_unit_test_setup_teardown(purgeevent, _setup2, _teardown), cmocka_unit_test_setup_teardown(task_shutdown, _setup4, _teardown),