diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index b621b7db64..869601f1ba 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -208,9 +208,10 @@ struct qpcnode { uint8_t : 0; /*% - * Used for dead node cleaning. The deadnodes queue is used - * for nodes that have no data any longer, but we can't unlink - * yet because we don't have a tree lock. + * Used for dead nodes cleaning. This linked list is used to mark nodes + * which have no data any longer, but we cannot unlink at that exact + * moment because we did not or could not obtain a write lock on the + * tree. */ isc_queue_node_t deadlink; }; @@ -221,8 +222,9 @@ struct qpcnode { * to reduce contention between threads. */ typedef struct qpcache_bucket { - /* - * Temporary storage for cache nodes that need to be deleted. + /*% + * Temporary storage for stale cache nodes and dynamically + * deleted nodes that await being cleaned up. */ isc_queue_t deadnodes; diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index af6fc14715..ff01b019af 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -806,7 +806,6 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) { } top_prev = current; } - if (!still_dirty) { node->dirty = false; } @@ -1450,8 +1449,10 @@ closeversion(dns_db_t *db, dns_dbversion_t **versionp, NODE_UNLOCK(nlock, &nlocktype); } - dns_qp_t *tree = NULL, *nsec = NULL, *nsec3 = NULL; - bool need_tree = false, need_nsec = false, need_nsec3 = false; + if (ISC_LIST_EMPTY(cleanup_list)) { + *versionp = NULL; + return; + } for (changed = HEAD(cleanup_list); changed != NULL; changed = next_changed) @@ -1467,100 +1468,14 @@ closeversion(dns_db_t *db, dns_dbversion_t **versionp, if (rollback) { rollback_node(node, serial); } - - qpznode_ref(node); qpznode_release(qpdb, node, least_serial, &nlocktype DNS__DB_FILELINE); - /* If the node is now empty, we can delete it. */ - if (commit && node->data == NULL) { - switch ((int)node->nsec) { - case DNS_DB_NSEC_HAS_NSEC: - /* - * Delete the matching node from the NSEC tree - * first, then fall through to the main tree. - */ - if (nsec == NULL) { - need_nsec = true; - next_changed = changed; - } else { - dns_qp_deletename(nsec, &node->name, - NULL, NULL); - } - FALLTHROUGH; - case DNS_DB_NSEC_NORMAL: - if (tree == NULL) { - need_tree = true; - next_changed = changed; - } else { - dns_qp_deletename(tree, &node->name, - NULL, NULL); - } - break; - case DNS_DB_NSEC_NSEC: - if (nsec == NULL) { - need_nsec = true; - next_changed = changed; - } else { - dns_qp_deletename(nsec, &node->name, - NULL, NULL); - } - break; - case DNS_DB_NSEC_NSEC3: - if (nsec3 == NULL) { - need_nsec3 = true; - next_changed = changed; - } else { - dns_qp_deletename(nsec3, &node->name, - NULL, NULL); - } - break; - default: - UNREACHABLE(); - } - } - - qpznode_detach(&node); - NODE_UNLOCK(nlock, &nlocktype); - if (next_changed == changed) { - /* - * We found a node to delete but didn't have a - * QP writer open, so we open one now, then go - * back to delete the node. If there's a next - * time, we'll already have the writer open, - * so we won't need this extra step. - */ - if (need_tree && tree == NULL) { - dns_qpmulti_write(qpdb->tree, &tree); - } - if (need_nsec && nsec == NULL) { - dns_qpmulti_write(qpdb->nsec, &nsec); - } - if (need_nsec3 && nsec3 == NULL) { - dns_qpmulti_write(qpdb->nsec3, &nsec3); - } - - continue; - } - isc_mem_put(qpdb->common.mctx, changed, sizeof(*changed)); } - if (tree != NULL) { - dns_qp_compact(tree, DNS_QPGC_MAYBE); - dns_qpmulti_commit(qpdb->tree, &tree); - } - if (nsec != NULL) { - dns_qp_compact(nsec, DNS_QPGC_MAYBE); - dns_qpmulti_commit(qpdb->nsec, &nsec); - } - if (nsec3 != NULL) { - dns_qp_compact(nsec3, DNS_QPGC_MAYBE); - dns_qpmulti_commit(qpdb->nsec3, &nsec3); - } - *versionp = NULL; }