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 c73eb7815f0c84874831fb5968820b073fd31f68..77aa285e0a62afe7965f0cf184a340d5db90a2fa 100644 (file)
--- 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 7633c4d5aa56bd1d9bb833efabb55086668de0a2..10d19e1b1b29d3c9070e15529f0949f89fc7806f 100644 (file)
--- 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 e9054da953d3a53a119cee5ee17a381bb10f17de..c0f69c35686e1a5966f5ea86e7ad84fcee102ad1 100644 (file)
--- 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 0b828d92e6b6b0ac198f236388a973f88aca2e8c..48a1ac46dad6a4bf7d3e4e2bd34bce2b485f878a 100644 (file)
--- 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 bef3e25558c45e9d1d4d79d1da59af309dc499d3..aba5dc1db827aea69c8f7cb09e5a0114095fbc85 100644 (file)
--- 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 ddbc83d37b6ec09cea04545647e90b30dd9ef41f..f9ebc49fe23b67c613005904a49e3baa8e000564 100644 (file)
--- 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 aff5eb3dc6b2c1e2d27eee50b0d6db04b9de87d1..392297746a1ddcbcc9a1df4ce11eb7a3979dd6a6 100644 (file)
--- 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,