Cleanup the __tsan_acquire/__tsan_release

With ThreadSanitizer support added to the Userspace RCU, we no longer
need to wrap the call_rcu and caa_container_of with
__tsan_{acquire,release} hints.  Remove the direct calls to
__tsan_{acquire,release} and the isc_urcu_{container,cleanup} macros.

diff --git a/lib/dns/qp.c b/lib/dns/qp.c
index afa513f5599a2cd1c1233a69a5f7069fe11f9a08..29259bdb2caace716f30cc8eb227de0949b0555b 100644 (file)
--- a/lib/dns/qp.c
+++ b/lib/dns/qp.c
@@ -628,7 +628,7 @@ recycle(dns_qp_t *qp) {
  */
 static void
 reclaim_chunks_cb(struct rcu_head *arg) {
-       qp_rcuctx_t *rcuctx = isc_urcu_container(arg, qp_rcuctx_t, rcu_head);
+       qp_rcuctx_t *rcuctx = caa_container_of(arg, qp_rcuctx_t, rcu_head);
        REQUIRE(QPRCU_VALID(rcuctx));
        dns_qpmulti_t *multi = rcuctx->multi;
        REQUIRE(QPMULTI_VALID(multi));
@@ -710,7 +710,7 @@ reclaim_chunks(dns_qpmulti_t *multi) {
                }
        }
 
-       isc_urcu_cleanup(rcuctx, rcu_head, reclaim_chunks_cb);
+       call_rcu(&rcuctx->rcu_head, reclaim_chunks_cb);
 
        LOG_STATS("qp will reclaim %u chunks", count);
 }
