From 814b87da648b2471947e4f9ac25069ab83f38ea3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 27 Jan 2025 18:06:17 +0100 Subject: [PATCH 1/4] Refactor decref() in both qpcache.c and qpzone.c Cleanup the pattern in the decref() functions in both qpcache.c and qpzone.c, so it follows the similar patter as we already have in newref() function. --- lib/dns/qpcache.c | 147 ++++++++++++++++++++++------------------------ lib/dns/qpzone.c | 106 ++++++++++++++++----------------- 2 files changed, 119 insertions(+), 134 deletions(-) diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index fd674facf6..aa645fbe44 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -625,30 +625,32 @@ qpcnode_newref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t nlocktype, func, file, line, node, refs + 1); #endif - if (refs == 0) { - /* - * this is the first external reference to the node. - * - * we need to hold the node or tree lock to avoid - * incrementing the reference count while also deleting - * the node. delete_node() is always protected by both - * tree and node locks being write-locked. - */ - 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 + if (refs > 0) { + return; } + + /* + * this is the first external reference to the node. + * + * we need to hold the node or tree lock to avoid + * incrementing the reference count while also deleting + * the node. delete_node() is always protected by both + * tree and node locks being write-locked. + */ + 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 } static void @@ -661,6 +663,33 @@ newref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t nlocktype, static void cleanup_deadnodes(void *arg); +static bool +qpcnode_decref(qpcache_t *qpdb, qpcnode_t *node DNS__DB_FLARG) { + uint_fast32_t refs = isc_refcount_decrement(&node->erefs); + +#if DNS_DB_NODETRACE + fprintf(stderr, "decr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n", + func, file, line, node, refs - 1); +#endif + if (refs > 1) { + 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 + + return true; +} + /* * Caller must be holding the node lock; either the read or write lock. * Note that the lock must be held even when node references are @@ -677,42 +706,22 @@ cleanup_deadnodes(void *arg); * to zero. (NOTE: Decrementing the reference count of a node to zero does * not mean it will be immediately freed.) */ -static bool +static void decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep, isc_rwlocktype_t *tlocktypep, bool tryupgrade DNS__DB_FLARG) { isc_result_t result; bool locked = *tlocktypep != isc_rwlocktype_none; bool write_locked = false; - int bucket = node->locknum; - db_nodelock_t *nodelock = &qpdb->node_locks[bucket]; - uint_fast32_t refs; REQUIRE(*nlocktypep != isc_rwlocktype_none); - refs = isc_refcount_decrement(&node->erefs); -#if DNS_DB_NODETRACE - fprintf(stderr, "decr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n", - func, file, line, node, refs - 1); -#endif - if (refs > 1) { - qpcnode_unref(node); - return false; + if (!qpcnode_decref(qpdb, node DNS__DB_FLARG_PASS)) { + goto unref; } - refs = isc_refcount_decrement(&nodelock->references); -#if DNS_DB_NODETRACE - fprintf(stderr, - "decr:nodelock:%s:%s:%u:%p:%p->references = " - "%" PRIuFAST32 "\n", - func, file, line, node, nodelock, refs - 1); -#else - UNUSED(refs); -#endif - /* Handle easy and typical case first. */ if (!node->dirty && (node->data != NULL || node == qpdb->origin_node)) { - qpcnode_unref(node); - return false; + goto unref; } /* @@ -724,18 +733,12 @@ decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep, /* Upgrade the lock? */ if (*nlocktypep == isc_rwlocktype_read) { - NODE_FORCEUPGRADE(&nodelock->lock, nlocktypep); + NODE_FORCEUPGRADE(&qpdb->node_locks[node->locknum].lock, + nlocktypep); } - refs = isc_refcount_decrement(&node->erefs); -#if DNS_DB_NODETRACE - fprintf(stderr, "decr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n", - func, file, line, node, refs - 1); -#endif - - if (refs > 1) { - qpcnode_unref(node); - return false; + if (!qpcnode_decref(qpdb, node DNS__DB_FLARG_PASS)) { + goto unref; } if (node->dirty) { @@ -775,21 +778,10 @@ decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep, write_locked = true; } - refs = isc_refcount_decrement(&nodelock->references); -#if DNS_DB_NODETRACE - fprintf(stderr, - "decr:nodelock:%s:%s:%u:%p:%p->references = %" PRIuFAST32 "\n", - func, file, line, node, nodelock, refs - 1); -#else - UNUSED(refs); -#endif - if (node->data != NULL || node == qpdb->origin_node) { goto restore_locks; } -#undef KEEP_NODE - if (write_locked) { /* * We can now delete the node. @@ -799,11 +791,12 @@ decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep, newref(qpdb, node, *nlocktypep, *tlocktypep DNS__DB_FLARG_PASS); isc_queue_node_init(&node->deadlink); - if (!isc_queue_enqueue_entry(&qpdb->deadnodes[bucket], node, - deadlink)) + if (!isc_queue_enqueue_entry(&qpdb->deadnodes[node->locknum], + node, deadlink)) { /* Queue was empty, trigger new cleaning */ - isc_loop_t *loop = isc_loop_get(qpdb->loopmgr, bucket); + isc_loop_t *loop = isc_loop_get(qpdb->loopmgr, + node->locknum); isc_async_run(loop, cleanup_deadnodes, qpdb); } @@ -817,8 +810,8 @@ restore_locks: TREE_UNLOCK(&qpdb->tree_lock, tlocktypep); } +unref: qpcnode_unref(node); - return true; } static void @@ -2752,13 +2745,11 @@ qpcache_detachnode(dns_db_t *db, dns_dbnode_t **targetp DNS__DB_FLARG) { NODE_RDLOCK(&nodelock->lock, &nlocktype); - if (decref(qpdb, node, &nlocktype, &tlocktype, true DNS__DB_FLARG_PASS)) + decref(qpdb, node, &nlocktype, &tlocktype, true DNS__DB_FLARG_PASS); + if (isc_refcount_current(&nodelock->references) == 0 && + nodelock->exiting) { - if (isc_refcount_current(&nodelock->references) == 0 && - nodelock->exiting) - { - inactive = true; - } + inactive = true; } NODE_UNLOCK(&nodelock->lock, &nlocktype); diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index 2b75a24a30..05c93a1fe2 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -734,20 +734,22 @@ qpznode_newref(qpzonedb_t *qpdb, qpznode_t *node DNS__DB_FLARG) { func, file, line, node, refs + 1); #endif - if (refs == 0) { - /* 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 + if (refs > 0) { + 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 } static void @@ -873,6 +875,33 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) { } } +static bool +qpznode_decref(qpzonedb_t *qpdb, qpznode_t *node DNS__DB_FLARG) { + uint_fast32_t refs = isc_refcount_decrement(&node->erefs); + +#if DNS_DB_NODETRACE + fprintf(stderr, "decr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n", + func, file, line, node, refs - 1); +#endif + if (refs > 1) { + 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 + + return true; +} + /* * Caller must be holding the node lock; either the read or write lock. * Note that the lock must be held even when node references are @@ -891,38 +920,17 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) { static void decref(qpzonedb_t *qpdb, qpznode_t *node, uint32_t least_serial, isc_rwlocktype_t *nlocktypep DNS__DB_FLARG) { - int bucket = node->locknum; - db_nodelock_t *nodelock = &qpdb->node_locks[bucket]; - uint_fast32_t refs; - REQUIRE(*nlocktypep != isc_rwlocktype_none); - refs = isc_refcount_decrement(&node->erefs); -#if DNS_DB_NODETRACE - fprintf(stderr, "decr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n", - func, file, line, node, refs - 1); -#endif - if (refs > 1) { - qpznode_unref(node); - return; + if (!qpznode_decref(qpdb, node DNS__DB_FLARG_PASS)) { + goto unref; } - refs = isc_refcount_decrement(&nodelock->references); -#if DNS_DB_NODETRACE - fprintf(stderr, - "decr:nodelock:%s:%s:%u:%p:%p->references = " - "%" PRIuFAST32 "\n", - func, file, line, node, nodelock, refs - 1); -#else - UNUSED(refs); -#endif - /* Handle easy and typical case first. */ if (!node->dirty && (node->data != NULL || node == qpdb->origin || node == qpdb->nsec3_origin)) { - qpznode_unref(node); - return; + goto unref; } /* @@ -934,17 +942,12 @@ decref(qpzonedb_t *qpdb, qpznode_t *node, uint32_t least_serial, /* Upgrade the lock? */ if (*nlocktypep == isc_rwlocktype_read) { - NODE_FORCEUPGRADE(&nodelock->lock, nlocktypep); + NODE_FORCEUPGRADE(&qpdb->node_locks[node->locknum].lock, + nlocktypep); } - refs = isc_refcount_decrement(&node->erefs); -#if DNS_DB_NODETRACE - fprintf(stderr, "decr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n", - func, file, line, node, refs - 1); -#endif - if (refs > 1) { - qpznode_unref(node); - return; + if (!qpznode_decref(qpdb, node DNS__DB_FLARG_PASS)) { + goto unref; } if (node->dirty) { @@ -960,16 +963,7 @@ decref(qpzonedb_t *qpdb, qpznode_t *node, uint32_t least_serial, clean_zone_node(node, least_serial); } - refs = isc_refcount_decrement(&nodelock->references); -#if DNS_DB_NODETRACE - fprintf(stderr, - "decr:nodelock:%s:%s:%u:%p:%p->references = " - "%" PRIuFAST32 "\n", - func, file, line, node, nodelock, refs - 1); -#else - UNUSED(refs); -#endif - +unref: qpznode_unref(node); } From 36a26bfa1a12f4711a1fd4e8aafe37fb0a355b1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 27 Jan 2025 18:13:38 +0100 Subject: [PATCH 2/4] Remove origin_node from qpcache The origin_node in qpcache was always NULL, so we can remove the getoriginode() function and origin_node pointer as the dns_db_getoriginnode() correctly returns ISC_R_NOTFOUND when the function is not implemented. --- lib/dns/qpcache.c | 32 ++------------------------------ 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index aa645fbe44..8b0bda1912 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -207,7 +207,6 @@ struct qpcache { /* Locks for individual tree nodes */ unsigned int node_lock_count; db_nodelock_t *node_locks; - qpcnode_t *origin_node; dns_stats_t *rrsetstats; /* cache DB only */ isc_stats_t *cachestats; /* cache DB only */ isc_stats_t *gluecachestats; /* zone DB only */ @@ -720,7 +719,7 @@ decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep, } /* Handle easy and typical case first. */ - if (!node->dirty && (node->data != NULL || node == qpdb->origin_node)) { + if (!node->dirty && node->data != NULL) { goto unref; } @@ -778,7 +777,7 @@ decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep, write_locked = true; } - if (node->data != NULL || node == qpdb->origin_node) { + if (node->data != NULL) { goto restore_locks; } @@ -2555,10 +2554,6 @@ qpcache_destroy(dns_db_t *arg) { unsigned int i; unsigned int inactive = 0; - if (qpdb->origin_node != NULL) { - qpcnode_detach(&qpdb->origin_node); - } - /* * Even though there are no external direct references, there still * may be nodes in use. @@ -3622,28 +3617,6 @@ nodecount(dns_db_t *db, dns_dbtree_t tree) { return mu.leaves; } -static isc_result_t -getoriginnode(dns_db_t *db, dns_dbnode_t **nodep DNS__DB_FLARG) { - qpcache_t *qpdb = (qpcache_t *)db; - qpcnode_t *onode = NULL; - isc_result_t result = ISC_R_SUCCESS; - - REQUIRE(VALID_QPDB(qpdb)); - REQUIRE(nodep != NULL && *nodep == NULL); - - /* Note that the access to origin_node doesn't require a DB lock */ - onode = (qpcnode_t *)qpdb->origin_node; - if (onode != NULL) { - newref(qpdb, onode, isc_rwlocktype_none, - isc_rwlocktype_none DNS__DB_FLARG_PASS); - *nodep = (dns_dbnode_t *)qpdb->origin_node; - } else { - result = ISC_R_NOTFOUND; - } - - return result; -} - static void locknode(dns_db_t *db, dns_dbnode_t *node, isc_rwlocktype_t type) { qpcache_t *qpdb = (qpcache_t *)db; @@ -4387,7 +4360,6 @@ static dns_dbmethods_t qpdb_cachemethods = { .addrdataset = qpcache_addrdataset, .deleterdataset = qpcache_deleterdataset, .nodecount = nodecount, - .getoriginnode = getoriginnode, .getrrsetstats = getrrsetstats, .setcachestats = setcachestats, .setservestalettl = setservestalettl, 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 3/4] 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)); } From d4f791793e2db06f63243eebb7c1a1de66d5792c Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Thu, 30 Jan 2025 14:42:57 -0800 Subject: [PATCH 4/4] Clarify reference counting in QP databases Change the names of the node reference counting functions and add comments to make the mechanism easier to understand: - newref() and decref() are now called qpcnode_acquire()/ qpznode_acquire() and qpcnode_release()/qpznode_release() respectively; this reflects the fact that they modify both the internal and external reference counters for a node. - qpcnode_newref() and qpznode_newref() are now called qpcnode_erefs_increment() and qpznode_erefs_increment(), and qpcnode_decref() and qpznode_decref() are now called qpcnode_erefs_decrement() and qpznode_erefs_decrement(), to reflect that they only increase and decrease the node's external reference counters, not internal. --- lib/dns/qpcache.c | 209 ++++++++++++++++++++++++++++------------------ lib/dns/qpzone.c | 155 +++++++++++++++++++++++----------- 2 files changed, 238 insertions(+), 126 deletions(-) diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index dfa03ba84a..cccf522694 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -171,6 +171,28 @@ struct qpcnode { uint16_t locknum; + /* + * 'erefs' counts external references held by a caller: for + * example, it could be incremented by dns_db_findnode(), + * and decremented by dns_db_detachnode(). + * + * 'references' counts internal references to the node object, + * including the one held by the QP trie so the node won't be + * deleted while it's quiescently stored in the database - even + * though 'erefs' may be zero because no external caller is + * using it at the time. + * + * Generally when 'erefs' is incremented or decremented, + * 'references' is too. When both go to zero (meaning callers + * and the database have both released the object) the object + * is freed. + * + * Whenever 'erefs' is incremented from zero, we also aquire a + * node use reference (see 'qpcache->references' below), and + * release it when 'erefs' goes back to zero. This prevents the + * database from being shut down until every caller has released + * all nodes. + */ isc_refcount_t references; isc_refcount_t erefs; void *data; @@ -204,15 +226,31 @@ struct qpcache { isc_rwlock_t lock; /* Locks the tree structure (prevents nodes appearing/disappearing) */ isc_rwlock_t tree_lock; + + /* + * NOTE: 'references' is NOT the global reference counter for + * the database object handled by dns_db_attach() and _detach(); + * that one is 'common.references'. + * + * Instead, 'references' counts the number of nodes being used by + * at least one external caller. (It's called 'references' to + * leverage the ISC_REFCOUNT_STATIC macros, but 'nodes_in_use' + * might be a clearer name.) + * + * One additional reference to this counter is held by the database + * object itself. When 'common.references' goes to zero, that + * reference is released. When in turn 'references' goes to zero, + * the database is shut down and freed. + */ + isc_refcount_t references; + /* Locks for individual tree nodes */ unsigned int node_lock_count; - dns_stats_t *rrsetstats; /* cache DB only */ - isc_stats_t *cachestats; /* cache DB only */ - isc_stats_t *gluecachestats; /* zone DB only */ - - isc_refcount_t references; isc_rwlock_t *node_locks; + dns_stats_t *rrsetstats; + isc_stats_t *cachestats; + uint32_t maxrrperset; /* Maximum RRs per RRset */ uint32_t maxtypepername; /* Maximum number of RR types per owner */ @@ -624,10 +662,18 @@ delete_node(qpcache_t *qpdb, qpcnode_t *node) { * It's okay for neither lock to be held if there are existing external * references to the node, but if this is the first external reference, * then the caller must be holding at least one lock. + * + * If incrementing erefs from zero, we also increment the node use counter + * in the qpcache object. + * + * This function is called from qpcnode_acquire(), so that internal + * and external references are acquired at the same time, and from + * qpcnode_release() when we only need to increase the internal references. */ static void -qpcnode_newref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t nlocktype, - isc_rwlocktype_t tlocktype DNS__DB_FLARG) { +qpcnode_erefs_increment(qpcache_t *qpdb, qpcnode_t *node, + isc_rwlocktype_t nlocktype, + isc_rwlocktype_t tlocktype DNS__DB_FLARG) { uint_fast32_t refs = isc_refcount_increment0(&node->erefs); #if DNS_DB_NODETRACE @@ -654,17 +700,23 @@ qpcnode_newref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t nlocktype, } static void -newref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t nlocktype, - isc_rwlocktype_t tlocktype DNS__DB_FLARG) { +qpcnode_acquire(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t nlocktype, + isc_rwlocktype_t tlocktype DNS__DB_FLARG) { qpcnode_ref(node); - qpcnode_newref(qpdb, node, nlocktype, tlocktype DNS__DB_FLARG_PASS); + qpcnode_erefs_increment(qpdb, node, nlocktype, + tlocktype DNS__DB_FLARG_PASS); } static void cleanup_deadnodes(void *arg); +/* + * Decrement the external references to a node. If the counter + * goes to zero, decrement the node use counter in the qpcache object + * as well, and return true. Otherwise return false. + */ static bool -qpcnode_decref(qpcache_t *qpdb, qpcnode_t *node DNS__DB_FLARG) { +qpcnode_erefs_decrement(qpcache_t *qpdb, qpcnode_t *node DNS__DB_FLARG) { uint_fast32_t refs = isc_refcount_decrement(&node->erefs); #if DNS_DB_NODETRACE @@ -676,37 +728,32 @@ qpcnode_decref(qpcache_t *qpdb, qpcnode_t *node DNS__DB_FLARG) { } qpcache_unref(qpdb); - return true; } /* - * Caller must be holding the node lock; either the read or write lock. + * Caller must be holding a node lock, either read or write. + * * Note that the lock must be held even when node references are * atomically modified; in that case the decrement operation itself does not * have to be protected, but we must avoid a race condition where multiple * threads are decreasing the reference to zero simultaneously and at least * one of them is going to free the node. * - * This decrements both the internal and external node reference counters. - * If the external reference count drops to zero, then the node lock - * reference count is also decremented. - * - * This function returns true if and only if the node reference decreases - * to zero. (NOTE: Decrementing the reference count of a node to zero does - * not mean it will be immediately freed.) + * This calls dec_erefs() to decrement the external node reference counter, + * (and possibly the node use counter), cleans up and deletes the node + * if necessary, then decrements the internal reference counter as well. */ static void -decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep, - isc_rwlocktype_t *tlocktypep, bool tryupgrade DNS__DB_FLARG) { +qpcnode_release(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep, + isc_rwlocktype_t *tlocktypep, bool tryupgrade DNS__DB_FLARG) { + REQUIRE(*nlocktypep != isc_rwlocktype_none); + 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); - - if (!qpcnode_decref(qpdb, node DNS__DB_FLARG_PASS)) { + if (!qpcnode_erefs_decrement(qpdb, node DNS__DB_FLARG_PASS)) { goto unref; } @@ -715,20 +762,23 @@ decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep, goto unref; } - /* - * Node lock ref has decremented to 0 and we may need to clean up the - * node. To clean it up, the node ref needs to decrement to 0 under the - * node write lock, so we regain the ref and try again. - */ - qpcnode_newref(qpdb, node, *nlocktypep, *tlocktypep DNS__DB_FLARG_PASS); - - /* Upgrade the lock? */ if (*nlocktypep == isc_rwlocktype_read) { + /* + * The external reference count went to zero and the node + * is dirty or has no data, so we might want to delete it. + * To do that, we'll need a write lock. If we don't already + * have one, we have to make sure nobody else has + * acquired a reference in the meantime, so we increment + * erefs (but NOT references!), upgrade the node lock, + * decrement erefs again, and see if it's still zero. + */ + isc_rwlock_t *nlock = &qpdb->node_locks[node->locknum]; + qpcnode_erefs_increment(qpdb, node, *nlocktypep, + *tlocktypep DNS__DB_FLARG_PASS); NODE_FORCEUPGRADE(nlock, nlocktypep); - } - - if (!qpcnode_decref(qpdb, node DNS__DB_FLARG_PASS)) { - goto unref; + if (!qpcnode_erefs_decrement(qpdb, node DNS__DB_FLARG_PASS)) { + goto unref; + } } if (node->dirty) { @@ -778,7 +828,8 @@ decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep, */ delete_node(qpdb, node); } else { - newref(qpdb, node, *nlocktypep, *tlocktypep DNS__DB_FLARG_PASS); + qpcnode_acquire(qpdb, node, *nlocktypep, + *tlocktypep DNS__DB_FLARG_PASS); isc_queue_node_init(&node->deadlink); if (!isc_queue_enqueue_entry(&qpdb->deadnodes[node->locknum], @@ -794,7 +845,7 @@ decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep, restore_locks: /* - * Relock a read lock, or unlock the write lock if no lock was held. + * Unlock the tree lock if it wasn't held previously. */ if (!locked && write_locked) { TREE_UNLOCK(&qpdb->tree_lock, tlocktypep); @@ -918,12 +969,12 @@ expireheader(dns_slabheader_t *header, isc_rwlocktype_t *nlocktypep, /* * If no one else is using the node, we can clean it up now. * We first need to gain a new reference to the node to meet a - * requirement of decref(). + * requirement of qpcnode_release(). */ - newref(qpdb, HEADERNODE(header), *nlocktypep, - *tlocktypep DNS__DB_FLARG_PASS); - decref(qpdb, HEADERNODE(header), nlocktypep, tlocktypep, - true DNS__DB_FLARG_PASS); + qpcnode_acquire(qpdb, HEADERNODE(header), *nlocktypep, + *tlocktypep DNS__DB_FLARG_PASS); + qpcnode_release(qpdb, HEADERNODE(header), nlocktypep, + tlocktypep, true DNS__DB_FLARG_PASS); if (qpdb->cachestats == NULL) { return; @@ -990,7 +1041,7 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header, return; } - newref(qpdb, node, nlocktype, tlocktype DNS__DB_FLARG_PASS); + qpcnode_acquire(qpdb, node, nlocktype, tlocktype DNS__DB_FLARG_PASS); INSIST(rdataset->methods == NULL); /* We must be disassociated. */ @@ -1205,7 +1256,7 @@ check_stale_header(qpcnode_t *node, dns_slabheader_t *header, /* * header->down can be non-NULL if the * refcount has just decremented to 0 - * but decref() has not + * but qpcnode_release() has not * performed clean_cache_node(), in * which case we need to purge the stale * headers first. @@ -1277,8 +1328,8 @@ check_zonecut(qpcnode_t *node, void *arg DNS__DB_FLARG) { * We increment the reference count on node to ensure that * search->zonecut_header will still be valid later. */ - newref(search->qpdb, node, nlocktype, - isc_rwlocktype_none DNS__DB_FLARG_PASS); + qpcnode_acquire(search->qpdb, node, nlocktype, + isc_rwlocktype_none DNS__DB_FLARG_PASS); search->zonecut = node; search->zonecut_header = dname_header; search->zonecut_sigheader = sigdname_header; @@ -1363,8 +1414,9 @@ find_deepest_zonecut(qpc_search_t *search, qpcnode_t *node, } result = DNS_R_DELEGATION; if (nodep != NULL) { - newref(search->qpdb, node, nlocktype, - isc_rwlocktype_none DNS__DB_FLARG_PASS); + qpcnode_acquire( + search->qpdb, node, nlocktype, + isc_rwlocktype_none DNS__DB_FLARG_PASS); *nodep = (dns_dbnode_t *)node; } bindrdataset(search->qpdb, node, found, search->now, @@ -1499,8 +1551,8 @@ find_coveringnsec(qpc_search_t *search, const dns_name_t *name, nlocktype, isc_rwlocktype_none, sigrdataset DNS__DB_FLARG_PASS); } - newref(search->qpdb, node, nlocktype, - isc_rwlocktype_none DNS__DB_FLARG_PASS); + qpcnode_acquire(search->qpdb, node, nlocktype, + isc_rwlocktype_none DNS__DB_FLARG_PASS); dns_name_copy(fname, foundname); @@ -1794,8 +1846,8 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, nsecheader != NULL) { if (nodep != NULL) { - newref(search.qpdb, node, nlocktype, - tlocktype DNS__DB_FLARG_PASS); + qpcnode_acquire(search.qpdb, node, nlocktype, + tlocktype DNS__DB_FLARG_PASS); *nodep = (dns_dbnode_t *)node; } bindrdataset(search.qpdb, node, nsecheader, search.now, @@ -1838,8 +1890,8 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, */ if (nsheader != NULL) { if (nodep != NULL) { - newref(search.qpdb, node, nlocktype, - tlocktype DNS__DB_FLARG_PASS); + qpcnode_acquire(search.qpdb, node, nlocktype, + tlocktype DNS__DB_FLARG_PASS); *nodep = (dns_dbnode_t *)node; } bindrdataset(search.qpdb, node, nsheader, search.now, @@ -1872,8 +1924,8 @@ qpcache_find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version, */ if (nodep != NULL) { - newref(search.qpdb, node, nlocktype, - tlocktype DNS__DB_FLARG_PASS); + qpcnode_acquire(search.qpdb, node, nlocktype, + tlocktype DNS__DB_FLARG_PASS); *nodep = (dns_dbnode_t *)node; } @@ -1949,8 +2001,8 @@ tree_exit: nlock = &search.qpdb->node_locks[node->locknum]; NODE_RDLOCK(nlock, &nlocktype); - decref(search.qpdb, node, &nlocktype, &tlocktype, - true DNS__DB_FLARG_PASS); + qpcnode_release(search.qpdb, node, &nlocktype, &tlocktype, + true DNS__DB_FLARG_PASS); NODE_UNLOCK(nlock, &nlocktype); INSIST(tlocktype == isc_rwlocktype_none); } @@ -2093,8 +2145,8 @@ qpcache_findzonecut(dns_db_t *db, const dns_name_t *name, unsigned int options, } if (nodep != NULL) { - newref(search.qpdb, node, nlocktype, - tlocktype DNS__DB_FLARG_PASS); + qpcnode_acquire(search.qpdb, node, nlocktype, + tlocktype DNS__DB_FLARG_PASS); *nodep = (dns_dbnode_t *)node; } @@ -2520,9 +2572,6 @@ qpcache__destroy(qpcache_t *qpdb) { if (qpdb->cachestats != NULL) { isc_stats_detach(&qpdb->cachestats); } - if (qpdb->gluecachestats != NULL) { - isc_stats_detach(&qpdb->gluecachestats); - } isc_mem_cput(qpdb->common.mctx, qpdb->node_locks, qpdb->node_lock_count, sizeof(qpdb->node_locks[0])); @@ -2577,7 +2626,7 @@ cleanup_deadnodes(void *arg) { 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); + qpcnode_release(qpdb, qpnode, &nlocktype, &tlocktype, false); } NODE_UNLOCK(nlock, &nlocktype); @@ -2600,7 +2649,7 @@ reactivate_node(qpcache_t *qpdb, qpcnode_t *node, isc_rwlock_t *nlock = &qpdb->node_locks[node->locknum]; NODE_RDLOCK(nlock, &nlocktype); - newref(qpdb, node, nlocktype, tlocktype DNS__DB_FLARG_PASS); + qpcnode_acquire(qpdb, node, nlocktype, tlocktype DNS__DB_FLARG_PASS); NODE_UNLOCK(nlock, &nlocktype); } @@ -2670,8 +2719,8 @@ qpcache_attachnode(dns_db_t *db, dns_dbnode_t *source, qpcache_t *qpdb = (qpcache_t *)db; qpcnode_t *node = (qpcnode_t *)source; - newref(qpdb, node, isc_rwlocktype_none, - isc_rwlocktype_none DNS__DB_FLARG_PASS); + qpcnode_acquire(qpdb, node, isc_rwlocktype_none, + isc_rwlocktype_none DNS__DB_FLARG_PASS); *targetp = source; } @@ -2692,13 +2741,15 @@ qpcache_detachnode(dns_db_t *db, dns_dbnode_t **nodep DNS__DB_FLARG) { nlock = &qpdb->node_locks[node->locknum]; /* - * We can't destroy qpcache under nodelock, so we need - * to reference it before acquiring the lock. + * We can't destroy qpcache while holding a nodelock, so + * we need to reference it before acquiring the lock + * and release it afterward. */ qpcache_ref(qpdb); NODE_RDLOCK(nlock, &nlocktype); - decref(qpdb, node, &nlocktype, &tlocktype, true DNS__DB_FLARG_PASS); + qpcnode_release(qpdb, node, &nlocktype, &tlocktype, + true DNS__DB_FLARG_PASS); NODE_UNLOCK(nlock, &nlocktype); qpcache_detach(&qpdb); @@ -2754,8 +2805,8 @@ qpcache_allrdatasets(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, iterator->common.now = now; iterator->current = NULL; - newref(qpdb, qpnode, isc_rwlocktype_none, - isc_rwlocktype_none DNS__DB_FLARG_PASS); + qpcnode_acquire(qpdb, qpnode, isc_rwlocktype_none, + isc_rwlocktype_none DNS__DB_FLARG_PASS); *iteratorp = (dns_rdatasetiter_t *)iterator; @@ -3896,8 +3947,8 @@ dereference_iter_node(qpc_dbit_t *qpdbiter DNS__DB_FLARG) { nlock = &qpdb->node_locks[node->locknum]; NODE_RDLOCK(nlock, &nlocktype); - decref(qpdb, node, &nlocktype, &qpdbiter->tree_locked, - false DNS__DB_FLARG_PASS); + qpcnode_release(qpdb, node, &nlocktype, &qpdbiter->tree_locked, + false DNS__DB_FLARG_PASS); NODE_UNLOCK(nlock, &nlocktype); INSIST(qpdbiter->tree_locked == tlocktype); @@ -4151,8 +4202,8 @@ dbiterator_current(dns_dbiterator_t *iterator, dns_dbnode_t **nodep, dns_name_copy(&node->name, name); } - newref(qpdb, node, isc_rwlocktype_none, - qpdbiter->tree_locked DNS__DB_FLARG_PASS); + qpcnode_acquire(qpdb, node, isc_rwlocktype_none, + qpdbiter->tree_locked DNS__DB_FLARG_PASS); *nodep = (dns_dbnode_t *)qpdbiter->node; return ISC_R_SUCCESS; diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index c00e50411d..96282b7852 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -150,8 +150,32 @@ typedef ISC_LIST(qpz_version_t) qpz_versionlist_t; struct qpznode { dns_name_t name; isc_mem_t *mctx; + + /* + * 'erefs' counts external references held by a caller: for + * example, it could be incremented by dns_db_findnode(), + * and decremented by dns_db_detachnode(). + * + * 'references' counts internal references to the node object, + * including the one held by the QP trie so the node won't be + * deleted while it's quiescently stored in the database - even + * though 'erefs' may be zero because no external caller is + * using it at the time. + * + * Generally when 'erefs' is incremented or decremented, + * 'references' is too. When both go to zero (meaning callers + * and the database have both released the object) the object + * is freed. + * + * Whenever 'erefs' is incremented from zero, we also aquire a + * node use reference (see 'qpzonedb->references' below), and + * release it when 'erefs' goes back to zero. This prevents the + * database from being shut down until every caller has released + * all nodes. + */ isc_refcount_t references; isc_refcount_t erefs; + uint16_t locknum; atomic_uint_fast8_t nsec; atomic_bool wild; @@ -165,10 +189,27 @@ struct qpzonedb { dns_db_t common; /* Locks the data in this struct */ isc_rwlock_t lock; + + /* + * NOTE: 'references' is NOT the global reference counter for + * the database object handled by dns_db_attach() and _detach(); + * that one is 'common.references'. + * + * Instead, 'references' counts the number of nodes being used by + * at least one external caller. (It's called 'references' to + * leverage the ISC_REFCOUNT_STATIC macros, but 'nodes_in_use' + * might be a clearer name.) + * + * One additional reference to this counter is held by the database + * object itself. When 'common.references' goes to zero, that + * reference is released. When in turn 'references' goes to zero, + * the database is shut down and freed. + */ + isc_refcount_t references; + /* Locks for tree nodes */ int node_lock_count; isc_rwlock_t *node_locks; - isc_refcount_t references; qpznode_t *origin; qpznode_t *nsec3_origin; @@ -698,8 +739,16 @@ dns__qpzone_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type, return ISC_R_SUCCESS; } +/* + * If incrementing erefs from zero, we also increment the node use counter + * in the qpzonedb object. + * + * This function is called from qpznode_acquire(), so that internal + * and external references are acquired at the same time, and from + * qpznode_release() when we only need to increase the internal references. + */ static void -qpznode_newref(qpzonedb_t *qpdb, qpznode_t *node DNS__DB_FLARG) { +qpznode_erefs_increment(qpzonedb_t *qpdb, qpznode_t *node DNS__DB_FLARG) { uint_fast32_t refs = isc_refcount_increment0(&node->erefs); #if DNS_DB_NODETRACE fprintf(stderr, "incr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n", @@ -714,9 +763,9 @@ qpznode_newref(qpzonedb_t *qpdb, qpznode_t *node DNS__DB_FLARG) { } static void -newref(qpzonedb_t *qpdb, qpznode_t *node DNS__DB_FLARG) { +qpznode_acquire(qpzonedb_t *qpdb, qpznode_t *node DNS__DB_FLARG) { qpznode_ref(node); - qpznode_newref(qpdb, node DNS__DB_FLARG_PASS); + qpznode_erefs_increment(qpdb, node DNS__DB_FLARG_PASS); } static void @@ -836,8 +885,13 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) { } } +/* + * Decrement the external references to a node. If the counter + * goes to zero, decrement the node use counter in the qpzonedb object + * as well, and return true. Otherwise return false. + */ static bool -qpznode_decref(qpzonedb_t *qpdb, qpznode_t *node DNS__DB_FLARG) { +qpznode_erefs_decrement(qpzonedb_t *qpdb, qpznode_t *node DNS__DB_FLARG) { uint_fast32_t refs = isc_refcount_decrement(&node->erefs); #if DNS_DB_NODETRACE @@ -861,19 +915,16 @@ qpznode_decref(qpzonedb_t *qpdb, qpznode_t *node DNS__DB_FLARG) { * threads are decreasing the reference to zero simultaneously and at least * one of them is going to free the node. * - * This decrements both the internal and external node reference counters. - * If the external reference count drops to zero, then the node lock - * reference count is also decremented. - * - * (NOTE: Decrementing the reference count of a node to zero does - * not mean it will be immediately freed.) + * This calls dec_erefs() to decrement the external node reference counter, + * (and possibly the node use counter), cleans up and deletes the node + * if necessary, then decrements the internal reference counter as well. */ static void -decref(qpzonedb_t *qpdb, qpznode_t *node, uint32_t least_serial, - isc_rwlocktype_t *nlocktypep DNS__DB_FLARG) { +qpznode_release(qpzonedb_t *qpdb, qpznode_t *node, uint32_t least_serial, + isc_rwlocktype_t *nlocktypep DNS__DB_FLARG) { REQUIRE(*nlocktypep != isc_rwlocktype_none); - if (!qpznode_decref(qpdb, node DNS__DB_FLARG_PASS)) { + if (!qpznode_erefs_decrement(qpdb, node DNS__DB_FLARG_PASS)) { goto unref; } @@ -884,21 +935,22 @@ decref(qpzonedb_t *qpdb, qpznode_t *node, uint32_t least_serial, goto unref; } - /* - * Node lock ref has decremented to 0 and we may need to clean up the - * node. To clean it up, the node ref needs to decrement to 0 under the - * node write lock, so we regain the ref and try again. - */ - qpznode_newref(qpdb, node DNS__DB_FLARG_PASS); - - /* Upgrade the lock? */ if (*nlocktypep == isc_rwlocktype_read) { + /* + * The external reference count went to zero and the node + * is dirty or has no data, so we might want to delete it. + * To do that, we'll need a write lock. If we don't already + * have one, we have to make sure nobody else has + * acquired a reference in the meantime, so we increment + * erefs (but NOT references!), upgrade the node lock, + * decrement erefs again, and see if it's still zero. + */ isc_rwlock_t *nlock = &qpdb->node_locks[node->locknum]; + qpznode_erefs_increment(qpdb, node DNS__DB_FLARG_PASS); NODE_FORCEUPGRADE(nlock, nlocktypep); - } - - if (!qpznode_decref(qpdb, node DNS__DB_FLARG_PASS)) { - goto unref; + if (!qpznode_erefs_decrement(qpdb, node DNS__DB_FLARG_PASS)) { + goto unref; + } } if (node->dirty) { @@ -925,7 +977,7 @@ bindrdataset(qpzonedb_t *qpdb, qpznode_t *node, dns_slabheader_t *header, return; } - newref(qpdb, node DNS__DB_FLARG_PASS); + qpznode_acquire(qpdb, node DNS__DB_FLARG_PASS); INSIST(rdataset->methods == NULL); /* We must be disassociated. */ @@ -1229,7 +1281,7 @@ resigndelete(qpzonedb_t *qpdb, qpz_version_t *version, RWUNLOCK(&qpdb->lock, isc_rwlocktype_write); header->heap_index = 0; - newref(qpdb, HEADERNODE(header) DNS__DB_FLARG_PASS); + qpznode_acquire(qpdb, HEADERNODE(header) DNS__DB_FLARG_PASS); ISC_LIST_APPEND(version->resigned_list, header, link); } @@ -1467,8 +1519,8 @@ closeversion(dns_db_t *db, dns_dbversion_t **versionp, if (rollback && !IGNORE(header)) { resigninsert(qpdb, header); } - decref(qpdb, HEADERNODE(header), least_serial, - &nlocktype DNS__DB_FLARG_PASS); + qpznode_release(qpdb, HEADERNODE(header), least_serial, + &nlocktype DNS__DB_FLARG_PASS); NODE_UNLOCK(nlock, &nlocktype); } @@ -1491,7 +1543,8 @@ closeversion(dns_db_t *db, dns_dbversion_t **versionp, if (rollback) { rollback_node(node, serial); } - decref(qpdb, node, least_serial, &nlocktype DNS__DB_FILELINE); + qpznode_release(qpdb, node, least_serial, + &nlocktype DNS__DB_FILELINE); NODE_UNLOCK(nlock, &nlocktype); @@ -1735,7 +1788,7 @@ add_changed(dns_slabheader_t *header, qpz_version_t *version DNS__DB_FLARG) { *changed = (qpz_changed_t){ .node = node }; ISC_LIST_INITANDAPPEND(version->changed_list, changed, link); - newref(qpdb, node DNS__DB_FLARG_PASS); + qpznode_acquire(qpdb, node DNS__DB_FLARG_PASS); RWUNLOCK(&qpdb->lock, isc_rwlocktype_write); return changed; @@ -2496,7 +2549,7 @@ findnodeintree(qpzonedb_t *qpdb, const dns_name_t *name, bool create, INSIST(node->nsec == DNS_DB_NSEC_NSEC3 || !nsec3); } - newref(qpdb, node DNS__DB_FLARG_PASS); + qpznode_acquire(qpdb, node DNS__DB_FLARG_PASS); if (create) { dns_qp_compact(qp, DNS_QPGC_MAYBE); @@ -3096,8 +3149,9 @@ again: */ dns_name_copy(name, foundname); if (nodep != NULL) { - newref(search->qpdb, - node DNS__DB_FLARG_PASS); + qpznode_acquire( + search->qpdb, + node DNS__DB_FLARG_PASS); *nodep = (dns_dbnode_t *)node; } bindrdataset(search->qpdb, node, found, @@ -3241,7 +3295,7 @@ qpzone_check_zonecut(qpznode_t *node, void *arg DNS__DB_FLARG) { * We increment the reference count on node to ensure that * search->zonecut_header will still be valid later. */ - newref(search->qpdb, node DNS__DB_FLARG_PASS); + qpznode_acquire(search->qpdb, node DNS__DB_FLARG_PASS); search->zonecut = node; search->zonecut_header = found; search->need_cleanup = true; @@ -3516,7 +3570,8 @@ found: * ensure that search->zonecut_header will * still be valid later. */ - newref(search.qpdb, node DNS__DB_FLARG_PASS); + qpznode_acquire(search.qpdb, + node DNS__DB_FLARG_PASS); search.zonecut = node; search.zonecut_header = header; search.zonecut_sigheader = NULL; @@ -3699,7 +3754,7 @@ found: goto tree_exit; } if (nodep != NULL) { - newref(search.qpdb, node DNS__DB_FLARG_PASS); + qpznode_acquire(search.qpdb, node DNS__DB_FLARG_PASS); *nodep = (dns_dbnode_t *)node; } if (search.version->secure && !search.version->havensec3) { @@ -3763,7 +3818,7 @@ found: if (nodep != NULL) { if (!at_zonecut) { - newref(search.qpdb, node DNS__DB_FLARG_PASS); + qpznode_acquire(search.qpdb, node DNS__DB_FLARG_PASS); } else { search.need_cleanup = false; } @@ -3803,7 +3858,8 @@ tree_exit: nlock = &search.qpdb->node_locks[node->locknum]; NODE_RDLOCK(nlock, &nlocktype); - decref(search.qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS); + qpznode_release(search.qpdb, node, 0, + &nlocktype DNS__DB_FLARG_PASS); NODE_UNLOCK(nlock, &nlocktype); } @@ -3843,7 +3899,7 @@ qpzone_allrdatasets(dns_db_t *db, dns_dbnode_t *dbnode, .common.magic = DNS_RDATASETITER_MAGIC, }; - newref(qpdb, node DNS__DB_FLARG_PASS); + qpznode_acquire(qpdb, node DNS__DB_FLARG_PASS); *iteratorp = (dns_rdatasetiter_t *)iterator; return ISC_R_SUCCESS; @@ -3858,7 +3914,7 @@ attachnode(dns_db_t *db, dns_dbnode_t *source, REQUIRE(VALID_QPZONE(qpdb)); REQUIRE(targetp != NULL && *targetp == NULL); - newref(qpdb, node DNS__DB_FLARG_PASS); + qpznode_acquire(qpdb, node DNS__DB_FLARG_PASS); *targetp = source; } @@ -3877,10 +3933,15 @@ detachnode(dns_db_t *db, dns_dbnode_t **nodep DNS__DB_FLARG) { *nodep = NULL; nlock = &qpdb->node_locks[node->locknum]; + /* + * We can't destroy qpzonedb while holding a nodelock, so + * we need to reference it before acquiring the lock + * and release it afterward. + */ qpzonedb_ref(qpdb); NODE_RDLOCK(nlock, &nlocktype); - decref(qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS); + qpznode_release(qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS); NODE_UNLOCK(nlock, &nlocktype); qpzonedb_detach(&qpdb); @@ -3939,7 +4000,7 @@ getoriginnode(dns_db_t *db, dns_dbnode_t **nodep DNS__DB_FLARG) { /* Note that the access to the origin node doesn't require a DB lock */ INSIST(qpdb->origin != NULL); - newref(qpdb, qpdb->origin DNS__DB_FLARG_PASS); + qpznode_acquire(qpdb, qpdb->origin DNS__DB_FLARG_PASS); *nodep = (dns_dbnode_t *)qpdb->origin; return ISC_R_SUCCESS; @@ -4143,7 +4204,7 @@ reference_iter_node(qpdb_dbiterator_t *iter DNS__DB_FLARG) { return; } - newref(qpdb, node DNS__DB_FLARG_PASS); + qpznode_acquire(qpdb, node DNS__DB_FLARG_PASS); } static void @@ -4161,7 +4222,7 @@ dereference_iter_node(qpdb_dbiterator_t *iter DNS__DB_FLARG) { nlock = &qpdb->node_locks[node->locknum]; NODE_RDLOCK(nlock, &nlocktype); - decref(qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS); + qpznode_release(qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS); NODE_UNLOCK(nlock, &nlocktype); } @@ -4505,7 +4566,7 @@ dbiterator_current(dns_dbiterator_t *iterator, dns_dbnode_t **nodep, dns_name_copy(&qpdbiter->node->name, name); } - newref(qpdb, node DNS__DB_FLARG_PASS); + qpznode_acquire(qpdb, node DNS__DB_FLARG_PASS); *nodep = (dns_dbnode_t *)qpdbiter->node;