[Qemu-devel] [PATCH v1 2/6] mem: make global dispatch table ready for rcu

[Liu Ping Fan]

From: Liu Ping Fan <address@hidden>

Currently, phys_node_map and phys_sections are shared by all
of the AddressSpaceDispatch. When updating mem topology, all
AddressSpaceDispatch will rebuild dispatch tables sequentially
on them. In order to use rcu-style, introducing next_node_map
and next_phys_sections, so that when rebuilding, the new dispatch
tables will locate on next_*. After all AddressSpaceDispatch
finished, we can switch to next_* and drop the previous stuff.

This patch still work in the scenario that readers and writer
can not run in parellel (ie, protected by biglock), so no extra
lock method needed till now, and the rcu style consistent issue
will be left to the following patches.

Signed-off-by: Liu Ping Fan <address@hidden>
---
 exec.c |  202 ++++++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 109 insertions(+), 93 deletions(-)

diff --git a/exec.c b/exec.c
index 6934c2d..e5335f5 100644
--- a/exec.c
+++ b/exec.c
@@ -80,16 +80,33 @@ int use_icount;
 
 #if !defined(CONFIG_USER_ONLY)
 
-static MemoryRegionSection *phys_sections;
-static unsigned phys_sections_nb, phys_sections_nb_alloc;
 #define phys_section_unassigned 0
 #define phys_section_notdirty 1
 #define phys_section_rom 2
 #define phys_section_watch 3
 
-/* Simple allocator for PhysPageEntry nodes */
-static PhysPageEntry (*phys_map_nodes)[L2_SIZE];
-static unsigned phys_map_nodes_nb, phys_map_nodes_nb_alloc;
+typedef PhysPageEntry Node[L2_SIZE];
+
+typedef struct AllocInfo {
+    unsigned phys_sections_nb;
+    unsigned phys_sections_nb_alloc;
+    unsigned phys_map_nodes_nb;
+    unsigned phys_map_nodes_nb_alloc;
+    /* Only used for release purpose */
+    Node *map;
+    MemoryRegionSection *sections;
+} AllocInfo;
+
+/* For performance, fetch page map related pointers directly, other than
+ * hiding them inside AllocInfo
+ */
+static MemoryRegionSection *cur_phys_sections;
+static Node *cur_map_nodes;
+static AllocInfo *cur_alloc_info;
+
+static MemoryRegionSection *next_phys_sections;
+static Node *next_map_nodes;
+static AllocInfo *next_alloc_info;
 
 #define PHYS_MAP_NODE_NIL (((uint16_t)~0) >> 1)
 
