From: Ondřej Surý <ondrej@isc.org>
Date: Fri, 12 Sep 2025 16:08:23 +0000 (+0200)
Subject: Convert slabheader to use the cds_list
X-Git-Tag: v9.21.14~32^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=28fff0045d7ae885aca6d77c237dc7c8887d1c3b;p=thirdparty%2Fbind9.git

Convert slabheader to use the cds_list

This is the second commit in series that aims to reduce the node locking
by replacing the single-linked list of slabheader(s) with CDS linked list.
This commit doesn't do anything else beyond replacing .next link with
the cds_list_head.  RCU semantics is going to be added in the subsequent
commits.
---

diff --git a/lib/dns/include/dns/rdataslab.h b/lib/dns/include/dns/rdataslab.h
index bacec3bcf70..36f99c74f09 100644
--- a/lib/dns/include/dns/rdataslab.h
+++ b/lib/dns/include/dns/rdataslab.h
@@ -75,9 +75,9 @@ struct dns_slabheader_proof {
 typedef struct dns_slabtop dns_slabtop_t;
 struct dns_slabtop {
 	struct cds_list_head types_link;
+	struct cds_list_head headers;
 
-	dns_slabheader_t *header;
-	dns_typepair_t	  typepair;
+	dns_typepair_t typepair;
 
 	/*% Used for SIEVE-LRU (cache) and changed_list (zone) */
 	ISC_LINK(struct dns_slabtop) link;
@@ -117,10 +117,9 @@ struct dns_slabheader {
 	dns_slabtop_t *top;
 
 	/*%
-	 * Points to the header for the next older version of
-	 * this rdataset.
+	 * Link to the other versions of this rdataset.
 	 */
-	struct dns_slabheader *down;
+	struct cds_list_head headers_link;
 
 	/*%
 	 * The database node objects containing this rdataset, if any.
diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c
index e90277cb33c..d64ba9c8802 100644
--- a/lib/dns/qpcache.c
+++ b/lib/dns/qpcache.c
@@ -467,7 +467,17 @@ rdataset_size(dns_slabheader_t *header) {
 
 static dns_slabheader_t *
 first_header(dns_slabtop_t *top) {
-	return top->header;
+	dns_slabheader_t *header = NULL;
+	cds_list_for_each_entry(header, &top->headers, headers_link) {
+		return header;
+	}
+	return NULL;
+}
+
+static dns_slabheader_t *
+next_header(dns_slabheader_t *header) {
+	return cds_list_entry((header)->headers_link.next, dns_slabheader_t,
+			      headers_link);
 }
 
 static dns_slabheader_t *
@@ -548,16 +558,17 @@ qpcache_hit(qpcache_t *qpdb ISC_ATTR_UNUSED, dns_slabheader_t *header) {
 
 static void
 clean_cache_headers(dns_slabtop_t *top) {
-	if (top->header == NULL) {
+	dns_slabheader_t *parent = first_header(top);
+	if (parent == NULL) {
 		return;
 	}
 
-	dns_slabheader_t *header = top->header, *header_down = NULL;
-	for (header = header->down; header != NULL; header = header_down) {
-		header_down = header->down;
+	dns_slabheader_t *header = next_header(parent), *header_next = NULL;
+	cds_list_for_each_entry_safe_from(header, header_next, &top->headers,
+					  headers_link) {
+		cds_list_del(&header->headers_link);
 		dns_slabheader_destroy(&header);
 	}
-	top->header->down = NULL;
 }
 
 static void
@@ -573,16 +584,22 @@ clean_cache_node(qpcache_t *qpdb, qpcnode_t *node) {
 		 * If current top header is nonexistent, ancient, or stale
 		 * and we are not keeping stale, we can clean it up too.
 		 */
-		if (!EXISTS(top->header) || ANCIENT(top->header) ||
-		    (STALE(top->header) && !KEEPSTALE(qpdb)))
+		dns_slabheader_t *header = first_header(top);
+		if (header == NULL) {
+			continue;
+		}
+
+		if (!EXISTS(header) || ANCIENT(header) ||
+		    (STALE(header) && !KEEPSTALE(qpdb)))
 		{
-			dns_slabheader_destroy(&top->header);
+			cds_list_del(&header->headers_link);
+			dns_slabheader_destroy(&header);
 		}
 
 		/*
 		 * If current slabtop is empty, we can clean it up.
 		 */
-		if (top->header == NULL) {
+		if (header == NULL) {
 			cds_list_del(&top->types_link);
 
 			if (ISC_LINK_LINKED(top, link)) {
@@ -594,7 +611,7 @@ clean_cache_node(qpcache_t *qpdb, qpcnode_t *node) {
 		}
 	}
 
-	node->dirty = 0;
+	node->dirty = false;
 }
 
 /*
@@ -2700,12 +2717,12 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
 
 		if (top->typepair == newheader->typepair) {
 			INSIST(oldheader == NULL);
-			oldheader = top->header;
+			oldheader = first_header(top);
 		}
 
 		if (sigpair != dns_rdatatype_none && top->typepair == sigpair) {
 			INSIST(oldsigheader == NULL);
-			oldsigheader = top->header;
+			oldsigheader = first_header(top);
 		}
 	}
 
@@ -2849,9 +2866,9 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
 			return DNS_R_UNCHANGED;
 		}
 
-		oldheader->top->header = newheader;
 		newheader->top = oldheader->top;
-		newheader->down = oldheader;
+		cds_list_add(&newheader->headers_link,
+			     &oldheader->top->headers);
 
 		ISC_SIEVE_UNLINK(qpdb->buckets[qpnode->locknum].sieve,
 				 oldheader->top, link);
@@ -2871,13 +2888,12 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader,
 		return DNS_R_UNCHANGED;
 	} else {
 		/* No rdatasets of the given type exist at the node. */
-		INSIST(newheader->down == NULL);
-
 		dns_slabtop_t *newtop = dns_slabtop_new(
 			((dns_db_t *)qpdb)->mctx, newheader->typepair);
-		newtop->header = newheader;
 		newheader->top = newtop;
 
+		cds_list_add(&newheader->headers_link, &newtop->headers);
+
 		qpcache_miss(qpdb, newheader, &nlocktype,
 			     &tlocktype DNS__DB_FLARG_PASS);
 
@@ -3828,12 +3844,13 @@ qpcnode_destroy(qpcnode_t *qpnode) {
 	qpcache_t *qpdb = qpnode->qpdb;
 
 	DNS_SLABTOP_FOREACH(top, qpnode->data) {
-		dns_slabheader_t *down = NULL, *down_next = NULL;
-		for (down = top->header; down != NULL; down = down_next) {
-			down_next = down->down;
-			dns_slabheader_destroy(&down);
+		dns_slabheader_t *header = NULL, *header_next = NULL;
+		cds_list_for_each_entry_safe(header, header_next, &top->headers,
+					     headers_link)
+		{
+			cds_list_del(&header->headers_link);
+			dns_slabheader_destroy(&header);
 		}
-		top->header = NULL;
 
 		if (ISC_LINK_LINKED(top, link)) {
 			ISC_SIEVE_UNLINK(qpdb->buckets[qpnode->locknum].sieve,
diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c
index 567603eba79..f6d0e30ee19 100644
--- a/lib/dns/qpzone.c
+++ b/lib/dns/qpzone.c
@@ -805,16 +805,48 @@ qpznode_acquire(qpznode_t *node DNS__DB_FLARG) {
 	qpznode_erefs_increment(node DNS__DB_FLARG_PASS);
 }
 
+static dns_slabheader_t *
+first_header(dns_slabtop_t *top) {
+	dns_slabheader_t *header = NULL;
+	cds_list_for_each_entry(header, &top->headers, headers_link) {
+		return header;
+	}
+	return NULL;
+}
+
+static dns_slabheader_t *
+next_header(dns_slabheader_t *header) {
+	return cds_list_entry((header)->headers_link.next, dns_slabheader_t,
+			      headers_link);
+}
+
+static dns_slabheader_t *
+first_existing_header(dns_slabtop_t *top, uint32_t serial) {
+	dns_slabheader_t *header = NULL;
+	cds_list_for_each_entry(header, &top->headers, headers_link) {
+		if (header->serial <= serial && !IGNORE(header)) {
+			if (EXISTS(header)) {
+				return header;
+			}
+			break;
+		}
+	}
+	return NULL;
+}
+
 static void
 clean_multiple_headers(dns_slabtop_t *top) {
-	dns_slabheader_t *parent = top->header;
-	dns_slabheader_t *header = NULL, *header_down = NULL;
+	dns_slabheader_t *parent = first_header(top);
+	if (parent == NULL) {
+		return;
+	}
 
-	for (header = parent->down; header != NULL; header = header_down) {
-		header_down = header->down;
+	dns_slabheader_t *header = next_header(parent), *header_next = NULL;
+	cds_list_for_each_entry_safe_from(header, header_next, &top->headers,
+					  headers_link) {
 		INSIST(header->serial <= parent->serial);
 		if (header->serial == parent->serial || IGNORE(header)) {
-			parent->down = header->down;
+			cds_list_del(&header->headers_link);
 			dns_slabheader_destroy(&header);
 		} else {
 			parent = header;
@@ -824,23 +856,26 @@ clean_multiple_headers(dns_slabtop_t *top) {
 
 static void
 check_top_header(dns_slabtop_t *top) {
-	dns_slabheader_t *header = top->header;
-	if (IGNORE(header)) {
-		top->header = header->down;
+	dns_slabheader_t *header = first_header(top);
+	if (header != NULL && IGNORE(header)) {
+		cds_list_del(&header->headers_link);
 		dns_slabheader_destroy(&header);
 	}
 }
 
 static bool
 clean_multiple_versions(dns_slabtop_t *top, uint32_t least_serial) {
-	dns_slabheader_t *parent = top->header;
-	dns_slabheader_t *header = NULL, *header_down = NULL;
-	bool multiple = false;
+	dns_slabheader_t *parent = first_header(top);
+	if (parent == NULL) {
+		return false;
+	}
 
-	for (header = parent->down; header != NULL; header = header_down) {
-		header_down = header->down;
+	bool multiple = false;
+	dns_slabheader_t *header = next_header(parent), *header_next = NULL;
+	cds_list_for_each_entry_safe_from(header, header_next, &top->headers,
+					  headers_link) {
 		if (header->serial < least_serial) {
-			parent->down = header->down;
+			cds_list_del(&header->headers_link);
 			dns_slabheader_destroy(&header);
 		} else {
 			multiple = true;
@@ -860,8 +895,6 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) {
 	REQUIRE(least_serial != 0);
 
 	DNS_SLABTOP_FOREACH(top, node->data) {
-		INSIST(top->header != NULL);
-
 		/*
 		 * First, we clean up any instances of multiple rdatasets
 		 * with the same serial number, or that have the IGNORE
@@ -875,7 +908,7 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) {
 		 */
 		check_top_header(top);
 
-		if (top->header == NULL) {
+		if (first_header(top) == NULL) {
 			cds_list_del(&top->types_link);
 			dns_slabtop_destroy(node->mctx, &top);
 		} else {
@@ -1013,28 +1046,6 @@ bindrdataset(qpzonedb_t *qpdb, qpznode_t *node, dns_slabheader_t *header,
 	}
 }
 
-static dns_slabheader_t *
-first_header(dns_slabtop_t *top, uint32_t serial) {
-	for (dns_slabheader_t *header = top->header; header != NULL;
-	     header = header->down)
-	{
-		if (header->serial <= serial && !IGNORE(header)) {
-			return header;
-		}
-	}
-
-	return NULL;
-}
-
-static dns_slabheader_t *
-first_existing_header(dns_slabtop_t *top, uint32_t serial) {
-	dns_slabheader_t *header = first_header(top, serial);
-	if (header != NULL && EXISTS(header)) {
-		return header;
-	}
-	return NULL;
-}
-
 static void
 setnsec3parameters(dns_db_t *db, qpz_version_t *version) {
 	qpznode_t *node = NULL;
@@ -1293,17 +1304,8 @@ rollback_node(qpznode_t *node, uint32_t serial) {
 	 * will be cleaned up; until that time, they will be ignored.
 	 */
 	DNS_SLABTOP_FOREACH(top, node->data) {
-		dns_slabheader_t *header = top->header;
-
-		if (header->serial == serial) {
-			DNS_SLABHEADER_SETATTR(header,
-					       DNS_SLABHEADERATTR_IGNORE);
-			make_dirty = true;
-		}
-
-		for (header = header->down; header != NULL;
-		     header = header->down)
-		{
+		dns_slabheader_t *header = NULL;
+		cds_list_for_each_entry(header, &top->headers, headers_link) {
 			if (header->serial == serial) {
 				DNS_SLABHEADER_SETATTR(
 					header, DNS_SLABHEADERATTR_IGNORE);
@@ -1822,12 +1824,14 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
 	 * IGNORE rdatasets between the top of the chain and the first real
 	 * data.  We skip over them.
 	 */
-	dns_slabheader_t *header = NULL, *header_prev = NULL;
+	dns_slabheader_t *header = NULL;
 	if (foundtop != NULL) {
-		header = foundtop->header;
-		while (header != NULL && IGNORE(header)) {
-			header_prev = header;
-			header = header->down;
+		dns_slabheader_t *tmp = NULL;
+		cds_list_for_each_entry(tmp, &top->headers, headers_link) {
+			if (!IGNORE(tmp)) {
+				header = tmp;
+				break;
+			}
 		}
 	}
 
@@ -1895,11 +1899,10 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
 			}
 		}
 
-		INSIST(version->serial >= foundtop->header->serial);
+		INSIST(version->serial >= header->serial);
 		INSIST(foundtop->typepair == newheader->typepair);
 
 		if (loading) {
-			newheader->down = NULL;
 			if (RESIGN(newheader)) {
 				resigninsert(newheader);
 				/* resigndelete not needed here */
@@ -1912,7 +1915,9 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
 			 * loading, we MUST clean up 'header' now.
 			 */
 			newheader->top = foundtop;
-			foundtop->header = newheader;
+			cds_list_del(&header->headers_link);
+			cds_list_add(&newheader->headers_link,
+				     &foundtop->headers);
 			maybe_update_recordsandsize(false, version, header,
 						    nodename->length);
 
@@ -1924,14 +1929,9 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
 					     header DNS__DB_FLARG_PASS);
 			}
 
-			if (header_prev != NULL) {
-				header_prev->down = newheader;
-			} else {
-				foundtop->header = newheader;
-			}
-
 			newheader->top = foundtop;
-			newheader->down = header;
+			cds_list_add(&newheader->headers_link,
+				     &foundtop->headers);
 
 			node->dirty = true;
 			if (changed != NULL) {
@@ -1967,10 +1967,11 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
 			 * we INSIST on it.
 			 */
 			INSIST(!loading);
-			INSIST(version->serial >= foundtop->header->serial);
+
 			newheader->top = foundtop;
-			newheader->down = foundtop->header;
-			foundtop->header = newheader;
+			cds_list_add(&newheader->headers_link,
+				     &foundtop->headers);
+
 			if (changed != NULL) {
 				changed->dirty = true;
 			}
@@ -1991,7 +1992,8 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
 				node->mctx, newheader->typepair);
 
 			newheader->top = newtop;
-			newtop->header = newheader;
+			cds_list_add(&newheader->headers_link,
+				     &newtop->headers);
 
 			if (prio_type(newheader->typepair)) {
 				/* This is a priority type, prepend it */
@@ -2130,7 +2132,6 @@ loading_addrdataset(void *arg, const dns_name_t *name,
 
 	newheader = (dns_slabheader_t *)region.base;
 	dns_slabheader_reset(newheader, (dns_dbnode_t *)node);
-
 	newheader->ttl = rdataset->ttl;
 	atomic_store(&newheader->trust, rdataset->trust);
 	newheader->serial = 1;
@@ -4766,9 +4767,12 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
 	 */
 	dns_slabheader_t *header = NULL;
 	if (foundtop != NULL) {
-		header = foundtop->header;
-		while (header != NULL && IGNORE(header)) {
-			header = header->down;
+		dns_slabheader_t *tmp = NULL;
+		cds_list_for_each_entry(tmp, &foundtop->headers, headers_link) {
+			if (!IGNORE(tmp)) {
+				header = tmp;
+				break;
+			}
 		}
 	}
 	if (header != NULL && EXISTS(header)) {
@@ -4834,13 +4838,11 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
 		/*
 		 * If we're here, we want to link newheader at the top.
 		 */
-		INSIST(version->serial >= foundtop->header->serial);
 		maybe_update_recordsandsize(false, version, header,
 					    nodename->length);
 
 		newheader->top = foundtop;
-		newheader->down = foundtop->header;
-		foundtop->header = newheader;
+		cds_list_add(&newheader->headers_link, &foundtop->headers);
 
 		node->dirty = true;
 		changed->dirty = true;
@@ -5291,12 +5293,13 @@ static dns_dbnode_methods_t qpznode_methods = (dns_dbnode_methods_t){
 static void
 destroy_qpznode(qpznode_t *node) {
 	DNS_SLABTOP_FOREACH(top, node->data) {
-		dns_slabheader_t *down = NULL, *down_next = NULL;
-		for (down = top->header; down != NULL; down = down_next) {
-			down_next = down->down;
-			dns_slabheader_destroy(&down);
+		dns_slabheader_t *header = NULL, *header_next = NULL;
+		cds_list_for_each_entry_safe(header, header_next, &top->headers,
+					     headers_link)
+		{
+			cds_list_del(&header->headers_link);
+			dns_slabheader_destroy(&header);
 		}
-		top->header = NULL;
 
 		dns_slabtop_destroy(node->mctx, &top);
 	}
diff --git a/lib/dns/rdataslab.c b/lib/dns/rdataslab.c
index beb5f38d019..703c2f6b1ea 100644
--- a/lib/dns/rdataslab.c
+++ b/lib/dns/rdataslab.c
@@ -323,6 +323,7 @@ dns_rdataslab_fromrdataset(dns_rdataset_t *rdataset, isc_mem_t *mctx,
 		}
 
 		*new = (dns_slabheader_t){
+			.headers_link = CDS_LIST_HEAD_INIT(new->headers_link),
 			.typepair = typepair,
 			.trust = rdataset->trust,
 			.ttl = rdataset->ttl,
@@ -1180,6 +1181,7 @@ dns_slabtop_new(isc_mem_t *mctx, dns_typepair_t typepair) {
 	dns_slabtop_t *top = isc_mem_get(mctx, sizeof(*top));
 	*top = (dns_slabtop_t){
 		.types_link = CDS_LIST_HEAD_INIT(top->types_link),
+		.headers = CDS_LIST_HEAD_INIT(top->headers),
 		.typepair = typepair,
 		.link = ISC_LINK_INITIALIZER,
 	};