upstream/bind9
1
0
Fork 0
mirror of https://github.com/isc-projects/bind9.git synced 2026-05-28 04:34:54 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

Merge branch 'ondrej-remove-task-privileged-mode' into 'main'

Browse source 
Remove task privileged mode

Closes #3253

See merge request isc-projects/bind9!6049
This commit is contained in:
Ondřej Surý 2022-04-01 23:01:48 +00:00
commit 635fbc7f93
8 changed files with 19 additions and 407 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
CHANGES
View file

@ -1,3 +1,7 @@
5847. [cleanup] Remove task privileged mode in favor of processing
all events in the loadzone task in a single run
by setting the quantum to UINT_MAX. [GL #3253]
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

29
bin/named/server.c
View file

@ -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,

1
lib/dns/zone.c
View file

@ -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,

62
lib/isc/include/isc/task.h
View file

@ -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);
/*%<

10
lib/isc/netmgr/netmgr-int.h
View file

@ -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);

45
lib/isc/netmgr/netmgr.c
View file

@ -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;

35
lib/isc/task.c
View file

@ -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));

240
lib/isc/tests/task_test.c
View file

@ -123,18 +123,6 @@ set(isc_task_t *task, isc_event_t *event) {
#include <isc/thread.h>
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),