From: Ondřej Surý Date: Fri, 31 Jan 2025 14:47:33 +0000 (+0100) Subject: Add .up pointer to slabheader X-Git-Tag: ondrej/lock-free-qpzone-reads-v1~47^2~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=15fe68e50d5d84f677950096762d3b27aa2425d2;p=thirdparty%2Fbind9.git Add .up pointer to slabheader The dns_slabheader object uses the 'next' pointer for two purposes. In the first header for any given type, 'next' points to the first header for the next type. But 'down' points to the next header of the same type, and in that record, 'next' points back up. This design made the code confusing to read. We now use a union so that the 'next' pointer can also be called 'up'. --- diff --git a/lib/dns/include/dns/rdataslab.h b/lib/dns/include/dns/rdataslab.h index cf2c8f1567d..c81fe9777f3 100644 --- a/lib/dns/include/dns/rdataslab.h +++ b/lib/dns/include/dns/rdataslab.h @@ -102,12 +102,16 @@ struct dns_slabheader { * both head and tail pointers, and is doubly linked. */ - struct dns_slabheader *next; + union { + struct dns_slabheader *next; + struct dns_slabheader *up; + }; /*%< * If this is the top header for an rdataset, 'next' points * to the top header for the next rdataset (i.e., the next type). - * Otherwise, it points up to the header whose down pointer points - * at this header. + * + * Otherwise 'up' points up to the header whose down pointer points at + * this header. */ struct dns_slabheader *down; @@ -322,3 +326,9 @@ dns_slabheader_freeproof(isc_mem_t *mctx, dns_slabheader_proof_t **proof); /*%< * Free all memory associated with a nonexistence proof. */ + +dns_slabheader_t * +dns_slabheader_top(dns_slabheader_t *header); +/*%< + * Return the top header for the type or the negtype + */ diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index d1bb246d2ca..feb0dddf57e 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -765,7 +765,7 @@ qpznode_acquire(qpzonedb_t *qpdb, qpznode_t *node DNS__DB_FLARG) { static void clean_zone_node(qpznode_t *node, uint32_t least_serial) { dns_slabheader_t *current = NULL, *dcurrent = NULL; - dns_slabheader_t *down_next = NULL, *dparent = NULL; + dns_slabheader_t *dcurrent_down = NULL, *dparent = NULL; dns_slabheader_t *top_prev = NULL, *top_next = NULL; bool still_dirty = false; @@ -784,17 +784,17 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) { */ dparent = current; for (dcurrent = current->down; dcurrent != NULL; - dcurrent = down_next) + dcurrent = dcurrent_down) { - down_next = dcurrent->down; + dcurrent_down = dcurrent->down; INSIST(dcurrent->serial <= dparent->serial); if (dcurrent->serial == dparent->serial || IGNORE(dcurrent)) { - if (down_next != NULL) { - down_next->next = dparent; + if (dcurrent_down != NULL) { + dcurrent_down->up = dparent; } - dparent->down = down_next; + dparent->down = dcurrent_down; dns_slabheader_destroy(&dcurrent); } else { dparent = dcurrent; @@ -805,9 +805,10 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) { * We've now eliminated all IGNORE datasets with the possible * exception of current, which we now check. */ - if (IGNORE(current)) { - down_next = current->down; - if (down_next == NULL) { + dcurrent = current; + if (IGNORE(dcurrent)) { + dcurrent_down = current->down; + if (dcurrent_down == NULL) { if (top_prev != NULL) { top_prev->next = current->next; } else { @@ -825,13 +826,13 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) { * current. */ if (top_prev != NULL) { - top_prev->next = down_next; + top_prev->next = dcurrent_down; } else { - node->data = down_next; + node->data = dcurrent_down; } - down_next->next = top_next; + dcurrent_down->next = top_next; dns_slabheader_destroy(¤t); - current = down_next; + current = dcurrent_down; } } @@ -841,9 +842,9 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) { */ dparent = current; for (dcurrent = current->down; dcurrent != NULL; - dcurrent = down_next) + dcurrent = dcurrent_down) { - down_next = dcurrent->down; + dcurrent_down = dcurrent->down; if (dcurrent->serial < least_serial) { break; } @@ -856,10 +857,10 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) { */ if (dcurrent != NULL) { do { - down_next = dcurrent->down; + dcurrent_down = dcurrent->down; INSIST(dcurrent->serial <= least_serial); dns_slabheader_destroy(&dcurrent); - dcurrent = down_next; + dcurrent = dcurrent_down; } while (dcurrent != NULL); dparent->down = NULL; } @@ -2056,7 +2057,7 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename, } newheader->next = topheader->next; newheader->down = topheader; - topheader->next = newheader; + topheader->up = newheader; node->dirty = true; if (changed != NULL) { changed->dirty = true; @@ -2099,7 +2100,7 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename, } newheader->next = topheader->next; newheader->down = topheader; - topheader->next = newheader; + topheader->up = newheader; if (changed != NULL) { changed->dirty = true; } @@ -4184,9 +4185,8 @@ rdatasetiter_next(dns_rdatasetiter_t *iterator DNS__DB_FLARG) { qpzonedb_t *qpdb = (qpzonedb_t *)(qrditer->common.db); qpznode_t *node = (qpznode_t *)qrditer->common.node; qpz_version_t *version = (qpz_version_t *)qrditer->common.version; - dns_slabheader_t *header = NULL, *top_next = NULL; - dns_typepair_t type, negtype; - dns_rdatatype_t rdtype; + dns_slabheader_t *header = NULL; + dns_slabheader_t *topheader, *topheader_next = NULL; isc_rwlocktype_t nlocktype = isc_rwlocktype_none; isc_rwlock_t *nlock = &qpdb->buckets[node->locknum].lock; @@ -4197,22 +4197,14 @@ rdatasetiter_next(dns_rdatasetiter_t *iterator DNS__DB_FLARG) { NODE_RDLOCK(nlock, &nlocktype); - type = header->type; - rdtype = DNS_TYPEPAIR_TYPE(header->type); - negtype = DNS_TYPEPAIR_VALUE(0, rdtype); - /* - * Find the start of the header chain for the next type - * by walking back up the list. + * Find the start of the header chain for the next type. */ - top_next = header->next; - while (top_next != NULL && - (top_next->type == type || top_next->type == negtype)) + topheader = dns_slabheader_top(header); + + for (header = topheader->next; header != NULL; header = topheader_next) { - top_next = top_next->next; - } - for (header = top_next; header != NULL; header = top_next) { - top_next = header->next; + topheader_next = header->next; do { if (header->serial <= version->serial && !IGNORE(header)) @@ -4228,15 +4220,11 @@ rdatasetiter_next(dns_rdatasetiter_t *iterator DNS__DB_FLARG) { if (header != NULL) { break; } + /* - * Find the start of the header chain for the next type - * by walking back up the list. + * Find the start of the header chain for the next type. */ - while (top_next != NULL && - (top_next->type == type || top_next->type == negtype)) - { - top_next = top_next->next; - } + topheader = topheader->next; } NODE_UNLOCK(nlock, &nlocktype); diff --git a/lib/dns/rdataslab.c b/lib/dns/rdataslab.c index c5e0ffbf914..ce0dfeb3f40 100644 --- a/lib/dns/rdataslab.c +++ b/lib/dns/rdataslab.c @@ -39,6 +39,9 @@ #define NONEXISTENT(header) \ ((atomic_load_acquire(&(header)->attributes) & \ DNS_SLABHEADERATTR_NONEXISTENT) != 0) +#define NEGATIVE(header) \ + ((atomic_load_acquire(&(header)->attributes) & \ + DNS_SLABHEADERATTR_NEGATIVE) != 0) /* * The rdataslab structure allows iteration to occur in both load order @@ -1188,3 +1191,30 @@ rdataset_getownercase(const dns_rdataset_t *rdataset, dns_name_t *name) { unlock: dns_db_unlocknode(header->db, header->node, isc_rwlocktype_read); } + +dns_slabheader_t * +dns_slabheader_top(dns_slabheader_t *header) { + dns_typepair_t type, negtype; + dns_rdatatype_t rdtype, covers; + + type = header->type; + rdtype = DNS_TYPEPAIR_TYPE(header->type); + if (NEGATIVE(header)) { + covers = DNS_TYPEPAIR_COVERS(header->type); + negtype = DNS_TYPEPAIR_VALUE(covers, 0); + } else { + negtype = DNS_TYPEPAIR_VALUE(0, rdtype); + } + + /* + * Find the start of the header chain for the next type + * by walking back up the list. + */ + while (header->up != NULL && + (header->up->type == type || header->up->type == negtype)) + { + header = header->up; + } + + return header; +}