@@ -1429,7 +1429,7 @@ dns_qp_destroy(dns_qp_t **qptp) {
 
 static void
 qpmulti_destroy_cb(struct rcu_head *arg) {
-       qp_rcuctx_t *rcuctx = isc_urcu_container(arg, qp_rcuctx_t, rcu_head);
+       qp_rcuctx_t *rcuctx = caa_container_of(arg, qp_rcuctx_t, rcu_head);
        REQUIRE(QPRCU_VALID(rcuctx));
        /* only nonzero for reclaim_chunks_cb() */
        REQUIRE(rcuctx->count == 0);
@@ -1476,7 +1476,7 @@ dns_qpmulti_destroy(dns_qpmulti_t **qpmp) {
                .multi = multi,
        };
        isc_mem_attach(qp->mctx, &rcuctx->mctx);
-       isc_urcu_cleanup(rcuctx, rcu_head, qpmulti_destroy_cb);
+       call_rcu(&rcuctx->rcu_head, qpmulti_destroy_cb);
 }
 
 /***********************************************************************

diff --git a/lib/dns/rbt-zonedb.c b/lib/dns/rbt-zonedb.c
index 110d7e5de966f0f2beea106ca0dbe777b6a6cee9..1bcb85979fcac04a6ebc973dcd5d4291dbd4506f 100644 (file)
--- a/lib/dns/rbt-zonedb.c
+++ b/lib/dns/rbt-zonedb.c
@@ -2390,7 +2390,6 @@ addglue(dns_db_t *db, dns_dbversion_t *version, dns_rdataset_t *rdataset,
                        dns__rbtdb_freeglue(glue);
                        glue = old_glue;
                } else if (glue != NULL) {
-                       __tsan_release(glue);
                        cds_wfs_push(&rbtversion->glue_stack,
                                     &header->wfs_node);
                }
@@ -2411,7 +2410,6 @@ addglue(dns_db_t *db, dns_dbversion_t *version, dns_rdataset_t *rdataset,
         * zone.
         */
        if (glue != (void *)-1) {
-               __tsan_acquire(glue);
                addglue_to_message(glue, msg);
        }
 

diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
index e3c0cae69539987e88719aae04f411d495c9af7c..bb54d2cb907fb897ef274a27250c93712d655fbb 100644 (file)
--- a/lib/dns/rbtdb.c
+++ b/lib/dns/rbtdb.c
@@ -4658,7 +4658,7 @@ dns__rbtdb_freeglue(dns_glue_t *glue_list) {
 
 static void
 free_gluelist_rcu(struct rcu_head *rcu_head) {
-       dns_glue_t *glue = isc_urcu_container(rcu_head, dns_glue_t, rcu_head);
+       dns_glue_t *glue = caa_container_of(rcu_head, dns_glue_t, rcu_head);
 
        dns__rbtdb_freeglue(glue);
 }
@@ -4674,7 +4674,7 @@ free_gluetable(dns_rbtdb_version_t *rbtversion) {
                        caa_container_of(node, dns_slabheader_t, wfs_node);
                dns_glue_t *glue = rcu_xchg_pointer(&header->glue_list, NULL);
 
-               isc_urcu_cleanup(glue, rcu_head, free_gluelist_rcu);
+               call_rcu(&glue->rcu_head, free_gluelist_rcu);
        }
        rcu_read_unlock();
 }

diff --git a/lib/isc/async.c b/lib/isc/async.c
index 1afa2a3027af3264279cd6b149b32b5d39692bff..351b213a14c9ce73424e1bc8fe0d64f54db7c063 100644 (file)
--- a/lib/isc/async.c
+++ b/lib/isc/async.c
@@ -57,7 +57,6 @@ isc_async_run(isc_loop_t *loop, isc_job_cb cb, void *cbarg) {
         * The function returns 'false' in case the queue was empty - in such
         * case we need to trigger the async callback.
         */
-       __tsan_release(job);
        if (!cds_wfcq_enqueue(&loop->async_jobs.head, &loop->async_jobs.tail,
                              &job->wfcq_node))
        {
@@ -107,7 +106,7 @@ isc__async_cb(uv_async_t *handle) {
         */
        struct cds_wfcq_node *node, *next;
        __cds_wfcq_for_each_blocking_safe(&jobs.head, &jobs.tail, node, next) {
-               isc_job_t *job = isc_urcu_container(node, isc_job_t, wfcq_node);
+               isc_job_t *job = caa_container_of(node, isc_job_t, wfcq_node);
 
                job->cb(job->cbarg);
 

diff --git a/lib/isc/include/isc/urcu.h b/lib/isc/include/isc/urcu.h
index d1acf991219654231c49e4117136f42fdec75f88..cf62934632be1d50d35c8fac30f13f955f38ac71 100644 (file)
--- a/lib/isc/include/isc/urcu.h
+++ b/lib/isc/include/isc/urcu.h
@@ -36,35 +36,6 @@
 
 #pragma GCC diagnostic pop
 
-/*
- * Help thread sanitizer to understand `call_rcu()`:
- *
- * The `rcu_head` argument to `call_rcu()` is expected to be embedded
- * in a structure defined by the caller, which is named `_rcuctx` in
- * these macros. The callback function uses `caa_container_of()` to
- * recover the `rcuctx` pointer from the `_rcu_head` pointer that is
- * passed to the callback.
- *
- * We explain the ordering dependency to tsan by releasing `_rcuctx`
- * pointer before `call_rcu()` and acquiring it in the callback
- * funtion. We pass the outer `_rcuctx` pointer to the `__tsan_`
- * barriers, because it should match a pointer that is known by tsan
- * to have been returned by `malloc()`.
- */
-
-#define isc_urcu_cleanup(ptr, member, func)     \
-       {                                       \
-               __tsan_release(ptr);            \
-               call_rcu(&(ptr)->member, func); \
-       }
-
-#define isc_urcu_container(ptr, type, member)                     \
-       ({                                                        \
-               type *_ptr = caa_container_of(ptr, type, member); \
-               __tsan_acquire(_ptr);                             \
-               _ptr;                                             \
-       })
-
 #if defined(RCU_QSBR)
 
 /*

diff --git a/lib/isc/include/isc/util.h b/lib/isc/include/isc/util.h
index c8b6e1c99f25199d79cb3c4ad5e666f6b9712a6b..25df4dbc34fd0da62b93d2c31ba47efa7153660f 100644 (file)
--- a/lib/isc/include/isc/util.h
+++ b/lib/isc/include/isc/util.h
@@ -261,19 +261,9 @@
 #endif /* if __has_feature(thread_sanitizer) */
 
 #if __SANITIZE_THREAD__
-/*
- * We should rather be including <sanitizer/tsan_interface.h>, but GCC 10
- * header is broken, so we just make the declarations by hand.
- */
-void
-__tsan_acquire(void *addr);
-void
-__tsan_release(void *addr);
 #define ISC_NO_SANITIZE_THREAD __attribute__((no_sanitize("thread")))
 #else /* if __SANITIZE_THREAD__ */
 #define ISC_NO_SANITIZE_THREAD
-#define __tsan_acquire(addr)
-#define __tsan_release(addr)
 #endif /* if __SANITIZE_THREAD__ */
 
 #if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR >= 6)

diff --git a/lib/isc/quota.c b/lib/isc/quota.c
index 5a05aa439e48d3d85046a933e1c0d39f1b3947f7..ad58f3381063d4e14801e97fab4978b774a50c77 100644 (file)
--- a/lib/isc/quota.c
+++ b/lib/isc/quota.c
@@ -79,7 +79,7 @@ isc_quota_release(isc_quota_t *quota) {
                return;
        }
 
-       isc_job_t *job = isc_urcu_container(node, isc_job_t, wfcq_node);
+       isc_job_t *job = caa_container_of(node, isc_job_t, wfcq_node);
        job->cb(job->cbarg);
 }
 
@@ -102,7 +102,6 @@ isc_quota_acquire_cb(isc_quota_t *quota, isc_job_t *job, isc_job_cb cb,
                         * The cds_wfcq_enqueue() is non-blocking (no internal
                         * mutex involved), so it offers a slight advantage.
                         */
-                       __tsan_release(job);
                        cds_wfcq_enqueue(&quota->jobs.head, &quota->jobs.tail,
                                         &job->wfcq_node);
                }

diff --git a/lib/isc/thread.c b/lib/isc/thread.c
index d0dc60c275fa83048b7286eddc14d4afebc994e6..4c0245dd36ff979f2305e728ffe50866d74642c2 100644 (file)
--- a/lib/isc/thread.c
+++ b/lib/isc/thread.c
@@ -62,7 +62,6 @@ thread_wrap(isc_threadfunc_t func, void *arg) {
                .func = func,
                .arg = arg,
        };
-       __tsan_release(wrap);
        return (wrap);
 }
 
@@ -81,8 +80,6 @@ thread_body(struct thread_wrap *wrap) {
        CMM_ACCESS_ONCE(jemalloc_enforce_init) = malloc(1);
        free(jemalloc_enforce_init);
 
-       /* Reassure Thread Sanitizer that it is safe to free the wrapper */
-       __tsan_acquire(wrap);
        free(wrap);
 
        ret = func(arg);