From: Tony Finch <fanf@isc.org>
Date: Mon, 15 May 2023 10:42:33 +0000 (+0100)
Subject: Fixes for liburcu-qsbr
X-Git-Tag: v9.19.14~47^2
X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=c319ccd4c9bb3f2d2b41ea7d67b0b79a33382fab;p=thirdparty%2Fbind9.git

Fixes for liburcu-qsbr

Move registration and deregistration of the main thread from
`isc_loopmgr_run()` into `isc__initialize()` / `isc__shutdown()`:
liburcu-qsbr fails an assertion if we try to use it from an
unregistered thread, and we need to be able to use it when the
event loops are not running.

Use `rcu_assign_pointer()` and `rcu_dereference()` in qp-trie
transactions so that they properly mark threads as online. The
RCU-protected pointer is no longer declared atomic because
liburcu does not (yet) use standard C atomics.

Fix the definition of `isc_qsbr_rcu_dereference()` to return
the referenced value, and to call the right function inside
liburcu.

Change the thread sanitizer suppressions to match any variant of
`rcu_*_barrier()`
---

diff --git a/.tsan-suppress b/.tsan-suppress
index c73eb7815f0..77aa285e0a6 100644
--- a/.tsan-suppress
+++ b/.tsan-suppress
@@ -1,4 +1,4 @@
 # be more selective with liburcu
 race:rcu_barrier
-race:rcu_memb_barrier
+race:rcu_*_barrier
 thread:*
diff --git a/lib/dns/qp.c b/lib/dns/qp.c
index 7633c4d5aa5..10d19e1b1b2 100644
--- a/lib/dns/qp.c
+++ b/lib/dns/qp.c
@@ -1169,8 +1169,7 @@ dns_qpmulti_commit(dns_qpmulti_t *multi, dns_qp_t **qptp) {
 	/* paired with chunk_free() */
 	isc_refcount_increment(&qp->base->refcount);
 
-	/* reader_open() below has the matching atomic_load_acquire() */
-	atomic_store_release(&multi->reader, reader); /* COMMIT */
+	rcu_assign_pointer(multi->reader, reader); /* COMMIT */
 
 	/* clean up what we can right now */
 	if (qp->transaction_mode == QP_UPDATE || QP_NEEDGC(qp)) {
@@ -1249,8 +1248,7 @@ dns_qpmulti_rollback(dns_qpmulti_t *multi, dns_qp_t **qptp) {
 static dns_qpmulti_t *
 reader_open(dns_qpmulti_t *multi, dns_qpreadable_t qpr) {
 	dns_qpreader_t *qp = dns_qpreader(qpr);
-	/* dns_qpmulti_commit() has the matching atomic_store_release() */
-	qp_node_t *reader = atomic_load_acquire(&multi->reader);
+	qp_node_t *reader = rcu_dereference(multi->reader);
 	if (reader == NULL) {
 		QP_INIT(qp, multi->writer.methods, multi->writer.uctx);
 	} else {
diff --git a/lib/dns/qp_p.h b/lib/dns/qp_p.h
index e9054da953d..c0f69c35686 100644
--- a/lib/dns/qp_p.h
+++ b/lib/dns/qp_p.h
@@ -549,8 +549,8 @@ struct dns_qp {
  */
 struct dns_qpmulti {
 	uint32_t magic;
-	/*% pointer to current packed reader */
-	atomic_ptr(qp_node_t) reader;
+	/*% RCU-protected pointer to current packed reader */
+	qp_node_t *reader;
 	/*% the mutex protects the rest of this structure */
 	isc_mutex_t mutex;
 	/*% ref_ptr(writer, reader_ref) == reader */
diff --git a/lib/isc/include/isc/urcu.h b/lib/isc/include/isc/urcu.h
index 0b828d92e6b..48a1ac46dad 100644
--- a/lib/isc/include/isc/urcu.h
+++ b/lib/isc/include/isc/urcu.h
@@ -109,12 +109,12 @@
 #define synchronize_rcu() isc_qsbr_syncronize_rcu()
 
 #define isc_qsbr_rcu_dereference(ptr)              \
-	{                                          \
+	({                                         \
 		if (!urcu_qsbr_read_ongoing()) {   \
 			urcu_qsbr_thread_online(); \
 		}                                  \
-		urcu_qsbr_dereference(ptr);        \
-	}
+		_rcu_dereference(ptr);             \
+	})
 
 #undef rcu_dereference
 #define rcu_dereference(ptr) isc_qsbr_rcu_dereference(ptr)
diff --git a/lib/isc/lib.c b/lib/isc/lib.c
index bef3e25558c..aba5dc1db82 100644
--- a/lib/isc/lib.c
+++ b/lib/isc/lib.c
@@ -18,6 +18,7 @@
 #include <isc/mem.h>
 #include <isc/os.h>
 #include <isc/tls.h>
+#include <isc/urcu.h>
 #include <isc/util.h>
 #include <isc/uv.h>
 #include <isc/xml.h>
@@ -51,6 +52,7 @@ isc__initialize(void) {
 	isc__md_initialize();
 	isc__iterated_hash_initialize();
 	(void)isc_os_ncpus();
+	rcu_register_thread();
 }
 
 void
@@ -63,4 +65,6 @@ isc__shutdown(void) {
 	isc__mem_shutdown();
 	isc__mutex_shutdown();
 	isc__os_shutdown();
+	/* should be after isc__mem_shutdown() which calls rcu_barrier() */
+	rcu_unregister_thread();
 }
diff --git a/lib/isc/loop.c b/lib/isc/loop.c
index ddbc83d37b6..f9ebc49fe23 100644
--- a/lib/isc/loop.c
+++ b/lib/isc/loop.c
@@ -436,8 +436,6 @@ isc_loopmgr_run(isc_loopmgr_t *loopmgr) {
 
 	bool free_call_rcu_data = !create_all_cpu_call_rcu_data(0);
 
-	rcu_register_thread();
-
 	/*
 	 * The thread 0 is this one.
 	 */
@@ -453,8 +451,6 @@ isc_loopmgr_run(isc_loopmgr_t *loopmgr) {
 
 	isc_thread_main(loop_thread, &loopmgr->loops[0]);
 
-	rcu_unregister_thread();
-
 	rcu_barrier();
 
 	if (free_call_rcu_data) {
diff --git a/tests/libtest/qp.c b/tests/libtest/qp.c
index aff5eb3dc6b..392297746a1 100644
--- a/tests/libtest/qp.c
+++ b/tests/libtest/qp.c
@@ -232,7 +232,7 @@ qp_test_dumpqp(dns_qp_t *qp) {
 void
 qp_test_dumpmulti(dns_qpmulti_t *multi) {
 	dns_qpreader_t qpr;
-	qp_node_t *reader = atomic_load(&multi->reader);
+	qp_node_t *reader = rcu_dereference(multi->reader);
 	dns_qpmulti_t *whence = unpack_reader(&qpr, reader);
 	dumpqp(&multi->writer, "qpmulti->writer");
 	printf("qpmulti->reader %p root_ref %u %u:%u base %p\n", reader,