From: Ondřej Surý <ondrej@isc.org>
Date: Sun, 2 Feb 2025 19:22:29 +0000 (+0100)
Subject: Refactor the search in qpcache_findrdataset()
X-Git-Tag: ondrej/lock-free-qpzone-reads-v1~51^2~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bfb219ac2dcaf5fe47464e9176d1368cafbaeaf9;p=thirdparty%2Fbind9.git

Refactor the search in qpcache_findrdataset()

Add new related_headers() function that simplifies the code
flow in qpcache_findrdataset().  Also use check_stale_header() function
to remove code duplication.
---

diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c
index 67f6a075f37..b8897141adb 100644
--- a/lib/dns/qpcache.c
+++ b/lib/dns/qpcache.c
@@ -1315,6 +1315,36 @@ check_stale_header(qpcnode_t *node, dns_slabheader_t *header,
 	return true;
 }
 
+static bool
+related_headers(dns_slabheader_t *header, dns_typepair_t type,
+		dns_typepair_t sigtype, dns_typepair_t negtype,
+		dns_slabheader_t **foundp, dns_slabheader_t **foundsigp) {
+	if (!EXISTS(header) || ANCIENT(header)) {
+		return false;
+	}
+
+	if (header->type == type) {
+		*foundp = header;
+		if (*foundsigp != NULL) {
+			return true;
+		}
+	} else if (header->type == sigtype) {
+		*foundsigp = header;
+		if (*foundp != NULL) {
+			return true;
+		}
+
+	} else if (header->type == RDATATYPE_NCACHEANY ||
+		   header->type == negtype)
+	{
+		*foundp = header;
+		*foundsigp = NULL;
+		return true;
+	}
+
+	return false;
+}
+
 static bool
 both_headers(dns_slabheader_t *header, dns_rdatatype_t type,
 	     dns_slabheader_t **foundp, dns_slabheader_t **foundsigp) {
@@ -2119,12 +2149,14 @@ qpcache_findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 		     dns_rdataset_t *sigrdataset DNS__DB_FLARG) {
 	qpcache_t *qpdb = (qpcache_t *)db;
 	qpcnode_t *qpnode = (qpcnode_t *)node;
-	dns_slabheader_t *header = NULL, *header_next = NULL;
+	dns_slabheader_t *header = NULL;
+	dns_slabheader_t *header_prev = NULL, *header_next = NULL;
 	dns_slabheader_t *found = NULL, *foundsig = NULL;
 	dns_typepair_t matchtype, sigmatchtype, negtype;
 	isc_result_t result;
 	isc_rwlock_t *nlock = NULL;
 	isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
+	qpc_search_t search;
 
 	REQUIRE(VALID_QPDB(qpdb));
 	REQUIRE(type != dns_rdatatype_any);
@@ -2137,6 +2169,11 @@ qpcache_findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 		now = isc_stdtime_now();
 	}
 
+	search = (qpc_search_t){
+		.qpdb = (qpcache_t *)db,
+		.now = now,
+	};
+
 	nlock = &qpdb->buckets[qpnode->locknum].lock;
 	NODE_RDLOCK(nlock, &nlocktype);
 
@@ -2150,35 +2187,17 @@ qpcache_findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 
 	for (header = qpnode->data; header != NULL; header = header_next) {
 		header_next = header->next;
-		if (!ACTIVE(header, now)) {
-			if ((header->expire + STALE_TTL(header, qpdb) <
-			     now - QPDB_VIRTUAL) &&
-			    (nlocktype == isc_rwlocktype_write ||
-			     NODE_TRYUPGRADE(nlock, &nlocktype) ==
-				     ISC_R_SUCCESS))
-			{
-				/*
-				 * We update the node's status only when we
-				 * can get write access.
-				 *
-				 * We don't check if refcurrent(qpnode) == 0
-				 * and try to free like we do in find(),
-				 * because refcurrent(qpnode) must be
-				 * non-zero.  This is so because 'node' is an
-				 * argument to the function.
-				 */
-				mark_ancient(header);
-			}
-		} else if (EXISTS(header) && !ANCIENT(header)) {
-			if (header->type == matchtype) {
-				found = header;
-			} else if (header->type == RDATATYPE_NCACHEANY ||
-				   header->type == negtype)
-			{
-				found = header;
-			} else if (header->type == sigmatchtype) {
-				foundsig = header;
-			}
+
+		if (check_stale_header(qpnode, header, &nlocktype, nlock,
+				       &search, &header_prev))
+		{
+			continue;
+		}
+
+		if (related_headers(header, matchtype, sigmatchtype, negtype,
+				    &found, &foundsig))
+		{
+			break;
 		}
 	}
 	if (found != NULL) {