[RFC,1/4] memory: Make memory_listeners RCU-safe for real

Commit Message

Peter Xu Feb. 25, 2023, 4:31 p.m. UTC

I think the plan was making memory_listeners rcu-safe, but maybe not
really.  This patch does it for real, by using RCU variances of qtailq
helpers when modifying memory_listeners.  The modification should be
serialized by BQL, add assertions to register/unregister functions.

Wait for a quiecent period before returning from unregister of memory
listener to make sure any rcu reader is accessing valid listeners.

AddressSpace.listeners are protected in the same way, so do the same.

Signed-off-by: Peter Xu <[e-mail]>
---
 softmmu/memory.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Patch

diff --git a/softmmu/memory.c b/softmmu/memory.c
index 9d64efca26..a63e0bcbb7 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -3016,30 +3016,32 @@  void memory_listener_register(MemoryListener *listener, AddressSpace *as)
 
     /* Only one of them can be defined for a listener */
     assert(!(listener->log_sync && listener->log_sync_global));
+    /* Ownership of memory_listeners & as->listeners for modifications */
+    assert(qemu_mutex_iothread_locked());
 
     listener->address_space = as;
     if (QTAILQ_EMPTY(&memory_listeners)
         || listener->priority >= QTAILQ_LAST(&memory_listeners)->priority) {
-        QTAILQ_INSERT_TAIL(&memory_listeners, listener, link);
+        QTAILQ_INSERT_TAIL_RCU(&memory_listeners, listener, link);
     } else {
         QTAILQ_FOREACH(other, &memory_listeners, link) {
             if (listener->priority < other->priority) {
                 break;
             }
         }
-        QTAILQ_INSERT_BEFORE(other, listener, link);
+        QTAILQ_INSERT_BEFORE_RCU(other, listener, link);
     }
 
     if (QTAILQ_EMPTY(&as->listeners)
         || listener->priority >= QTAILQ_LAST(&as->listeners)->priority) {
-        QTAILQ_INSERT_TAIL(&as->listeners, listener, link_as);
+        QTAILQ_INSERT_TAIL_RCU(&as->listeners, listener, link_as);
     } else {
         QTAILQ_FOREACH(other, &as->listeners, link_as) {
             if (listener->priority < other->priority) {
                 break;
             }
         }
-        QTAILQ_INSERT_BEFORE(other, listener, link_as);
+        QTAILQ_INSERT_BEFORE_RCU(other, listener, link_as);
     }
 
     listener_add_address_space(listener, as);
@@ -3051,9 +3053,14 @@  void memory_listener_unregister(MemoryListener *listener)
         return;
     }
 
+    /* Ownership of memory_listeners & as->listeners for modifications */
+    assert(qemu_mutex_iothread_locked());
+
     listener_del_address_space(listener, listener->address_space);
-    QTAILQ_REMOVE(&memory_listeners, listener, link);
-    QTAILQ_REMOVE(&listener->address_space->listeners, listener, link_as);
+    QTAILQ_REMOVE_RCU(&memory_listeners, listener, link);
+    QTAILQ_REMOVE_RCU(&listener->address_space->listeners, listener, link_as);
+    /* Wait for RCU readers to finish.  NOTE!  this may release BQL */
+    drain_call_rcu();
     listener->address_space = NULL;
 }