From: Ondřej Surý Date: Wed, 19 Jul 2023 06:58:31 +0000 (+0200) Subject: Cleanup the __tsan_acquire/__tsan_release X-Git-Tag: v9.19.16~17^2 X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=b6b0d81a362b18bdbd5f6dc7f6aa3c470685d4ab;p=thirdparty%2Fbind9.git 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 afa513f5599..29259bdb2ca 100644 --- 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 110d7e5de96..1bcb85979fc 100644 --- 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 e3c0cae6953..bb54d2cb907 100644 --- 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 1afa2a3027a..351b213a14c 100644 --- 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 d1acf991219..cf62934632b 100644 --- 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 c8b6e1c99f2..25df4dbc34f 100644 --- 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 , 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 5a05aa439e4..ad58f338106 100644 --- 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("a->jobs.head, "a->jobs.tail, &job->wfcq_node); } diff --git a/lib/isc/thread.c b/lib/isc/thread.c index d0dc60c275f..4c0245dd36f 100644 --- 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);