@@ -104,13 +121,15 @@ static MemoryRegion io_mem_watch;
 
 static void phys_map_node_reserve(unsigned nodes)
 {
-    if (phys_map_nodes_nb + nodes > phys_map_nodes_nb_alloc) {
-        typedef PhysPageEntry Node[L2_SIZE];
-        phys_map_nodes_nb_alloc = MAX(phys_map_nodes_nb_alloc * 2, 16);
-        phys_map_nodes_nb_alloc = MAX(phys_map_nodes_nb_alloc,
-                                      phys_map_nodes_nb + nodes);
-        phys_map_nodes = g_renew(Node, phys_map_nodes,
-                                 phys_map_nodes_nb_alloc);
+    AllocInfo *info = next_alloc_info;
+
+    if (info->phys_map_nodes_nb + nodes > info->phys_map_nodes_nb_alloc) {
+        info->phys_map_nodes_nb_alloc = MAX(info->phys_map_nodes_nb_alloc * 2,
+                                            16);
+        info->phys_map_nodes_nb_alloc = MAX(info->phys_map_nodes_nb_alloc,
+                                      info->phys_map_nodes_nb + nodes);
+        next_map_nodes = g_renew(Node, next_map_nodes,
+                                 info->phys_map_nodes_nb_alloc);
     }
 }
 
@@ -118,23 +137,18 @@ static uint16_t phys_map_node_alloc(void)
 {
     unsigned i;
     uint16_t ret;
+    AllocInfo *info = next_alloc_info;
 
-    ret = phys_map_nodes_nb++;
+    ret = info->phys_map_nodes_nb++;
     assert(ret != PHYS_MAP_NODE_NIL);
-    assert(ret != phys_map_nodes_nb_alloc);
+    assert(ret != info->phys_map_nodes_nb_alloc);
     for (i = 0; i < L2_SIZE; ++i) {
-        phys_map_nodes[ret][i].is_leaf = 0;
-        phys_map_nodes[ret][i].ptr = PHYS_MAP_NODE_NIL;
+        next_map_nodes[ret][i].is_leaf = 0;
+        next_map_nodes[ret][i].ptr = PHYS_MAP_NODE_NIL;
     }
     return ret;
 }
 
-static void phys_map_nodes_reset(void)
-{
-    phys_map_nodes_nb = 0;
-}
-
-
 static void phys_page_set_level(PhysPageEntry *lp, hwaddr *index,
                                 hwaddr *nb, uint16_t leaf,
                                 int level)
@@ -145,7 +159,7 @@ static void phys_page_set_level(PhysPageEntry *lp, hwaddr 
*index,
 
     if (!lp->is_leaf && lp->ptr == PHYS_MAP_NODE_NIL) {
         lp->ptr = phys_map_node_alloc();
-        p = phys_map_nodes[lp->ptr];
+        p = next_map_nodes[lp->ptr];
         if (level == 0) {
             for (i = 0; i < L2_SIZE; i++) {
                 p[i].is_leaf = 1;
@@ -153,7 +167,7 @@ static void phys_page_set_level(PhysPageEntry *lp, hwaddr 
*index,
             }
         }
     } else {
-        p = phys_map_nodes[lp->ptr];
+        p = next_map_nodes[lp->ptr];
     }
     lp = &p[(*index >> (level * L2_BITS)) & (L2_SIZE - 1)];
 
@@ -180,20 +194,29 @@ static void phys_page_set(AddressSpaceDispatch *d,
     phys_page_set_level(&d->phys_map, &index, &nb, leaf, P_L2_LEVELS - 1);
 }
 
-static MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr 
index)
+static MemoryRegionSection *phys_page_find(AddressSpaceDispatch *d, hwaddr 
index, bool cur)
 {
     PhysPageEntry lp = d->phys_map;
     PhysPageEntry *p;
     int i;
+    Node *map_nodes;
+    MemoryRegionSection *sections;
 
+    if (likely(cur)) {
+        map_nodes = cur_map_nodes;
+        sections = cur_phys_sections;
+    } else {
+        map_nodes = next_map_nodes;
+        sections = next_phys_sections;
+    }
     for (i = P_L2_LEVELS - 1; i >= 0 && !lp.is_leaf; i--) {
         if (lp.ptr == PHYS_MAP_NODE_NIL) {
-            return &phys_sections[phys_section_unassigned];
+            return &sections[phys_section_unassigned];
         }
-        p = phys_map_nodes[lp.ptr];
+        p = map_nodes[lp.ptr];
         lp = p[(index >> (i * L2_BITS)) & (L2_SIZE - 1)];
     }
-    return &phys_sections[lp.ptr];
+    return &sections[lp.ptr];
 }
 
 bool memory_region_is_unassigned(MemoryRegion *mr)
@@ -205,7 +228,7 @@ bool memory_region_is_unassigned(MemoryRegion *mr)
 static MemoryRegionSection *address_space_lookup_region(AddressSpace *as,
                                                         hwaddr addr)
 {
-    return phys_page_find(as->dispatch, addr >> TARGET_PAGE_BITS);
+    return phys_page_find(as->dispatch, addr >> TARGET_PAGE_BITS, true);
 }
 
 MemoryRegionSection *address_space_translate(AddressSpace *as, hwaddr addr,
@@ -213,7 +236,7 @@ MemoryRegionSection *address_space_translate(AddressSpace 
*as, hwaddr addr,
                                              bool is_write)
 {
     IOMMUTLBEntry iotlb;
-    MemoryRegionSection *section;
+    MemoryRegionSection *section, *base = cur_phys_sections;
     hwaddr len = *plen;
 
     for (;;) {
@@ -235,7 +258,7 @@ MemoryRegionSection *address_space_translate(AddressSpace 
*as, hwaddr addr,
                 | (addr & iotlb.addr_mask));
         len = MIN(len, (addr | iotlb.addr_mask) - addr + 1);
         if (!(iotlb.perm & (1 << is_write))) {
-            section = &phys_sections[phys_section_unassigned];
+            section = &base[phys_section_unassigned];
             break;
         }
 
@@ -677,7 +700,7 @@ hwaddr memory_region_section_get_iotlb(CPUArchState *env,
             iotlb |= phys_section_rom;
         }
     } else {
-        iotlb = section - phys_sections;
+        iotlb = section - cur_phys_sections;
         iotlb += xlat;
     }
 
@@ -710,67 +733,31 @@ typedef struct subpage_t {
 static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
                              uint16_t section);
 static subpage_t *subpage_init(hwaddr base);
-static void destroy_page_desc(uint16_t section_index)
-{
-    MemoryRegionSection *section = &phys_sections[section_index];
-    MemoryRegion *mr = section->mr;
-
-    if (mr->subpage) {
-        subpage_t *subpage = container_of(mr, subpage_t, iomem);
-        memory_region_destroy(&subpage->iomem);
-        g_free(subpage);
-    }
-}
-
-static void destroy_l2_mapping(PhysPageEntry *lp, unsigned level)
-{
-    unsigned i;
-    PhysPageEntry *p;
-
-    if (lp->ptr == PHYS_MAP_NODE_NIL) {
-        return;
-    }
-
-    p = phys_map_nodes[lp->ptr];
-    for (i = 0; i < L2_SIZE; ++i) {
-        if (!p[i].is_leaf) {
-            destroy_l2_mapping(&p[i], level - 1);
-        } else {
-            destroy_page_desc(p[i].ptr);
-        }
-    }
-    lp->is_leaf = 0;
-    lp->ptr = PHYS_MAP_NODE_NIL;
-}
-
-static void destroy_all_mappings(AddressSpaceDispatch *d)
-{
-    destroy_l2_mapping(&d->phys_map, P_L2_LEVELS - 1);
-    phys_map_nodes_reset();
-}
 
 static uint16_t phys_section_add(MemoryRegionSection *section)
 {
+    AllocInfo *info = next_alloc_info;
     /* The physical section number is ORed with a page-aligned
      * pointer to produce the iotlb entries.  Thus it should
      * never overflow into the page-aligned value.
      */
-    assert(phys_sections_nb < TARGET_PAGE_SIZE);
+    assert(info->phys_sections_nb < TARGET_PAGE_SIZE);
 
-    if (phys_sections_nb == phys_sections_nb_alloc) {
-        phys_sections_nb_alloc = MAX(phys_sections_nb_alloc * 2, 16);
-        phys_sections = g_renew(MemoryRegionSection, phys_sections,
-                                phys_sections_nb_alloc);
+    if (info->phys_sections_nb == info->phys_sections_nb_alloc) {
+        info->phys_sections_nb_alloc = MAX(info->phys_sections_nb_alloc * 2,
+                                            16);
+        next_phys_sections = g_renew(MemoryRegionSection, next_phys_sections,
+                                info->phys_sections_nb_alloc);
     }
-    phys_sections[phys_sections_nb] = *section;
+    next_phys_sections[info->phys_sections_nb] = *section;
     memory_region_ref(section->mr);
-    return phys_sections_nb++;
+    return info->phys_sections_nb++;
 }
 
-static void phys_sections_clear(void)
+static void phys_sections_clear(unsigned cnt, MemoryRegionSection *mrs)
 {
-    while (phys_sections_nb > 0) {
-        MemoryRegionSection *section = &phys_sections[--phys_sections_nb];
+    while (cnt > 0) {
+        MemoryRegionSection *section = &mrs[--cnt];
         memory_region_unref(section->mr);
     }
 }
@@ -780,7 +767,8 @@ static void register_subpage(AddressSpaceDispatch *d, 
MemoryRegionSection *secti
     subpage_t *subpage;
     hwaddr base = section->offset_within_address_space
         & TARGET_PAGE_MASK;
-    MemoryRegionSection *existing = phys_page_find(d, base >> 
TARGET_PAGE_BITS);
+    MemoryRegionSection *existing = phys_page_find(d,
+                                    base >> TARGET_PAGE_BITS, false);
     MemoryRegionSection subsection = {
         .offset_within_address_space = base,
         .size = TARGET_PAGE_SIZE,
@@ -1572,13 +1560,13 @@ static uint64_t subpage_read(void *opaque, hwaddr addr,
     unsigned int idx = SUBPAGE_IDX(addr);
     uint64_t val;
 
-    MemoryRegionSection *section;
+    MemoryRegionSection *section, *base = cur_phys_sections;
 #if defined(DEBUG_SUBPAGE)
     printf("%s: subpage %p len %d addr " TARGET_FMT_plx " idx %d\n", __func__,
            mmio, len, addr, idx);
 #endif
 
-    section = &phys_sections[mmio->sub_section[idx]];
+    section = &base[mmio->sub_section[idx]];
     addr += mmio->base;
     addr -= section->offset_within_address_space;
     addr += section->offset_within_region;
@@ -1591,14 +1579,14 @@ static void subpage_write(void *opaque, hwaddr addr,
 {
     subpage_t *mmio = opaque;
     unsigned int idx = SUBPAGE_IDX(addr);
-    MemoryRegionSection *section;
+    MemoryRegionSection *section, *base = cur_phys_sections;
 #if defined(DEBUG_SUBPAGE)
     printf("%s: subpage %p len %d addr " TARGET_FMT_plx
            " idx %d value %"PRIx64"\n",
            __func__, mmio, len, addr, idx, value);
 #endif
 
-    section = &phys_sections[mmio->sub_section[idx]];
+    section = &base[mmio->sub_section[idx]];
     addr += mmio->base;
     addr -= section->offset_within_address_space;
     addr += section->offset_within_region;
@@ -1610,14 +1598,14 @@ static bool subpage_accepts(void *opaque, hwaddr addr,
 {
     subpage_t *mmio = opaque;
     unsigned int idx = SUBPAGE_IDX(addr);
-    MemoryRegionSection *section;
+    MemoryRegionSection *section, *base = cur_phys_sections;;
 #if defined(DEBUG_SUBPAGE)
     printf("%s: subpage %p %c len %d addr " TARGET_FMT_plx
            " idx %d\n", __func__, mmio,
            is_write ? 'w' : 'r', len, addr, idx);
 #endif
 
-    section = &phys_sections[mmio->sub_section[idx]];
+    section = &base[mmio->sub_section[idx]];
     addr += mmio->base;
     addr -= section->offset_within_address_space;
     addr += section->offset_within_region;
@@ -1676,8 +1664,8 @@ static int subpage_register (subpage_t *mmio, uint32_t 
start, uint32_t end,
     printf("%s: %p start %08x end %08x idx %08x eidx %08x mem %ld\n", __func__,
            mmio, start, end, idx, eidx, memory);
 #endif
-    if (memory_region_is_ram(phys_sections[section].mr)) {
-        MemoryRegionSection new_section = phys_sections[section];
+    if (memory_region_is_ram(next_phys_sections[section].mr)) {
+        MemoryRegionSection new_section = next_phys_sections[section];
         new_section.mr = &io_mem_subpage_ram;
         section = phys_section_add(&new_section);
     }
@@ -1721,7 +1709,7 @@ static uint16_t dummy_section(MemoryRegion *mr)
 
 MemoryRegion *iotlb_to_region(hwaddr index)
 {
-    return phys_sections[index & ~TARGET_PAGE_MASK].mr;
+    return cur_phys_sections[index & ~TARGET_PAGE_MASK].mr;
 }
 
 static void io_mem_init(void)
@@ -1741,7 +1729,6 @@ static void mem_begin(MemoryListener *listener)
 {
     AddressSpaceDispatch *d = container_of(listener, AddressSpaceDispatch, 
listener);
 
-    destroy_all_mappings(d);
     d->phys_map.ptr = PHYS_MAP_NODE_NIL;
 }
 
@@ -1749,7 +1736,7 @@ static void core_begin(MemoryListener *listener)
 {
     uint16_t n;
 
-    phys_sections_clear();
+    next_alloc_info = g_malloc0(sizeof(AllocInfo));
     n = dummy_section(&io_mem_unassigned);
     assert(phys_section_unassigned == n);
     n = dummy_section(&io_mem_notdirty);
@@ -1760,6 +1747,35 @@ static void core_begin(MemoryListener *listener)
     assert(phys_section_watch == n);
 }
 
+static void release_dispatch_map(AllocInfo *info)
+{
+    phys_sections_clear(info->phys_sections_nb, info->sections);
+    g_free(info->map);
+    g_free(info->sections);
+    g_free(info);
+}
+
+/* This listener's commit run after the other AddressSpaceDispatch listeners'.
+ * It means that AddressSpaceDispatch's deleter has finished, so it can be
+ * the place for call_rcu()
+ */
+static void core_commit(MemoryListener *listener)
+{
+    AllocInfo *info = cur_alloc_info;
+    info->map = cur_map_nodes;
+    info->sections = cur_phys_sections;
+
+    /* Fix me, in future, will be protected by wr seqlock when in parellel
+     * with reader
+     */
+    cur_map_nodes = next_map_nodes;
+    cur_phys_sections = next_phys_sections;
+    cur_alloc_info = next_alloc_info;
+
+    /* Fix me, will changed to call_rcu */
+    release_dispatch_map(info);
+}
+
 static void tcg_commit(MemoryListener *listener)
 {
     CPUArchState *env;
@@ -1802,6 +1818,7 @@ static void io_region_del(MemoryListener *listener,
 
 static MemoryListener core_memory_listener = {
     .begin = core_begin,
+    .commit = core_commit,
     .log_global_start = core_log_global_start,
     .log_global_stop = core_log_global_stop,
     .priority = 1,
@@ -1837,7 +1854,6 @@ void address_space_destroy_dispatch(AddressSpace *as)
     AddressSpaceDispatch *d = as->dispatch;
 
     memory_listener_unregister(&d->listener);
-    destroy_l2_mapping(&d->phys_map, P_L2_LEVELS - 1);
     g_free(d);
     as->dispatch = NULL;
 }
-- 
1.7.4.4