Make the dns_slabheaders in the cache reference counted

Instead of only reference counting the enclosing qpcnode, add the
reference counting directly to the slabheaders.  This will allow us to
simplify the data model, as the slabheaders reference is now incremented
when we bind the rdataset and decremented when we disassociate the
rdataset.  This will allow us to remove the slabheader from the slabtop
directly instead of waiting for qpcnode to become completely
unreferenced.

diff --git a/lib/dns/include/dns/rdataslab.h b/lib/dns/include/dns/rdataslab.h
index a8d1a09a36ee6e2891ab77fb14f60e77d3766979..a1cfeedd49f1a97794f918b5cfa3160d1016bb77 100644 (file)
--- a/lib/dns/include/dns/rdataslab.h
+++ b/lib/dns/include/dns/rdataslab.h
@@ -41,6 +41,8 @@
  *** Imports
  ***/
 
+/* Add -DDNS_SLABHEADER_TRACE=1 to CFLAGS for detailed reference tracing */
+
 #include <stdalign.h>
 #include <stdbool.h>
 
@@ -90,6 +92,8 @@ struct dns_slabheader {
        _Atomic(uint16_t)    attributes;
        _Atomic(dns_trust_t) trust;
 
+       isc_refcount_t references;
+
        /*%
         * Locked by the owning node's lock.
         */
@@ -140,6 +144,20 @@ struct dns_slabheader {
        alignas(sizeof(void *)) unsigned char raw[];
 };
 
+#if DNS_SLABHEADER_TRACE
+#define dns_slabheader_ref(ptr) \
+       dns_slabheader__ref(ptr, __func__, __FILE__, __LINE__)
+#define dns_slabheader_unref(ptr) \
+       dns_slabheader__unref(ptr, __func__, __FILE__, __LINE__)
+#define dns_slabheader_attach(ptr, ptrp) \
+       dns_slabheader__attach(ptr, ptrp, __func__, __FILE__, __LINE__)
+#define dns_slabheader_detach(ptrp) \
+       dns_slabheader__detach(ptrp, __func__, __FILE__, __LINE__)
+ISC_REFCOUNT_TRACE_DECL(dns_slabheader);
+#else
+ISC_REFCOUNT_DECL(dns_slabheader);
+#endif
+
 enum {
        DNS_SLABHEADERATTR_NONEXISTENT = 1 << 0,
        DNS_SLABHEADERATTR_STALE = 1 << 1,
@@ -172,9 +190,14 @@ extern dns_rdatasetmethods_t dns_rdataslab_rdatasetmethods;
  *** Functions
  ***/
 
+#define dns_rdataslab_fromrdataset(rdataset, mctx, region, limit)            \
+       dns_rdataslab__fromrdataset(rdataset, mctx, region, limit, __func__, \
+                                   __FILE__, __LINE__)
 isc_result_t
-dns_rdataslab_fromrdataset(dns_rdataset_t *rdataset, isc_mem_t *mctx,
-                          isc_region_t *region, uint32_t limit);
+dns_rdataslab__fromrdataset(dns_rdataset_t *rdataset, isc_mem_t *mctx,
+                           isc_region_t *region, uint32_t limit,
+                           const char *func, const char *file,
+                           const unsigned int line);
 /*%<
  * Allocate space for a slab to hold the data in rdataset, and copy the
  * data into it.  The resulting slab will be returned in 'region'.
@@ -248,26 +271,26 @@ dns_rdataslab_equalx(dns_slabheader_t *header1, dns_slabheader_t *header2,
  *\li  true if the slabs are equal, #false otherwise.
  */
 
+#define dns_slabheader_reset(header, node) \
+       dns_slabheader__reset(header, node, __func__, __FILE__, __LINE__)
 void
-dns_slabheader_reset(dns_slabheader_t *h, dns_dbnode_t *node);
+dns_slabheader__reset(dns_slabheader_t *h, dns_dbnode_t *node, const char *func,
+                     const char *file, const unsigned int line);
 /*%<
  * Reset an rdataslab header 'h' so it can be used to store data in
  * database node 'node'.
  */
 
+#define dns_slabheader_new(mctx, node) \
+       dns_slabheader__new(mctx, node, __func__, __FILE__, __LINE__)
 dns_slabheader_t *
-dns_slabheader_new(isc_mem_t *mctx, dns_dbnode_t *node);
+dns_slabheader__new(isc_mem_t *mctx, dns_dbnode_t *node, const char *func,
+                   const char *file, const unsigned int line);
 /*%<
  * Allocate memory for an rdataslab header and initialize it for use
  * in database node 'node'.
  */
 
-void
-dns_slabheader_destroy(dns_slabheader_t **headerp);
-/*%<
- * Free all memory associated with '*headerp'.
- */
-
 void
 dns_slabheader_freeproof(isc_mem_t *mctx, dns_slabheader_proof_t **proof);
 /*%<

diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c
index 8250405ed1f438b2e914f6bd983c0c28c0540aaf..c31572fdc1e097dde37f2be6c38f9fa4fb4cb9b1 100644 (file)
--- a/lib/dns/qpcache.c
+++ b/lib/dns/qpcache.c
@@ -538,7 +538,7 @@ clean_cache_headers(dns_slabtop_t *top) {
        cds_list_for_each_entry_safe_from(header, header_next, &top->headers,
                                          headers_link) {
                cds_list_del(&header->headers_link);
-               dns_slabheader_destroy(&header);
+               dns_slabheader_detach(&header);
        }
 }
 
@@ -574,7 +574,7 @@ clean_cache_node(qpcache_t *qpdb, qpcnode_t *node) {
                    (STALE(header) && !KEEPSTALE(qpdb)))
                {
                        cds_list_del(&header->headers_link);
-                       dns_slabheader_destroy(&header);
+                       dns_slabheader_detach(&header);
                }
 
                /*
@@ -978,6 +978,8 @@ bindrdataset(qpcache_t *qpdb, qpcnode_t *node, dns_slabheader_t *header,
                return;
        }
 
+       dns_slabheader_ref(header);
+
        qpcnode_acquire(qpdb, node, nlocktype, tlocktype DNS__DB_FLARG_PASS);
 
        INSIST(rdataset->methods == NULL); /* We must be disassociated. */
@@ -2867,7 +2869,7 @@ qpcache_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
                update_rrsetstats(qpdb->rrsetstats, newheader->typepair,
                                  newheader->attributes, true);
        } else {
-               dns_slabheader_destroy(&newheader);
+               dns_slabheader_detach(&newheader);
        }
 
        NODE_UNLOCK(nlock, &nlocktype);
@@ -2884,7 +2886,7 @@ qpcache_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 
        return result;
 cleanup:
-       dns_slabheader_destroy(&newheader);
+       dns_slabheader_detach(&newheader);
        return result;
 }
 
