From: Ondřej Surý Date: Thu, 18 Jun 2026 07:59:51 +0000 (+0200) Subject: Make the dns_slabheaders in the cache reference counted X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=db750e75af4104a3f96ef0f8abe6da4dae3a5a25;p=thirdparty%2Fbind9.git 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 a8d1a09a36..a1cfeedd49 100644 --- 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 #include @@ -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 8250405ed1..c31572fdc1 100644 --- 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 06fe3487dc..b917fdce33 100644 --- 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