From b54c721894397f623837ea9931a2d2c05b18e92a Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 26 Oct 2022 11:12:36 -0700 Subject: [PATCH 1/3] refactor dns_master_dump*async() to use loop callbacks Asynchronous zone dumping now uses loop callbacks instead of task events. --- bin/named/server.c | 36 +++++--------- lib/dns/include/dns/masterdump.h | 5 +- lib/dns/masterdump.c | 82 ++++++++++---------------------- lib/dns/zone.c | 2 +- 4 files changed, 39 insertions(+), 86 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index e988d2c8d4..a1e61713d7 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -265,7 +265,7 @@ struct dumpcontext { dns_dumpctx_t *mdctx; dns_db_t *db; dns_db_t *cache; - isc_task_t *task; + isc_loop_t *loop; dns_dbversion_t *version; }; @@ -11555,9 +11555,6 @@ dumpcontext_destroy(struct dumpcontext *dctx) { if (dctx->cache != NULL) { dns_db_detach(&dctx->cache); } - if (dctx->task != NULL) { - isc_task_detach(&dctx->task); - } if (dctx->fp != NULL) { (void)isc_stdio_close(dctx->fp); } @@ -11613,7 +11610,7 @@ resume: dns_cache_getname(dctx->view->view->cache)); result = dns_master_dumptostreamasync( dctx->mctx, dctx->cache, NULL, style, dctx->fp, - dctx->task, dumpdone, dctx, &dctx->mdctx); + named_g_mainloop, dumpdone, dctx, &dctx->mdctx); if (result == DNS_R_CONTINUE) { return; } @@ -11673,7 +11670,7 @@ resume: dns_db_currentversion(dctx->db, &dctx->version); result = dns_master_dumptostreamasync( dctx->mctx, dctx->db, dctx->version, style, - dctx->fp, dctx->task, dumpdone, dctx, + dctx->fp, named_g_mainloop, dumpdone, dctx, &dctx->mdctx); if (result == DNS_R_CONTINUE) { return; @@ -11732,25 +11729,14 @@ named_server_dumpdb(named_server_t *server, isc_lex_t *lex, } dctx = isc_mem_get(server->mctx, sizeof(*dctx)); - - dctx->mctx = server->mctx; - dctx->dumpcache = true; - dctx->dumpadb = true; - dctx->dumpbad = true; - dctx->dumpexpired = false; - dctx->dumpfail = true; - dctx->dumpzones = false; - dctx->fp = NULL; - ISC_LIST_INIT(dctx->viewlist); - dctx->view = NULL; - dctx->zone = NULL; - dctx->cache = NULL; - dctx->mdctx = NULL; - dctx->db = NULL; - dctx->cache = NULL; - dctx->task = NULL; - dctx->version = NULL; - isc_task_attach(server->task, &dctx->task); + *dctx = (struct dumpcontext){ + .mctx = server->mctx, + .dumpcache = true, + .dumpadb = true, + .dumpbad = true, + .dumpfail = true, + .viewlist = ISC_LIST_INITIALIZER, + }; CHECKMF(isc_stdio_open(server->dumpfile, "w", &dctx->fp), "could not open dump file", server->dumpfile); diff --git a/lib/dns/include/dns/masterdump.h b/lib/dns/include/dns/masterdump.h index 056780ba84..e20816e87d 100644 --- a/lib/dns/include/dns/masterdump.h +++ b/lib/dns/include/dns/masterdump.h @@ -245,7 +245,7 @@ isc_result_t dns_master_dumptostreamasync(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, const dns_master_style_t *style, FILE *f, - isc_task_t *task, dns_dumpdonefunc_t done, + isc_loop_t *loop, dns_dumpdonefunc_t done, void *done_arg, dns_dumpctx_t **dctxp); isc_result_t @@ -264,7 +264,6 @@ dns_master_dumptostream(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, * Temporary dynamic memory may be allocated from 'mctx'. * * Require: - *\li 'task' to be valid. *\li 'done' to be non NULL. *\li 'dctxp' to be non NULL && '*dctxp' to be NULL. * @@ -281,7 +280,7 @@ dns_master_dumptostream(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, isc_result_t dns_master_dumpasync(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, const dns_master_style_t *style, const char *filename, - isc_task_t *task, dns_dumpdonefunc_t done, void *done_arg, + isc_loop_t *loop, dns_dumpdonefunc_t done, void *done_arg, dns_dumpctx_t **dctxp, dns_masterformat_t format, dns_masterrawheader_t *header); diff --git a/lib/dns/masterdump.c b/lib/dns/masterdump.c index 7ab6021aca..981668450f 100644 --- a/lib/dns/masterdump.c +++ b/lib/dns/masterdump.c @@ -17,10 +17,12 @@ #include #include +#include #include #include #include #include +#include #include #include #include @@ -28,7 +30,6 @@ #include #include #include -#include #include #include #include @@ -261,7 +262,7 @@ struct dns_dumpctx { dns_dbversion_t *version; dns_dbiterator_t *dbiter; dns_totext_ctx_t tctx; - isc_task_t *task; + isc_loop_t *loop; dns_dumpdonefunc_t done; void *done_arg; /* dns_master_dumpasync() */ @@ -1334,8 +1335,8 @@ dumpctx_destroy(dns_dumpctx_t *dctx) { dns_db_closeversion(dctx->db, &dctx->version, false); } dns_db_detach(&dctx->db); - if (dctx->task != NULL) { - isc_task_detach(&dctx->task); + if (dctx->loop != NULL) { + isc_loop_detach(&dctx->loop); } if (dctx->file != NULL) { isc_mem_free(dctx->mctx, dctx->file); @@ -1496,7 +1497,7 @@ master_dump_cb(void *data) { } /* - * This will run in a network/task manager thread when the dump is complete. + * This will run in a loop manager thread when the dump is complete. */ static void master_dump_done_cb(void *data) { @@ -1507,33 +1508,15 @@ master_dump_done_cb(void *data) { } /* - * This must be run from a network/task manager thread. + * This must be run from a loop manager thread. */ static void -setup_dump(isc_task_t *task, isc_event_t *event) { - dns_dumpctx_t *dctx = NULL; - isc_loopmgr_t *loopmgr = isc_task_getloopmgr(task); - isc_loop_t *loop = isc_loop_current(loopmgr); - - REQUIRE(event != NULL); - - dctx = event->ev_arg; +setup_dump(void *arg) { + dns_dumpctx_t *dctx = (dns_dumpctx_t *)arg; REQUIRE(DNS_DCTX_VALID(dctx)); - isc_work_enqueue(loop, master_dump_cb, master_dump_done_cb, dctx); - - isc_event_free(&event); -} - -static isc_result_t -task_send(dns_dumpctx_t *dctx) { - isc_event_t *event; - - event = isc_event_allocate(dctx->mctx, NULL, DNS_EVENT_DUMPQUANTUM, - setup_dump, dctx, sizeof(*event)); - isc_task_send(dctx->task, &event); - return (ISC_R_SUCCESS); + isc_work_enqueue(dctx->loop, master_dump_cb, master_dump_done_cb, dctx); } static isc_result_t @@ -1545,19 +1528,11 @@ dumpctx_create(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, unsigned int options; dctx = isc_mem_get(mctx, sizeof(*dctx)); + *dctx = (dns_dumpctx_t){ + .f = f, + .format = format, + }; - dctx->mctx = NULL; - dctx->f = f; - dctx->dbiter = NULL; - dctx->db = NULL; - dctx->version = NULL; - dctx->done = NULL; - dctx->done_arg = NULL; - dctx->task = NULL; - atomic_init(&dctx->canceled, false); - dctx->file = NULL; - dctx->tmpfile = NULL; - dctx->format = format; if (header == NULL) { dns_master_initrawheader(&dctx->header); } else { @@ -1772,12 +1747,12 @@ isc_result_t dns_master_dumptostreamasync(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, const dns_master_style_t *style, FILE *f, - isc_task_t *task, dns_dumpdonefunc_t done, + isc_loop_t *loop, dns_dumpdonefunc_t done, void *done_arg, dns_dumpctx_t **dctxp) { dns_dumpctx_t *dctx = NULL; isc_result_t result; - REQUIRE(task != NULL); + REQUIRE(loop != NULL); REQUIRE(f != NULL); REQUIRE(done != NULL); @@ -1786,18 +1761,13 @@ dns_master_dumptostreamasync(isc_mem_t *mctx, dns_db_t *db, if (result != ISC_R_SUCCESS) { return (result); } - isc_task_attach(task, &dctx->task); + isc_loop_attach(loop, &dctx->loop); dctx->done = done; dctx->done_arg = done_arg; - result = task_send(dctx); - if (result == ISC_R_SUCCESS) { - dns_dumpctx_attach(dctx, dctxp); - return (DNS_R_CONTINUE); - } - - dns_dumpctx_detach(&dctx); - return (result); + isc_async_run(dctx->loop, setup_dump, dctx); + dns_dumpctx_attach(dctx, dctxp); + return (DNS_R_CONTINUE); } isc_result_t @@ -1867,7 +1837,7 @@ cleanup: isc_result_t dns_master_dumpasync(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, const dns_master_style_t *style, const char *filename, - isc_task_t *task, dns_dumpdonefunc_t done, void *done_arg, + isc_loop_t *loop, dns_dumpdonefunc_t done, void *done_arg, dns_dumpctx_t **dctxp, dns_masterformat_t format, dns_masterrawheader_t *header) { FILE *f = NULL; @@ -1891,7 +1861,7 @@ dns_master_dumpasync(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, goto cleanup; } - isc_task_attach(task, &dctx->task); + isc_loop_attach(loop, &dctx->loop); dctx->done = done; dctx->done_arg = done_arg; dctx->file = file; @@ -1899,11 +1869,9 @@ dns_master_dumpasync(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, dctx->tmpfile = tempname; tempname = NULL; - result = task_send(dctx); - if (result == ISC_R_SUCCESS) { - dns_dumpctx_attach(dctx, dctxp); - return (DNS_R_CONTINUE); - } + isc_async_run(dctx->loop, setup_dump, dctx); + dns_dumpctx_attach(dctx, dctxp); + return (DNS_R_CONTINUE); cleanup: if (dctx != NULL) { diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 632a8f641c..5f488ca441 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -2691,7 +2691,7 @@ zone_gotwritehandle(isc_task_t *task, isc_event_t *event) { } result = dns_master_dumpasync( zone->mctx, db, version, output_style, zone->masterfile, - zone->task, dump_done, zone, &zone->dctx, + zone->loop, dump_done, zone, &zone->dctx, zone->masterformat, &rawdata); dns_db_closeversion(db, &version, false); } else { From 04670889bc855c20be0ea95dd8007e95ff6f94c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 26 Oct 2022 20:01:29 +0200 Subject: [PATCH 2/3] Refactor dns_master_dump*async() to use offloaded work The dns_master_dump*async() functions were using isc_async_run() to schedule work on the active loop; use isc_work_enqueue() instead. --- bin/named/server.c | 8 +++--- lib/dns/include/dns/zone.h | 10 +++++++ lib/dns/masterdump.c | 55 +++++++++++--------------------------- lib/dns/zone.c | 11 +++++--- 4 files changed, 37 insertions(+), 47 deletions(-) diff --git a/bin/named/server.c b/bin/named/server.c index a1e61713d7..4568fdd1e1 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -11611,7 +11611,7 @@ resume: result = dns_master_dumptostreamasync( dctx->mctx, dctx->cache, NULL, style, dctx->fp, named_g_mainloop, dumpdone, dctx, &dctx->mdctx); - if (result == DNS_R_CONTINUE) { + if (result == ISC_R_SUCCESS) { return; } if (result == ISC_R_NOTIMPLEMENTED) { @@ -11670,9 +11670,9 @@ resume: dns_db_currentversion(dctx->db, &dctx->version); result = dns_master_dumptostreamasync( dctx->mctx, dctx->db, dctx->version, style, - dctx->fp, named_g_mainloop, dumpdone, dctx, - &dctx->mdctx); - if (result == DNS_R_CONTINUE) { + dctx->fp, dns_zone_getloop(dctx->zone->zone), + dumpdone, dctx, &dctx->mdctx); + if (result == ISC_R_SUCCESS) { return; } if (result == ISC_R_NOTIMPLEMENTED) { diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index 845afe8c09..ae84a5ff67 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -2753,6 +2753,16 @@ dns_zone_gettid(dns_zone_t *zone); * \return thread id associated with the zone */ +isc_loop_t * +dns_zone_getloop(dns_zone_t *zone); +/**< + * \brief Return loop associated with the zone. + * + * \param valid dns_zone_t object + * + * \return loop associated with the zone + */ + bool dns_zone_check_dnskey_nsec3(dns_zone_t *zone, dns_db_t *db, dns_dbversion_t *ver, dns_diff_t *diff, diff --git a/lib/dns/masterdump.c b/lib/dns/masterdump.c index 981668450f..d46fd6e758 100644 --- a/lib/dns/masterdump.c +++ b/lib/dns/masterdump.c @@ -262,7 +262,6 @@ struct dns_dumpctx { dns_dbversion_t *version; dns_dbiterator_t *dbiter; dns_totext_ctx_t tctx; - isc_loop_t *loop; dns_dumpdonefunc_t done; void *done_arg; /* dns_master_dumpasync() */ @@ -1072,7 +1071,6 @@ again: sorted[i] = &rdatasets[i]; } n = i; - INSIST(n <= MAXSORT); qsort(sorted, n, sizeof(sorted[0]), dump_order_compare); @@ -1335,9 +1333,6 @@ dumpctx_destroy(dns_dumpctx_t *dctx) { dns_db_closeversion(dctx->db, &dctx->version, false); } dns_db_detach(&dctx->db); - if (dctx->loop != NULL) { - isc_loop_detach(&dctx->loop); - } if (dctx->file != NULL) { isc_mem_free(dctx->mctx, dctx->file); } @@ -1507,18 +1502,6 @@ master_dump_done_cb(void *data) { dns_dumpctx_detach(&dctx); } -/* - * This must be run from a loop manager thread. - */ -static void -setup_dump(void *arg) { - dns_dumpctx_t *dctx = (dns_dumpctx_t *)arg; - - REQUIRE(DNS_DCTX_VALID(dctx)); - - isc_work_enqueue(dctx->loop, master_dump_cb, master_dump_done_cb, dctx); -} - static isc_result_t dumpctx_create(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, const dns_master_style_t *style, FILE *f, dns_dumpctx_t **dctxp, @@ -1761,13 +1744,13 @@ dns_master_dumptostreamasync(isc_mem_t *mctx, dns_db_t *db, if (result != ISC_R_SUCCESS) { return (result); } - isc_loop_attach(loop, &dctx->loop); dctx->done = done; dctx->done_arg = done_arg; - isc_async_run(dctx->loop, setup_dump, dctx); dns_dumpctx_attach(dctx, dctxp); - return (DNS_R_CONTINUE); + isc_work_enqueue(loop, master_dump_cb, master_dump_done_cb, dctx); + + return (ISC_R_SUCCESS); } isc_result_t @@ -1850,39 +1833,33 @@ dns_master_dumpasync(isc_mem_t *mctx, dns_db_t *db, dns_dbversion_t *version, result = opentmp(mctx, format, filename, &tempname, &f); if (result != ISC_R_SUCCESS) { - goto cleanup; + goto cleanup_file; } result = dumpctx_create(mctx, db, version, style, f, &dctx, format, header); if (result != ISC_R_SUCCESS) { - (void)isc_stdio_close(f); - (void)isc_file_remove(tempname); - goto cleanup; + goto cleanup_tempname; } - isc_loop_attach(loop, &dctx->loop); dctx->done = done; dctx->done_arg = done_arg; dctx->file = file; - file = NULL; dctx->tmpfile = tempname; - tempname = NULL; - isc_async_run(dctx->loop, setup_dump, dctx); dns_dumpctx_attach(dctx, dctxp); - return (DNS_R_CONTINUE); + isc_work_enqueue(loop, master_dump_cb, master_dump_done_cb, dctx); + + return (ISC_R_SUCCESS); + +cleanup_tempname: + (void)isc_stdio_close(f); + (void)isc_file_remove(tempname); + isc_mem_free(mctx, tempname); + +cleanup_file: + isc_mem_free(mctx, file); -cleanup: - if (dctx != NULL) { - dns_dumpctx_detach(&dctx); - } - if (file != NULL) { - isc_mem_free(mctx, file); - } - if (tempname != NULL) { - isc_mem_free(mctx, tempname); - } return (result); } diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 5f488ca441..a0055db617 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -2701,7 +2701,7 @@ zone_gotwritehandle(isc_task_t *task, isc_event_t *event) { dns_db_detach(&db); } UNLOCK_ZONE(zone); - if (result != DNS_R_CONTINUE) { + if (result != ISC_R_SUCCESS) { goto fail; } return; @@ -12174,8 +12174,6 @@ redo: &zone->writeio); if (result != ISC_R_SUCCESS) { zone_idetach(&dummy); - } else { - result = DNS_R_CONTINUE; } UNLOCK_ZONE(zone); } else { @@ -12205,7 +12203,7 @@ fail: } masterfile = NULL; - if (result == DNS_R_CONTINUE) { + if (result == ISC_R_SUCCESS) { return (ISC_R_SUCCESS); /* XXXMPA */ } @@ -23674,3 +23672,8 @@ unsigned int dns_zone_gettid(dns_zone_t *zone) { return (zone->tid); } + +isc_loop_t * +dns_zone_getloop(dns_zone_t *zone) { + return (zone->loop); +} From 8fc229c17a356cba89c4d478d02669439759cc9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ondr=CC=8Cej=20Sury=CC=81?= Date: Mon, 31 Oct 2022 10:19:36 +0000 Subject: [PATCH 3/3] Add CHANGES note for [GL #3628] --- CHANGES | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGES b/CHANGES index 8a86367eee..9afbfe405f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +6006. [cleanup] The zone dumping was using isc_task API to launch + the zonedump on the offloaded threadpool. Remove + the task and launch the offloaded work directly. + [GL #3628] + 6005. [func] The zone loading has been moved to the offload threadpool instead of doing incremental repeated tasks, so zone loading scheduling is now driven