@@ -2925,7 +2927,7 @@ qpcache_deleterdataset(dns_db_t *db, dns_dbnode_t *node,
        result = add(qpdb, qpnode, newheader, DNS_DBADD_FORCE, NULL, 0,
                     nlocktype, isc_rwlocktype_none DNS__DB_FLARG_PASS);
        if (result != ISC_R_SUCCESS) {
-               dns_slabheader_destroy(&newheader);
+               dns_slabheader_detach(&newheader);
        }
        NODE_UNLOCK(nlock, &nlocktype);
 
@@ -3516,7 +3518,7 @@ qpcnode_destroy(qpcnode_t *qpnode) {
                                             headers_link)
                {
                        cds_list_del(&header->headers_link);
-                       dns_slabheader_destroy(&header);
+                       dns_slabheader_detach(&header);
                }
 
                if (ISC_SIEVE_LINKED(top, link)) {

diff --git a/lib/dns/rdataslab.c b/lib/dns/rdataslab.c
index 06fe3487dcdfe7bf66abefbb1e582d507263380c..b917fdce33588d2604e015c6c36661ce1921d6ea 100644 (file)
--- a/lib/dns/rdataslab.c
+++ b/lib/dns/rdataslab.c
@@ -134,7 +134,8 @@ compare_rdata(const void *p1, const void *p2) {
 
 static unsigned char *
 newslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
-       uint16_t nitems, size_t size) {
+       uint16_t nitems, size_t size, const char *func, const char *file,
+       const unsigned int line) {
        dns_slabheader_t *header = isc_mem_get(mctx, size);
 
        *header = (dns_slabheader_t){
@@ -142,8 +143,20 @@ newslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
                .trust = rdataset->trust,
                .dirtylink = ISC_LINK_INITIALIZER,
                .nitems = nitems,
+               .references = ISC_REFCOUNT_INITIALIZER(1),
        };
 
+#if DNS_SLABHEADER_TRACE
+       fprintf(stderr,
+               "%s:%s:%s:%u:t%" PRItid ":%p->references = %" PRIuFAST32 "\n",
+               __func__, func, file, line, isc_tid(), header,
+               header->references);
+#else
+       UNUSED(func);
+       UNUSED(file);
+       UNUSED(line);
+#endif
+
        region->base = (unsigned char *)header;
        region->length = size;
 
@@ -152,7 +165,8 @@ newslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
 
 static isc_result_t
 makeslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
-        uint32_t maxrrperset) {
+        uint32_t maxrrperset, const char *func, const char *file,
+        const unsigned int line) {
        /*
         * Use &removed as a sentinel pointer for duplicate
         * rdata as rdata.data == NULL is valid.
@@ -178,8 +192,8 @@ makeslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
                dns_slabheader_t *header = rdataset_getheader(rdataset);
                buflen = dns_rdataslab_size(header);
 
-               rawbuf = newslab(rdataset, mctx, region, header->nitems,
-                                buflen);
+               rawbuf = newslab(rdataset, mctx, region, header->nitems, buflen,
+                                func, file, line);
 
                INSIST(headerlen <= buflen);
                memmove(rawbuf, (unsigned char *)header + headerlen,
@@ -196,7 +210,8 @@ makeslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
                if (rdataset->type != 0) {
                        return ISC_R_FAILURE;
                }
-               rawbuf = newslab(rdataset, mctx, region, 0, buflen);
+               rawbuf = newslab(rdataset, mctx, region, 0, buflen, func, file,
+                                line);
                return ISC_R_SUCCESS;
        }
 
@@ -308,7 +323,8 @@ makeslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
         * Allocate the memory, set up a buffer, start copying in
         * data.
         */
-       rawbuf = newslab(rdataset, mctx, region, nitems, buflen);
+       rawbuf = newslab(rdataset, mctx, region, nitems, buflen, func, file,
+                        line);
 
        for (i = 0; i < nalloc; i++) {
                if (rdata[i].data == &removed) {
@@ -344,15 +360,18 @@ free_rdatas:
 }
 
 isc_result_t
-dns_rdataslab_fromrdataset(dns_rdataset_t *rdataset, isc_mem_t *mctx,
-                          isc_region_t *region, uint32_t maxrrperset) {
+dns_rdataslab__fromrdataset(dns_rdataset_t *rdataset, isc_mem_t *mctx,
+                           isc_region_t *region, uint32_t maxrrperset,
+                           const char *func, const char *file,
+                           const unsigned int line) {
        if (rdataset->type == dns_rdatatype_none &&
            rdataset->covers == dns_rdatatype_none)
        {
                return DNS_R_DISALLOWED;
        }
 
-       isc_result_t result = makeslab(rdataset, mctx, region, maxrrperset);
+       isc_result_t result = makeslab(rdataset, mctx, region, maxrrperset,
+                                      func, file, line);
        if (result != ISC_R_SUCCESS) {
                return result;
        }
@@ -496,37 +515,60 @@ dns_rdataslab_equalx(dns_slabheader_t *slab1, dns_slabheader_t *slab2,
 }
 
 void
-dns_slabheader_reset(dns_slabheader_t *h, dns_dbnode_t *node) {
+dns_slabheader__reset(dns_slabheader_t *h, dns_dbnode_t *node, const char *func,
+                     const char *file, const unsigned int line) {
        h->node = node;
 
        atomic_init(&h->attributes, 0);
        atomic_init(&h->last_refresh_fail_ts, 0);
+       isc_refcount_init(&h->references, 1);
 
        ISC_LINK_INIT(h, dirtylink);
 
        STATIC_ASSERT(sizeof(h->attributes) == 2,
                      "The .attributes field of dns_slabheader_t needs to be "
                      "16-bit int type exactly.");
+
+#if DNS_SLABHEADER_TRACE
+       fprintf(stderr,
+               "%s:%s:%s:%u:t%" PRItid ":%p->references = %" PRIuFAST32 "\n",
+               __func__, func, file, line, isc_tid(), h, h->references);
+#else
+       UNUSED(func);
+       UNUSED(file);
+       UNUSED(line);
+#endif
 }
 
 dns_slabheader_t *
-dns_slabheader_new(isc_mem_t *mctx, dns_dbnode_t *node) {
+dns_slabheader__new(isc_mem_t *mctx, dns_dbnode_t *node, const char *func,
+                   const char *file, const unsigned int line) {
        dns_slabheader_t *h = NULL;
 
        h = isc_mem_get(mctx, sizeof(*h));
        *h = (dns_slabheader_t){
+               .headers_link = CDS_LIST_HEAD_INIT(h->headers_link),
                .node = node,
                .dirtylink = ISC_LINK_INITIALIZER,
+               .references = ISC_REFCOUNT_INITIALIZER(1),
        };
+
+#if DNS_SLABHEADER_TRACE
+       fprintf(stderr,
+               "%s:%s:%s:%u:t%" PRItid ":%p->references = %" PRIuFAST32 "\n",
+               __func__, func, file, line, isc_tid(), h, h->references);
+#else
+       UNUSED(func);
+       UNUSED(file);
+       UNUSED(line);
+#endif
+
        return h;
 }
 
-void
-dns_slabheader_destroy(dns_slabheader_t **headerp) {
+static void
+slabheader_destroy(dns_slabheader_t *header) {
        unsigned int size;
-       dns_slabheader_t *header = *headerp;
-
-       *headerp = NULL;
 
        isc_mem_t *mctx = header->node->mctx;
        dns_db_deletedata(header->node, header);
@@ -567,6 +609,12 @@ dns_slabheader_freeproof(isc_mem_t *mctx, dns_slabheader_proof_t **proofp) {
        isc_mem_put(mctx, proof, sizeof(*proof));
 }
 
+#if DNS_SLABHEADER_TRACE
+ISC_REFCOUNT_TRACE_IMPL(dns_slabheader, slabheader_destroy);
+#else
+ISC_REFCOUNT_IMPL(dns_slabheader, slabheader_destroy);
+#endif
+
 /* Fixed RRSet helper macros */
 
 static void
@@ -574,6 +622,8 @@ rdataset_disassociate(dns_rdataset_t *rdataset DNS__DB_FLARG) {
        dns_slabheader_t *header = rdataset_getheader(rdataset);
        dns_dbnode_t *node = header->node;
 
+       dns_slabheader_detach(&header);
+
        dns__db_detachnode(&node DNS__DB_FLARG_PASS);
 }
 
@@ -665,6 +715,8 @@ rdataset_clone(const dns_rdataset_t *source,
 
        target->slab.iter_pos = NULL;
        target->slab.iter_count = 0;
+
+       dns_slabheader_ref(header);
 }
 
 static unsigned int