Re: [Qemu-devel] [PATCH v2 00/11] convert CPU list to RCU

[Paolo Bonzini <pbonzini@redhat.com>]

On 19/08/2018 11:13, Emilio G. Cota wrote:
> - Add some fixes for test-rcu-list. I wanted to be able to get no
>   races with ThreadSanitizer, but it still warns about two races.
>   I'm appending the report just in case, but I think tsan is getting
>   confused.

I cannot understand the first.  The second might be fixed by this untested
patch (the second hunk; the first is an optimization and clarification):

diff --git a/util/rcu.c b/util/rcu.c
index 5676c22bd1..314b7db1a6 100644
--- a/util/rcu.c
+++ b/util/rcu.c
@@ -192,7 +192,9 @@ static void enqueue(struct rcu_head *node)
 
     node->next = NULL;
     old_tail = atomic_xchg(&tail, &node->next);
-    atomic_mb_set(old_tail, node);
+
+    /* Pairs with smb_mb_acquire() in try_dequeue.  */
+    atomic_store_release(old_tail, node);
 }
 
 static struct rcu_head *try_dequeue(void)
@@ -213,7 +215,10 @@ retry:
      * wrong and we need to wait until its enqueuer finishes the update.
      */
     node = head;
-    next = atomic_mb_read(&head->next);
+    smp_mb_acquire();
+
+    /* atomic_read is enough because the pointer is never dereferenced.  */
+    next = atomic_read(&head->next);
     if (!next) {
         return NULL;
     }


The idea being that enqueue() writes so to speak node->prev->next in
the atomic_store_release when it enqueues node; try_dequeue() instead reads
node->next, so there is no synchronizes-with edge.  However, I'm not that
convinced that it's necessary...

Paolo