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 fd674facf68a35d6c4bb16e94694171d27cc8688..aa645fbe44594ea386de48c52df2429672cc0aab 100644 (file)
--- 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 2b75a24a302246332b2ad7bb8e6fc9208112b00f..05c93a1fe20e3bba96d0abeca7723059fbcf3394 100644 (file)
--- 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);
 }