[Qemu-devel] [PATCH v2 2/6] hostmem: AddressSpace has its own map and maintained by RCU prepared style

[Liu Ping Fan <qemulist@gmail.com>]

From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>

Each address space will have a map between its hwaddr and hva,
this is expressed as the struct AddrSpaceMem. Currently only
address space of system memory's map is used by virtio device,
and the map is stored in AddrSpaceMem *system_mem.

The map is maintained by RCU prepared style, cur_hostmem,
next_hostmem, cur_lock fields in AddrSpaceMem help to access
the aim. cur_hostmem is used to search, next_hostmem is used to
update and will substitue cur_hostmem when done.

Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
---
 hw/virtio/dataplane/hostmem.c         |  133 +++++++++++++++++++++++----------
 include/hw/virtio/dataplane/hostmem.h |   20 +++--
 2 files changed, 103 insertions(+), 50 deletions(-)

diff --git a/hw/virtio/dataplane/hostmem.c b/hw/virtio/dataplane/hostmem.c
index 756b09f..1fd3e06 100644
--- a/hw/virtio/dataplane/hostmem.c
+++ b/hw/virtio/dataplane/hostmem.c
@@ -14,7 +14,7 @@
 #include "exec/address-spaces.h"
 #include "hw/virtio/dataplane/hostmem.h"
 
-HostMem *system_mem;
+static AddrSpaceMem *system_mem;
 
 static void hostmem_finalize(void);
 
@@ -32,17 +32,39 @@ static int hostmem_lookup_cmp(const void *phys_, const void *region_)
     }
 }
 
