From 431513d8b3cc595a7612479fd7811e959501e6dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 27 Jan 2025 21:07:11 +0100 Subject: [PATCH] Remove db_nodelock_t in favor of reference counted qpdb This removes the db_nodelock_t structure and changes the node_locks array to be composed only of isc_rwlock_t pointers. The .reference member has been moved to qpdb->references in addition to common.references that's external to dns_db API users. The .exiting members has been completely removed as it has no use when the reference counting is used correctly. --- lib/dns/db_p.h | 8 - lib/dns/qpcache.c | 335 ++++++++++++++++--------------------- lib/dns/qpzone.c | 354 +++++++++++++++++----------------------- tests/dns/qpzone_test.c | 12 +- 4 files changed, 296 insertions(+), 413 deletions(-) diff --git a/lib/dns/db_p.h b/lib/dns/db_p.h index b1e0460b5d..132f8e0464 100644 --- a/lib/dns/db_p.h +++ b/lib/dns/db_p.h @@ -143,14 +143,6 @@ typedef struct dns_glue_additionaldata_ctx { dns_glue_t *glue; } dns_glue_additionaldata_ctx_t; -typedef struct { - isc_rwlock_t lock; - /* Protected in the refcount routines. */ - isc_refcount_t references; - /* Locked by lock. */ - bool exiting; -} db_nodelock_t; - static inline bool prio_type(dns_typepair_t type) { switch (type) { diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index 8b0bda1912..dfa03ba84a 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -206,12 +206,12 @@ struct qpcache { isc_rwlock_t tree_lock; /* Locks for individual tree nodes */ unsigned int node_lock_count; - db_nodelock_t *node_locks; dns_stats_t *rrsetstats; /* cache DB only */ isc_stats_t *cachestats; /* cache DB only */ isc_stats_t *gluecachestats; /* zone DB only */ - /* Locked by lock. */ - unsigned int active; + + isc_refcount_t references; + isc_rwlock_t *node_locks; uint32_t maxrrperset; /* Maximum RRs per RRset */ uint32_t maxtypepername; /* Maximum number of RR types per owner */ @@ -261,6 +261,17 @@ struct qpcache { dns_qp_t *nsec; }; +#ifdef DNS_DB_NODETRACE +#define qpcache_ref(ptr) qpcache__ref(ptr, __func__, __FILE__, __LINE__) +#define qpcache_unref(ptr) qpcache_unref(ptr, __func__, __FILE__, __LINE__) +#define qpcache_attach(ptr, ptrp) \ + qpcache__attach(ptr, ptrp, __func__, __FILE__, __LINE__) +#define qpcache_detach(ptrp) qpcache__detach(ptrp, __func__, __FILE__, __LINE__) +ISC_REFCOUNT_STATIC_TRACE_DECL(qpcache); +#else +ISC_REFCOUNT_STATIC_DECL(qpcache); +#endif + /*% * Search Context */ @@ -398,7 +409,7 @@ typedef struct qpc_dbit { } qpc_dbit_t; static void -free_qpdb(qpcache_t *qpdb, bool log); +qpcache__destroy(qpcache_t *qpdb); static dns_dbmethods_t qpdb_cachemethods; @@ -639,17 +650,7 @@ qpcnode_newref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t nlocktype, INSIST(nlocktype != isc_rwlocktype_none || tlocktype != isc_rwlocktype_none); - refs = isc_refcount_increment0( - &qpdb->node_locks[node->locknum].references); -#if DNS_DB_NODETRACE - fprintf(stderr, - "incr:nodelock:%s:%s:%u:%p:%p->references = " - "%" PRIuFAST32 "\n", - func, file, line, node, &qpdb->node_locks[node->locknum], - refs + 1); -#else - UNUSED(refs); -#endif + qpcache_ref(qpdb); } static void @@ -674,17 +675,7 @@ qpcnode_decref(qpcache_t *qpdb, qpcnode_t *node DNS__DB_FLARG) { return false; } - refs = isc_refcount_decrement( - &qpdb->node_locks[node->locknum].references); -#if DNS_DB_NODETRACE - fprintf(stderr, - "decr:nodelock:%s:%s:%u:%p:%p->references = " - "%" PRIuFAST32 "\n", - func, file, line, node, &qpdb->node_locks[node->locknum], - refs - 1); -#else - UNUSED(refs); -#endif + qpcache_unref(qpdb); return true; } @@ -711,6 +702,7 @@ decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep, isc_result_t result; bool locked = *tlocktypep != isc_rwlocktype_none; bool write_locked = false; + isc_rwlock_t *nlock = &qpdb->node_locks[node->locknum]; REQUIRE(*nlocktypep != isc_rwlocktype_none); @@ -732,8 +724,7 @@ decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep, /* Upgrade the lock? */ if (*nlocktypep == isc_rwlocktype_read) { - NODE_FORCEUPGRADE(&qpdb->node_locks[node->locknum].lock, - nlocktypep); + NODE_FORCEUPGRADE(nlock, nlocktypep); } if (!qpcnode_decref(qpdb, node DNS__DB_FLARG_PASS)) { @@ -1113,8 +1104,8 @@ setup_delegation(qpc_search_t *search, dns_dbnode_t **nodep, } if (rdataset != NULL) { isc_rwlocktype_t nlocktype = isc_rwlocktype_none; - NODE_RDLOCK(&(search->qpdb->node_locks[node->locknum].lock), - &nlocktype); + isc_rwlock_t *nlock = &search->qpdb->node_locks[node->locknum]; + NODE_RDLOCK(nlock, &nlocktype); bindrdataset(search->qpdb, node, search->zonecut_header, search->now, nlocktype, tlocktype, rdataset DNS__DB_FLARG_PASS); @@ -1124,8 +1115,7 @@ setup_delegation(qpc_search_t *search, dns_dbnode_t **nodep, nlocktype, tlocktype, sigrdataset DNS__DB_FLARG_PASS); } - NODE_UNLOCK(&(search->qpdb->node_locks[node->locknum].lock), - &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); } if (type == dns_rdatatype_dname) { @@ -1136,7 +1126,7 @@ setup_delegation(qpc_search_t *search, dns_dbnode_t **nodep, static bool check_stale_header(qpcnode_t *node, dns_slabheader_t *header, - isc_rwlocktype_t *nlocktypep, isc_rwlock_t *lock, + isc_rwlocktype_t *nlocktypep, isc_rwlock_t *nlock, qpc_search_t *search, dns_slabheader_t **header_prev) { if (!ACTIVE(header, search->now)) { dns_ttl_t stale = header->ttl + STALE_TTL(header, search->qpdb); @@ -1200,7 +1190,7 @@ check_stale_header(qpcnode_t *node, dns_slabheader_t *header, */ if ((header->ttl < search->now - QPDB_VIRTUAL) && (*nlocktypep == isc_rwlocktype_write || - NODE_TRYUPGRADE(lock, nlocktypep) == ISC_R_SUCCESS)) + NODE_TRYUPGRADE(nlock, nlocktypep) == ISC_R_SUCCESS)) { /* * We update the node's status only when we can @@ -1247,20 +1237,20 @@ check_zonecut(qpcnode_t *node, void *arg DNS__DB_FLARG) { dns_slabheader_t *header_prev = NULL, *header_next = NULL; dns_slabheader_t *dname_header = NULL, *sigdname_header = NULL; isc_result_t result; - isc_rwlock_t *lock = NULL; + isc_rwlock_t *nlock = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; REQUIRE(search->zonecut == NULL); - lock = &(search->qpdb->node_locks[node->locknum].lock); - NODE_RDLOCK(lock, &nlocktype); + nlock = &search->qpdb->node_locks[node->locknum]; + NODE_RDLOCK(nlock, &nlocktype); /* * Look for a DNAME or RRSIG DNAME rdataset. */ for (header = node->data; header != NULL; header = header_next) { header_next = header->next; - if (check_stale_header(node, header, &nlocktype, lock, search, + if (check_stale_header(node, header, &nlocktype, nlock, search, &header_prev)) { /* Do nothing. */ @@ -1298,7 +1288,7 @@ check_zonecut(qpcnode_t *node, void *arg DNS__DB_FLARG) { result = DNS_R_CONTINUE; } - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); return result; } @@ -1321,13 +1311,13 @@ find_deepest_zonecut(qpc_search_t *search, qpcnode_t *node, dns_slabheader_t *header = NULL; dns_slabheader_t *header_prev = NULL, *header_next = NULL; dns_slabheader_t *found = NULL, *foundsig = NULL; - isc_rwlock_t *lock = NULL; + isc_rwlock_t *nlock = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; dns_qpchain_node(&search->chain, i, NULL, (void **)&node, NULL); - lock = &qpdb->node_locks[node->locknum].lock; + nlock = &qpdb->node_locks[node->locknum]; - NODE_RDLOCK(lock, &nlocktype); + NODE_RDLOCK(nlock, &nlocktype); /* * Look for NS and RRSIG NS rdatasets. @@ -1335,7 +1325,7 @@ find_deepest_zonecut(qpc_search_t *search, qpcnode_t *node, for (header = node->data; header != NULL; header = header_next) { header_next = header->next; - if (check_stale_header(node, header, &nlocktype, lock, + if (check_stale_header(node, header, &nlocktype, nlock, search, &header_prev)) { /* Do nothing. */ @@ -1391,7 +1381,7 @@ find_deepest_zonecut(qpc_search_t *search, qpcnode_t *node, need_headerupdate(foundsig, search->now))) { if (nlocktype != isc_rwlocktype_write) { - NODE_FORCEUPGRADE(lock, &nlocktype); + NODE_FORCEUPGRADE(nlock, &nlocktype); POST(nlocktype); } if (need_headerupdate(found, search->now)) { @@ -1407,7 +1397,7 @@ find_deepest_zonecut(qpc_search_t *search, qpcnode_t *node, } } - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); if (found != NULL) { break; @@ -1435,7 +1425,7 @@ find_coveringnsec(qpc_search_t *search, const dns_name_t *name, dns_qpiter_t iter; isc_result_t result; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; - isc_rwlock_t *lock = NULL; + isc_rwlock_t *nlock = NULL; dns_typepair_t matchtype, sigmatchtype; dns_slabheader_t *found = NULL, *foundsig = NULL; dns_slabheader_t *header = NULL; @@ -1474,11 +1464,11 @@ find_coveringnsec(qpc_search_t *search, const dns_name_t *name, } dns_name_copy(&node->name, fname); - lock = &(search->qpdb->node_locks[node->locknum].lock); - NODE_RDLOCK(lock, &nlocktype); + nlock = &search->qpdb->node_locks[node->locknum]; + NODE_RDLOCK(nlock, &nlocktype); for (header = node->data; header != NULL; header = header_next) { header_next = header->next; - if (check_stale_header(node, header, &nlocktype, lock, search, + if (check_stale_header(node, header, &nlocktype, nlock, search, &header_prev)) { continue; @@ -1519,7 +1509,7 @@ find_coveringnsec(qpc_search_t *search, const dns_name_t *name, } else { result = ISC_R_NOTFOUND; } - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); return result; } @@ -1536,7 +1526,7 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, bool found_noqname = false; bool all_negative = true; bool empty_node; - isc_rwlock_t *lock = NULL; + isc_rwlock_t *nlock = NULL; isc_rwlocktype_t tlocktype = isc_rwlocktype_none; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; dns_slabheader_t *header = NULL; @@ -1652,8 +1642,8 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, * We now go looking for rdata... */ - lock = &(search.qpdb->node_locks[node->locknum].lock); - NODE_RDLOCK(lock, &nlocktype); + nlock = &search.qpdb->node_locks[node->locknum]; + NODE_RDLOCK(nlock, &nlocktype); /* * These pointers need to be reset here in case we did @@ -1672,7 +1662,7 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, header_prev = NULL; for (header = node->data; header != NULL; header = header_next) { header_next = header->next; - if (check_stale_header(node, header, &nlocktype, lock, &search, + if (check_stale_header(node, header, &nlocktype, nlock, &search, &header_prev)) { /* Do nothing. */ @@ -1774,7 +1764,7 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, * extant rdatasets. That means that this node doesn't * meaningfully exist, and that we really have a partial match. */ - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); if ((search.options & DNS_DBFIND_COVERINGNSEC) != 0) { result = find_coveringnsec( &search, name, nodep, now, foundname, rdataset, @@ -1832,7 +1822,7 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, if (found == NULL && (found_noqname || all_negative) && (search.options & DNS_DBFIND_COVERINGNSEC) != 0) { - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); result = find_coveringnsec( &search, name, nodep, now, foundname, rdataset, sigrdataset DNS__DB_FLARG_PASS); @@ -1873,7 +1863,7 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, /* * Go find the deepest zone cut. */ - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); goto find_ns; } @@ -1934,7 +1924,7 @@ node_exit: if ((update != NULL || updatesig != NULL) && nlocktype != isc_rwlocktype_write) { - NODE_FORCEUPGRADE(lock, &nlocktype); + NODE_FORCEUPGRADE(nlock, &nlocktype); POST(nlocktype); } if (update != NULL && need_headerupdate(update, search.now)) { @@ -1944,7 +1934,7 @@ node_exit: update_header(search.qpdb, updatesig, search.now); } - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); tree_exit: TREE_UNLOCK(&search.qpdb->tree_lock, &tlocktype); @@ -1956,12 +1946,12 @@ tree_exit: if (search.need_cleanup) { node = search.zonecut; INSIST(node != NULL); - lock = &(search.qpdb->node_locks[node->locknum].lock); + nlock = &search.qpdb->node_locks[node->locknum]; - NODE_RDLOCK(lock, &nlocktype); + NODE_RDLOCK(nlock, &nlocktype); decref(search.qpdb, node, &nlocktype, &tlocktype, true DNS__DB_FLARG_PASS); - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); INSIST(tlocktype == isc_rwlocktype_none); } @@ -1976,7 +1966,7 @@ qpcache_findzonecut(dns_db_t *db, const dns_name_t *name, unsigned int options, dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset DNS__DB_FLARG) { qpcnode_t *node = NULL; - isc_rwlock_t *lock = NULL; + isc_rwlock_t *nlock = NULL; isc_result_t result; qpc_search_t search; dns_slabheader_t *header = NULL; @@ -2040,12 +2030,12 @@ qpcache_findzonecut(dns_db_t *db, const dns_name_t *name, unsigned int options, * We now go looking for an NS rdataset at the node. */ - lock = &(search.qpdb->node_locks[node->locknum].lock); - NODE_RDLOCK(lock, &nlocktype); + nlock = &search.qpdb->node_locks[node->locknum]; + NODE_RDLOCK(nlock, &nlocktype); for (header = node->data; header != NULL; header = header_next) { header_next = header->next; - if (check_stale_header(node, header, &nlocktype, lock, &search, + if (check_stale_header(node, header, &nlocktype, nlock, &search, &header_prev)) { /* @@ -2058,7 +2048,7 @@ qpcache_findzonecut(dns_db_t *db, const dns_name_t *name, unsigned int options, * zonecut we know about. If so, find the deepest * zonecut from this node up and return that instead. */ - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); result = find_deepest_zonecut( &search, node, nodep, foundname, rdataset, sigrdataset DNS__DB_FLARG_PASS); @@ -2095,7 +2085,7 @@ qpcache_findzonecut(dns_db_t *db, const dns_name_t *name, unsigned int options, /* * No NS records here. */ - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); result = find_deepest_zonecut(&search, node, nodep, foundname, rdataset, sigrdataset DNS__DB_FLARG_PASS); @@ -2119,7 +2109,7 @@ qpcache_findzonecut(dns_db_t *db, const dns_name_t *name, unsigned int options, (foundsig != NULL && need_headerupdate(foundsig, search.now))) { if (nlocktype != isc_rwlocktype_write) { - NODE_FORCEUPGRADE(lock, &nlocktype); + NODE_FORCEUPGRADE(nlock, &nlocktype); POST(nlocktype); } if (need_headerupdate(found, search.now)) { @@ -2131,7 +2121,7 @@ qpcache_findzonecut(dns_db_t *db, const dns_name_t *name, unsigned int options, } } - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); tree_exit: TREE_UNLOCK(&search.qpdb->tree_lock, &tlocktype); @@ -2156,7 +2146,7 @@ qpcache_findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, dns_slabheader_t *found = NULL, *foundsig = NULL; dns_typepair_t matchtype, sigmatchtype, negtype; isc_result_t result; - isc_rwlock_t *lock = NULL; + isc_rwlock_t *nlock = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; REQUIRE(VALID_QPDB(qpdb)); @@ -2170,8 +2160,8 @@ qpcache_findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, now = isc_stdtime_now(); } - lock = &qpdb->node_locks[qpnode->locknum].lock; - NODE_RDLOCK(lock, &nlocktype); + nlock = &qpdb->node_locks[qpnode->locknum]; + NODE_RDLOCK(nlock, &nlocktype); matchtype = DNS_TYPEPAIR_VALUE(type, covers); negtype = DNS_TYPEPAIR_VALUE(0, type); @@ -2187,7 +2177,7 @@ qpcache_findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, if ((header->ttl + STALE_TTL(header, qpdb) < now - QPDB_VIRTUAL) && (nlocktype == isc_rwlocktype_write || - NODE_TRYUPGRADE(lock, &nlocktype) == + NODE_TRYUPGRADE(nlock, &nlocktype) == ISC_R_SUCCESS)) { /* @@ -2225,7 +2215,7 @@ qpcache_findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, } } - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); if (found == NULL) { return ISC_R_NOTFOUND; @@ -2316,11 +2306,12 @@ expiredata(dns_db_t *db, dns_dbnode_t *node, void *data) { dns_slabheader_t *header = data; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; isc_rwlocktype_t tlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = &qpdb->node_locks[qpnode->locknum]; - NODE_WRLOCK(&qpdb->node_locks[qpnode->locknum].lock, &nlocktype); + NODE_WRLOCK(nlock, &nlocktype); expireheader(header, &nlocktype, &tlocktype, dns_expire_flush DNS__DB_FILELINE); - NODE_UNLOCK(&qpdb->node_locks[qpnode->locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); INSIST(tlocktype == isc_rwlocktype_none); } @@ -2397,7 +2388,8 @@ overmem(qpcache_t *qpdb, dns_slabheader_t *newheader, again: do { isc_rwlocktype_t nlocktype = isc_rwlocktype_none; - NODE_WRLOCK(&qpdb->node_locks[locknum].lock, &nlocktype); + isc_rwlock_t *nlock = &qpdb->node_locks[locknum]; + NODE_WRLOCK(nlock, &nlocktype); purged += expire_lru_headers( qpdb, locknum, &nlocktype, tlocktypep, @@ -2413,7 +2405,7 @@ again: { min_last_used = header->last_used; } - NODE_UNLOCK(&qpdb->node_locks[locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); locknum = (locknum + 1) % qpdb->node_lock_count; } while (locknum != locknum_start && purged <= purgesize); @@ -2454,7 +2446,7 @@ set_index(void *what, unsigned int idx) { } static void -free_qpdb(qpcache_t *qpdb, bool log) { +qpcache__destroy(qpcache_t *qpdb) { unsigned int i; char buf[DNS_NAME_FORMATSIZE]; dns_qp_t **treep = NULL; @@ -2475,21 +2467,19 @@ free_qpdb(qpcache_t *qpdb, bool log) { INSIST(*treep == NULL); } - if (log) { - if (dns_name_dynamic(&qpdb->common.origin)) { - dns_name_format(&qpdb->common.origin, buf, sizeof(buf)); - } else { - strlcpy(buf, "", sizeof(buf)); - } - isc_log_write(DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_CACHE, - ISC_LOG_DEBUG(1), "done free_qpdb(%s)", buf); + if (dns_name_dynamic(&qpdb->common.origin)) { + dns_name_format(&qpdb->common.origin, buf, sizeof(buf)); + } else { + strlcpy(buf, "", sizeof(buf)); } + isc_log_write(DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_CACHE, + ISC_LOG_DEBUG(1), "done %s(%s)", __func__, buf); + if (dns_name_dynamic(&qpdb->common.origin)) { dns_name_free(&qpdb->common.origin, qpdb->common.mctx); } for (i = 0; i < qpdb->node_lock_count; i++) { - isc_refcount_destroy(&qpdb->node_locks[i].references); - NODE_DESTROYLOCK(&qpdb->node_locks[i].lock); + NODE_DESTROYLOCK(&qpdb->node_locks[i]); } /* @@ -2535,8 +2525,9 @@ free_qpdb(qpcache_t *qpdb, bool log) { } isc_mem_cput(qpdb->common.mctx, qpdb->node_locks, qpdb->node_lock_count, - sizeof(db_nodelock_t)); + sizeof(qpdb->node_locks[0])); TREE_DESTROYLOCK(&qpdb->tree_lock); + isc_refcount_destroy(&qpdb->references); isc_refcount_destroy(&qpdb->common.references); isc_rwlock_destroy(&qpdb->lock); @@ -2550,46 +2541,8 @@ free_qpdb(qpcache_t *qpdb, bool log) { static void qpcache_destroy(dns_db_t *arg) { qpcache_t *qpdb = (qpcache_t *)arg; - bool want_free = false; - unsigned int i; - unsigned int inactive = 0; - /* - * Even though there are no external direct references, there still - * may be nodes in use. - */ - for (i = 0; i < qpdb->node_lock_count; i++) { - isc_rwlocktype_t nodelock = isc_rwlocktype_none; - NODE_WRLOCK(&qpdb->node_locks[i].lock, &nodelock); - qpdb->node_locks[i].exiting = true; - if (isc_refcount_current(&qpdb->node_locks[i].references) == 0) - { - inactive++; - } - NODE_UNLOCK(&qpdb->node_locks[i].lock, &nodelock); - } - - if (inactive != 0) { - RWLOCK(&qpdb->lock, isc_rwlocktype_write); - qpdb->active -= inactive; - if (qpdb->active == 0) { - want_free = true; - } - RWUNLOCK(&qpdb->lock, isc_rwlocktype_write); - if (want_free) { - char buf[DNS_NAME_FORMATSIZE]; - if (dns_name_dynamic(&qpdb->common.origin)) { - dns_name_format(&qpdb->common.origin, buf, - sizeof(buf)); - } else { - strlcpy(buf, "", sizeof(buf)); - } - isc_log_write(DNS_LOGCATEGORY_DATABASE, - DNS_LOGMODULE_CACHE, ISC_LOG_DEBUG(1), - "calling free_qpdb(%s)", buf); - free_qpdb(qpdb, true); - } - } + qpcache_detach(&qpdb); } static void @@ -2611,6 +2564,7 @@ cleanup_deadnodes(void *arg) { uint16_t locknum = isc_tid(); isc_rwlocktype_t tlocktype = isc_rwlocktype_none; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = &qpdb->node_locks[locknum]; qpcnode_t *qpnode = NULL, *qpnext = NULL; isc_queue_t deadnodes; @@ -2619,14 +2573,14 @@ cleanup_deadnodes(void *arg) { isc_queue_init(&deadnodes); TREE_WRLOCK(&qpdb->tree_lock, &tlocktype); - NODE_WRLOCK(&qpdb->node_locks[locknum].lock, &nlocktype); + NODE_WRLOCK(nlock, &nlocktype); RUNTIME_CHECK(isc_queue_splice(&deadnodes, &qpdb->deadnodes[locknum])); isc_queue_for_each_entry_safe(&deadnodes, qpnode, qpnext, deadlink) { decref(qpdb, qpnode, &nlocktype, &tlocktype, false); } - NODE_UNLOCK(&qpdb->node_locks[locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); TREE_UNLOCK(&qpdb->tree_lock, &tlocktype); } @@ -2643,11 +2597,11 @@ static void reactivate_node(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t tlocktype ISC_ATTR_UNUSED DNS__DB_FLARG) { isc_rwlocktype_t nlocktype = isc_rwlocktype_none; - isc_rwlock_t *nodelock = &qpdb->node_locks[node->locknum].lock; + isc_rwlock_t *nlock = &qpdb->node_locks[node->locknum]; - NODE_RDLOCK(nodelock, &nlocktype); + NODE_RDLOCK(nlock, &nlocktype); newref(qpdb, node, nlocktype, tlocktype DNS__DB_FLARG_PASS); - NODE_UNLOCK(nodelock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); } static qpcnode_t * @@ -2723,56 +2677,31 @@ qpcache_attachnode(dns_db_t *db, dns_dbnode_t *source, } static void -qpcache_detachnode(dns_db_t *db, dns_dbnode_t **targetp DNS__DB_FLARG) { +qpcache_detachnode(dns_db_t *db, dns_dbnode_t **nodep DNS__DB_FLARG) { qpcache_t *qpdb = (qpcache_t *)db; qpcnode_t *node = NULL; - bool want_free = false; - bool inactive = false; - db_nodelock_t *nodelock = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; isc_rwlocktype_t tlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = NULL; REQUIRE(VALID_QPDB(qpdb)); - REQUIRE(targetp != NULL && *targetp != NULL); + REQUIRE(nodep != NULL && *nodep != NULL); - node = (qpcnode_t *)(*targetp); - nodelock = &qpdb->node_locks[node->locknum]; + node = (qpcnode_t *)(*nodep); + *nodep = NULL; + nlock = &qpdb->node_locks[node->locknum]; - NODE_RDLOCK(&nodelock->lock, &nlocktype); + /* + * We can't destroy qpcache under nodelock, so we need + * to reference it before acquiring the lock. + */ + qpcache_ref(qpdb); + NODE_RDLOCK(nlock, &nlocktype); decref(qpdb, node, &nlocktype, &tlocktype, true DNS__DB_FLARG_PASS); - if (isc_refcount_current(&nodelock->references) == 0 && - nodelock->exiting) - { - inactive = true; - } + NODE_UNLOCK(nlock, &nlocktype); - NODE_UNLOCK(&nodelock->lock, &nlocktype); - INSIST(tlocktype == isc_rwlocktype_none); - - *targetp = NULL; - - if (inactive) { - RWLOCK(&qpdb->lock, isc_rwlocktype_write); - qpdb->active--; - if (qpdb->active == 0) { - want_free = true; - } - RWUNLOCK(&qpdb->lock, isc_rwlocktype_write); - if (want_free) { - char buf[DNS_NAME_FORMATSIZE]; - if (dns_name_dynamic(&qpdb->common.origin)) { - dns_name_format(&qpdb->common.origin, buf, - sizeof(buf)); - } else { - strlcpy(buf, "", sizeof(buf)); - } - isc_log_write(DNS_LOGCATEGORY_DATABASE, - DNS_LOGMODULE_CACHE, ISC_LOG_DEBUG(1), - "calling free_qpdb(%s)", buf); - free_qpdb(qpdb, true); - } - } + qpcache_detach(&qpdb); } static isc_result_t @@ -3389,6 +3318,7 @@ qpcache_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, bool newnsec; isc_rwlocktype_t tlocktype = isc_rwlocktype_none; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = NULL; bool cache_is_overmem = false; dns_fixedname_t fixed; dns_name_t *name = NULL; @@ -3500,7 +3430,9 @@ qpcache_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, overmem(qpdb, newheader, &tlocktype DNS__DB_FLARG_PASS); } - NODE_WRLOCK(&qpdb->node_locks[qpnode->locknum].lock, &nlocktype); + nlock = &qpdb->node_locks[qpnode->locknum]; + + NODE_WRLOCK(nlock, &nlocktype); if (qpdb->rrsetstats != NULL) { DNS_SLABHEADER_SETATTR(newheader, DNS_SLABHEADERATTR_STATCOUNT); @@ -3549,7 +3481,7 @@ qpcache_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, qpnode->delegating = 1; } - NODE_UNLOCK(&qpdb->node_locks[qpnode->locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); if (tlocktype != isc_rwlocktype_none) { TREE_UNLOCK(&qpdb->tree_lock, &tlocktype); @@ -3568,6 +3500,7 @@ qpcache_deleterdataset(dns_db_t *db, dns_dbnode_t *node, isc_result_t result; dns_slabheader_t *newheader = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = NULL; REQUIRE(VALID_QPDB(qpdb)); REQUIRE(version == NULL); @@ -3584,11 +3517,12 @@ qpcache_deleterdataset(dns_db_t *db, dns_dbnode_t *node, setttl(newheader, 0); atomic_init(&newheader->attributes, DNS_SLABHEADERATTR_NONEXISTENT); - NODE_WRLOCK(&qpdb->node_locks[qpnode->locknum].lock, &nlocktype); + nlock = &qpdb->node_locks[qpnode->locknum]; + NODE_WRLOCK(nlock, &nlocktype); result = add(qpdb, qpnode, NULL, newheader, DNS_DBADD_FORCE, false, NULL, 0, nlocktype, isc_rwlocktype_none DNS__DB_FLARG_PASS); - NODE_UNLOCK(&qpdb->node_locks[qpnode->locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); return result; } @@ -3622,7 +3556,7 @@ locknode(dns_db_t *db, dns_dbnode_t *node, isc_rwlocktype_t type) { qpcache_t *qpdb = (qpcache_t *)db; qpcnode_t *qpnode = (qpcnode_t *)node; - RWLOCK(&qpdb->node_locks[qpnode->locknum].lock, type); + RWLOCK(&qpdb->node_locks[qpnode->locknum], type); } static void @@ -3630,7 +3564,7 @@ unlocknode(dns_db_t *db, dns_dbnode_t *node, isc_rwlocktype_t type) { qpcache_t *qpdb = (qpcache_t *)db; qpcnode_t *qpnode = (qpcnode_t *)node; - RWUNLOCK(&qpdb->node_locks[qpnode->locknum].lock, type); + RWUNLOCK(&qpdb->node_locks[qpnode->locknum], type); } isc_result_t @@ -3653,11 +3587,11 @@ dns__qpcache_create(isc_mem_t *mctx, const dns_name_t *origin, .common.origin = DNS_NAME_INITEMPTY, .common.rdclass = rdclass, .common.attributes = DNS_DBATTR_CACHE, + .common.references = 1, .loopmgr = isc_loop_getloopmgr(loop), + .references = 1, }; - isc_refcount_init(&qpdb->common.references, 1); - /* * If argv[0] exists, it points to a memory context to use for heap */ @@ -3670,7 +3604,7 @@ dns__qpcache_create(isc_mem_t *mctx, const dns_name_t *origin, qpdb->node_lock_count = isc_loopmgr_nloops(qpdb->loopmgr); qpdb->node_locks = isc_mem_cget(mctx, qpdb->node_lock_count, - sizeof(db_nodelock_t)); + sizeof(qpdb->node_locks[0])); dns_rdatasetstats_create(mctx, &qpdb->rrsetstats); qpdb->lru = isc_mem_cget(mctx, qpdb->node_lock_count, @@ -3698,12 +3632,8 @@ dns__qpcache_create(isc_mem_t *mctx, const dns_name_t *origin, isc_queue_init(&qpdb->deadnodes[i]); } - qpdb->active = qpdb->node_lock_count; - for (i = 0; i < (int)(qpdb->node_lock_count); i++) { - NODE_INITLOCK(&qpdb->node_locks[i].lock); - isc_refcount_init(&qpdb->node_locks[i].references, 0); - qpdb->node_locks[i].exiting = false; + NODE_INITLOCK(&qpdb->node_locks[i]); } /* @@ -3786,8 +3716,9 @@ rdatasetiter_first(dns_rdatasetiter_t *it DNS__DB_FLARG) { qpcnode_t *qpnode = (qpcnode_t *)iterator->common.node; dns_slabheader_t *header = NULL, *top_next = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = &qpdb->node_locks[qpnode->locknum]; - NODE_RDLOCK(&qpdb->node_locks[qpnode->locknum].lock, &nlocktype); + NODE_RDLOCK(nlock, &nlocktype); for (header = qpnode->data; header != NULL; header = top_next) { top_next = header->next; @@ -3811,7 +3742,7 @@ rdatasetiter_first(dns_rdatasetiter_t *it DNS__DB_FLARG) { } } - NODE_UNLOCK(&qpdb->node_locks[qpnode->locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); iterator->current = header; @@ -3831,6 +3762,7 @@ rdatasetiter_next(dns_rdatasetiter_t *it DNS__DB_FLARG) { dns_typepair_t type, negtype; dns_rdatatype_t rdtype, covers; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = &qpdb->node_locks[qpnode->locknum]; bool expiredok = EXPIREDOK(iterator); header = iterator->current; @@ -3838,7 +3770,7 @@ rdatasetiter_next(dns_rdatasetiter_t *it DNS__DB_FLARG) { return ISC_R_NOMORE; } - NODE_RDLOCK(&qpdb->node_locks[qpnode->locknum].lock, &nlocktype); + NODE_RDLOCK(nlock, &nlocktype); type = header->type; rdtype = DNS_TYPEPAIR_TYPE(header->type); @@ -3899,7 +3831,7 @@ rdatasetiter_next(dns_rdatasetiter_t *it DNS__DB_FLARG) { } } - NODE_UNLOCK(&qpdb->node_locks[qpnode->locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); iterator->current = header; @@ -3918,16 +3850,17 @@ rdatasetiter_current(dns_rdatasetiter_t *it, qpcnode_t *qpnode = (qpcnode_t *)iterator->common.node; dns_slabheader_t *header = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = &qpdb->node_locks[qpnode->locknum]; header = iterator->current; REQUIRE(header != NULL); - NODE_RDLOCK(&qpdb->node_locks[qpnode->locknum].lock, &nlocktype); + NODE_RDLOCK(nlock, &nlocktype); bindrdataset(qpdb, qpnode, header, iterator->common.now, nlocktype, isc_rwlocktype_none, rdataset DNS__DB_FLARG_PASS); - NODE_UNLOCK(&qpdb->node_locks[qpnode->locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); } /* @@ -3951,7 +3884,7 @@ static void dereference_iter_node(qpc_dbit_t *qpdbiter DNS__DB_FLARG) { qpcache_t *qpdb = (qpcache_t *)qpdbiter->common.db; qpcnode_t *node = qpdbiter->node; - isc_rwlock_t *lock = NULL; + isc_rwlock_t *nlock = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; isc_rwlocktype_t tlocktype = qpdbiter->tree_locked; @@ -3961,11 +3894,11 @@ dereference_iter_node(qpc_dbit_t *qpdbiter DNS__DB_FLARG) { REQUIRE(tlocktype != isc_rwlocktype_write); - lock = &qpdb->node_locks[node->locknum].lock; - NODE_RDLOCK(lock, &nlocktype); + nlock = &qpdb->node_locks[node->locknum]; + NODE_RDLOCK(nlock, &nlocktype); decref(qpdb, node, &nlocktype, &qpdbiter->tree_locked, false DNS__DB_FLARG_PASS); - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); INSIST(qpdbiter->tree_locked == tlocktype); @@ -4400,3 +4333,9 @@ ISC_REFCOUNT_STATIC_TRACE_IMPL(qpcnode, qpcnode_destroy); #else ISC_REFCOUNT_STATIC_IMPL(qpcnode, qpcnode_destroy); #endif + +#ifdef DNS_DB_NODETRACE +ISC_REFCOUNT_STATIC_TRACE_IMPL(qpcache, qpcache__destroy); +#else +ISC_REFCOUNT_STATIC_IMPL(qpcache, qpcache__destroy); +#endif diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index 05c93a1fe2..c00e50411d 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -167,7 +167,9 @@ struct qpzonedb { isc_rwlock_t lock; /* Locks for tree nodes */ int node_lock_count; - db_nodelock_t *node_locks; + isc_rwlock_t *node_locks; + isc_refcount_t references; + qpznode_t *origin; qpznode_t *nsec3_origin; isc_stats_t *gluecachestats; @@ -192,6 +194,18 @@ struct qpzonedb { dns_qpmulti_t *nsec3; /* NSEC3 nodes only */ }; +#ifdef DNS_DB_NODETRACE +#define qpzonedb_ref(ptr) qpzonedb__ref(ptr, __func__, __FILE__, __LINE__) +#define qpzonedb_unref(ptr) qpzonedb_unref(ptr, __func__, __FILE__, __LINE__) +#define qpzonedb_attach(ptr, ptrp) \ + qpzonedb__attach(ptr, ptrp, __func__, __FILE__, __LINE__) +#define qpzonedb_detach(ptrp) \ + qpzonedb__detach(ptrp, __func__, __FILE__, __LINE__) +ISC_REFCOUNT_STATIC_TRACE_DECL(qpzonedb); +#else +ISC_REFCOUNT_STATIC_DECL(qpzonedb); +#endif + /*% * Search Context */ @@ -447,8 +461,7 @@ free_db_rcu(struct rcu_head *rcu_head) { dns_name_free(&qpdb->common.origin, qpdb->common.mctx); } for (int i = 0; i < qpdb->node_lock_count; i++) { - isc_refcount_destroy(&qpdb->node_locks[i].references); - NODE_DESTROYLOCK(&qpdb->node_locks[i].lock); + NODE_DESTROYLOCK(&qpdb->node_locks[i]); } isc_heap_destroy(&qpdb->heap); @@ -458,13 +471,15 @@ free_db_rcu(struct rcu_head *rcu_head) { } isc_mem_cput(qpdb->common.mctx, qpdb->node_locks, qpdb->node_lock_count, - sizeof(db_nodelock_t)); - isc_refcount_destroy(&qpdb->common.references); + sizeof(qpdb->node_locks[0])); if (qpdb->loop != NULL) { isc_loop_detach(&qpdb->loop); } isc_rwlock_destroy(&qpdb->lock); + isc_refcount_destroy(&qpdb->references); + isc_refcount_destroy(&qpdb->common.references); + qpdb->common.magic = 0; qpdb->common.impmagic = 0; @@ -476,7 +491,7 @@ free_db_rcu(struct rcu_head *rcu_head) { } static void -free_qpdb(qpzonedb_t *qpdb, bool log) { +qpzone_destroy(qpzonedb_t *qpdb) { REQUIRE(qpdb->future_version == NULL); isc_refcount_decrementz(&qpdb->current_version->references); @@ -492,16 +507,14 @@ free_qpdb(qpzonedb_t *qpdb, bool log) { dns_qpmulti_destroy(&qpdb->nsec); dns_qpmulti_destroy(&qpdb->nsec3); - if (log) { - char buf[DNS_NAME_FORMATSIZE]; - if (dns_name_dynamic(&qpdb->common.origin)) { - dns_name_format(&qpdb->common.origin, buf, sizeof(buf)); - } else { - strlcpy(buf, "", sizeof(buf)); - } - isc_log_write(DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_DB, - ISC_LOG_DEBUG(1), "called free_qpdb(%s)", buf); + char buf[DNS_NAME_FORMATSIZE]; + if (dns_name_dynamic(&qpdb->common.origin)) { + dns_name_format(&qpdb->common.origin, buf, sizeof(buf)); + } else { + strlcpy(buf, "", sizeof(buf)); } + isc_log_write(DNS_LOGCATEGORY_DATABASE, DNS_LOGMODULE_DB, + ISC_LOG_DEBUG(1), "called %s(%s)", __func__, buf); call_rcu(&qpdb->rcu_head, free_db_rcu); } @@ -509,7 +522,6 @@ free_qpdb(qpzonedb_t *qpdb, bool log) { static void qpdb_destroy(dns_db_t *arg) { qpzonedb_t *qpdb = (qpzonedb_t *)arg; - unsigned int inactive = 0; if (qpdb->origin != NULL) { qpznode_detach(&qpdb->origin); @@ -527,45 +539,7 @@ qpdb_destroy(dns_db_t *arg) { cleanup_gluelists(&qpdb->current_version->glue_stack); } - /* - * Even though there are no external direct references, there still - * may be nodes in use. - */ - for (int i = 0; i < qpdb->node_lock_count; i++) { - isc_rwlocktype_t nodelock = isc_rwlocktype_none; - NODE_WRLOCK(&qpdb->node_locks[i].lock, &nodelock); - qpdb->node_locks[i].exiting = true; - if (isc_refcount_current(&qpdb->node_locks[i].references) == 0) - { - inactive++; - } - NODE_UNLOCK(&qpdb->node_locks[i].lock, &nodelock); - } - - if (inactive != 0) { - bool want_free = false; - - RWLOCK(&qpdb->lock, isc_rwlocktype_write); - qpdb->active -= inactive; - if (qpdb->active == 0) { - want_free = true; - } - RWUNLOCK(&qpdb->lock, isc_rwlocktype_write); - - if (want_free) { - char buf[DNS_NAME_FORMATSIZE]; - if (dns_name_dynamic(&qpdb->common.origin)) { - dns_name_format(&qpdb->common.origin, buf, - sizeof(buf)); - } else { - strlcpy(buf, "", sizeof(buf)); - } - isc_log_write(DNS_LOGCATEGORY_DATABASE, - DNS_LOGMODULE_DB, ISC_LOG_DEBUG(1), - "calling free_qpdb(%s)", buf); - free_qpdb(qpdb, true); - } - } + qpzonedb_detach(&qpdb); } static qpznode_t * @@ -596,11 +570,11 @@ allocate_version(isc_mem_t *mctx, uint32_t serial, unsigned int references, .changed_list = ISC_LIST_INITIALIZER, .resigned_list = ISC_LIST_INITIALIZER, .link = ISC_LINK_INITIALIZER, + .references = ISC_REFCOUNT_INITIALIZER(references), }; cds_wfs_init(&version->glue_stack); isc_rwlock_init(&version->rwlock); - isc_refcount_init(&version->references, references); return version; } @@ -618,15 +592,15 @@ dns__qpzone_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type, *qpdb = (qpzonedb_t){ .common.origin = DNS_NAME_INITEMPTY, .common.rdclass = rdclass, + .common.references = ISC_REFCOUNT_INITIALIZER(1), .node_lock_count = DEFAULT_NODE_LOCK_COUNT, .current_serial = 1, .least_serial = 1, .next_serial = 2, .open_versions = ISC_LIST_INITIALIZER, + .references = ISC_REFCOUNT_INITIALIZER(1), }; - isc_refcount_init(&qpdb->common.references, 1); - qpdb->common.methods = &qpdb_zonemethods; if (type == dns_dbtype_stub) { qpdb->common.attributes |= DNS_DBATTR_STUB; @@ -635,7 +609,7 @@ dns__qpzone_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type, isc_rwlock_init(&qpdb->lock); qpdb->node_locks = isc_mem_cget(mctx, qpdb->node_lock_count, - sizeof(db_nodelock_t)); + sizeof(qpdb->node_locks[0])); qpdb->common.update_listeners = cds_lfht_new(16, 16, 0, 0, NULL); @@ -644,9 +618,7 @@ dns__qpzone_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type, qpdb->active = qpdb->node_lock_count; for (int i = 0; i < qpdb->node_lock_count; i++) { - NODE_INITLOCK(&qpdb->node_locks[i].lock); - isc_refcount_init(&qpdb->node_locks[i].references, 0); - qpdb->node_locks[i].exiting = false; + NODE_INITLOCK(&qpdb->node_locks[i]); } /* @@ -691,7 +663,7 @@ dns__qpzone_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type, if (result != ISC_R_SUCCESS) { INSIST(result != ISC_R_EXISTS); - free_qpdb(qpdb, false); + qpzonedb_detach(&qpdb); return result; } @@ -708,7 +680,7 @@ dns__qpzone_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type, if (result != ISC_R_SUCCESS) { INSIST(result != ISC_R_EXISTS); - free_qpdb(qpdb, false); + qpzonedb_detach(&qpdb); return result; } @@ -738,18 +710,7 @@ qpznode_newref(qpzonedb_t *qpdb, qpznode_t *node DNS__DB_FLARG) { return; } - /* this is the first reference to the node */ - refs = isc_refcount_increment0( - &qpdb->node_locks[node->locknum].references); -#if DNS_DB_NODETRACE - fprintf(stderr, - "incr:nodelock:%s:%s:%u:%p:%p->references = " - "%" PRIuFAST32 "\n", - func, file, line, node, &qpdb->node_locks[node->locknum], - refs + 1); -#else - UNUSED(refs); -#endif + qpzonedb_ref(qpdb); } static void @@ -887,17 +848,7 @@ qpznode_decref(qpzonedb_t *qpdb, qpznode_t *node DNS__DB_FLARG) { return false; } - refs = isc_refcount_decrement( - &qpdb->node_locks[node->locknum].references); -#if DNS_DB_NODETRACE - fprintf(stderr, - "decr:nodelock:%s:%s:%u:%p:%p->references = " - "%" PRIuFAST32 "\n", - func, file, line, node, &qpdb->node_locks[node->locknum], - refs - 1); -#else - UNUSED(refs); -#endif + qpzonedb_unref(qpdb); return true; } @@ -942,8 +893,8 @@ decref(qpzonedb_t *qpdb, qpznode_t *node, uint32_t least_serial, /* Upgrade the lock? */ if (*nlocktypep == isc_rwlocktype_read) { - NODE_FORCEUPGRADE(&qpdb->node_locks[node->locknum].lock, - nlocktypep); + isc_rwlock_t *nlock = &qpdb->node_locks[node->locknum]; + NODE_FORCEUPGRADE(nlock, nlocktypep); } if (!qpznode_decref(qpdb, node DNS__DB_FLARG_PASS)) { @@ -1032,10 +983,12 @@ setnsec3parameters(dns_db_t *db, qpz_version_t *version) { unsigned int count, length; qpzonedb_t *qpdb = (qpzonedb_t *)db; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = NULL; version->havensec3 = false; node = qpdb->origin; - NODE_RDLOCK(&(qpdb->node_locks[node->locknum].lock), &nlocktype); + nlock = &qpdb->node_locks[node->locknum]; + NODE_RDLOCK(nlock, &nlocktype); for (header = node->data; header != NULL; header = header_next) { header_next = header->next; do { @@ -1102,7 +1055,7 @@ setnsec3parameters(dns_db_t *db, qpz_version_t *version) { } } unlock: - NODE_UNLOCK(&(qpdb->node_locks[node->locknum].lock), &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); } static void @@ -1504,19 +1457,19 @@ closeversion(dns_db_t *db, dns_dbversion_t **versionp, for (header = HEAD(resigned_list); header != NULL; header = HEAD(resigned_list)) { - isc_rwlock_t *lock = NULL; + isc_rwlock_t *nlock = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; ISC_LIST_UNLINK(resigned_list, header, link); - lock = &qpdb->node_locks[HEADERNODE(header)->locknum].lock; - NODE_WRLOCK(lock, &nlocktype); + nlock = &qpdb->node_locks[HEADERNODE(header)->locknum]; + NODE_WRLOCK(nlock, &nlocktype); if (rollback && !IGNORE(header)) { resigninsert(qpdb, header); } decref(qpdb, HEADERNODE(header), least_serial, &nlocktype DNS__DB_FLARG_PASS); - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); } if (EMPTY(cleanup_list)) { @@ -1527,20 +1480,20 @@ closeversion(dns_db_t *db, dns_dbversion_t **versionp, for (changed = HEAD(cleanup_list); changed != NULL; changed = next_changed) { - isc_rwlock_t *lock = NULL; + isc_rwlock_t *nlock = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; next_changed = NEXT(changed, link); node = changed->node; - lock = &qpdb->node_locks[node->locknum].lock; + nlock = &qpdb->node_locks[node->locknum]; - NODE_WRLOCK(lock, &nlocktype); + NODE_WRLOCK(nlock, &nlocktype); if (rollback) { rollback_node(node, serial); } decref(qpdb, node, least_serial, &nlocktype DNS__DB_FILELINE); - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); isc_mem_put(qpdb->common.mctx, changed, sizeof(*changed)); } @@ -1563,6 +1516,7 @@ qpzone_findrdataset(dns_db_t *db, dns_dbnode_t *dbnode, bool close_version = false; dns_typepair_t matchtype, sigmatchtype; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = NULL; REQUIRE(VALID_QPZONE(qpdb)); REQUIRE(type != dns_rdatatype_any); @@ -1574,7 +1528,8 @@ qpzone_findrdataset(dns_db_t *db, dns_dbnode_t *dbnode, } serial = version->serial; - NODE_RDLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + nlock = &qpdb->node_locks[node->locknum]; + NODE_RDLOCK(nlock, &nlocktype); matchtype = DNS_TYPEPAIR_VALUE(type, covers); if (covers == 0) { @@ -1621,7 +1576,7 @@ qpzone_findrdataset(dns_db_t *db, dns_dbnode_t *dbnode, } } - NODE_UNLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); if (close_version) { closeversion(db, (dns_dbversion_t **)&version, @@ -2120,6 +2075,7 @@ loading_addrdataset(void *arg, const dns_name_t *name, isc_region_t region; dns_slabheader_t *newheader = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = NULL; REQUIRE(rdataset->rdclass == qpdb->common.rdclass); @@ -2190,10 +2146,11 @@ loading_addrdataset(void *arg, const dns_name_t *name, newheader->resign_lsb = rdataset->resign & 0x1; } - NODE_WRLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + nlock = &qpdb->node_locks[node->locknum]; + NODE_WRLOCK(nlock, &nlocktype); result = add(qpdb, node, name, qpdb->current_version, newheader, DNS_DBADD_MERGE, true, NULL, 0 DNS__DB_FLARG_PASS); - NODE_UNLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); if (result == ISC_R_SUCCESS && delegating_type(qpdb, node, rdataset->type)) @@ -2390,6 +2347,7 @@ setsigningtime(dns_db_t *db, dns_rdataset_t *rdataset, isc_stdtime_t resign) { qpzonedb_t *qpdb = (qpzonedb_t *)db; dns_slabheader_t *header = NULL, oldheader; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = NULL; REQUIRE(VALID_QPZONE(qpdb)); REQUIRE(rdataset != NULL); @@ -2397,8 +2355,8 @@ setsigningtime(dns_db_t *db, dns_rdataset_t *rdataset, isc_stdtime_t resign) { header = dns_slabheader_fromrdataset(rdataset); - NODE_WRLOCK(&qpdb->node_locks[HEADERNODE(header)->locknum].lock, - &nlocktype); + nlock = &qpdb->node_locks[HEADERNODE(header)->locknum]; + NODE_WRLOCK(nlock, &nlocktype); oldheader = *header; @@ -2429,8 +2387,7 @@ setsigningtime(dns_db_t *db, dns_rdataset_t *rdataset, isc_stdtime_t resign) { DNS_SLABHEADER_SETATTR(header, DNS_SLABHEADERATTR_RESIGN); resigninsert(qpdb, header); } - NODE_UNLOCK(&qpdb->node_locks[HEADERNODE(header)->locknum].lock, - &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); return ISC_R_SUCCESS; } @@ -2440,6 +2397,7 @@ getsigningtime(dns_db_t *db, isc_stdtime_t *resign, dns_name_t *foundname, qpzonedb_t *qpdb = (qpzonedb_t *)db; dns_slabheader_t *header = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = NULL; uint16_t locknum; isc_result_t result = ISC_R_NOTFOUND; @@ -2458,13 +2416,15 @@ getsigningtime(dns_db_t *db, isc_stdtime_t *resign, dns_name_t *foundname, RWUNLOCK(&qpdb->lock, isc_rwlocktype_read); again: - NODE_RDLOCK(&qpdb->node_locks[locknum].lock, &nlocktype); + nlock = &qpdb->node_locks[locknum]; + + NODE_RDLOCK(nlock, &nlocktype); RWLOCK(&qpdb->lock, isc_rwlocktype_read); header = isc_heap_element(qpdb->heap, 1); if (header != NULL && HEADERNODE(header)->locknum != locknum) { RWUNLOCK(&qpdb->lock, isc_rwlocktype_read); - NODE_UNLOCK(&qpdb->node_locks[locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); locknum = HEADERNODE(header)->locknum; goto again; } @@ -2478,7 +2438,7 @@ again: result = ISC_R_SUCCESS; } RWUNLOCK(&qpdb->lock, isc_rwlocktype_read); - NODE_UNLOCK(&qpdb->node_locks[locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); return result; } @@ -2651,8 +2611,8 @@ qpzone_setup_delegation(qpz_search_t *search, dns_dbnode_t **nodep, } if (rdataset != NULL) { isc_rwlocktype_t nlocktype = isc_rwlocktype_none; - NODE_RDLOCK(&(search->qpdb->node_locks[node->locknum].lock), - &nlocktype); + isc_rwlock_t *nlock = &search->qpdb->node_locks[node->locknum]; + NODE_RDLOCK(nlock, &nlocktype); bindrdataset(search->qpdb, node, search->zonecut_header, search->now, rdataset DNS__DB_FLARG_PASS); if (sigrdataset != NULL && search->zonecut_sigheader != NULL) { @@ -2660,8 +2620,7 @@ qpzone_setup_delegation(qpz_search_t *search, dns_dbnode_t **nodep, search->zonecut_sigheader, search->now, sigrdataset DNS__DB_FLARG_PASS); } - NODE_UNLOCK(&(search->qpdb->node_locks[node->locknum].lock), - &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); } if (type == dns_rdatatype_dname) { @@ -2692,10 +2651,10 @@ step(qpz_search_t *search, dns_qpiter_t *it, direction_t direction, result = dns_qpiter_current(it, nodename, (void **)&node, NULL); while (result == ISC_R_SUCCESS) { - isc_rwlock_t *nodelock = &qpdb->node_locks[node->locknum].lock; + isc_rwlock_t *nlock = &qpdb->node_locks[node->locknum]; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; - NODE_RDLOCK(nodelock, &nlocktype); + NODE_RDLOCK(nlock, &nlocktype); for (header = node->data; header != NULL; header = header->next) { if (header->serial <= search->serial && @@ -2704,7 +2663,7 @@ step(qpz_search_t *search, dns_qpiter_t *it, direction_t direction, break; } } - NODE_UNLOCK(nodelock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); if (header != NULL) { break; } @@ -2839,14 +2798,14 @@ find_wildcard(qpz_search_t *search, qpznode_t **nodep, */ for (int i = dns_qpchain_length(&search->chain) - 1; i >= 0; i--) { qpznode_t *node = NULL; - isc_rwlock_t *lock = NULL; + isc_rwlock_t *nlock = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; bool wild, active; dns_qpchain_node(&search->chain, i, NULL, (void **)&node, NULL); - lock = &qpdb->node_locks[node->locknum].lock; - NODE_RDLOCK(lock, &nlocktype); + nlock = &qpdb->node_locks[node->locknum]; + NODE_RDLOCK(nlock, &nlocktype); /* * First we try to figure out if this node is active in * the search's version. We do this now, even though we @@ -2864,7 +2823,7 @@ find_wildcard(qpz_search_t *search, qpznode_t **nodep, active = (header != NULL); wild = node->wild; - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); if (wild) { qpznode_t *wnode = NULL; @@ -2889,8 +2848,8 @@ find_wildcard(qpz_search_t *search, qpznode_t **nodep, * is active in the search's version, we're * done. */ - lock = &qpdb->node_locks[wnode->locknum].lock; - NODE_RDLOCK(lock, &nlocktype); + nlock = &qpdb->node_locks[wnode->locknum]; + NODE_RDLOCK(nlock, &nlocktype); for (header = wnode->data; header != NULL; header = header->next) { @@ -2901,7 +2860,7 @@ find_wildcard(qpz_search_t *search, qpznode_t **nodep, break; } } - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); if (header != NULL || activeempty(search, &wit, wname)) { @@ -3072,8 +3031,8 @@ again: do { dns_slabheader_t *found = NULL, *foundsig = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; - NODE_RDLOCK(&(search->qpdb->node_locks[node->locknum].lock), - &nlocktype); + isc_rwlock_t *nlock = &search->qpdb->node_locks[node->locknum]; + NODE_RDLOCK(nlock, &nlocktype); empty_node = true; for (header = node->data; header != NULL; header = header_next) { @@ -3179,8 +3138,7 @@ again: &prevnode, &nseciter, &first); } - NODE_UNLOCK(&(search->qpdb->node_locks[node->locknum].lock), - &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); node = prevnode; prevnode = NULL; } while (empty_node && result == ISC_R_SUCCESS); @@ -3215,9 +3173,9 @@ qpzone_check_zonecut(qpznode_t *node, void *arg DNS__DB_FLARG) { dns_slabheader_t *found = NULL; isc_result_t result = DNS_R_CONTINUE; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = &search->qpdb->node_locks[node->locknum]; - NODE_RDLOCK(&(search->qpdb->node_locks[node->locknum].lock), - &nlocktype); + NODE_RDLOCK(nlock, &nlocktype); /* * Look for an NS or DNAME rdataset active in our version. @@ -3327,8 +3285,7 @@ qpzone_check_zonecut(qpznode_t *node, void *arg DNS__DB_FLARG) { } } - NODE_UNLOCK(&(search->qpdb->node_locks[node->locknum].lock), - &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); return result; } @@ -3352,7 +3309,7 @@ qpzone_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, dns_slabheader_t *foundsig = NULL, *cnamesig = NULL, *nsecsig = NULL; dns_typepair_t sigtype; bool active; - isc_rwlock_t *lock = NULL; + isc_rwlock_t *nlock = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; REQUIRE(VALID_QPZONE((qpzonedb_t *)db)); @@ -3482,8 +3439,8 @@ found: * have matched a wildcard. */ - lock = &search.qpdb->node_locks[node->locknum].lock; - NODE_RDLOCK(lock, &nlocktype); + nlock = &search.qpdb->node_locks[node->locknum]; + NODE_RDLOCK(nlock, &nlocktype); if (search.zonecut != NULL) { /* @@ -3596,7 +3553,7 @@ found: if (header->type == dns_rdatatype_nsec3 && !matchparams(header, &search)) { - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); goto partial_match; } /* @@ -3689,7 +3646,7 @@ found: if (!wild) { unsigned int len = search.chain.len - 1; if (len > 0) { - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); dns_qpchain_node(&search.chain, len - 1, NULL, (void **)&node, NULL); dns_name_copy(&node->name, foundname); @@ -3709,7 +3666,7 @@ found: * * Return the delegation. */ - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); result = qpzone_setup_delegation( &search, nodep, foundname, rdataset, sigrdataset DNS__DB_FLARG_PASS); @@ -3731,7 +3688,7 @@ found: goto node_exit; } - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); result = find_closest_nsec( &search, nodep, foundname, rdataset, sigrdataset, false, @@ -3827,7 +3784,7 @@ found: } node_exit: - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); tree_exit: if (nsec3) { @@ -3843,11 +3800,11 @@ tree_exit: if (search.need_cleanup) { node = search.zonecut; INSIST(node != NULL); - lock = &(search.qpdb->node_locks[node->locknum].lock); + nlock = &search.qpdb->node_locks[node->locknum]; - NODE_RDLOCK(lock, &nlocktype); + NODE_RDLOCK(nlock, &nlocktype); decref(search.qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS); - NODE_UNLOCK(lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); } if (close_version) { @@ -3907,52 +3864,26 @@ attachnode(dns_db_t *db, dns_dbnode_t *source, } static void -detachnode(dns_db_t *db, dns_dbnode_t **targetp DNS__DB_FLARG) { +detachnode(dns_db_t *db, dns_dbnode_t **nodep DNS__DB_FLARG) { qpzonedb_t *qpdb = (qpzonedb_t *)db; qpznode_t *node = NULL; - bool want_free = false; - bool inactive = false; - db_nodelock_t *nodelock = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = NULL; REQUIRE(VALID_QPZONE(qpdb)); - REQUIRE(targetp != NULL && *targetp != NULL); + REQUIRE(nodep != NULL && *nodep != NULL); - node = (qpznode_t *)(*targetp); - nodelock = &qpdb->node_locks[node->locknum]; + node = (qpznode_t *)(*nodep); + *nodep = NULL; + nlock = &qpdb->node_locks[node->locknum]; - NODE_RDLOCK(&nodelock->lock, &nlocktype); + qpzonedb_ref(qpdb); + + NODE_RDLOCK(nlock, &nlocktype); decref(qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS); - if (isc_refcount_current(&nodelock->references) == 0 && - nodelock->exiting) - { - inactive = true; - } - NODE_UNLOCK(&nodelock->lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); - *targetp = NULL; - - if (inactive) { - RWLOCK(&qpdb->lock, isc_rwlocktype_write); - qpdb->active--; - if (qpdb->active == 0) { - want_free = true; - } - RWUNLOCK(&qpdb->lock, isc_rwlocktype_write); - if (want_free) { - char buf[DNS_NAME_FORMATSIZE]; - if (dns_name_dynamic(&qpdb->common.origin)) { - dns_name_format(&qpdb->common.origin, buf, - sizeof(buf)); - } else { - strlcpy(buf, "", sizeof(buf)); - } - isc_log_write(DNS_LOGCATEGORY_DATABASE, - DNS_LOGMODULE_DB, ISC_LOG_DEBUG(1), - "calling free_qpdb(%s)", buf); - free_qpdb(qpdb, true); - } - } + qpzonedb_detach(&qpdb); } static unsigned int @@ -4019,7 +3950,7 @@ locknode(dns_db_t *db, dns_dbnode_t *dbnode, isc_rwlocktype_t type) { qpzonedb_t *qpdb = (qpzonedb_t *)db; qpznode_t *node = (qpznode_t *)dbnode; - RWLOCK(&qpdb->node_locks[node->locknum].lock, type); + RWLOCK(&qpdb->node_locks[node->locknum], type); } static void @@ -4027,7 +3958,7 @@ unlocknode(dns_db_t *db, dns_dbnode_t *dbnode, isc_rwlocktype_t type) { qpzonedb_t *qpdb = (qpzonedb_t *)db; qpznode_t *node = (qpznode_t *)dbnode; - RWUNLOCK(&qpdb->node_locks[node->locknum].lock, type); + RWUNLOCK(&qpdb->node_locks[node->locknum], type); } static void @@ -4073,8 +4004,9 @@ rdatasetiter_first(dns_rdatasetiter_t *iterator DNS__DB_FLARG) { qpz_version_t *version = (qpz_version_t *)qrditer->common.version; dns_slabheader_t *header = NULL, *top_next = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = &qpdb->node_locks[node->locknum]; - NODE_RDLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + NODE_RDLOCK(nlock, &nlocktype); for (header = node->data; header != NULL; header = top_next) { top_next = header->next; @@ -4095,7 +4027,7 @@ rdatasetiter_first(dns_rdatasetiter_t *iterator DNS__DB_FLARG) { } } - NODE_UNLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); qrditer->current = header; @@ -4116,13 +4048,14 @@ rdatasetiter_next(dns_rdatasetiter_t *iterator DNS__DB_FLARG) { dns_typepair_t type, negtype; dns_rdatatype_t rdtype; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = &qpdb->node_locks[node->locknum]; header = qrditer->current; if (header == NULL) { return ISC_R_NOMORE; } - NODE_RDLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + NODE_RDLOCK(nlock, &nlocktype); type = header->type; rdtype = DNS_TYPEPAIR_TYPE(header->type); @@ -4166,7 +4099,7 @@ rdatasetiter_next(dns_rdatasetiter_t *iterator DNS__DB_FLARG) { } } - NODE_UNLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); qrditer->current = header; @@ -4185,16 +4118,17 @@ rdatasetiter_current(dns_rdatasetiter_t *iterator, qpznode_t *node = (qpznode_t *)qrditer->common.node; dns_slabheader_t *header = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = &qpdb->node_locks[node->locknum]; header = qrditer->current; REQUIRE(header != NULL); - NODE_RDLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + NODE_RDLOCK(nlock, &nlocktype); bindrdataset(qpdb, node, header, qrditer->common.now, rdataset DNS__DB_FLARG_PASS); - NODE_UNLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); } /* @@ -4216,18 +4150,19 @@ static void dereference_iter_node(qpdb_dbiterator_t *iter DNS__DB_FLARG) { qpzonedb_t *qpdb = (qpzonedb_t *)iter->common.db; qpznode_t *node = iter->node; - isc_rwlock_t *lock = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = NULL; if (node == NULL) { return; } - lock = &qpdb->node_locks[node->locknum].lock; - NODE_RDLOCK(lock, &nlocktype); - decref(qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS); - NODE_UNLOCK(lock, &nlocktype); iter->node = NULL; + nlock = &qpdb->node_locks[node->locknum]; + + NODE_RDLOCK(nlock, &nlocktype); + decref(qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS); + NODE_UNLOCK(nlock, &nlocktype); } static void @@ -4646,6 +4581,7 @@ qpzone_addrdataset(dns_db_t *db, dns_dbnode_t *dbnode, isc_region_t region; dns_slabheader_t *newheader = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = NULL; dns_fixedname_t fn; dns_name_t *name = dns_fixedname_initname(&fn); dns_qp_t *nsec = NULL; @@ -4724,7 +4660,9 @@ qpzone_addrdataset(dns_db_t *db, dns_dbnode_t *dbnode, * (Note: node lock must be acquired after starting * the QPDB transaction and released before committing.) */ - NODE_WRLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + nlock = &qpdb->node_locks[node->locknum]; + + NODE_WRLOCK(nlock, &nlocktype); result = ISC_R_SUCCESS; if (nsec != NULL) { @@ -4755,7 +4693,7 @@ qpzone_addrdataset(dns_db_t *db, dns_dbnode_t *dbnode, node->delegating = true; } - NODE_UNLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); if (nsec != NULL) { dns_qpmulti_commit(qpdb->nsec, &nsec); @@ -4781,6 +4719,7 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode, isc_result_t result; qpz_changed_t *changed = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock; REQUIRE(VALID_QPZONE(qpdb)); REQUIRE(version != NULL && version->qpdb == qpdb); @@ -4825,7 +4764,8 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode, newheader->resign_lsb = 0; } - NODE_WRLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + nlock = &qpdb->node_locks[node->locknum]; + NODE_WRLOCK(nlock, &nlocktype); changed = add_changed(newheader, version DNS__DB_FLARG_PASS); for (topheader = node->data; topheader != NULL; @@ -4952,7 +4892,7 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode, } unlock: - NODE_UNLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); return result; } @@ -4968,6 +4908,7 @@ qpzone_deleterdataset(dns_db_t *db, dns_dbnode_t *dbnode, isc_result_t result; dns_slabheader_t *newheader = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = NULL; REQUIRE(VALID_QPZONE(qpdb)); REQUIRE(version != NULL && version->qpdb == qpdb); @@ -4987,10 +4928,11 @@ qpzone_deleterdataset(dns_db_t *db, dns_dbnode_t *dbnode, dns_name_copy(&node->name, nodename); - NODE_WRLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + nlock = &qpdb->node_locks[node->locknum]; + NODE_WRLOCK(nlock, &nlocktype); result = add(qpdb, node, nodename, version, newheader, DNS_DBADD_FORCE, false, NULL, 0 DNS__DB_FLARG_PASS); - NODE_UNLOCK(&qpdb->node_locks[node->locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); return result; } @@ -4999,14 +4941,18 @@ nodefullname(dns_db_t *db, dns_dbnode_t *node, dns_name_t *name) { qpzonedb_t *qpdb = (qpzonedb_t *)db; qpznode_t *qpnode = (qpznode_t *)node; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; + isc_rwlock_t *nlock = NULL; REQUIRE(VALID_QPZONE(qpdb)); REQUIRE(node != NULL); REQUIRE(name != NULL); - NODE_RDLOCK(&qpdb->node_locks[qpnode->locknum].lock, &nlocktype); + nlock = &qpdb->node_locks[qpnode->locknum]; + + NODE_RDLOCK(nlock, &nlocktype); dns_name_copy(&qpnode->name, name); - NODE_UNLOCK(&qpdb->node_locks[qpnode->locknum].lock, &nlocktype); + NODE_UNLOCK(nlock, &nlocktype); + return ISC_R_SUCCESS; } @@ -5415,6 +5361,12 @@ ISC_REFCOUNT_STATIC_TRACE_IMPL(qpznode, destroy_qpznode); ISC_REFCOUNT_STATIC_IMPL(qpznode, destroy_qpznode); #endif +#ifdef DNS_DB_NODETRACE +ISC_REFCOUNT_STATIC_TRACE_IMPL(qpzonedb, qpzone_destroy); +#else +ISC_REFCOUNT_STATIC_IMPL(qpzonedb, qpzone_destroy); +#endif + static void qp_attach(void *uctx ISC_ATTR_UNUSED, void *pval, uint32_t ival ISC_ATTR_UNUSED) { diff --git a/tests/dns/qpzone_test.c b/tests/dns/qpzone_test.c index 2b0537022c..1186217fa2 100644 --- a/tests/dns/qpzone_test.c +++ b/tests/dns/qpzone_test.c @@ -101,7 +101,7 @@ const char *ownercase_vectors[12][2] = { static bool ownercase_test_one(const char *str1, const char *str2) { isc_result_t result; - db_nodelock_t node_locks[1]; + isc_rwlock_t node_locks[1]; qpzonedb_t qpdb = { .common.methods = &qpdb_zonemethods, .common.mctx = mctx, @@ -128,7 +128,7 @@ ownercase_test_one(const char *str1, const char *str2) { memset(node_locks, 0, sizeof(node_locks)); /* Minimal initialization of the mock objects */ - NODE_INITLOCK(&qpdb.node_locks[0].lock); + NODE_INITLOCK(&qpdb.node_locks[0]); isc_buffer_constinit(&b, str1, strlen(str1)); isc_buffer_add(&b, strlen(str1)); @@ -148,7 +148,7 @@ ownercase_test_one(const char *str1, const char *str2) { /* Retrieve the case to name2 */ dns_rdataset_getownercase(&rdataset, name2); - NODE_DESTROYLOCK(&qpdb.node_locks[0].lock); + NODE_DESTROYLOCK(&qpdb.node_locks[0]); return dns_name_caseequal(name1, name2); } @@ -169,7 +169,7 @@ ISC_RUN_TEST_IMPL(ownercase) { ISC_RUN_TEST_IMPL(setownercase) { isc_result_t result; - db_nodelock_t node_locks[1]; + isc_rwlock_t node_locks[1]; qpzonedb_t qpdb = { .common.methods = &qpdb_zonemethods, .common.mctx = mctx, @@ -200,7 +200,7 @@ ISC_RUN_TEST_IMPL(setownercase) { /* Minimal initialization of the mock objects */ memset(node_locks, 0, sizeof(node_locks)); - NODE_INITLOCK(&qpdb.node_locks[0].lock); + NODE_INITLOCK(&qpdb.node_locks[0]); isc_buffer_constinit(&b, str1, strlen(str1)); isc_buffer_add(&b, strlen(str1)); @@ -217,7 +217,7 @@ ISC_RUN_TEST_IMPL(setownercase) { /* Retrieve the case to name2 */ dns_rdataset_getownercase(&rdataset, name2); - NODE_DESTROYLOCK(&qpdb.node_locks[0].lock); + NODE_DESTROYLOCK(&qpdb.node_locks[0]); assert_true(dns_name_caseequal(name1, name2)); }