mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-31 14:35:26 +00:00
Code Issues Releases Wiki Activity

Abstract bucket lock selection logic

Browse Source 
Recovering the node lock from a pointer to the header and a pointer to
the db is a common operation. This commit abstracts it away into a
function, so that the node lock selection logic may be modified more
easily.
This commit is contained in:
Alessio Podda
2025-07-08 15:52:35 +02:00
parent 720fa14670
commit c2a84bb17a
2 changed files with 60 additions and 57 deletions
Download Patch File Download Diff File Expand all files Collapse all files

114
lib/dns/qpzone.c
View File

@@ -91,7 +91,9 @@
#define QPDB_ATTR_LOADED 0x01
#define QPDB_ATTR_LOADING 0x02
#ifndef DEFAULT_BUCKETS_COUNT
#define DEFAULT_BUCKETS_COUNT 17 /*%< Should be prime. */
#endif
#define QPDBITER_NSEC3_ORIGIN_NODE(qpdb, iterator) \
((iterator)->current == &(iterator)->nsec3iter && \
@@ -108,6 +110,15 @@
typedef struct qpzonedb qpzonedb_t;
typedef struct qpznode qpznode_t;
typedef struct qpzone_bucket {
/* Per-bucket lock. */
isc_rwlock_t lock;
/* Padding to prevent false sharing between locks. */
uint8_t __padding[ISC_OS_CACHELINE_SIZE -
(sizeof(isc_rwlock_t)) % ISC_OS_CACHELINE_SIZE];
} qpzone_bucket_t;
typedef struct qpz_changed {
qpznode_t *node;
bool dirty;
@@ -185,15 +196,6 @@ struct qpznode {
void *data;
};
typedef struct qpzone_bucket {
/* Per-bucket lock. */
isc_rwlock_t lock;
/* Padding to prevent false sharing between locks. */
uint8_t __padding[ISC_OS_CACHELINE_SIZE -
(sizeof(isc_rwlock_t)) % ISC_OS_CACHELINE_SIZE];
} qpzone_bucket_t;
struct qpzonedb {
/* Unlocked. */
dns_db_t common;
@@ -239,8 +241,7 @@ struct qpzonedb {
dns_qpmulti_t *nsec; /* NSEC nodes only */
dns_qpmulti_t *nsec3; /* NSEC3 nodes only */
size_t buckets_count;
qpzone_bucket_t buckets[]; /* attribute((counted_by(buckets_count))) */
qpzone_bucket_t buckets[DEFAULT_BUCKETS_COUNT];
};
#ifdef DNS_DB_NODETRACE
@@ -404,6 +405,16 @@ static atomic_uint_fast16_t init_count = 0;
* Failure to follow this hierarchy can result in deadlock.
*/
static isc_rwlock_t *
qpzone_get_lock(qpzonedb_t *qpdb, qpznode_t *node) {
return &qpdb->buckets[node->locknum].lock;
}
static uint16_t
qpzone_get_locknum(void) {
return isc_random_uniform(DEFAULT_BUCKETS_COUNT);
}
/*%
* Return which RRset should be resigned sooner. If the RRsets have the
* same signing time, prefer the other RRset over the SOA RRset.
@@ -507,7 +518,7 @@ free_db_rcu(struct rcu_head *rcu_head) {
if (dns_name_dynamic(&qpdb->common.origin)) {
dns_name_free(&qpdb->common.origin, qpdb->common.mctx);
}
for (size_t i = 0; i < qpdb->buckets_count; i++) {
for (size_t i = 0; i < DEFAULT_BUCKETS_COUNT; i++) {
NODE_DESTROYLOCK(&qpdb->buckets[i].lock);
}
@@ -532,9 +543,7 @@ free_db_rcu(struct rcu_head *rcu_head) {
INSIST(!cds_lfht_destroy(qpdb->common.update_listeners, NULL));
}
isc_mem_putanddetach(&qpdb->common.mctx, qpdb,
sizeof(*qpdb) + qpdb->buckets_count *
sizeof(qpdb->buckets[0]));
isc_mem_putanddetach(&qpdb->common.mctx, qpdb, sizeof(*qpdb));
}
static void
@@ -595,7 +604,7 @@ new_qpznode(qpzonedb_t *qpdb, const dns_name_t *name) {
*newdata = (qpznode_t){
.name = DNS_NAME_INITEMPTY,
.references = ISC_REFCOUNT_INITIALIZER(1),
.locknum = isc_random_uniform(qpdb->buckets_count),
.locknum = qpzone_get_locknum(),
};
isc_mem_attach(qpdb->common.mctx, &newdata->mctx);
@@ -636,14 +645,11 @@ dns__qpzone_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type,
isc_result_t result;
dns_qp_t *qp = NULL;
qpdb = isc_mem_get(mctx,
sizeof(*qpdb) + DEFAULT_BUCKETS_COUNT *
sizeof(qpdb->buckets[0]));
qpdb = isc_mem_get(mctx, sizeof(*qpdb));
*qpdb = (qpzonedb_t){
.common.origin = DNS_NAME_INITEMPTY,
.common.rdclass = rdclass,
.common.references = ISC_REFCOUNT_INITIALIZER(1),
.buckets_count = DEFAULT_BUCKETS_COUNT,
.current_serial = 1,
.least_serial = 1,
.next_serial = 2,
@@ -662,7 +668,7 @@ dns__qpzone_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type,
isc_heap_create(mctx, resign_sooner, set_index, 0, &qpdb->heap);
for (size_t i = 0; i < qpdb->buckets_count; i++) {
for (size_t i = 0; i < DEFAULT_BUCKETS_COUNT; i++) {
NODE_INITLOCK(&qpdb->buckets[i].lock);
}
@@ -944,7 +950,7 @@ qpznode_release(qpzonedb_t *qpdb, qpznode_t *node, uint32_t least_serial,
* erefs_increment. If another thread acquires reference it
* will be larger than 0, if it doesn't it is going to be 0.
*/
isc_rwlock_t *nlock = &qpdb->buckets[node->locknum].lock;
isc_rwlock_t *nlock = qpzone_get_lock(qpdb, node);
qpznode_erefs_increment(qpdb, node DNS__DB_FLARG_PASS);
NODE_FORCEUPGRADE(nlock, nlocktypep);
if (!qpznode_erefs_decrement(qpdb, node DNS__DB_FLARG_PASS)) {
@@ -1038,7 +1044,7 @@ setnsec3parameters(dns_db_t *db, qpz_version_t *version) {
version->havensec3 = false;
node = qpdb->origin;
nlock = &qpdb->buckets[node->locknum].lock;
nlock = qpzone_get_lock(qpdb, node);
NODE_RDLOCK(nlock, &nlocktype);
for (header = node->data; header != NULL; header = header_next) {
header_next = header->next;
@@ -1498,7 +1504,7 @@ closeversion(dns_db_t *db, dns_dbversion_t **versionp,
ISC_LIST_UNLINK(resigned_list, header, link);
nlock = &qpdb->buckets[HEADERNODE(header)->locknum].lock;
nlock = qpzone_get_lock(qpdb, HEADERNODE(header));
NODE_WRLOCK(nlock, &nlocktype);
if (rollback && !IGNORE(header)) {
resigninsert(qpdb, header);
@@ -1518,7 +1524,7 @@ closeversion(dns_db_t *db, dns_dbversion_t **versionp,
isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
node = changed->node;
nlock = &qpdb->buckets[node->locknum].lock;
nlock = qpzone_get_lock(qpdb, node);
NODE_WRLOCK(nlock, &nlocktype);
if (rollback) {
@@ -1562,7 +1568,7 @@ qpzone_findrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
}
serial = version->serial;
nlock = &qpdb->buckets[node->locknum].lock;
nlock = qpzone_get_lock(qpdb, node);
NODE_RDLOCK(nlock, &nlocktype);
matchtype = DNS_TYPEPAIR_VALUE(type, covers);
@@ -2168,7 +2174,7 @@ loading_addrdataset(void *arg, const dns_name_t *name,
newheader->resign_lsb = rdataset->resign & 0x1;
}
nlock = &qpdb->buckets[node->locknum].lock;
nlock = qpzone_get_lock(qpdb, node);
NODE_WRLOCK(nlock, &nlocktype);
result = add(qpdb, node, name, qpdb->current_version, newheader,
DNS_DBADD_MERGE, true, NULL, 0 DNS__DB_FLARG_PASS);
@@ -2377,7 +2383,7 @@ setsigningtime(dns_db_t *db, dns_rdataset_t *rdataset, isc_stdtime_t resign) {
header = dns_rdataset_getheader(rdataset);
nlock = &qpdb->buckets[HEADERNODE(header)->locknum].lock;
nlock = qpzone_get_lock(qpdb, HEADERNODE(header));
NODE_WRLOCK(nlock, &nlocktype);
oldheader = *header;
@@ -2420,7 +2426,6 @@ getsigningtime(dns_db_t *db, isc_stdtime_t *resign, dns_name_t *foundname,
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;
REQUIRE(VALID_QPZONE(qpdb));
@@ -2434,20 +2439,21 @@ getsigningtime(dns_db_t *db, isc_stdtime_t *resign, dns_name_t *foundname,
RWUNLOCK(&qpdb->lock, isc_rwlocktype_read);
return ISC_R_NOTFOUND;
}
locknum = HEADERNODE(header)->locknum;
nlock = qpzone_get_lock(qpdb, HEADERNODE(header));
RWUNLOCK(&qpdb->lock, isc_rwlocktype_read);
again:
nlock = &qpdb->buckets[locknum].lock;
NODE_RDLOCK(nlock, &nlocktype);
RWLOCK(&qpdb->lock, isc_rwlocktype_read);
header = isc_heap_element(qpdb->heap, 1);
if (header != NULL && HEADERNODE(header)->locknum != locknum) {
if (header != NULL &&
qpzone_get_lock(qpdb, HEADERNODE(header)) != nlock)
{
RWUNLOCK(&qpdb->lock, isc_rwlocktype_read);
NODE_UNLOCK(nlock, &nlocktype);
locknum = HEADERNODE(header)->locknum;
nlock = qpzone_get_lock(qpdb, HEADERNODE(header));
goto again;
}
@@ -2632,8 +2638,7 @@ qpzone_setup_delegation(qpz_search_t *search, dns_dbnode_t **nodep,
}
if (rdataset != NULL) {
isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
isc_rwlock_t *nlock =
&search->qpdb->buckets[node->locknum].lock;
isc_rwlock_t *nlock = qpzone_get_lock(search->qpdb, node);
NODE_RDLOCK(nlock, &nlocktype);
bindrdataset(search->qpdb, node, search->zonecut_header,
rdataset DNS__DB_FLARG_PASS);
@@ -2673,7 +2678,7 @@ 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 *nlock = &qpdb->buckets[node->locknum].lock;
isc_rwlock_t *nlock = qpzone_get_lock(qpdb, node);
isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
dns_slabheader_t *header_next = NULL;
@@ -2838,7 +2843,7 @@ find_wildcard(qpz_search_t *search, qpznode_t **nodep,
dns_qpchain_node(&search->chain, i, NULL, (void **)&node, NULL);
nlock = &qpdb->buckets[node->locknum].lock;
nlock = qpzone_get_lock(qpdb, node);
NODE_RDLOCK(nlock, &nlocktype);
/*
* First we try to figure out if this node is active in
@@ -2882,7 +2887,7 @@ find_wildcard(qpz_search_t *search, qpznode_t **nodep,
* is active in the search's version, we're
* done.
*/
nlock = &qpdb->buckets[wnode->locknum].lock;
nlock = qpzone_get_lock(qpdb, wnode);
NODE_RDLOCK(nlock, &nlocktype);
for (header = wnode->data; header != NULL;
header = header->next)
@@ -3065,8 +3070,7 @@ again:
do {
dns_slabheader_t *found = NULL, *foundsig = NULL;
isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
isc_rwlock_t *nlock =
&search->qpdb->buckets[node->locknum].lock;
isc_rwlock_t *nlock = qpzone_get_lock(search->qpdb, node);
NODE_RDLOCK(nlock, &nlocktype);
empty_node = true;
for (header = node->data; header != NULL; header = header_next)
@@ -3209,7 +3213,7 @@ 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->buckets[node->locknum].lock;
isc_rwlock_t *nlock = qpzone_get_lock(search->qpdb, node);
NODE_RDLOCK(nlock, &nlocktype);
@@ -3498,7 +3502,7 @@ found:
* have matched a wildcard.
*/
nlock = &search.qpdb->buckets[node->locknum].lock;
nlock = qpzone_get_lock(search.qpdb, node);
NODE_RDLOCK(nlock, &nlocktype);
if (search.zonecut != NULL) {
@@ -3860,7 +3864,7 @@ tree_exit:
if (search.need_cleanup) {
node = search.zonecut;
INSIST(node != NULL);
nlock = &search.qpdb->buckets[node->locknum].lock;
nlock = qpzone_get_lock(search.qpdb, node);
NODE_RDLOCK(nlock, &nlocktype);
qpznode_release(search.qpdb, node, 0,
@@ -3936,7 +3940,7 @@ qpzone_detachnode(dns_db_t *db, dns_dbnode_t **nodep DNS__DB_FLARG) {
node = (qpznode_t *)(*nodep);
*nodep = NULL;
nlock = &qpdb->buckets[node->locknum].lock;
nlock = qpzone_get_lock(qpdb, node);
/*
* qpzone_destroy() uses call_rcu() API to destroy the node locks, so it
@@ -4020,7 +4024,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->buckets[node->locknum].lock, type);
RWLOCK(qpzone_get_lock(qpdb, node), type);
}
static void
@@ -4028,7 +4032,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->buckets[node->locknum].lock, type);
RWUNLOCK(qpzone_get_lock(qpdb, node), type);
}
static void
@@ -4074,7 +4078,7 @@ 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->buckets[node->locknum].lock;
isc_rwlock_t *nlock = qpzone_get_lock(qpdb, node);
NODE_RDLOCK(nlock, &nlocktype);
@@ -4117,7 +4121,7 @@ rdatasetiter_next(dns_rdatasetiter_t *iterator DNS__DB_FLARG) {
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;
isc_rwlock_t *nlock = qpzone_get_lock(qpdb, node);
header = qrditer->current;
if (header == NULL) {
@@ -4175,7 +4179,7 @@ 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->buckets[node->locknum].lock;
isc_rwlock_t *nlock = qpzone_get_lock(qpdb, node);
header = qrditer->current;
REQUIRE(header != NULL);
@@ -4214,7 +4218,7 @@ dereference_iter_node(qpdb_dbiterator_t *iter DNS__DB_FLARG) {
}
iter->node = NULL;
nlock = &qpdb->buckets[node->locknum].lock;
nlock = qpzone_get_lock(qpdb, node);
NODE_RDLOCK(nlock, &nlocktype);
qpznode_release(qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS);
@@ -4710,7 +4714,7 @@ 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.)
*/
nlock = &qpdb->buckets[node->locknum].lock;
nlock = qpzone_get_lock(qpdb, node);
NODE_WRLOCK(nlock, &nlocktype);
@@ -4804,7 +4808,7 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
newheader->resign_lsb = rdataset->resign & 0x1;
}
nlock = &qpdb->buckets[node->locknum].lock;
nlock = qpzone_get_lock(qpdb, node);
NODE_WRLOCK(nlock, &nlocktype);
changed = add_changed(newheader, version DNS__DB_FLARG_PASS);
@@ -4966,7 +4970,7 @@ qpzone_deleterdataset(dns_db_t *db, dns_dbnode_t *dbnode,
dns_name_copy(&node->name, nodename);
nlock = &qpdb->buckets[node->locknum].lock;
nlock = qpzone_get_lock(qpdb, node);
NODE_WRLOCK(nlock, &nlocktype);
result = add(qpdb, node, nodename, version, newheader, DNS_DBADD_FORCE,
false, NULL, 0 DNS__DB_FLARG_PASS);
@@ -4985,7 +4989,7 @@ nodefullname(dns_db_t *db, dns_dbnode_t *node, dns_name_t *name) {
REQUIRE(node != NULL);
REQUIRE(name != NULL);
nlock = &qpdb->buckets[qpnode->locknum].lock;
nlock = qpzone_get_lock(qpdb, qpnode);
NODE_RDLOCK(nlock, &nlocktype);
dns_name_copy(&qpnode->name, name);

3
tests/dns/qpzone_test.c
View File

@@ -39,6 +39,7 @@
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wshadow"
#undef CHECK
#define DEFAULT_BUCKETS_COUNT 1 /*%< Should be prime. */
#include "qpzone.c"
#pragma GCC diagnostic pop
@@ -108,7 +109,6 @@ ownercase_test_one(const char *str1, const char *str2) {
*qpdb = (qpzonedb_t){
.common.methods = &qpdb_zonemethods,
.common.mctx = mctx,
.buckets_count = 1,
};
qpznode_t node = { .locknum = 0 };
dns_slabheader_t header = {
@@ -176,7 +176,6 @@ ISC_RUN_TEST_IMPL(setownercase) {
*qpdb = (qpzonedb_t){
.common.methods = &qpdb_zonemethods,
.common.mctx = mctx,
.buckets_count = 1,
};
qpznode_t node = { .locknum = 0 };
dns_slabheader_t header = {