-/**
- * Map guest physical address to host pointer
- */
-void *hostmem_lookup(hwaddr phys, hwaddr len, bool is_write)
+static void hostmem_ref(HostMem *hostmem)
+{
+    int t;
+
+    t = __sync_add_and_fetch(&hostmem->ref, 1);
+    assert(t > 0);
+}
+
+static void hostmem_unref(HostMem *hostmem)
+{
+    int t;
+
+    t = __sync_sub_and_fetch(&hostmem->ref, 1);
+    assert(t >= 0);
+    if (!t) {
+        g_free(hostmem->current_regions);
+        g_free(hostmem);
+    }
+}
+
+static void *address_space_mem_lookup(AddrSpaceMem *as_mem, hwaddr phys,
+    hwaddr len, bool is_write)
 {
     HostMemRegion *region;
     void *host_addr = NULL;
     hwaddr offset_within_region;
-    HostMem *hostmem = system_mem;
+    HostMem *hostmem;
+
+    qemu_mutex_lock(&as_mem->cur_lock);
+    hostmem = as_mem->cur_hostmem;
+    hostmem_ref(hostmem);
+    qemu_mutex_unlock(&as_mem->cur_lock);
 
-    qemu_mutex_lock(&hostmem->current_regions_lock);
     region = bsearch(&phys, hostmem->current_regions,
                      hostmem->num_current_regions,
                      sizeof(hostmem->current_regions[0]),
@@ -57,28 +79,45 @@ void *hostmem_lookup(hwaddr phys, hwaddr len, bool is_write)
     if (len <= region->size - offset_within_region) {
         host_addr = region->host_addr + offset_within_region;
     }
-out:
-    qemu_mutex_unlock(&hostmem->current_regions_lock);
 
+out:
+    hostmem_unref(hostmem);
     return host_addr;
 }
 
 /**
- * Install new regions list
+ * Map guest physical address to host pointer
  */
-static void hostmem_listener_commit(MemoryListener *listener)
+void *hostmem_lookup(hwaddr phys, hwaddr len, bool is_write)
 {
-    HostMem *hostmem = container_of(listener, HostMem, listener);
+    return address_space_mem_lookup(system_mem, phys, len, is_write);
+}
 
-    qemu_mutex_lock(&hostmem->current_regions_lock);
-    g_free(hostmem->current_regions);
-    hostmem->current_regions = hostmem->new_regions;
-    hostmem->num_current_regions = hostmem->num_new_regions;
-    qemu_mutex_unlock(&hostmem->current_regions_lock);
+static void hostmem_listener_begin(MemoryListener *listener)
+{
+    AddrSpaceMem *as_mem = container_of(listener, AddrSpaceMem, listener);
+
+    as_mem->next_hostmem = g_new0(HostMem, 1);
+    as_mem->next_hostmem->ref = 1;
+}
 
-    /* Reset new regions list */
-    hostmem->new_regions = NULL;
-    hostmem->num_new_regions = 0;
+/**
+ * Install new regions list
+ */
+static void hostmem_listener_commit(MemoryListener *listener)
+{
+    HostMem *tmp;
+    AddrSpaceMem *as_mem = container_of(listener, AddrSpaceMem, listener);
+
+    /* writer of cur_hostmem &next_hostmem is serialed by biglock
+         * in hotplug path. So only take care of r/w on cur_hostmem
+         */
+    tmp = as_mem->cur_hostmem;
+    qemu_mutex_lock(&as_mem->cur_lock);
+    as_mem->cur_hostmem = as_mem->next_hostmem;
+    qemu_mutex_unlock(&as_mem->cur_lock);
+    as_mem->next_hostmem = NULL;
+    hostmem_unref(tmp);
 }
 
 /**
@@ -88,23 +127,23 @@ static void hostmem_append_new_region(HostMem *hostmem,
                                       MemoryRegionSection *section)
 {
     void *ram_ptr = memory_region_get_ram_ptr(section->mr);
-    size_t num = hostmem->num_new_regions;
-    size_t new_size = (num + 1) * sizeof(hostmem->new_regions[0]);
+    size_t num = hostmem->num_current_regions;
+    size_t new_size = (num + 1) * sizeof(hostmem->current_regions[0]);
 
-    hostmem->new_regions = g_realloc(hostmem->new_regions, new_size);
-    hostmem->new_regions[num] = (HostMemRegion){
+    hostmem->current_regions = g_realloc(hostmem->current_regions, new_size);
+    hostmem->current_regions[num] = (HostMemRegion){
         .host_addr = ram_ptr + section->offset_within_region,
         .guest_addr = section->offset_within_address_space,
         .size = section->size,
         .readonly = section->readonly,
     };
-    hostmem->num_new_regions++;
+    hostmem->num_current_regions++;
 }
 
 static void hostmem_listener_append_region(MemoryListener *listener,
                                            MemoryRegionSection *section)
 {
-    HostMem *hostmem = container_of(listener, HostMem, listener);
+    AddrSpaceMem *as_mem = container_of(listener, AddrSpaceMem, listener);
 
     /* Ignore non-RAM regions, we may not be able to map them */
     if (!memory_region_is_ram(section->mr)) {
@@ -116,7 +155,7 @@ static void hostmem_listener_append_region(MemoryListener *listener,
         return;
     }
 
-    hostmem_append_new_region(hostmem, section);
+    hostmem_append_new_region(as_mem->next_hostmem, section);
 }
 
 /* We don't implement most MemoryListener callbacks, use these nop stubs */
@@ -142,13 +181,13 @@ static void hostmem_listener_coalesced_mmio_dummy(MemoryListener *listener,
 {
 }
 
-void hostmem_init(void)
+static AddrSpaceMem *address_space_mem_create(AddressSpace *as)
 {
-    system_mem = g_new0(HostMem, 1);
-    qemu_mutex_init(&system_mem->current_regions_lock);
+    AddrSpaceMem *as_mem = g_new0(AddrSpaceMem, 1);
 
-    system_mem->listener = (MemoryListener) {
-        .begin = hostmem_listener_dummy,
+    qemu_mutex_init(&as_mem->cur_lock);
+    as_mem->listener = (MemoryListener) {
+        .begin = hostmem_listener_begin,
         .commit = hostmem_listener_commit,
         .region_add = hostmem_listener_append_region,
         .region_del = hostmem_listener_section_dummy,
@@ -164,18 +203,30 @@ void hostmem_init(void)
         .coalesced_mmio_del = hostmem_listener_coalesced_mmio_dummy,
         .priority = 10,
     };
+    as_mem->cur_hostmem = g_new0(HostMem, 1);
+    as_mem->cur_hostmem->ref = 1;
+    memory_listener_register(&as_mem->listener, as);
 
-    memory_listener_register(&system_mem->listener, &address_space_memory);
-    if (system_mem->num_new_regions > 0) {
-        hostmem_listener_commit(&system_mem->listener);
-    }
+    return as_mem;
+}
+
+static void address_space_mem_destroy(AddrSpaceMem *as_mem)
+{
+    memory_listener_unregister(&as_mem->listener);
+    qemu_mutex_destroy(&as_mem->cur_lock);
+    hostmem_unref(as_mem->cur_hostmem);
+    assert(!as_mem->next_hostmem);
+    g_free(as_mem);
+}
+
+void hostmem_init(void)
+{
+    system_mem = address_space_mem_create(&address_space_memory);
     atexit(hostmem_finalize);
 }
 
-static void hostmem_finalize(void)
+void hostmem_finalize(void)
 {
-    memory_listener_unregister(&system_mem->listener);
-    g_free(system_mem->new_regions);
-    g_free(system_mem->current_regions);
-    qemu_mutex_destroy(&system_mem->current_regions_lock);
+    address_space_mem_destroy(system_mem);
+    system_mem = NULL;
 }
diff --git a/include/hw/virtio/dataplane/hostmem.h b/include/hw/virtio/dataplane/hostmem.h
index 7f967a5..51afa8f 100644
--- a/include/hw/virtio/dataplane/hostmem.h
+++ b/include/hw/virtio/dataplane/hostmem.h
@@ -28,19 +28,21 @@ typedef struct {
     /* The listener is invoked when regions change and a new list of regions is
      * built up completely before they are installed.
      */
-    MemoryListener listener;
-    HostMemRegion *new_regions;
-    size_t num_new_regions;
-
-    /* Current regions are accessed from multiple threads either to lookup
-     * addresses or to install a new list of regions.  The lock protects the
-     * pointer and the regions.
-     */
-    QemuMutex current_regions_lock;
+    int ref;
     HostMemRegion *current_regions;
     size_t num_current_regions;
 } HostMem;
 
+/* Helper to map address space's hw_addr to hva */
+typedef struct {
+    /* lock to protect the r/w access to cur_hostmem */
+    QemuMutex cur_lock;
+    /* switch from cur_hostmem to next_hostmem to adopt RCU */
+    HostMem *cur_hostmem;
+    HostMem *next_hostmem;
+    MemoryListener listener;
+} AddrSpaceMem;
+
 /**
  * Map a guest physical address to a pointer
  *
-- 
1.7.4.4