Run RPZ and catalog zones tasks in exclusive mode

All the heavy RPZ and CATZ work is already running with offloaded
threads, and running the remaining small functions in exclusive mode
offers more synchronization guaranties.

Move the update notify registration code from the offloaded
dns__catz_update_cb() function into dns__catz_done_cb().

After this change, it should be safe to remove the lock/unlock code
from the dns_catz_dbupdate_register() and dns_catz_dbupdate_unregister()
functions, as they were causing a benign TSAN lock-order-inversion
report.
This commit is contained in:
Ondřej Surý 2023-07-04 15:15:12 +02:00 committed by Aram Sargsyan
parent 471a2c0dd5
commit 26bb402b44
2 changed files with 22 additions and 24 deletions

View file

@ -25,6 +25,7 @@
#include <isc/print.h>
#include <isc/result.h>
#include <isc/task.h>
#include <isc/thread.h>
#include <isc/util.h>
#include <dns/catz.h>
@ -790,7 +791,7 @@ dns_catz_new_zones(isc_mem_t *mctx, isc_taskmgr_t *taskmgr,
.zmm = zmm,
.magic = DNS_CATZ_ZONES_MAGIC };
result = isc_task_create_bound(taskmgr, 0, &catzs->updater, 0);
result = isc_taskmgr_excltask(taskmgr, &catzs->updater);
if (result != ISC_R_SUCCESS) {
goto cleanup_task;
}
@ -1019,7 +1020,7 @@ dns__catz_zones_destroy(dns_catz_zones_t *catzs) {
REQUIRE(catzs->zones == NULL);
catzs->magic = 0;
isc_task_destroy(&catzs->updater);
isc_task_detach(&catzs->updater);
isc_mutex_destroy(&catzs->lock);
isc_refcount_destroy(&catzs->references);
@ -2227,9 +2228,7 @@ dns_catz_dbupdate_unregister(dns_db_t *db, dns_catz_zones_t *catzs) {
REQUIRE(DNS_DB_VALID(db));
REQUIRE(DNS_CATZ_ZONES_VALID(catzs));
LOCK(&catzs->lock);
dns_db_updatenotify_unregister(db, dns_catz_dbupdate_callback, catzs);
UNLOCK(&catzs->lock);
}
void
@ -2237,9 +2236,7 @@ dns_catz_dbupdate_register(dns_db_t *db, dns_catz_zones_t *catzs) {
REQUIRE(DNS_DB_VALID(db));
REQUIRE(DNS_CATZ_ZONES_VALID(catzs));
LOCK(&catzs->lock);
dns_db_updatenotify_register(db, dns_catz_dbupdate_callback, catzs);
UNLOCK(&catzs->lock);
}
static bool
@ -2305,6 +2302,8 @@ dns__catz_update_cb(void *data) {
goto exit;
}
INSIST(catz == oldcatz);
if (!is_active) {
/* This can happen during a reconfiguration. */
isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
@ -2540,22 +2539,6 @@ final:
ISC_LOG_DEBUG(3),
"catz: update_from_db: new zone merged");
/*
* When we're doing reconfig and setting a new catalog zone
* from an existing zone we won't have a chance to set up
* update callback in zone_startload or axfr_makedb, but we will
* call onupdate() artificially so we can register the callback here.
*/
LOCK(&catzs->lock);
if (!oldcatz->db_registered) {
result = dns_db_updatenotify_register(
updb, dns_catz_dbupdate_callback, oldcatz->catzs);
if (result == ISC_R_SUCCESS) {
oldcatz->db_registered = true;
}
}
UNLOCK(&catzs->lock);
exit:
catz->updateresult = result;
}
@ -2576,6 +2559,21 @@ dns__catz_done_cb(void *data, isc_result_t result) {
dns_name_format(&catz->name, dname, DNS_NAME_FORMATSIZE);
/*
* When we're doing reconfig and setting a new catalog zone
* from an existing zone we won't have a chance to set up
* update callback in zone_startload or axfr_makedb, but we will
* call onupdate() artificially so we can register the callback
* here.
*/
if (result == ISC_R_SUCCESS && !catz->db_registered) {
result = dns_db_updatenotify_register(
catz->db, dns_catz_dbupdate_callback, catz->catzs);
if (result == ISC_R_SUCCESS) {
catz->db_registered = true;
}
}
/* If there's no update pending, or if shutting down, finish. */
if (!catz->updatepending || atomic_load(&catz->catzs->shuttingdown)) {
goto done;

View file

@ -1474,7 +1474,7 @@ dns_rpz_new_zones(isc_mem_t *mctx, isc_taskmgr_t *taskmgr,
goto cleanup_rbt;
}
result = isc_task_create_bound(taskmgr, 0, &rpzs->updater, 0);
result = isc_taskmgr_excltask(taskmgr, &rpzs->updater);
if (result != ISC_R_SUCCESS) {
goto cleanup_task;
}
@ -2120,7 +2120,7 @@ dns__rpz_zones_destroy(dns_rpz_zones_t *rpzs) {
if (rpzs->rbt != NULL) {
dns_rbt_destroy(&rpzs->rbt);
}
isc_task_destroy(&rpzs->updater);
isc_task_detach(&rpzs->updater);
isc_mutex_destroy(&rpzs->maint_lock);
isc_rwlock_destroy(&rpzs->search_lock);
isc_mem_putanddetach(&rpzs->mctx, rpzs, sizeof(*rpzs));