Improve qp-trie refcount debugging

Add some qp-trie tracing macros which can be enabled by a
developer. These print a message when a leaf is attached or
detached, indicating which part of the qp-trie implementation
did so. The refcount methods must now return the refcount value
so it can be printed by the trace macros.

diff --git a/fuzz/dns_qp.c b/fuzz/dns_qp.c
index 3f0b674c140e64cb268459ba27d8ea859afaba01..a528019597c119ae9c6bc3b22b66cf19c6bf558a 100644 (file)
--- a/fuzz/dns_qp.c
+++ b/fuzz/dns_qp.c
@@ -57,18 +57,19 @@ static struct {
        dns_qpkey_t ascii;
 } item[256 * 256 / 4];
 
-static void
+static uint32_t
 fuzz_attach(void *ctx, void *pval, uint32_t ival) {
        assert(ctx == NULL);
        assert(pval == &item[ival]);
-       item[ival].refcount++;
+       return (item[ival].refcount++);
 }
 
-static void
+static uint32_t
 fuzz_detach(void *ctx, void *pval, uint32_t ival) {
        assert(ctx == NULL);
        assert(pval == &item[ival]);
-       item[ival].refcount--;
+       assert(item[ival].refcount > 0);
+       return (item[ival].refcount--);
 }
 
 static size_t

diff --git a/lib/dns/include/dns/qp.h b/lib/dns/include/dns/qp.h
index 4604d6cee0d93291b1be3ea9ad0223e7dc8edbab..ef676de7d435e06554074e1128c04b13813c221c 100644 (file)
--- a/lib/dns/include/dns/qp.h
+++ b/lib/dns/include/dns/qp.h
@@ -180,8 +180,9 @@ typedef uint8_t dns_qpkey_t[512];
  * The `attach` and `detach` methods adjust reference counts on value
  * objects. They support copy-on-write and safe memory reclamation
  * needed for multi-version concurrency. The methods are only called
- * when the `dns_qpmulti_t` mutex is held, so they only need to use
- * atomic ops if the refcounts are used by code other than the qp-trie.
+ * when the `dns_qpmulti_t` mutex is held. For tracing purposes, they
+ * should return the same value as `isc_refcount_increment()` or
+ * `isc_refcount_decrement()`, respectively
  *
  * Note: When a value object reference count is greater than one, the
  * object is in use by concurrent readers so it must not be modified. A
