Refactor decref() in both qpcache.c and qpzone.c

Cleanup the pattern in the decref() functions in both qpcache.c and
qpzone.c, so it follows the similar patter as we already have in
newref() function.

(cherry picked from commit 814b87da64)
This commit is contained in:
Ondřej Surý 2025-01-27 18:06:17 +01:00
parent 8662424442
commit d1d444d2ab
No known key found for this signature in database
GPG key ID: 2820F37E873DEA41
2 changed files with 119 additions and 134 deletions

View file

@ -626,30 +626,32 @@ qpcnode_newref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t nlocktype,
func, file, line, node, refs + 1);
#endif
if (refs == 0) {
/*
* this is the first external reference to the node.
*
* we need to hold the node or tree lock to avoid
* incrementing the reference count while also deleting
* the node. delete_node() is always protected by both
* tree and node locks being write-locked.
*/
INSIST(nlocktype != isc_rwlocktype_none ||
tlocktype != isc_rwlocktype_none);
refs = isc_refcount_increment0(
&qpdb->node_locks[node->locknum].references);
#if DNS_DB_NODETRACE
fprintf(stderr,
"incr:nodelock:%s:%s:%u:%p:%p->references = "
"%" PRIuFAST32 "\n",
func, file, line, node,
&qpdb->node_locks[node->locknum], refs + 1);
#else
UNUSED(refs);
#endif
if (refs > 0) {
return;
}
/*
* this is the first external reference to the node.
*
* we need to hold the node or tree lock to avoid
* incrementing the reference count while also deleting
* the node. delete_node() is always protected by both
* tree and node locks being write-locked.
*/
INSIST(nlocktype != isc_rwlocktype_none ||
tlocktype != isc_rwlocktype_none);
refs = isc_refcount_increment0(
&qpdb->node_locks[node->locknum].references);
#if DNS_DB_NODETRACE
fprintf(stderr,
"incr:nodelock:%s:%s:%u:%p:%p->references = "
"%" PRIuFAST32 "\n",
func, file, line, node, &qpdb->node_locks[node->locknum],
refs + 1);
#else
UNUSED(refs);
#endif
}
static void
@ -662,6 +664,33 @@ newref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t nlocktype,
static void
cleanup_deadnodes(void *arg);
static bool
qpcnode_decref(qpcache_t *qpdb, qpcnode_t *node DNS__DB_FLARG) {
uint_fast32_t refs = isc_refcount_decrement(&node->erefs);
#if DNS_DB_NODETRACE
fprintf(stderr, "decr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n",
func, file, line, node, refs - 1);
#endif
if (refs > 1) {
return false;
}
refs = isc_refcount_decrement(
&qpdb->node_locks[node->locknum].references);
#if DNS_DB_NODETRACE
fprintf(stderr,
"decr:nodelock:%s:%s:%u:%p:%p->references = "
"%" PRIuFAST32 "\n",
func, file, line, node, &qpdb->node_locks[node->locknum],
refs - 1);
#else
UNUSED(refs);
#endif
return true;
}
/*
* Caller must be holding the node lock; either the read or write lock.
* Note that the lock must be held even when node references are
@ -678,42 +707,22 @@ cleanup_deadnodes(void *arg);
* to zero. (NOTE: Decrementing the reference count of a node to zero does
* not mean it will be immediately freed.)
*/
static bool
static void
decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep,
isc_rwlocktype_t *tlocktypep, bool tryupgrade DNS__DB_FLARG) {
isc_result_t result;
bool locked = *tlocktypep != isc_rwlocktype_none;
bool write_locked = false;
int bucket = node->locknum;
db_nodelock_t *nodelock = &qpdb->node_locks[bucket];
uint_fast32_t refs;
REQUIRE(*nlocktypep != isc_rwlocktype_none);
refs = isc_refcount_decrement(&node->erefs);
#if DNS_DB_NODETRACE
fprintf(stderr, "decr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n",
func, file, line, node, refs - 1);
#endif
if (refs > 1) {
qpcnode_unref(node);
return false;
if (!qpcnode_decref(qpdb, node DNS__DB_FLARG_PASS)) {
goto unref;
}
refs = isc_refcount_decrement(&nodelock->references);
#if DNS_DB_NODETRACE
fprintf(stderr,
"decr:nodelock:%s:%s:%u:%p:%p->references = "
"%" PRIuFAST32 "\n",
func, file, line, node, nodelock, refs - 1);
#else
UNUSED(refs);
#endif
/* Handle easy and typical case first. */
if (!node->dirty && (node->data != NULL || node == qpdb->origin_node)) {
qpcnode_unref(node);
return false;
goto unref;
}
/*
@ -725,18 +734,12 @@ decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep,
/* Upgrade the lock? */
if (*nlocktypep == isc_rwlocktype_read) {
NODE_FORCEUPGRADE(&nodelock->lock, nlocktypep);
NODE_FORCEUPGRADE(&qpdb->node_locks[node->locknum].lock,
nlocktypep);
}
refs = isc_refcount_decrement(&node->erefs);
#if DNS_DB_NODETRACE
fprintf(stderr, "decr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n",
func, file, line, node, refs - 1);
#endif
if (refs > 1) {
qpcnode_unref(node);
return false;
if (!qpcnode_decref(qpdb, node DNS__DB_FLARG_PASS)) {
goto unref;
}
if (node->dirty) {
@ -776,21 +779,10 @@ decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep,
write_locked = true;
}
refs = isc_refcount_decrement(&nodelock->references);
#if DNS_DB_NODETRACE
fprintf(stderr,
"decr:nodelock:%s:%s:%u:%p:%p->references = %" PRIuFAST32 "\n",
func, file, line, node, nodelock, refs - 1);
#else
UNUSED(refs);
#endif
if (node->data != NULL || node == qpdb->origin_node) {
goto restore_locks;
}
#undef KEEP_NODE
if (write_locked) {
/*
* We can now delete the node.
@ -800,11 +792,12 @@ decref(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep,
newref(qpdb, node, *nlocktypep, *tlocktypep DNS__DB_FLARG_PASS);
isc_queue_node_init(&node->deadlink);
if (!isc_queue_enqueue_entry(&qpdb->deadnodes[bucket], node,
deadlink))
if (!isc_queue_enqueue_entry(&qpdb->deadnodes[node->locknum],
node, deadlink))
{
/* Queue was empty, trigger new cleaning */
isc_loop_t *loop = isc_loop_get(qpdb->loopmgr, bucket);
isc_loop_t *loop = isc_loop_get(qpdb->loopmgr,
node->locknum);
isc_async_run(loop, cleanup_deadnodes, qpdb);
}
@ -818,8 +811,8 @@ restore_locks:
TREE_UNLOCK(&qpdb->tree_lock, tlocktypep);
}
unref:
qpcnode_unref(node);
return true;
}
static void
@ -2752,13 +2745,11 @@ detachnode(dns_db_t *db, dns_dbnode_t **targetp DNS__DB_FLARG) {
NODE_RDLOCK(&nodelock->lock, &nlocktype);
if (decref(qpdb, node, &nlocktype, &tlocktype, true DNS__DB_FLARG_PASS))
decref(qpdb, node, &nlocktype, &tlocktype, true DNS__DB_FLARG_PASS);
if (isc_refcount_current(&nodelock->references) == 0 &&
nodelock->exiting)
{
if (isc_refcount_current(&nodelock->references) == 0 &&
nodelock->exiting)
{
inactive = true;
}
inactive = true;
}
NODE_UNLOCK(&nodelock->lock, &nlocktype);

View file

@ -665,20 +665,22 @@ qpznode_newref(qpzonedb_t *qpdb, qpznode_t *node DNS__DB_FLARG) {
func, file, line, node, refs + 1);
#endif
if (refs == 0) {
/* this is the first reference to the node */
refs = isc_refcount_increment0(
&qpdb->node_locks[node->locknum].references);
#if DNS_DB_NODETRACE
fprintf(stderr,
"incr:nodelock:%s:%s:%u:%p:%p->references = "
"%" PRIuFAST32 "\n",
func, file, line, node,
&qpdb->node_locks[node->locknum], refs + 1);
#else
UNUSED(refs);
#endif
if (refs > 0) {
return;
}
/* this is the first reference to the node */
refs = isc_refcount_increment0(
&qpdb->node_locks[node->locknum].references);
#if DNS_DB_NODETRACE
fprintf(stderr,
"incr:nodelock:%s:%s:%u:%p:%p->references = "
"%" PRIuFAST32 "\n",
func, file, line, node, &qpdb->node_locks[node->locknum],
refs + 1);
#else
UNUSED(refs);
#endif
}
static void
@ -804,6 +806,33 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) {
}
}
static bool
qpznode_decref(qpzonedb_t *qpdb, qpznode_t *node DNS__DB_FLARG) {
uint_fast32_t refs = isc_refcount_decrement(&node->erefs);
#if DNS_DB_NODETRACE
fprintf(stderr, "decr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n",
func, file, line, node, refs - 1);
#endif
if (refs > 1) {
return false;
}
refs = isc_refcount_decrement(
&qpdb->node_locks[node->locknum].references);
#if DNS_DB_NODETRACE
fprintf(stderr,
"decr:nodelock:%s:%s:%u:%p:%p->references = "
"%" PRIuFAST32 "\n",
func, file, line, node, &qpdb->node_locks[node->locknum],
refs - 1);
#else
UNUSED(refs);
#endif
return true;
}
/*
* Caller must be holding the node lock; either the read or write lock.
* Note that the lock must be held even when node references are
@ -822,38 +851,17 @@ clean_zone_node(qpznode_t *node, uint32_t least_serial) {
static void
decref(qpzonedb_t *qpdb, qpznode_t *node, uint32_t least_serial,
isc_rwlocktype_t *nlocktypep DNS__DB_FLARG) {
int bucket = node->locknum;
db_nodelock_t *nodelock = &qpdb->node_locks[bucket];
uint_fast32_t refs;
REQUIRE(*nlocktypep != isc_rwlocktype_none);
refs = isc_refcount_decrement(&node->erefs);
#if DNS_DB_NODETRACE
fprintf(stderr, "decr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n",
func, file, line, node, refs - 1);
#endif
if (refs > 1) {
qpznode_unref(node);
return;
if (!qpznode_decref(qpdb, node DNS__DB_FLARG_PASS)) {
goto unref;
}
refs = isc_refcount_decrement(&nodelock->references);
#if DNS_DB_NODETRACE
fprintf(stderr,
"decr:nodelock:%s:%s:%u:%p:%p->references = "
"%" PRIuFAST32 "\n",
func, file, line, node, nodelock, refs - 1);
#else
UNUSED(refs);
#endif
/* Handle easy and typical case first. */
if (!node->dirty && (node->data != NULL || node == qpdb->origin ||
node == qpdb->nsec3_origin))
{
qpznode_unref(node);
return;
goto unref;
}
/*
@ -865,17 +873,12 @@ decref(qpzonedb_t *qpdb, qpznode_t *node, uint32_t least_serial,
/* Upgrade the lock? */
if (*nlocktypep == isc_rwlocktype_read) {
NODE_FORCEUPGRADE(&nodelock->lock, nlocktypep);
NODE_FORCEUPGRADE(&qpdb->node_locks[node->locknum].lock,
nlocktypep);
}
refs = isc_refcount_decrement(&node->erefs);
#if DNS_DB_NODETRACE
fprintf(stderr, "decr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n",
func, file, line, node, refs - 1);
#endif
if (refs > 1) {
qpznode_unref(node);
return;
if (!qpznode_decref(qpdb, node DNS__DB_FLARG_PASS)) {
goto unref;
}
if (node->dirty) {
@ -891,16 +894,7 @@ decref(qpzonedb_t *qpdb, qpznode_t *node, uint32_t least_serial,
clean_zone_node(node, least_serial);
}
refs = isc_refcount_decrement(&nodelock->references);
#if DNS_DB_NODETRACE
fprintf(stderr,
"decr:nodelock:%s:%s:%u:%p:%p->references = "
"%" PRIuFAST32 "\n",
func, file, line, node, nodelock, refs - 1);
#else
UNUSED(refs);
#endif
unref:
qpznode_unref(node);
}