From: Ondřej Surý Date: Mon, 27 Jan 2025 17:06:17 +0000 (+0100) Subject: Refactor decref() in both qpcache.c and qpzone.c X-Git-Tag: v9.21.5~16^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=814b87da648b2471947e4f9ac25069ab83f38ea3;p=thirdparty%2Fbind9.git 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. --- diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index fd674facf68..aa645fbe445 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -625,30 +625,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); + if (refs > 0) { + return; + } - refs = isc_refcount_increment0( - &qpdb->node_locks[node->locknum].references); + /* + * 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); + 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); + UNUSED(refs); #endif - } } static void @@ -661,6 +663,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 @@ -677,42 +706,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; } /* @@ -724,18 +733,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) { @@ -775,21 +778,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. @@ -799,11 +791,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); } @@ -817,8 +810,8 @@ restore_locks: TREE_UNLOCK(&qpdb->tree_lock, tlocktypep); } +unref: qpcnode_unref(node); - return true; } static void @@ -2752,13 +2745,11 @@ qpcache_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); diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index 2b75a24a302..05c93a1fe20 100644 --- a/lib/dns/qpzone.c +++ b/lib/dns/qpzone.c @@ -734,20 +734,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 (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); + 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); + UNUSED(refs); #endif - } } static void @@ -873,6 +875,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 @@ -891,38 +920,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; } /* @@ -934,17 +942,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) { @@ -960,16 +963,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); }