@@ -199,8 +200,8 @@ typedef uint8_t dns_qpkey_t[512];
  * readable identifier into `buf` which has max length `size`.
  */
 typedef struct dns_qpmethods {
-       void (*attach)(void *uctx, void *pval, uint32_t ival);
-       void (*detach)(void *uctx, void *pval, uint32_t ival);
+       uint32_t (*attach)(void *uctx, void *pval, uint32_t ival);
+       uint32_t (*detach)(void *uctx, void *pval, uint32_t ival);
        size_t (*makekey)(dns_qpkey_t key, void *uctx, void *pval,
                          uint32_t ival);
        void (*triename)(void *uctx, char *buf, size_t size);

diff --git a/lib/dns/qp_p.h b/lib/dns/qp_p.h
index cbaefb6f880a0aec5c3a16cc401b2bbad5e29225..5fc4cdf8018f1b3b56b8833b903258b0943daa78 100644 (file)
--- a/lib/dns/qp_p.h
+++ b/lib/dns/qp_p.h
@@ -890,6 +890,34 @@ unpack_reader(dns_qpreader_t *qp, qp_node_t *reader) {
  *  method invocation helpers
  */
 
+#if 0
+
+#define attach_leaf(qp, n)                                                  \
+       do {                                                                \
+               uint32_t iv = leaf_ival(n);                                 \
+               void *pv = leaf_pval(n);                                    \
+               uint32_t r = qp->methods->attach(qp->uctx, pv, iv);         \
+               fprintf(stderr,                                             \
+                       "%s:%u:%s():t%u qp %p node %p leaf %p %u "          \
+                       "(%u -> %u)\n",                                     \
+                       __FILE__, __LINE__, __func__, isc_tid(), qp, n, pv, \
+                       iv, r, r + 1);                                      \
+       } while (0)
+
+#define detach_leaf(qp, n)                                                  \
+       do {                                                                \
+               uint32_t iv = leaf_ival(n);                                 \
+               void *pv = leaf_pval(n);                                    \
+               uint32_t r = qp->methods->detach(qp->uctx, pv, iv);         \
+               fprintf(stderr,                                             \
+                       "%s:%u:%s():t%u qp %p node %p leaf %p %u "          \
+                       "(%u -> %u)\n",                                     \
+                       __FILE__, __LINE__, __func__, isc_tid(), qp, n, pv, \
+                       iv, r, r - 1);                                      \
+       } while (0)
+
+#else
+
 static inline void
 attach_leaf(dns_qpreadable_t qpr, qp_node_t *n) {
        dns_qpreader_t *qp = dns_qpreader(qpr);
@@ -902,6 +930,8 @@ detach_leaf(dns_qpreadable_t qpr, qp_node_t *n) {
        qp->methods->detach(qp->uctx, leaf_pval(n), leaf_ival(n));
 }
 
+#endif
+
 static inline size_t
 leaf_qpkey(dns_qpreadable_t qpr, qp_node_t *n, dns_qpkey_t key) {
        dns_qpreader_t *qp = dns_qpreader(qpr);

diff --git a/tests/bench/load-names.c b/tests/bench/load-names.c
index 455f20928fdeae82454a4255e4e8872973bcb4cc..99c5325dd4b2cfeb994cab9c9a3e906679fef3e9 100644 (file)
--- a/tests/bench/load-names.c
+++ b/tests/bench/load-names.c
@@ -37,10 +37,11 @@ struct {
        dns_fixedname_t fixed;
 } item[1024 * 1024];
 
-static void
+static uint32_t
 item_check(void *ctx, void *pval, uint32_t ival) {
        UNUSED(ctx);
        assert(pval == &item[ival]);
+       return (1);
 }
 
 static size_t

diff --git a/tests/bench/qp-dump.c b/tests/bench/qp-dump.c
index 18c83851d4d15f33ee503f5daf372f34a43de602..515048ea97fdde7c87b1294092cb42e0f892f0fd 100644 (file)
--- a/tests/bench/qp-dump.c
+++ b/tests/bench/qp-dump.c
@@ -95,17 +95,19 @@ qpkey_from_smallname(dns_qpkey_t key, void *ctx, void *pval, uint32_t ival) {
        return (dns_qpkey_fromname(key, &name));
 }
 
-static void
+static uint32_t
 smallname_attach(void *ctx, void *pval, uint32_t ival) {
        UNUSED(ctx);
-       isc_refcount_increment0(smallname_refcount(pval, ival));
+       return (isc_refcount_increment0(smallname_refcount(pval, ival)));
 }
 
-static void
+static uint32_t
 smallname_detach(void *ctx, void *pval, uint32_t ival) {
-       if (isc_refcount_decrement(smallname_refcount(pval, ival)) == 1) {
+       uint32_t refs = isc_refcount_decrement(smallname_refcount(pval, ival));
+       if (refs == 1) {
                isc_mem_free(ctx, pval);
        }
+       return (refs);
 }
 
 static void

diff --git a/tests/bench/qpmulti.c b/tests/bench/qpmulti.c
index ed072804a97d198f7b557965c5a82a600ab6b8c5..59c9a11a3efdafa49f552fda95eed412abc96d07 100644 (file)
--- a/tests/bench/qpmulti.c
+++ b/tests/bench/qpmulti.c
@@ -89,11 +89,12 @@ static struct {
        dns_qpkey_t key;
 } *item;
 
-static void
+static uint32_t
 item_refcount(void *ctx, void *pval, uint32_t ival) {
        UNUSED(ctx);
        UNUSED(pval);
        UNUSED(ival);
+       return (1);
 }
 
 static size_t

diff --git a/tests/dns/qpmulti_test.c b/tests/dns/qpmulti_test.c
index dede4951ba04a55e71d332c2795873ff3ed205c0..28605080e0a6ddce8f5d4a43dc14a8e162a92199 100644 (file)
--- a/tests/dns/qpmulti_test.c
+++ b/tests/dns/qpmulti_test.c
@@ -104,18 +104,19 @@ static struct {
        dns_qpkey_t ascii;
 } item[ITEM_COUNT];
 
-static void
+static uint32_t
 item_attach(void *ctx, void *pval, uint32_t ival) {
-       INSIST(ctx == NULL);
-       INSIST(pval == &item[ival]);
-       item[ival].refcount++;
+       assert_null(ctx);
+       assert_ptr_equal(pval, &item[ival]);
+       return (item[ival].refcount++);
 }
 
-static void
+static uint32_t
 item_detach(void *ctx, void *pval, uint32_t ival) {
        assert_null(ctx);
        assert_ptr_equal(pval, &item[ival]);
-       item[ival].refcount--;
+       assert_int_not_equal(item[ival].refcount, 0);
+       return (item[ival].refcount--);
 }
 
 static size_t