All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use
@ 2017-09-20 11:46 Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 01/18] exec: Explicitly export target AS from address_space_translate_internal Alexey Kardashevskiy
                   ` (17 more replies)
  0 siblings, 18 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini


This was inspired by https://bugzilla.redhat.com/show_bug.cgi?id=1481593
Previous versions:
v1: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg01559.html
v2: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg04069.html
v3: https://lists.gnu.org/archive/html/qemu-devel/2017-09/msg04523.html

This patchset tries to reduce amount of memory used by FlatViews
and tries share as many FVs between address spaces as possible.

Changelog:
v4:
* total rework again to provide grounds for new 15/18, 17/18, 18/18

v3:
* addressed comments from v2, mainly simplified the code

v2:
* total rework


This is based on sha1
c51700273a Peter Maydell "Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20170919-v2' into staging".

Please comment. Thanks.



Alexey Kardashevskiy (18):
  exec: Explicitly export target AS from
    address_space_translate_internal
  memory: Open code FlatView rendering
  memory: Move FlatView allocation to a helper
  memory: Move AddressSpaceDispatch from AddressSpace to FlatView
  memory: Remove AddressSpace pointer from AddressSpaceDispatch
  memory: Switch memory from using AddressSpace to FlatView
  memory: Cleanup after switching to FlatView
  memory: Rename mem_begin/mem_commit/mem_add helpers
  memory: Store physical root MR in FlatView
  memory: Alloc dispatch tree where topology is generared
  memory: Move address_space_update_ioeventfds
  memory: Share FlatView's and dispatch trees between address spaces
  memory: Do not allocate FlatView in address_space_init
  memory: Add flat views to HMP "info mtree"
  memory: Share special empty FlatView
  memory: Get rid of address_space_init_shareable
  memory: Create FlatView directly
  memory: Give memory_region_transaction_commit a hint

 include/exec/memory-internal.h |  16 +-
 include/exec/memory.h          |  76 ++++----
 include/hw/arm/armv7m.h        |   2 +-
 cpus.c                         |   5 +-
 exec.c                         | 330 +++++++++++++++++++++--------------
 hw/arm/armv7m.c                |   9 +-
 hw/intc/openpic_kvm.c          |   2 +-
 memory.c                       | 383 +++++++++++++++++++++++++++++++----------
 monitor.c                      |   3 +-
 target/arm/cpu.c               |  16 +-
 target/i386/cpu.c              |   5 +-
 hmp-commands-info.hx           |   7 +-
 12 files changed, 570 insertions(+), 284 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 01/18] exec: Explicitly export target AS from address_space_translate_internal
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 02/18] memory: Open code FlatView rendering Alexey Kardashevskiy
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

This adds an AS** parameter to address_space_do_translate()
to make it easier for the next patch to share FlatViews.

This should cause no behavioural change.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 exec.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/exec.c b/exec.c
index a25a4c6018..fd8994b25d 100644
--- a/exec.c
+++ b/exec.c
@@ -476,7 +476,8 @@ static MemoryRegionSection address_space_do_translate(AddressSpace *as,
                                                       hwaddr *xlat,
                                                       hwaddr *plen,
                                                       bool is_write,
-                                                      bool is_mmio)
+                                                      bool is_mmio,
+                                                      AddressSpace **target_as)
 {
     IOMMUTLBEntry iotlb;
     MemoryRegionSection *section;
@@ -503,6 +504,7 @@ static MemoryRegionSection address_space_do_translate(AddressSpace *as,
         }
 
         as = iotlb.target_as;
+        *target_as = iotlb.target_as;
     }
 
     *xlat = addr;
@@ -525,7 +527,7 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
 
     /* This can never be MMIO. */
     section = address_space_do_translate(as, addr, &xlat, &plen,
-                                         is_write, false);
+                                         is_write, false, &as);
 
     /* Illegal translation */
     if (section.mr == &io_mem_unassigned) {
@@ -548,7 +550,7 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
     plen -= 1;
 
     return (IOMMUTLBEntry) {
-        .target_as = section.address_space,
+        .target_as = as,
         .iova = addr & ~plen,
         .translated_addr = xlat & ~plen,
         .addr_mask = plen,
@@ -569,7 +571,8 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
     MemoryRegionSection section;
 
     /* This can be MMIO, so setup MMIO bit. */
-    section = address_space_do_translate(as, addr, xlat, plen, is_write, true);
+    section = address_space_do_translate(as, addr, xlat, plen, is_write, true,
+                                         &as);
     mr = section.mr;
 
     if (xen_enabled() && memory_access_is_direct(mr, is_write)) {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 02/18] memory: Open code FlatView rendering
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 01/18] exec: Explicitly export target AS from address_space_translate_internal Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 03/18] memory: Move FlatView allocation to a helper Alexey Kardashevskiy
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

We are going to share FlatView's between AddressSpace's and per-AS
memory listeners won't suit the purpose anymore so open code
the dispatch tree rendering.

Since there is a good chance that dispatch_listener was the only
listener, this avoids address_space_update_topology_pass() if there is
no registered listeners; this should improve starting time.

This should cause no behavioural change.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v4:
* address_space_update_topology_pass() is called now under
if (!QTAILQ_EMPTY(&as->listeners))

v3:
* inlined & simplified address_space_update_flatview
---
 include/exec/memory-internal.h |  6 ++++--
 include/exec/memory.h          |  1 -
 exec.c                         | 27 +++------------------------
 memory.c                       | 19 ++++++++++++++-----
 4 files changed, 21 insertions(+), 32 deletions(-)

diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index fb467acdba..9abde2f11c 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -22,8 +22,6 @@
 #ifndef CONFIG_USER_ONLY
 typedef struct AddressSpaceDispatch AddressSpaceDispatch;
 
-void address_space_init_dispatch(AddressSpace *as);
-void address_space_unregister(AddressSpace *as);
 void address_space_destroy_dispatch(AddressSpace *as);
 
 extern const MemoryRegionOps unassigned_mem_ops;
@@ -31,5 +29,9 @@ extern const MemoryRegionOps unassigned_mem_ops;
 bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr,
                                 unsigned size, bool is_write);
 
+void mem_add(AddressSpace *as, MemoryRegionSection *section);
+void mem_begin(AddressSpace *as);
+void mem_commit(AddressSpace *as);
+
 #endif
 #endif
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1dcd3122d7..9581f7a7db 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -318,7 +318,6 @@ struct AddressSpace {
     struct MemoryRegionIoeventfd *ioeventfds;
     struct AddressSpaceDispatch *dispatch;
     struct AddressSpaceDispatch *next_dispatch;
-    MemoryListener dispatch_listener;
     QTAILQ_HEAD(memory_listeners_as, MemoryListener) listeners;
     QTAILQ_ENTRY(AddressSpace) address_spaces_link;
 };
diff --git a/exec.c b/exec.c
index fd8994b25d..1626d254bb 100644
--- a/exec.c
+++ b/exec.c
@@ -1347,9 +1347,8 @@ static void register_multipage(AddressSpaceDispatch *d,
     phys_page_set(d, start_addr >> TARGET_PAGE_BITS, num_pages, section_index);
 }
 
-static void mem_add(MemoryListener *listener, MemoryRegionSection *section)
+void mem_add(AddressSpace *as, MemoryRegionSection *section)
 {
-    AddressSpace *as = container_of(listener, AddressSpace, dispatch_listener);
     AddressSpaceDispatch *d = as->next_dispatch;
     MemoryRegionSection now = *section, remain = *section;
     Int128 page_size = int128_make64(TARGET_PAGE_SIZE);
@@ -2673,9 +2672,8 @@ static void io_mem_init(void)
                           NULL, UINT64_MAX);
 }
 
-static void mem_begin(MemoryListener *listener)
+void mem_begin(AddressSpace *as)
 {
-    AddressSpace *as = container_of(listener, AddressSpace, dispatch_listener);
     AddressSpaceDispatch *d = g_new0(AddressSpaceDispatch, 1);
     uint16_t n;
 
@@ -2699,9 +2697,8 @@ static void address_space_dispatch_free(AddressSpaceDispatch *d)
     g_free(d);
 }
 
-static void mem_commit(MemoryListener *listener)
+void mem_commit(AddressSpace *as)
 {
-    AddressSpace *as = container_of(listener, AddressSpace, dispatch_listener);
     AddressSpaceDispatch *cur = as->dispatch;
     AddressSpaceDispatch *next = as->next_dispatch;
 
@@ -2731,24 +2728,6 @@ static void tcg_commit(MemoryListener *listener)
     tlb_flush(cpuas->cpu);
 }
 
-void address_space_init_dispatch(AddressSpace *as)
-{
-    as->dispatch = NULL;
-    as->dispatch_listener = (MemoryListener) {
-        .begin = mem_begin,
-        .commit = mem_commit,
-        .region_add = mem_add,
-        .region_nop = mem_add,
-        .priority = 0,
-    };
-    memory_listener_register(&as->dispatch_listener, as);
-}
-
-void address_space_unregister(AddressSpace *as)
-{
-    memory_listener_unregister(&as->dispatch_listener);
-}
-
 void address_space_destroy_dispatch(AddressSpace *as)
 {
     AddressSpaceDispatch *d = as->dispatch;
diff --git a/memory.c b/memory.c
index b9920a6540..101d5f4d60 100644
--- a/memory.c
+++ b/memory.c
@@ -879,14 +879,24 @@ static void address_space_update_topology_pass(AddressSpace *as,
     }
 }
 
-
 static void address_space_update_topology(AddressSpace *as)
 {
     FlatView *old_view = address_space_get_flatview(as);
     FlatView *new_view = generate_memory_topology(as->root);
+    int i;
 
-    address_space_update_topology_pass(as, old_view, new_view, false);
-    address_space_update_topology_pass(as, old_view, new_view, true);
+    mem_begin(as);
+    for (i = 0; i < new_view->nr; i++) {
+        MemoryRegionSection mrs =
+            section_from_flat_range(&new_view->ranges[i], as);
+        mem_add(as, &mrs);
+    }
+    mem_commit(as);
+
+    if (!QTAILQ_EMPTY(&as->listeners)) {
+        address_space_update_topology_pass(as, old_view, new_view, false);
+        address_space_update_topology_pass(as, old_view, new_view, true);
+    }
 
     /* Writes are protected by the BQL.  */
     atomic_rcu_set(&as->current_map, new_view);
@@ -2621,7 +2631,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
     QTAILQ_INIT(&as->listeners);
     QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
     as->name = g_strdup(name ? name : "anonymous");
-    address_space_init_dispatch(as);
+    as->dispatch = NULL;
     memory_region_update_pending |= root->enabled;
     memory_region_transaction_commit();
 }
@@ -2672,7 +2682,6 @@ void address_space_destroy(AddressSpace *as)
     as->root = NULL;
     memory_region_transaction_commit();
     QTAILQ_REMOVE(&address_spaces, as, address_spaces_link);
-    address_space_unregister(as);
 
     /* At this point, as->dispatch and as->current_map are dummy
      * entries that the guest should never use.  Wait for the old
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 03/18] memory: Move FlatView allocation to a helper
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 01/18] exec: Explicitly export target AS from address_space_translate_internal Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 02/18] memory: Open code FlatView rendering Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 04/18] memory: Move AddressSpaceDispatch from AddressSpace to FlatView Alexey Kardashevskiy
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

This moves a FlatView allocation and initialization to a helper.
While we are nere, replace g_new with g_new0 to not to bother if we add
new fields in the future.

This should cause no behavioural change.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* s/flatview_alloc/flatview_new/
---
 memory.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/memory.c b/memory.c
index 101d5f4d60..9babd7d423 100644
--- a/memory.c
+++ b/memory.c
@@ -258,12 +258,14 @@ static bool flatrange_equal(FlatRange *a, FlatRange *b)
         && a->readonly == b->readonly;
 }
 
-static void flatview_init(FlatView *view)
+static FlatView *flatview_new(void)
 {
+    FlatView *view;
+
+    view = g_new0(FlatView, 1);
     view->ref = 1;
-    view->ranges = NULL;
-    view->nr = 0;
-    view->nr_allocated = 0;
+
+    return view;
 }
 
 /* Insert a range into a given position.  Caller is responsible for maintaining
@@ -706,8 +708,7 @@ static FlatView *generate_memory_topology(MemoryRegion *mr)
 {
     FlatView *view;
 
-    view = g_new(FlatView, 1);
-    flatview_init(view);
+    view = flatview_new();
 
     if (mr) {
         render_memory_region(view, mr, int128_zero(),
@@ -2624,8 +2625,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
     as->ref_count = 1;
     as->root = root;
     as->malloced = false;
-    as->current_map = g_new(FlatView, 1);
-    flatview_init(as->current_map);
+    as->current_map = flatview_new();
     as->ioeventfd_nb = 0;
     as->ioeventfds = NULL;
     QTAILQ_INIT(&as->listeners);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 04/18] memory: Move AddressSpaceDispatch from AddressSpace to FlatView
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
                   ` (2 preceding siblings ...)
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 03/18] memory: Move FlatView allocation to a helper Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 05/18] memory: Remove AddressSpace pointer from AddressSpaceDispatch Alexey Kardashevskiy
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

As we are going to share FlatView's between AddressSpace's,
and AddressSpaceDispatch is a structure to perform quick lookup
in FlatView, this moves ASD to FlatView.

After previosly open coded ASD rendering, we can also remove
as->next_dispatch as the new FlatView pointer is stored
on a stack and set to an AS atomically.

flatview_destroy() is executed under RCU instead of
address_space_dispatch_free() now.

This makes mem_begin/mem_commit to work with ASD and mem_add with FV
as later on mem_add will be taking FV as an argument anyway.

This should cause no behavioural change.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v4:
* since FlatView::rcu is used now to dispose FV, call_rcu() in
address_space_update_topology() is replaced with direct call to
flatview_unref()
---
 include/exec/memory-internal.h | 12 +++++++-----
 include/exec/memory.h          |  2 --
 exec.c                         | 41 +++++++++++------------------------------
 memory.c                       | 31 ++++++++++++++++++++++++-------
 4 files changed, 42 insertions(+), 44 deletions(-)

diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index 9abde2f11c..6e08eda256 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -22,16 +22,18 @@
 #ifndef CONFIG_USER_ONLY
 typedef struct AddressSpaceDispatch AddressSpaceDispatch;
 
-void address_space_destroy_dispatch(AddressSpace *as);
-
 extern const MemoryRegionOps unassigned_mem_ops;
 
 bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr,
                                 unsigned size, bool is_write);
 
-void mem_add(AddressSpace *as, MemoryRegionSection *section);
-void mem_begin(AddressSpace *as);
-void mem_commit(AddressSpace *as);
+void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section);
+AddressSpaceDispatch *mem_begin(AddressSpace *as);
+void mem_commit(AddressSpaceDispatch *d);
+
+AddressSpaceDispatch *address_space_to_dispatch(AddressSpace *as);
+AddressSpaceDispatch *flatview_to_dispatch(FlatView *fv);
+void address_space_dispatch_free(AddressSpaceDispatch *d);
 
 #endif
 #endif
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 9581f7a7db..2346f8b863 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -316,8 +316,6 @@ struct AddressSpace {
 
     int ioeventfd_nb;
     struct MemoryRegionIoeventfd *ioeventfds;
-    struct AddressSpaceDispatch *dispatch;
-    struct AddressSpaceDispatch *next_dispatch;
     QTAILQ_HEAD(memory_listeners_as, MemoryListener) listeners;
     QTAILQ_ENTRY(AddressSpace) address_spaces_link;
 };
diff --git a/exec.c b/exec.c
index 1626d254bb..afd64127e6 100644
--- a/exec.c
+++ b/exec.c
@@ -187,8 +187,6 @@ typedef struct PhysPageMap {
 } PhysPageMap;
 
 struct AddressSpaceDispatch {
-    struct rcu_head rcu;
-
     MemoryRegionSection *mru_section;
     /* This is a multi-level map on the physical address space.
      * The bottom level has pointers to MemoryRegionSections.
@@ -485,7 +483,7 @@ static MemoryRegionSection address_space_do_translate(AddressSpace *as,
     IOMMUMemoryRegionClass *imrc;
 
     for (;;) {
-        AddressSpaceDispatch *d = atomic_rcu_read(&as->dispatch);
+        AddressSpaceDispatch *d = address_space_to_dispatch(as);
         section = address_space_translate_internal(d, addr, &addr, plen, is_mmio);
 
         iommu_mr = memory_region_get_iommu(section->mr);
@@ -1222,7 +1220,7 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
     } else {
         AddressSpaceDispatch *d;
 
-        d = atomic_rcu_read(&section->address_space->dispatch);
+        d = address_space_to_dispatch(section->address_space);
         iotlb = section - d->map.sections;
         iotlb += xlat;
     }
@@ -1347,9 +1345,9 @@ static void register_multipage(AddressSpaceDispatch *d,
     phys_page_set(d, start_addr >> TARGET_PAGE_BITS, num_pages, section_index);
 }
 
-void mem_add(AddressSpace *as, MemoryRegionSection *section)
+void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section)
 {
-    AddressSpaceDispatch *d = as->next_dispatch;
+    AddressSpaceDispatch *d = flatview_to_dispatch(fv);
     MemoryRegionSection now = *section, remain = *section;
     Int128 page_size = int128_make64(TARGET_PAGE_SIZE);
 
@@ -2672,7 +2670,7 @@ static void io_mem_init(void)
                           NULL, UINT64_MAX);
 }
 
-void mem_begin(AddressSpace *as)
+AddressSpaceDispatch *mem_begin(AddressSpace *as)
 {
     AddressSpaceDispatch *d = g_new0(AddressSpaceDispatch, 1);
     uint16_t n;
@@ -2688,26 +2686,19 @@ void mem_begin(AddressSpace *as)
 
     d->phys_map  = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .skip = 1 };
     d->as = as;
-    as->next_dispatch = d;
+
+    return d;
 }
 
-static void address_space_dispatch_free(AddressSpaceDispatch *d)
+void address_space_dispatch_free(AddressSpaceDispatch *d)
 {
     phys_sections_free(&d->map);
     g_free(d);
 }
 
-void mem_commit(AddressSpace *as)
+void mem_commit(AddressSpaceDispatch *d)
 {
-    AddressSpaceDispatch *cur = as->dispatch;
-    AddressSpaceDispatch *next = as->next_dispatch;
-
-    phys_page_compact_all(next, next->map.nodes_nb);
-
-    atomic_rcu_set(&as->dispatch, next);
-    if (cur) {
-        call_rcu(cur, address_space_dispatch_free, rcu);
-    }
+    phys_page_compact_all(d, d->map.nodes_nb);
 }
 
 static void tcg_commit(MemoryListener *listener)
@@ -2723,21 +2714,11 @@ static void tcg_commit(MemoryListener *listener)
      * We reload the dispatch pointer now because cpu_reloading_memory_map()
      * may have split the RCU critical section.
      */
-    d = atomic_rcu_read(&cpuas->as->dispatch);
+    d = address_space_to_dispatch(cpuas->as);
     atomic_rcu_set(&cpuas->memory_dispatch, d);
     tlb_flush(cpuas->cpu);
 }
 
-void address_space_destroy_dispatch(AddressSpace *as)
-{
-    AddressSpaceDispatch *d = as->dispatch;
-
-    atomic_rcu_set(&as->dispatch, NULL);
-    if (d) {
-        call_rcu(d, address_space_dispatch_free, rcu);
-    }
-}
-
 static void memory_map_init(void)
 {
     system_memory = g_malloc(sizeof(*system_memory));
diff --git a/memory.c b/memory.c
index 9babd7d423..27d7aeffc2 100644
--- a/memory.c
+++ b/memory.c
@@ -229,6 +229,7 @@ struct FlatView {
     FlatRange *ranges;
     unsigned nr;
     unsigned nr_allocated;
+    struct AddressSpaceDispatch *dispatch;
 };
 
 typedef struct AddressSpaceOps AddressSpaceOps;
@@ -289,6 +290,9 @@ static void flatview_destroy(FlatView *view)
 {
     int i;
 
+    if (view->dispatch) {
+        address_space_dispatch_free(view->dispatch);
+    }
     for (i = 0; i < view->nr; i++) {
         memory_region_unref(view->ranges[i].mr);
     }
@@ -304,10 +308,25 @@ static void flatview_ref(FlatView *view)
 static void flatview_unref(FlatView *view)
 {
     if (atomic_fetch_dec(&view->ref) == 1) {
-        flatview_destroy(view);
+        call_rcu(view, flatview_destroy, rcu);
     }
 }
 
+static FlatView *address_space_to_flatview(AddressSpace *as)
+{
+    return atomic_rcu_read(&as->current_map);
+}
+
+AddressSpaceDispatch *flatview_to_dispatch(FlatView *fv)
+{
+    return fv->dispatch;
+}
+
+AddressSpaceDispatch *address_space_to_dispatch(AddressSpace *as)
+{
+    return flatview_to_dispatch(address_space_to_flatview(as));
+}
+
 static bool can_merge(FlatRange *r1, FlatRange *r2)
 {
     return int128_eq(addrrange_end(r1->addr), r2->addr.start)
@@ -886,13 +905,13 @@ static void address_space_update_topology(AddressSpace *as)
     FlatView *new_view = generate_memory_topology(as->root);
     int i;
 
-    mem_begin(as);
+    new_view->dispatch = mem_begin(as);
     for (i = 0; i < new_view->nr; i++) {
         MemoryRegionSection mrs =
             section_from_flat_range(&new_view->ranges[i], as);
-        mem_add(as, &mrs);
+        mem_add(as, new_view, &mrs);
     }
-    mem_commit(as);
+    mem_commit(new_view->dispatch);
 
     if (!QTAILQ_EMPTY(&as->listeners)) {
         address_space_update_topology_pass(as, old_view, new_view, false);
@@ -901,7 +920,7 @@ static void address_space_update_topology(AddressSpace *as)
 
     /* Writes are protected by the BQL.  */
     atomic_rcu_set(&as->current_map, new_view);
-    call_rcu(old_view, flatview_unref, rcu);
+    flatview_unref(old_view);
 
     /* Note that all the old MemoryRegions are still alive up to this
      * point.  This relieves most MemoryListeners from the need to
@@ -2631,7 +2650,6 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
     QTAILQ_INIT(&as->listeners);
     QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
     as->name = g_strdup(name ? name : "anonymous");
-    as->dispatch = NULL;
     memory_region_update_pending |= root->enabled;
     memory_region_transaction_commit();
 }
@@ -2640,7 +2658,6 @@ static void do_address_space_destroy(AddressSpace *as)
 {
     bool do_free = as->malloced;
 
-    address_space_destroy_dispatch(as);
     assert(QTAILQ_EMPTY(&as->listeners));
 
     flatview_unref(as->current_map);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 05/18] memory: Remove AddressSpace pointer from AddressSpaceDispatch
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
                   ` (3 preceding siblings ...)
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 04/18] memory: Move AddressSpaceDispatch from AddressSpace to FlatView Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 06/18] memory: Switch memory from using AddressSpace to FlatView Alexey Kardashevskiy
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

AS in ASD is only used to pass AS from mem_begin() to register_subpage()
to store it in MemoryRegionSection, we can do this directly now.

This should cause no behavioural change.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 exec.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/exec.c b/exec.c
index afd64127e6..a54dde7835 100644
--- a/exec.c
+++ b/exec.c
@@ -193,7 +193,6 @@ struct AddressSpaceDispatch {
      */
     PhysPageEntry phys_map;
     PhysPageMap map;
-    AddressSpace *as;
 };
 
 #define SUBPAGE_IDX(addr) ((addr) & ~TARGET_PAGE_MASK)
@@ -1303,7 +1302,8 @@ static void phys_sections_free(PhysPageMap *map)
     g_free(map->nodes);
 }
 
-static void register_subpage(AddressSpaceDispatch *d, MemoryRegionSection *section)
+static void register_subpage(AddressSpace *as, AddressSpaceDispatch *d,
+                             MemoryRegionSection *section)
 {
     subpage_t *subpage;
     hwaddr base = section->offset_within_address_space
@@ -1318,8 +1318,8 @@ static void register_subpage(AddressSpaceDispatch *d, MemoryRegionSection *secti
     assert(existing->mr->subpage || existing->mr == &io_mem_unassigned);
 
     if (!(existing->mr->subpage)) {
-        subpage = subpage_init(d->as, base);
-        subsection.address_space = d->as;
+        subpage = subpage_init(as, base);
+        subsection.address_space = as;
         subsection.mr = &subpage->iomem;
         phys_page_set(d, base >> TARGET_PAGE_BITS, 1,
                       phys_section_add(&d->map, &subsection));
@@ -1356,7 +1356,7 @@ void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section)
                        - now.offset_within_address_space;
 
         now.size = int128_min(int128_make64(left), now.size);
-        register_subpage(d, &now);
+        register_subpage(as, d, &now);
     } else {
         now.size = int128_zero();
     }
@@ -1366,10 +1366,10 @@ void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section)
         remain.offset_within_region += int128_get64(now.size);
         now = remain;
         if (int128_lt(remain.size, page_size)) {
-            register_subpage(d, &now);
+            register_subpage(as, d, &now);
         } else if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
             now.size = page_size;
-            register_subpage(d, &now);
+            register_subpage(as, d, &now);
         } else {
             now.size = int128_and(now.size, int128_neg(page_size));
             register_multipage(d, &now);
@@ -2685,7 +2685,6 @@ AddressSpaceDispatch *mem_begin(AddressSpace *as)
     assert(n == PHYS_SECTION_WATCH);
 
     d->phys_map  = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .skip = 1 };
-    d->as = as;
 
     return d;
 }
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 06/18] memory: Switch memory from using AddressSpace to FlatView
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
                   ` (4 preceding siblings ...)
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 05/18] memory: Remove AddressSpace pointer from AddressSpaceDispatch Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 07/18] memory: Cleanup after switching " Alexey Kardashevskiy
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

FlatView's will be shared between AddressSpace's and subpage_t
and MemoryRegionSection cannot store AS anymore, hence this change.

In particular, for:

 typedef struct subpage_t {
     MemoryRegion iomem;
-    AddressSpace *as;
+    FlatView *fv;
     hwaddr base;
     uint16_t sub_section[];
 } subpage_t;

  struct MemoryRegionSection {
     MemoryRegion *mr;
-    AddressSpace *address_space;
+    FlatView *fv;
     hwaddr offset_within_region;
     Int128 size;
     hwaddr offset_within_address_space;
     bool readonly;
 };

This should cause no behavioural change.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v4:
* fixed compile warning/error about duplicated "typedef struct FlatView
FlatView"
---
 include/exec/memory-internal.h |   2 +-
 include/exec/memory.h          |  51 ++++++++----
 exec.c                         | 180 ++++++++++++++++++++++++-----------------
 hw/intc/openpic_kvm.c          |   2 +-
 memory.c                       |  29 +++----
 5 files changed, 157 insertions(+), 107 deletions(-)

diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index 6e08eda256..1cf8ad9869 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -27,7 +27,7 @@ extern const MemoryRegionOps unassigned_mem_ops;
 bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr,
                                 unsigned size, bool is_write);
 
-void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section);
+void mem_add(FlatView *fv, MemoryRegionSection *section);
 AddressSpaceDispatch *mem_begin(AddressSpace *as);
 void mem_commit(AddressSpaceDispatch *d);
 
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2346f8b863..7816e5d655 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -48,6 +48,7 @@
 
 typedef struct MemoryRegionOps MemoryRegionOps;
 typedef struct MemoryRegionMmio MemoryRegionMmio;
+typedef struct FlatView FlatView;
 
 struct MemoryRegionMmio {
     CPUReadMemoryFunc *read[3];
@@ -320,6 +321,8 @@ struct AddressSpace {
     QTAILQ_ENTRY(AddressSpace) address_spaces_link;
 };
 
+FlatView *address_space_to_flatview(AddressSpace *as);
+
 /**
  * MemoryRegionSection: describes a fragment of a #MemoryRegion
  *
@@ -333,7 +336,7 @@ struct AddressSpace {
  */
 struct MemoryRegionSection {
     MemoryRegion *mr;
-    AddressSpace *address_space;
+    FlatView *fv;
     hwaddr offset_within_region;
     Int128 size;
     hwaddr offset_within_address_space;
@@ -1842,9 +1845,17 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
  * @len: pointer to length
  * @is_write: indicates the transfer direction
  */
-MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
-                                      hwaddr *xlat, hwaddr *len,
-                                      bool is_write);
+MemoryRegion *flatview_translate(FlatView *fv,
+                                 hwaddr addr, hwaddr *xlat,
+                                 hwaddr *len, bool is_write);
+
+static inline MemoryRegion *address_space_translate(AddressSpace *as,
+                                                    hwaddr addr, hwaddr *xlat,
+                                                    hwaddr *len, bool is_write)
+{
+    return flatview_translate(address_space_to_flatview(as),
+                              addr, xlat, len, is_write);
+}
 
 /* address_space_access_valid: check for validity of accessing an address
  * space range
@@ -1895,12 +1906,13 @@ void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
 
 
 /* Internal functions, part of the implementation of address_space_read.  */
-MemTxResult address_space_read_continue(AddressSpace *as, hwaddr addr,
-                                        MemTxAttrs attrs, uint8_t *buf,
-                                        int len, hwaddr addr1, hwaddr l,
-					MemoryRegion *mr);
-MemTxResult address_space_read_full(AddressSpace *as, hwaddr addr,
-                                    MemTxAttrs attrs, uint8_t *buf, int len);
+MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
+                                   MemTxAttrs attrs, uint8_t *buf,
+                                   int len, hwaddr addr1, hwaddr l,
+                                   MemoryRegion *mr);
+
+MemTxResult flatview_read_full(FlatView *fv, hwaddr addr,
+                               MemTxAttrs attrs, uint8_t *buf, int len);
 void *qemu_map_ram_ptr(RAMBlock *ram_block, ram_addr_t addr);
 
 static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
@@ -1927,8 +1939,8 @@ static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
  * @buf: buffer with the data transferred
  */
 static inline __attribute__((__always_inline__))
-MemTxResult address_space_read(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
-                               uint8_t *buf, int len)
+MemTxResult flatview_read(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
+                          uint8_t *buf, int len)
 {
     MemTxResult result = MEMTX_OK;
     hwaddr l, addr1;
@@ -1939,22 +1951,29 @@ MemTxResult address_space_read(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
         if (len) {
             rcu_read_lock();
             l = len;
-            mr = address_space_translate(as, addr, &addr1, &l, false);
+            mr = flatview_translate(fv, addr, &addr1, &l, false);
             if (len == l && memory_access_is_direct(mr, false)) {
                 ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
                 memcpy(buf, ptr, len);
             } else {
-                result = address_space_read_continue(as, addr, attrs, buf, len,
-                                                     addr1, l, mr);
+                result = flatview_read_continue(fv, addr, attrs, buf, len,
+                                                addr1, l, mr);
             }
             rcu_read_unlock();
         }
     } else {
-        result = address_space_read_full(as, addr, attrs, buf, len);
+        result = flatview_read_full(fv, addr, attrs, buf, len);
     }
     return result;
 }
 
+static inline MemTxResult address_space_read(AddressSpace *as, hwaddr addr,
+                                             MemTxAttrs attrs, uint8_t *buf,
+                                             int len)
+{
+    return flatview_read(address_space_to_flatview(as), addr, attrs, buf, len);
+}
+
 /**
  * address_space_read_cached: read from a cached RAM region
  *
diff --git a/exec.c b/exec.c
index a54dde7835..d2b9f60494 100644
--- a/exec.c
+++ b/exec.c
@@ -198,7 +198,7 @@ struct AddressSpaceDispatch {
 #define SUBPAGE_IDX(addr) ((addr) & ~TARGET_PAGE_MASK)
 typedef struct subpage_t {
     MemoryRegion iomem;
-    AddressSpace *as;
+    FlatView *fv;
     hwaddr base;
     uint16_t sub_section[];
 } subpage_t;
@@ -468,13 +468,13 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
 }
 
 /* Called from RCU critical section */
-static MemoryRegionSection address_space_do_translate(AddressSpace *as,
-                                                      hwaddr addr,
-                                                      hwaddr *xlat,
-                                                      hwaddr *plen,
-                                                      bool is_write,
-                                                      bool is_mmio,
-                                                      AddressSpace **target_as)
+static MemoryRegionSection flatview_do_translate(FlatView *fv,
+                                                 hwaddr addr,
+                                                 hwaddr *xlat,
+                                                 hwaddr *plen,
+                                                 bool is_write,
+                                                 bool is_mmio,
+                                                 AddressSpace **target_as)
 {
     IOMMUTLBEntry iotlb;
     MemoryRegionSection *section;
@@ -482,8 +482,9 @@ static MemoryRegionSection address_space_do_translate(AddressSpace *as,
     IOMMUMemoryRegionClass *imrc;
 
     for (;;) {
-        AddressSpaceDispatch *d = address_space_to_dispatch(as);
-        section = address_space_translate_internal(d, addr, &addr, plen, is_mmio);
+        section = address_space_translate_internal(
+                flatview_to_dispatch(fv), addr, &addr,
+                plen, is_mmio);
 
         iommu_mr = memory_region_get_iommu(section->mr);
         if (!iommu_mr) {
@@ -500,7 +501,7 @@ static MemoryRegionSection address_space_do_translate(AddressSpace *as,
             goto translate_fail;
         }
 
-        as = iotlb.target_as;
+        fv = address_space_to_flatview(iotlb.target_as);
         *target_as = iotlb.target_as;
     }
 
@@ -523,8 +524,8 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
     plen = (hwaddr)-1;
 
     /* This can never be MMIO. */
-    section = address_space_do_translate(as, addr, &xlat, &plen,
-                                         is_write, false, &as);
+    section = flatview_do_translate(address_space_to_flatview(as), addr,
+                                    &xlat, &plen, is_write, false, &as);
 
     /* Illegal translation */
     if (section.mr == &io_mem_unassigned) {
@@ -560,16 +561,15 @@ iotlb_fail:
 }
 
 /* Called from RCU critical section */
-MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
-                                      hwaddr *xlat, hwaddr *plen,
-                                      bool is_write)
+MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr *xlat,
+                                 hwaddr *plen, bool is_write)
 {
     MemoryRegion *mr;
     MemoryRegionSection section;
+    AddressSpace *as = NULL;
 
     /* This can be MMIO, so setup MMIO bit. */
-    section = address_space_do_translate(as, addr, xlat, plen, is_write, true,
-                                         &as);
+    section = flatview_do_translate(fv, addr, xlat, plen, is_write, true, &as);
     mr = section.mr;
 
     if (xen_enabled() && memory_access_is_direct(mr, is_write)) {
@@ -1219,7 +1219,7 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
     } else {
         AddressSpaceDispatch *d;
 
-        d = address_space_to_dispatch(section->address_space);
+        d = flatview_to_dispatch(section->fv);
         iotlb = section - d->map.sections;
         iotlb += xlat;
     }
@@ -1245,7 +1245,7 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
 
 static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
                              uint16_t section);
-static subpage_t *subpage_init(AddressSpace *as, hwaddr base);
+static subpage_t *subpage_init(FlatView *fv, hwaddr base);
 
 static void *(*phys_mem_alloc)(size_t size, uint64_t *align) =
                                qemu_anon_ram_alloc;
@@ -1302,7 +1302,7 @@ static void phys_sections_free(PhysPageMap *map)
     g_free(map->nodes);
 }
 
-static void register_subpage(AddressSpace *as, AddressSpaceDispatch *d,
+static void register_subpage(FlatView *fv, AddressSpaceDispatch *d,
                              MemoryRegionSection *section)
 {
     subpage_t *subpage;
@@ -1318,8 +1318,8 @@ static void register_subpage(AddressSpace *as, AddressSpaceDispatch *d,
     assert(existing->mr->subpage || existing->mr == &io_mem_unassigned);
 
     if (!(existing->mr->subpage)) {
-        subpage = subpage_init(as, base);
-        subsection.address_space = as;
+        subpage = subpage_init(fv, base);
+        subsection.fv = fv;
         subsection.mr = &subpage->iomem;
         phys_page_set(d, base >> TARGET_PAGE_BITS, 1,
                       phys_section_add(&d->map, &subsection));
@@ -1345,7 +1345,7 @@ static void register_multipage(AddressSpaceDispatch *d,
     phys_page_set(d, start_addr >> TARGET_PAGE_BITS, num_pages, section_index);
 }
 
-void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section)
+void mem_add(FlatView *fv, MemoryRegionSection *section)
 {
     AddressSpaceDispatch *d = flatview_to_dispatch(fv);
     MemoryRegionSection now = *section, remain = *section;
@@ -1356,7 +1356,7 @@ void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section)
                        - now.offset_within_address_space;
 
         now.size = int128_min(int128_make64(left), now.size);
-        register_subpage(as, d, &now);
+        register_subpage(fv, d, &now);
     } else {
         now.size = int128_zero();
     }
@@ -1366,10 +1366,10 @@ void mem_add(AddressSpace *as, FlatView *fv, MemoryRegionSection *section)
         remain.offset_within_region += int128_get64(now.size);
         now = remain;
         if (int128_lt(remain.size, page_size)) {
-            register_subpage(as, d, &now);
+            register_subpage(fv, d, &now);
         } else if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
             now.size = page_size;
-            register_subpage(as, d, &now);
+            register_subpage(fv, d, &now);
         } else {
             now.size = int128_and(now.size, int128_neg(page_size));
             register_multipage(d, &now);
@@ -2500,6 +2500,11 @@ static const MemoryRegionOps watch_mem_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
+                                  const uint8_t *buf, int len);
+static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len,
+                                  bool is_write);
+
 static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data,
                                 unsigned len, MemTxAttrs attrs)
 {
@@ -2511,8 +2516,7 @@ static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data,
     printf("%s: subpage %p len %u addr " TARGET_FMT_plx "\n", __func__,
            subpage, len, addr);
 #endif
-    res = address_space_read(subpage->as, addr + subpage->base,
-                             attrs, buf, len);
+    res = flatview_read(subpage->fv, addr + subpage->base, attrs, buf, len);
     if (res) {
         return res;
     }
@@ -2561,8 +2565,7 @@ static MemTxResult subpage_write(void *opaque, hwaddr addr,
     default:
         abort();
     }
-    return address_space_write(subpage->as, addr + subpage->base,
-                               attrs, buf, len);
+    return flatview_write(subpage->fv, addr + subpage->base, attrs, buf, len);
 }
 
 static bool subpage_accepts(void *opaque, hwaddr addr,
@@ -2574,8 +2577,8 @@ static bool subpage_accepts(void *opaque, hwaddr addr,
            __func__, subpage, is_write ? 'w' : 'r', len, addr);
 #endif
 
-    return address_space_access_valid(subpage->as, addr + subpage->base,
-                                      len, is_write);
+    return flatview_access_valid(subpage->fv, addr + subpage->base,
+                                 len, is_write);
 }
 
 static const MemoryRegionOps subpage_ops = {
@@ -2609,12 +2612,12 @@ static int subpage_register (subpage_t *mmio, uint32_t start, uint32_t end,
     return 0;
 }
 
-static subpage_t *subpage_init(AddressSpace *as, hwaddr base)
+static subpage_t *subpage_init(FlatView *fv, hwaddr base)
 {
     subpage_t *mmio;
 
     mmio = g_malloc0(sizeof(subpage_t) + TARGET_PAGE_SIZE * sizeof(uint16_t));
-    mmio->as = as;
+    mmio->fv = fv;
     mmio->base = base;
     memory_region_init_io(&mmio->iomem, NULL, &subpage_ops, mmio,
                           NULL, TARGET_PAGE_SIZE);
@@ -2628,12 +2631,11 @@ static subpage_t *subpage_init(AddressSpace *as, hwaddr base)
     return mmio;
 }
 
-static uint16_t dummy_section(PhysPageMap *map, AddressSpace *as,
-                              MemoryRegion *mr)
+static uint16_t dummy_section(PhysPageMap *map, FlatView *fv, MemoryRegion *mr)
 {
-    assert(as);
+    assert(fv);
     MemoryRegionSection section = {
-        .address_space = as,
+        .fv = fv,
         .mr = mr,
         .offset_within_address_space = 0,
         .offset_within_region = 0,
@@ -2672,16 +2674,17 @@ static void io_mem_init(void)
 
 AddressSpaceDispatch *mem_begin(AddressSpace *as)
 {
+    FlatView *fv = address_space_to_flatview(as);
     AddressSpaceDispatch *d = g_new0(AddressSpaceDispatch, 1);
     uint16_t n;
 
-    n = dummy_section(&d->map, as, &io_mem_unassigned);
+    n = dummy_section(&d->map, fv, &io_mem_unassigned);
     assert(n == PHYS_SECTION_UNASSIGNED);
-    n = dummy_section(&d->map, as, &io_mem_notdirty);
+    n = dummy_section(&d->map, fv, &io_mem_notdirty);
     assert(n == PHYS_SECTION_NOTDIRTY);
-    n = dummy_section(&d->map, as, &io_mem_rom);
+    n = dummy_section(&d->map, fv, &io_mem_rom);
     assert(n == PHYS_SECTION_ROM);
-    n = dummy_section(&d->map, as, &io_mem_watch);
+    n = dummy_section(&d->map, fv, &io_mem_watch);
     assert(n == PHYS_SECTION_WATCH);
 
     d->phys_map  = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .skip = 1 };
@@ -2861,11 +2864,11 @@ static bool prepare_mmio_access(MemoryRegion *mr)
 }
 
 /* Called within RCU critical section.  */
-static MemTxResult address_space_write_continue(AddressSpace *as, hwaddr addr,
-                                                MemTxAttrs attrs,
-                                                const uint8_t *buf,
-                                                int len, hwaddr addr1,
-                                                hwaddr l, MemoryRegion *mr)
+static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr,
+                                           MemTxAttrs attrs,
+                                           const uint8_t *buf,
+                                           int len, hwaddr addr1,
+                                           hwaddr l, MemoryRegion *mr)
 {
     uint8_t *ptr;
     uint64_t val;
@@ -2927,14 +2930,14 @@ static MemTxResult address_space_write_continue(AddressSpace *as, hwaddr addr,
         }
 
         l = len;
-        mr = address_space_translate(as, addr, &addr1, &l, true);
+        mr = flatview_translate(fv, addr, &addr1, &l, true);
     }
 
     return result;
 }
 
-MemTxResult address_space_write(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
-                                const uint8_t *buf, int len)
+static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
+                                  const uint8_t *buf, int len)
 {
     hwaddr l;
     hwaddr addr1;
@@ -2944,20 +2947,27 @@ MemTxResult address_space_write(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
     if (len > 0) {
         rcu_read_lock();
         l = len;
-        mr = address_space_translate(as, addr, &addr1, &l, true);
-        result = address_space_write_continue(as, addr, attrs, buf, len,
-                                              addr1, l, mr);
+        mr = flatview_translate(fv, addr, &addr1, &l, true);
+        result = flatview_write_continue(fv, addr, attrs, buf, len,
+                                         addr1, l, mr);
         rcu_read_unlock();
     }
 
     return result;
 }
 
+MemTxResult address_space_write(AddressSpace *as, hwaddr addr,
+                                              MemTxAttrs attrs,
+                                              const uint8_t *buf, int len)
+{
+    return flatview_write(address_space_to_flatview(as), addr, attrs, buf, len);
+}
+
 /* Called within RCU critical section.  */
-MemTxResult address_space_read_continue(AddressSpace *as, hwaddr addr,
-                                        MemTxAttrs attrs, uint8_t *buf,
-                                        int len, hwaddr addr1, hwaddr l,
-                                        MemoryRegion *mr)
+MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
+                                   MemTxAttrs attrs, uint8_t *buf,
+                                   int len, hwaddr addr1, hwaddr l,
+                                   MemoryRegion *mr)
 {
     uint8_t *ptr;
     uint64_t val;
@@ -3017,14 +3027,14 @@ MemTxResult address_space_read_continue(AddressSpace *as, hwaddr addr,
         }
 
         l = len;
-        mr = address_space_translate(as, addr, &addr1, &l, false);
+        mr = flatview_translate(fv, addr, &addr1, &l, false);
     }
 
     return result;
 }
 
-MemTxResult address_space_read_full(AddressSpace *as, hwaddr addr,
-                                    MemTxAttrs attrs, uint8_t *buf, int len)
+MemTxResult flatview_read_full(FlatView *fv, hwaddr addr,
+                               MemTxAttrs attrs, uint8_t *buf, int len)
 {
     hwaddr l;
     hwaddr addr1;
@@ -3034,25 +3044,33 @@ MemTxResult address_space_read_full(AddressSpace *as, hwaddr addr,
     if (len > 0) {
         rcu_read_lock();
         l = len;
-        mr = address_space_translate(as, addr, &addr1, &l, false);
-        result = address_space_read_continue(as, addr, attrs, buf, len,
-                                             addr1, l, mr);
+        mr = flatview_translate(fv, addr, &addr1, &l, false);
+        result = flatview_read_continue(fv, addr, attrs, buf, len,
+                                        addr1, l, mr);
         rcu_read_unlock();
     }
 
     return result;
 }
 
-MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
-                             uint8_t *buf, int len, bool is_write)
+static MemTxResult flatview_rw(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
+                               uint8_t *buf, int len, bool is_write)
 {
     if (is_write) {
-        return address_space_write(as, addr, attrs, (uint8_t *)buf, len);
+        return flatview_write(fv, addr, attrs, (uint8_t *)buf, len);
     } else {
-        return address_space_read(as, addr, attrs, (uint8_t *)buf, len);
+        return flatview_read(fv, addr, attrs, (uint8_t *)buf, len);
     }
 }
 
+MemTxResult address_space_rw(AddressSpace *as, hwaddr addr,
+                             MemTxAttrs attrs, uint8_t *buf,
+                             int len, bool is_write)
+{
+    return flatview_rw(address_space_to_flatview(as),
+                       addr, attrs, buf, len, is_write);
+}
+
 void cpu_physical_memory_rw(hwaddr addr, uint8_t *buf,
                             int len, int is_write)
 {
@@ -3210,7 +3228,8 @@ static void cpu_notify_map_clients(void)
     qemu_mutex_unlock(&map_client_list_lock);
 }
 
-bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len, bool is_write)
+static bool flatview_access_valid(FlatView *fv, hwaddr addr, int len,
+                                  bool is_write)
 {
     MemoryRegion *mr;
     hwaddr l, xlat;
@@ -3218,7 +3237,7 @@ bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len, bool is_
     rcu_read_lock();
     while (len > 0) {
         l = len;
-        mr = address_space_translate(as, addr, &xlat, &l, is_write);
+        mr = flatview_translate(fv, addr, &xlat, &l, is_write);
         if (!memory_access_is_direct(mr, is_write)) {
             l = memory_access_size(mr, l, addr);
             if (!memory_region_access_valid(mr, xlat, l, is_write)) {
@@ -3234,8 +3253,16 @@ bool address_space_access_valid(AddressSpace *as, hwaddr addr, int len, bool is_
     return true;
 }
 
+bool address_space_access_valid(AddressSpace *as, hwaddr addr,
+                                int len, bool is_write)
+{
+    return flatview_access_valid(address_space_to_flatview(as),
+                                 addr, len, is_write);
+}
+
 static hwaddr
-address_space_extend_translation(AddressSpace *as, hwaddr addr, hwaddr target_len,
+flatview_extend_translation(FlatView *fv, hwaddr addr,
+                                 hwaddr target_len,
                                  MemoryRegion *mr, hwaddr base, hwaddr len,
                                  bool is_write)
 {
@@ -3252,7 +3279,8 @@ address_space_extend_translation(AddressSpace *as, hwaddr addr, hwaddr target_le
         }
 
         len = target_len;
-        this_mr = address_space_translate(as, addr, &xlat, &len, is_write);
+        this_mr = flatview_translate(fv, addr, &xlat,
+                                                   &len, is_write);
         if (this_mr != mr || xlat != base + done) {
             return done;
         }
@@ -3275,6 +3303,7 @@ void *address_space_map(AddressSpace *as,
     hwaddr l, xlat;
     MemoryRegion *mr;
     void *ptr;
+    FlatView *fv = address_space_to_flatview(as);
 
     if (len == 0) {
         return NULL;
@@ -3282,7 +3311,7 @@ void *address_space_map(AddressSpace *as,
 
     l = len;
     rcu_read_lock();
-    mr = address_space_translate(as, addr, &xlat, &l, is_write);
+    mr = flatview_translate(fv, addr, &xlat, &l, is_write);
 
     if (!memory_access_is_direct(mr, is_write)) {
         if (atomic_xchg(&bounce.in_use, true)) {
@@ -3298,7 +3327,7 @@ void *address_space_map(AddressSpace *as,
         memory_region_ref(mr);
         bounce.mr = mr;
         if (!is_write) {
-            address_space_read(as, addr, MEMTXATTRS_UNSPECIFIED,
+            flatview_read(fv, addr, MEMTXATTRS_UNSPECIFIED,
                                bounce.buffer, l);
         }
 
@@ -3309,7 +3338,8 @@ void *address_space_map(AddressSpace *as,
 
 
     memory_region_ref(mr);
-    *plen = address_space_extend_translation(as, addr, len, mr, xlat, l, is_write);
+    *plen = flatview_extend_translation(fv, addr, len, mr, xlat,
+                                             l, is_write);
     ptr = qemu_ram_ptr_length(mr->ram_block, xlat, plen, true);
     rcu_read_unlock();
 
diff --git a/hw/intc/openpic_kvm.c b/hw/intc/openpic_kvm.c
index 0518e017c4..fa83420254 100644
--- a/hw/intc/openpic_kvm.c
+++ b/hw/intc/openpic_kvm.c
@@ -124,7 +124,7 @@ static void kvm_openpic_region_add(MemoryListener *listener,
     uint64_t reg_base;
     int ret;
 
-    if (section->address_space != &address_space_memory) {
+    if (section->fv != address_space_to_flatview(&address_space_memory)) {
         abort();
     }
 
diff --git a/memory.c b/memory.c
index 27d7aeffc2..1364500eb6 100644
--- a/memory.c
+++ b/memory.c
@@ -154,7 +154,8 @@ enum ListenerDirection { Forward, Reverse };
 /* No need to ref/unref .mr, the FlatRange keeps it alive.  */
 #define MEMORY_LISTENER_UPDATE_REGION(fr, as, dir, callback, _args...)  \
     do {                                                                \
-        MemoryRegionSection mrs = section_from_flat_range(fr, as);      \
+        MemoryRegionSection mrs = section_from_flat_range(fr,           \
+                address_space_to_flatview(as));                         \
         MEMORY_LISTENER_CALL(as, callback, dir, &mrs, ##_args);         \
     } while(0)
 
@@ -208,7 +209,6 @@ static bool memory_region_ioeventfd_equal(MemoryRegionIoeventfd a,
 }
 
 typedef struct FlatRange FlatRange;
-typedef struct FlatView FlatView;
 
 /* Range of memory in the global map.  Addresses are absolute. */
 struct FlatRange {
@@ -238,11 +238,11 @@ typedef struct AddressSpaceOps AddressSpaceOps;
     for (var = (view)->ranges; var < (view)->ranges + (view)->nr; ++var)
 
 static inline MemoryRegionSection
-section_from_flat_range(FlatRange *fr, AddressSpace *as)
+section_from_flat_range(FlatRange *fr, FlatView *fv)
 {
     return (MemoryRegionSection) {
         .mr = fr->mr,
-        .address_space = as,
+        .fv = fv,
         .offset_within_region = fr->offset_in_region,
         .size = fr->addr.size,
         .offset_within_address_space = int128_get64(fr->addr.start),
@@ -312,7 +312,7 @@ static void flatview_unref(FlatView *view)
     }
 }
 
-static FlatView *address_space_to_flatview(AddressSpace *as)
+FlatView *address_space_to_flatview(AddressSpace *as)
 {
     return atomic_rcu_read(&as->current_map);
 }
@@ -760,7 +760,7 @@ static void address_space_add_del_ioeventfds(AddressSpace *as,
                                                   fds_new[inew]))) {
             fd = &fds_old[iold];
             section = (MemoryRegionSection) {
-                .address_space = as,
+                .fv = address_space_to_flatview(as),
                 .offset_within_address_space = int128_get64(fd->addr.start),
                 .size = fd->addr.size,
             };
@@ -773,7 +773,7 @@ static void address_space_add_del_ioeventfds(AddressSpace *as,
                                                          fds_old[iold]))) {
             fd = &fds_new[inew];
             section = (MemoryRegionSection) {
-                .address_space = as,
+                .fv = address_space_to_flatview(as),
                 .offset_within_address_space = int128_get64(fd->addr.start),
                 .size = fd->addr.size,
             };
@@ -908,8 +908,8 @@ static void address_space_update_topology(AddressSpace *as)
     new_view->dispatch = mem_begin(as);
     for (i = 0; i < new_view->nr; i++) {
         MemoryRegionSection mrs =
-            section_from_flat_range(&new_view->ranges[i], as);
-        mem_add(as, new_view, &mrs);
+            section_from_flat_range(&new_view->ranges[i], new_view);
+        mem_add(new_view, &mrs);
     }
     mem_commit(new_view->dispatch);
 
@@ -1865,7 +1865,7 @@ void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
         view = address_space_get_flatview(as);
         FOR_EACH_FLAT_RANGE(fr, view) {
             if (fr->mr == mr) {
-                MemoryRegionSection mrs = section_from_flat_range(fr, as);
+                MemoryRegionSection mrs = section_from_flat_range(fr, view);
                 listener->log_sync(listener, &mrs);
             }
         }
@@ -1968,7 +1968,7 @@ static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpa
     FOR_EACH_FLAT_RANGE(fr, view) {
         if (fr->mr == mr) {
             section = (MemoryRegionSection) {
-                .address_space = as,
+                .fv = view,
                 .offset_within_address_space = int128_get64(fr->addr.start),
                 .size = fr->addr.size,
             };
@@ -2330,7 +2330,7 @@ static MemoryRegionSection memory_region_find_rcu(MemoryRegion *mr,
     }
 
     ret.mr = fr->mr;
-    ret.address_space = as;
+    ret.fv = view;
     range = addrrange_intersection(range, fr->addr);
     ret.offset_within_region = fr->offset_in_region;
     ret.offset_within_region += int128_get64(int128_sub(range.start,
@@ -2379,7 +2379,8 @@ void memory_global_dirty_log_sync(void)
         view = address_space_get_flatview(as);
         FOR_EACH_FLAT_RANGE(fr, view) {
             if (fr->dirty_log_mask) {
-                MemoryRegionSection mrs = section_from_flat_range(fr, as);
+                MemoryRegionSection mrs = section_from_flat_range(fr, view);
+
                 listener->log_sync(listener, &mrs);
             }
         }
@@ -2464,7 +2465,7 @@ static void listener_add_address_space(MemoryListener *listener,
     FOR_EACH_FLAT_RANGE(fr, view) {
         MemoryRegionSection section = {
             .mr = fr->mr,
-            .address_space = as,
+            .fv = view,
             .offset_within_region = fr->offset_in_region,
             .size = fr->addr.size,
             .offset_within_address_space = int128_get64(fr->addr.start),
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 07/18] memory: Cleanup after switching to FlatView
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
                   ` (5 preceding siblings ...)
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 06/18] memory: Switch memory from using AddressSpace to FlatView Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 08/18] memory: Rename mem_begin/mem_commit/mem_add helpers Alexey Kardashevskiy
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

We store AddressSpaceDispatch* in FlatView anyway so there is no need
to carry it from mem_add() to register_subpage/register_multipage.

This should cause no behavioural change.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 exec.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/exec.c b/exec.c
index d2b9f60494..548ec71b4c 100644
--- a/exec.c
+++ b/exec.c
@@ -1302,9 +1302,9 @@ static void phys_sections_free(PhysPageMap *map)
     g_free(map->nodes);
 }
 
-static void register_subpage(FlatView *fv, AddressSpaceDispatch *d,
-                             MemoryRegionSection *section)
+static void register_subpage(FlatView *fv, MemoryRegionSection *section)
 {
+    AddressSpaceDispatch *d = flatview_to_dispatch(fv);
     subpage_t *subpage;
     hwaddr base = section->offset_within_address_space
         & TARGET_PAGE_MASK;
@@ -1333,9 +1333,10 @@ static void register_subpage(FlatView *fv, AddressSpaceDispatch *d,
 }
 
 
-static void register_multipage(AddressSpaceDispatch *d,
+static void register_multipage(FlatView *fv,
                                MemoryRegionSection *section)
 {
+    AddressSpaceDispatch *d = flatview_to_dispatch(fv);
     hwaddr start_addr = section->offset_within_address_space;
     uint16_t section_index = phys_section_add(&d->map, section);
     uint64_t num_pages = int128_get64(int128_rshift(section->size,
@@ -1347,7 +1348,6 @@ static void register_multipage(AddressSpaceDispatch *d,
 
 void mem_add(FlatView *fv, MemoryRegionSection *section)
 {
-    AddressSpaceDispatch *d = flatview_to_dispatch(fv);
     MemoryRegionSection now = *section, remain = *section;
     Int128 page_size = int128_make64(TARGET_PAGE_SIZE);
 
@@ -1356,7 +1356,7 @@ void mem_add(FlatView *fv, MemoryRegionSection *section)
                        - now.offset_within_address_space;
 
         now.size = int128_min(int128_make64(left), now.size);
-        register_subpage(fv, d, &now);
+        register_subpage(fv, &now);
     } else {
         now.size = int128_zero();
     }
@@ -1366,13 +1366,13 @@ void mem_add(FlatView *fv, MemoryRegionSection *section)
         remain.offset_within_region += int128_get64(now.size);
         now = remain;
         if (int128_lt(remain.size, page_size)) {
-            register_subpage(fv, d, &now);
+            register_subpage(fv, &now);
         } else if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
             now.size = page_size;
-            register_subpage(fv, d, &now);
+            register_subpage(fv, &now);
         } else {
             now.size = int128_and(now.size, int128_neg(page_size));
-            register_multipage(d, &now);
+            register_multipage(fv, &now);
         }
     }
 }
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 08/18] memory: Rename mem_begin/mem_commit/mem_add helpers
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
                   ` (6 preceding siblings ...)
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 07/18] memory: Cleanup after switching " Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 09/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

This renames some helpers to reflect better what they do.

This should cause no behavioural change.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* s/flatview_mem_add/flatview_add_to_dispatch/
* s/phys_page_compact_all/address_space_dispatch_compact/
* s/address_space_dispatch_alloc/address_space_dispatch_new/
---
 include/exec/memory-internal.h |  6 +++---
 exec.c                         | 12 +++---------
 memory.c                       |  6 +++---
 3 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index 1cf8ad9869..d4a35c6e96 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -27,9 +27,9 @@ extern const MemoryRegionOps unassigned_mem_ops;
 bool memory_region_access_valid(MemoryRegion *mr, hwaddr addr,
                                 unsigned size, bool is_write);
 
-void mem_add(FlatView *fv, MemoryRegionSection *section);
-AddressSpaceDispatch *mem_begin(AddressSpace *as);
-void mem_commit(AddressSpaceDispatch *d);
+void flatview_add_to_dispatch(FlatView *fv, MemoryRegionSection *section);
+AddressSpaceDispatch *address_space_dispatch_new(FlatView *fv);
+void address_space_dispatch_compact(AddressSpaceDispatch *d);
 
 AddressSpaceDispatch *address_space_to_dispatch(AddressSpace *as);
 AddressSpaceDispatch *flatview_to_dispatch(FlatView *fv);
diff --git a/exec.c b/exec.c
index 548ec71b4c..b085f82503 100644
--- a/exec.c
+++ b/exec.c
@@ -358,7 +358,7 @@ static void phys_page_compact(PhysPageEntry *lp, Node *nodes)
     }
 }
 
-static void phys_page_compact_all(AddressSpaceDispatch *d, int nodes_nb)
+void address_space_dispatch_compact(AddressSpaceDispatch *d)
 {
     if (d->phys_map.skip) {
         phys_page_compact(&d->phys_map, d->map.nodes);
@@ -1346,7 +1346,7 @@ static void register_multipage(FlatView *fv,
     phys_page_set(d, start_addr >> TARGET_PAGE_BITS, num_pages, section_index);
 }
 
-void mem_add(FlatView *fv, MemoryRegionSection *section)
+void flatview_add_to_dispatch(FlatView *fv, MemoryRegionSection *section)
 {
     MemoryRegionSection now = *section, remain = *section;
     Int128 page_size = int128_make64(TARGET_PAGE_SIZE);
@@ -2672,9 +2672,8 @@ static void io_mem_init(void)
                           NULL, UINT64_MAX);
 }
 
-AddressSpaceDispatch *mem_begin(AddressSpace *as)
+AddressSpaceDispatch *address_space_dispatch_new(FlatView *fv)
 {
-    FlatView *fv = address_space_to_flatview(as);
     AddressSpaceDispatch *d = g_new0(AddressSpaceDispatch, 1);
     uint16_t n;
 
@@ -2698,11 +2697,6 @@ void address_space_dispatch_free(AddressSpaceDispatch *d)
     g_free(d);
 }
 
-void mem_commit(AddressSpaceDispatch *d)
-{
-    phys_page_compact_all(d, d->map.nodes_nb);
-}
-
 static void tcg_commit(MemoryListener *listener)
 {
     CPUAddressSpace *cpuas;
diff --git a/memory.c b/memory.c
index 1364500eb6..990e9b8478 100644
--- a/memory.c
+++ b/memory.c
@@ -905,13 +905,13 @@ static void address_space_update_topology(AddressSpace *as)
     FlatView *new_view = generate_memory_topology(as->root);
     int i;
 
-    new_view->dispatch = mem_begin(as);
+    new_view->dispatch = address_space_dispatch_new(new_view);
     for (i = 0; i < new_view->nr; i++) {
         MemoryRegionSection mrs =
             section_from_flat_range(&new_view->ranges[i], new_view);
-        mem_add(new_view, &mrs);
+        flatview_add_to_dispatch(new_view, &mrs);
     }
-    mem_commit(new_view->dispatch);
+    address_space_dispatch_compact(new_view->dispatch);
 
     if (!QTAILQ_EMPTY(&as->listeners)) {
         address_space_update_topology_pass(as, old_view, new_view, false);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 09/18] memory: Store physical root MR in FlatView
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
                   ` (7 preceding siblings ...)
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 08/18] memory: Rename mem_begin/mem_commit/mem_add helpers Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 17:15   ` Paolo Bonzini
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 10/18] memory: Alloc dispatch tree where topology is generared Alexey Kardashevskiy
                   ` (8 subsequent siblings)
  17 siblings, 1 reply; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

Address spaces get to keep a root MR (alias or not) but FlatView stores
the actual MR as this is going to be used later on to decide whether to
share a particular FlatView or not.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v4:
* s/memory_region_unalias_entire/memory_region_get_flatview_root/
---
 memory.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/memory.c b/memory.c
index 990e9b8478..98b0b95e6e 100644
--- a/memory.c
+++ b/memory.c
@@ -230,6 +230,7 @@ struct FlatView {
     unsigned nr;
     unsigned nr_allocated;
     struct AddressSpaceDispatch *dispatch;
+    MemoryRegion *root;
 };
 
 typedef struct AddressSpaceOps AddressSpaceOps;
@@ -259,12 +260,14 @@ static bool flatrange_equal(FlatRange *a, FlatRange *b)
         && a->readonly == b->readonly;
 }
 
-static FlatView *flatview_new(void)
+static FlatView *flatview_new(MemoryRegion *mr_root)
 {
     FlatView *view;
 
     view = g_new0(FlatView, 1);
     view->ref = 1;
+    view->root = mr_root;
+    memory_region_ref(mr_root);
 
     return view;
 }
@@ -297,6 +300,7 @@ static void flatview_destroy(FlatView *view)
         memory_region_unref(view->ranges[i].mr);
     }
     g_free(view->ranges);
+    memory_region_unref(view->root);
     g_free(view);
 }
 
@@ -722,12 +726,25 @@ static void render_memory_region(FlatView *view,
     }
 }
 
+static MemoryRegion *memory_region_get_flatview_root(MemoryRegion *mr)
+{
+    while (mr->alias && !mr->alias_offset &&
+           int128_ge(mr->size, mr->alias->size)) {
+        /* The alias is included in its entirety.  Use it as
+         * the "real" root, so that we can share more FlatViews.
+         */
+        mr = mr->alias;
+    }
+
+    return mr;
+}
+
 /* Render a memory topology into a list of disjoint absolute ranges. */
 static FlatView *generate_memory_topology(MemoryRegion *mr)
 {
     FlatView *view;
 
-    view = flatview_new();
+    view = flatview_new(mr);
 
     if (mr) {
         render_memory_region(view, mr, int128_zero(),
@@ -902,7 +919,8 @@ static void address_space_update_topology_pass(AddressSpace *as,
 static void address_space_update_topology(AddressSpace *as)
 {
     FlatView *old_view = address_space_get_flatview(as);
-    FlatView *new_view = generate_memory_topology(as->root);
+    MemoryRegion *physmr = memory_region_get_flatview_root(old_view->root);
+    FlatView *new_view = generate_memory_topology(physmr);
     int i;
 
     new_view->dispatch = address_space_dispatch_new(new_view);
@@ -2645,7 +2663,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
     as->ref_count = 1;
     as->root = root;
     as->malloced = false;
-    as->current_map = flatview_new();
+    as->current_map = flatview_new(root);
     as->ioeventfd_nb = 0;
     as->ioeventfds = NULL;
     QTAILQ_INIT(&as->listeners);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 10/18] memory: Alloc dispatch tree where topology is generared
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
                   ` (8 preceding siblings ...)
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 09/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 11/18] memory: Move address_space_update_ioeventfds Alexey Kardashevskiy
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

This is to make next patches simpler.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 memory.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/memory.c b/memory.c
index 98b0b95e6e..eb730cd619 100644
--- a/memory.c
+++ b/memory.c
@@ -742,6 +742,7 @@ static MemoryRegion *memory_region_get_flatview_root(MemoryRegion *mr)
 /* Render a memory topology into a list of disjoint absolute ranges. */
 static FlatView *generate_memory_topology(MemoryRegion *mr)
 {
+    int i;
     FlatView *view;
 
     view = flatview_new(mr);
@@ -752,6 +753,14 @@ static FlatView *generate_memory_topology(MemoryRegion *mr)
     }
     flatview_simplify(view);
 
+    view->dispatch = address_space_dispatch_new(view);
+    for (i = 0; i < view->nr; i++) {
+        MemoryRegionSection mrs =
+            section_from_flat_range(&view->ranges[i], view);
+        flatview_add_to_dispatch(view, &mrs);
+    }
+    address_space_dispatch_compact(view->dispatch);
+
     return view;
 }
 
@@ -921,15 +930,6 @@ static void address_space_update_topology(AddressSpace *as)
     FlatView *old_view = address_space_get_flatview(as);
     MemoryRegion *physmr = memory_region_get_flatview_root(old_view->root);
     FlatView *new_view = generate_memory_topology(physmr);
-    int i;
-
-    new_view->dispatch = address_space_dispatch_new(new_view);
-    for (i = 0; i < new_view->nr; i++) {
-        MemoryRegionSection mrs =
-            section_from_flat_range(&new_view->ranges[i], new_view);
-        flatview_add_to_dispatch(new_view, &mrs);
-    }
-    address_space_dispatch_compact(new_view->dispatch);
 
     if (!QTAILQ_EMPTY(&as->listeners)) {
         address_space_update_topology_pass(as, old_view, new_view, false);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 11/18] memory: Move address_space_update_ioeventfds
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
                   ` (9 preceding siblings ...)
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 10/18] memory: Alloc dispatch tree where topology is generared Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 12/18] memory: Share FlatView's and dispatch trees between address spaces Alexey Kardashevskiy
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

So it is called (twice) from the same function. This is to make the next
patches a bit simpler.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 memory.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/memory.c b/memory.c
index eb730cd619..9868d0f57c 100644
--- a/memory.c
+++ b/memory.c
@@ -947,8 +947,6 @@ static void address_space_update_topology(AddressSpace *as)
      * counting is necessary.
      */
     flatview_unref(old_view);
-
-    address_space_update_ioeventfds(as);
 }
 
 void memory_region_transaction_begin(void)
@@ -971,6 +969,7 @@ void memory_region_transaction_commit(void)
 
             QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
                 address_space_update_topology(as);
+                address_space_update_ioeventfds(as);
             }
             memory_region_update_pending = false;
             MEMORY_LISTENER_CALL_GLOBAL(commit, Forward);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 12/18] memory: Share FlatView's and dispatch trees between address spaces
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
                   ` (10 preceding siblings ...)
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 11/18] memory: Move address_space_update_ioeventfds Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 17:18   ` Paolo Bonzini
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 13/18] memory: Do not allocate FlatView in address_space_init Alexey Kardashevskiy
                   ` (5 subsequent siblings)
  17 siblings, 1 reply; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

This allows sharing flat views between address spaces (AS) when
the same root memory region is used when creating a new address space.
This is done by walking through all ASes and caching one FlatView per
a physical root MR (i.e. not aliased).

This removes search for duplicates from address_space_init_shareable() as
FlatViews are shared elsewhere and keeping as::ref_count correct seems
an unnecessary and useless complication.

This should cause no change and memory use or boot time yet.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v4:
* used g_hash_table_new_full() instead of g_hash_table_new() +
g_hash_table_foreach_remove()
* cached last hash table of FV globally

v3:
* got rid of global and per-AS lists of FlatView objects
---
 memory.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 46 insertions(+), 11 deletions(-)

diff --git a/memory.c b/memory.c
index 9868d0f57c..1083fe9130 100644
--- a/memory.c
+++ b/memory.c
@@ -47,6 +47,8 @@ static QTAILQ_HEAD(memory_listeners, MemoryListener) memory_listeners
 static QTAILQ_HEAD(, AddressSpace) address_spaces
     = QTAILQ_HEAD_INITIALIZER(address_spaces);
 
+static GHashTable *flat_views;
+
 typedef struct AddrRange AddrRange;
 
 /*
@@ -925,11 +927,49 @@ static void address_space_update_topology_pass(AddressSpace *as,
     }
 }
 
-static void address_space_update_topology(AddressSpace *as)
+static void flatviews_init(void)
+{
+    if (flat_views) {
+        return;
+    }
+
+    flat_views = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL,
+                                       (GDestroyNotify) flatview_unref);
+}
+
+static void flatviews_reset(void)
+{
+    AddressSpace *as;
+
+    if (flat_views) {
+        g_hash_table_unref(flat_views);
+        flat_views = NULL;
+    }
+    flatviews_init();
+
+    /* Render unique FVs */
+    QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
+        MemoryRegion *physmr = memory_region_get_flatview_root(as->root);
+        FlatView *new_view = g_hash_table_lookup(flat_views, physmr);
+
+        if (new_view) {
+            continue;
+        }
+
+        new_view = generate_memory_topology(physmr);
+        g_hash_table_insert(flat_views, physmr, new_view);
+    }
+}
+
+static void flatview_set_to_address_space(AddressSpace *as)
 {
     FlatView *old_view = address_space_get_flatview(as);
-    MemoryRegion *physmr = memory_region_get_flatview_root(old_view->root);
-    FlatView *new_view = generate_memory_topology(physmr);
+    MemoryRegion *physmr = memory_region_get_flatview_root(as->root);
+    FlatView *new_view = g_hash_table_lookup(flat_views, physmr);
+
+    assert(new_view);
+
+    flatview_ref(new_view);
 
     if (!QTAILQ_EMPTY(&as->listeners)) {
         address_space_update_topology_pass(as, old_view, new_view, false);
@@ -965,10 +1005,12 @@ void memory_region_transaction_commit(void)
     --memory_region_transaction_depth;
     if (!memory_region_transaction_depth) {
         if (memory_region_update_pending) {
+            flatviews_reset();
+
             MEMORY_LISTENER_CALL_GLOBAL(begin, Forward);
 
             QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
-                address_space_update_topology(as);
+                flatview_set_to_address_space(as);
                 address_space_update_ioeventfds(as);
             }
             memory_region_update_pending = false;
@@ -2691,13 +2733,6 @@ AddressSpace *address_space_init_shareable(MemoryRegion *root, const char *name)
 {
     AddressSpace *as;
 
-    QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
-        if (root == as->root && as->malloced) {
-            as->ref_count++;
-            return as;
-        }
-    }
-
     as = g_malloc0(sizeof *as);
     address_space_init(as, root, name);
     as->malloced = true;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 13/18] memory: Do not allocate FlatView in address_space_init
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
                   ` (11 preceding siblings ...)
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 12/18] memory: Share FlatView's and dispatch trees between address spaces Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 14/18] memory: Add flat views to HMP "info mtree" Alexey Kardashevskiy
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

This creates a new AS object without any FlatView as
memory_region_transaction_commit() may want to reuse the empty FV.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

I do not really want to add view!=NULL into
flatview_ref()/flatview_unref() as this is quite special case when
view==NULL.
---
 memory.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/memory.c b/memory.c
index 1083fe9130..aa8a49539d 100644
--- a/memory.c
+++ b/memory.c
@@ -963,22 +963,37 @@ static void flatviews_reset(void)
 
 static void flatview_set_to_address_space(AddressSpace *as)
 {
-    FlatView *old_view = address_space_get_flatview(as);
+    FlatView *old_view = address_space_to_flatview(as);
     MemoryRegion *physmr = memory_region_get_flatview_root(as->root);
     FlatView *new_view = g_hash_table_lookup(flat_views, physmr);
 
     assert(new_view);
 
+    if (old_view == new_view) {
+        return;
+    }
+
+    if (old_view) {
+        flatview_ref(old_view);
+    }
+
     flatview_ref(new_view);
 
     if (!QTAILQ_EMPTY(&as->listeners)) {
-        address_space_update_topology_pass(as, old_view, new_view, false);
-        address_space_update_topology_pass(as, old_view, new_view, true);
+        FlatView tmpview = { 0 }, *old_view2 = old_view;
+
+        if (!old_view2) {
+            old_view2 = &tmpview;
+        }
+        address_space_update_topology_pass(as, old_view2, new_view, false);
+        address_space_update_topology_pass(as, old_view2, new_view, true);
     }
 
     /* Writes are protected by the BQL.  */
     atomic_rcu_set(&as->current_map, new_view);
-    flatview_unref(old_view);
+    if (old_view) {
+        flatview_unref(old_view);
+    }
 
     /* Note that all the old MemoryRegions are still alive up to this
      * point.  This relieves most MemoryListeners from the need to
@@ -986,7 +1001,9 @@ static void flatview_set_to_address_space(AddressSpace *as)
      * outside the iothread mutex, in which case precise reference
      * counting is necessary.
      */
-    flatview_unref(old_view);
+    if (old_view) {
+        flatview_unref(old_view);
+    }
 }
 
 void memory_region_transaction_begin(void)
@@ -2704,7 +2721,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
     as->ref_count = 1;
     as->root = root;
     as->malloced = false;
-    as->current_map = flatview_new(root);
+    as->current_map = NULL;
     as->ioeventfd_nb = 0;
     as->ioeventfds = NULL;
     QTAILQ_INIT(&as->listeners);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 14/18] memory: Add flat views to HMP "info mtree"
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
                   ` (12 preceding siblings ...)
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 13/18] memory: Do not allocate FlatView in address_space_init Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 15/18] memory: Share special empty FlatView Alexey Kardashevskiy
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

This adds a new "-d" switch to "info mtree" to print dispatch tree
internals.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* reimplemented as there is no more global FlatView list

---

Example:

QEMU 2.10.50 monitor - type 'help' for more information
(qemu) info mtree -f -d
FlatView #0
 AS "memory", root: system
 AS "cpu-memory", root: system
 AS "cpu-memory", root: system
 AS "cpu-memory", root: system
 AS "cpu-memory", root: system
 AS "cpu-memory", root: system
 AS "cpu-memory", root: system
 AS "cpu-memory", root: system
 AS "cpu-memory", root: system
 Root memory region: system
  0000000000000000-000000007fffffff (prio 0, ram): ppc_spapr.ram
  Dispatch
    Physical sections
      #0 @0000000000000000..ffffffffffffffff (noname) [unassigned]
      #1 @0000000000000000..ffffffffffffffff (noname) [not dirty]
      #2 @0000000000000000..ffffffffffffffff (noname) [ROM]
      #3 @0000000000000000..ffffffffffffffff (noname) [watch]
      #4 @0000000000000000..000000007fffffff ppc_spapr.ram [MRU]
    Nodes (9 bits per level, 6 levels) ptr=[3] skip=4
      [0]
	  0       skip=3  ptr=[3]
	  1..511  skip=1  ptr=NIL
      [1]
	  0       skip=2  ptr=[3]
	  1..511  skip=1  ptr=NIL
      [2]
	  0       skip=1  ptr=[3]
	  1..511  skip=1  ptr=NIL
      [3]
	  0..1    skip=0  ptr=#4
	  2..511  skip=1  ptr=NIL

FlatView #1
 AS "I/O", root: io
 Root memory region: io
  0000000000000000-000000000000ffff (prio 0, i/o): io
  Dispatch
    Physical sections
      #0 @0000000000000000..ffffffffffffffff (noname) [unassigned]
      #1 @0000000000000000..ffffffffffffffff (noname) [not dirty]
      #2 @0000000000000000..ffffffffffffffff (noname) [ROM]
      #3 @0000000000000000..ffffffffffffffff (noname) [watch]
      #4 @0000000000000000..000000000000ffff io [ROOT]
    Nodes (9 bits per level, 6 levels) ptr=[5] skip=6
      [0]
	  0       skip=5  ptr=[5]
	  1..511  skip=1  ptr=NIL
      [1]
	  0       skip=4  ptr=[5]
	  1..511  skip=1  ptr=NIL
      [2]
	  0       skip=3  ptr=[5]
	  1..511  skip=1  ptr=NIL
      [3]
	  0       skip=2  ptr=[5]
	  1..511  skip=1  ptr=NIL
      [4]
	  0       skip=1  ptr=[5]
	  1..511  skip=1  ptr=NIL
      [5]
	  0..15   skip=0  ptr=#4
	 16..511  skip=0  ptr=#0

FlatView #2
 AS "pci@800000020000000", root: pci@800000020000000.iommu-root
 Root memory region: pci@800000020000000.iommu-root
  0000000000000000-000000003fffffff (prio 0, i/o): tce-iommu-80000000
  0000040000000000-000004000000ffff (prio 0, i/o): msi
  Dispatch
    Physical sections
      #0 @0000000000000000..ffffffffffffffff (noname) [unassigned]
      #1 @0000000000000000..ffffffffffffffff (noname) [not dirty]
      #2 @0000000000000000..ffffffffffffffff (noname) [ROM]
      #3 @0000000000000000..ffffffffffffffff (noname) [watch]
      #4 @0000000000000000..000000003fffffff tce-iommu-80000000 [iommu]
      #5 @0000040000000000..000004000000ffff msi
    Nodes (9 bits per level, 6 levels) ptr=[2] skip=3
      [0]
	  0       skip=2  ptr=[2]
	  1..511  skip=1  ptr=NIL
      [1]
	  0       skip=1  ptr=[2]
	  1..511  skip=1  ptr=NIL
      [2]
	  0       skip=0  ptr=#4
	  1..7    skip=1  ptr=NIL
	  8       skip=3  ptr=[6]
	  9..511  skip=1  ptr=NIL
      [3]
	  0       skip=0  ptr=#4
	  1..511  skip=1  ptr=NIL
      [4]
	  0       skip=2  ptr=[6]
	  1..511  skip=1  ptr=NIL
      [5]
	  0       skip=1  ptr=[6]
	  1..511  skip=1  ptr=NIL
      [6]
	  0..15   skip=0  ptr=#5
	 16..511  skip=0  ptr=#0

FlatView #3
 AS "e1000", root: bus master container
 AS "e1000", root: bus master container
 AS "e1000", root: bus master container
 AS "e1000", root: bus master container
 AS "e1000", root: bus master container
 AS "e1000", root: bus master container
 AS "e1000", root: bus master container
 AS "e1000", root: bus master container
 AS "e1000", root: bus master container
 AS "e1000", root: bus master container
 Root memory region: (none)
  No rendered FlatView
---
 include/exec/memory-internal.h |  4 ++
 include/exec/memory.h          |  3 +-
 exec.c                         | 84 +++++++++++++++++++++++++++++++++++++++
 memory.c                       | 90 +++++++++++++++++++++++++++++++++++++-----
 monitor.c                      |  3 +-
 hmp-commands-info.hx           |  7 ++--
 6 files changed, 176 insertions(+), 15 deletions(-)

diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index d4a35c6e96..647e9bd5c4 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -35,5 +35,9 @@ AddressSpaceDispatch *address_space_to_dispatch(AddressSpace *as);
 AddressSpaceDispatch *flatview_to_dispatch(FlatView *fv);
 void address_space_dispatch_free(AddressSpaceDispatch *d);
 
+void mtree_print_dispatch(fprintf_function mon, void *f,
+                          struct AddressSpaceDispatch *d,
+                          MemoryRegion *root);
+
 #endif
 #endif
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 7816e5d655..2f4f56cf40 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1515,7 +1515,8 @@ void memory_global_dirty_log_start(void);
  */
 void memory_global_dirty_log_stop(void);
 
-void mtree_info(fprintf_function mon_printf, void *f, bool flatview);
+void mtree_info(fprintf_function mon_printf, void *f, bool flatview,
+                bool dispatch_tree);
 
 /**
  * memory_region_request_mmio_ptr: request a pointer to an mmio
diff --git a/exec.c b/exec.c
index b085f82503..7a80460725 100644
--- a/exec.c
+++ b/exec.c
@@ -3616,3 +3616,87 @@ void page_size_init(void)
     }
     qemu_host_page_mask = -(intptr_t)qemu_host_page_size;
 }
+
+#if !defined(CONFIG_USER_ONLY)
+
+static void mtree_print_phys_entries(fprintf_function mon, void *f,
+                                     int start, int end, int skip, int ptr)
+{
+    if (start == end - 1) {
+        mon(f, "\t%3d      ", start);
+    } else {
+        mon(f, "\t%3d..%-3d ", start, end - 1);
+    }
+    mon(f, " skip=%d ", skip);
+    if (ptr == PHYS_MAP_NODE_NIL) {
+        mon(f, " ptr=NIL");
+    } else if (!skip) {
+        mon(f, " ptr=#%d", ptr);
+    } else {
+        mon(f, " ptr=[%d]", ptr);
+    }
+    mon(f, "\n");
+}
+
+#define MR_SIZE(size) (int128_nz(size) ? (hwaddr)int128_get64( \
+                           int128_sub((size), int128_one())) : 0)
+
+void mtree_print_dispatch(fprintf_function mon, void *f,
+                          AddressSpaceDispatch *d, MemoryRegion *root)
+{
+    int i;
+
+    mon(f, "  Dispatch\n");
+    mon(f, "    Physical sections\n");
+
+    for (i = 0; i < d->map.sections_nb; ++i) {
+        MemoryRegionSection *s = d->map.sections + i;
+        const char *names[] = { " [unassigned]", " [not dirty]",
+                                " [ROM]", " [watch]" };
+
+        mon(f, "      #%d @" TARGET_FMT_plx ".." TARGET_FMT_plx " %s%s%s%s%s",
+            i,
+            s->offset_within_address_space,
+            s->offset_within_address_space + MR_SIZE(s->mr->size),
+            s->mr->name ? s->mr->name : "(noname)",
+            i < ARRAY_SIZE(names) ? names[i] : "",
+            s->mr == root ? " [ROOT]" : "",
+            s == d->mru_section ? " [MRU]" : "",
+            s->mr->is_iommu ? " [iommu]" : "");
+
+        if (s->mr->alias) {
+            mon(f, " alias=%s", s->mr->alias->name ?
+                    s->mr->alias->name : "noname");
+        }
+        mon(f, "\n");
+    }
+
+    mon(f, "    Nodes (%d bits per level, %d levels) ptr=[%d] skip=%d\n",
+               P_L2_BITS, P_L2_LEVELS, d->phys_map.ptr, d->phys_map.skip);
+    for (i = 0; i < d->map.nodes_nb; ++i) {
+        int j, jprev;
+        PhysPageEntry prev;
+        Node *n = d->map.nodes + i;
+
+        mon(f, "      [%d]\n", i);
+
+        for (j = 0, jprev = 0, prev = *n[0]; j < ARRAY_SIZE(*n); ++j) {
+            PhysPageEntry *pe = *n + j;
+
+            if (pe->ptr == prev.ptr && pe->skip == prev.skip) {
+                continue;
+            }
+
+            mtree_print_phys_entries(mon, f, jprev, j, prev.skip, prev.ptr);
+
+            jprev = j;
+            prev = *pe;
+        }
+
+        if (jprev != ARRAY_SIZE(*n)) {
+            mtree_print_phys_entries(mon, f, jprev, j, prev.skip, prev.ptr);
+        }
+    }
+}
+
+#endif
diff --git a/memory.c b/memory.c
index aa8a49539d..4add0fd030 100644
--- a/memory.c
+++ b/memory.c
@@ -2903,18 +2903,44 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
     }
 }
 
-static void mtree_print_flatview(fprintf_function p, void *f,
-                                 AddressSpace *as)
+struct FlatViewInfo {
+    fprintf_function mon_printf;
+    void *f;
+    int counter;
+    bool dispatch_tree;
+};
+
+static void mtree_print_flatview(gpointer key, gpointer value,
+                                 gpointer user_data)
 {
-    FlatView *view = address_space_get_flatview(as);
+    FlatView *view = key;
+    GArray *fv_address_spaces = value;
+    struct FlatViewInfo *fvi = user_data;
+    fprintf_function p = fvi->mon_printf;
+    void *f = fvi->f;
     FlatRange *range = &view->ranges[0];
     MemoryRegion *mr;
     int n = view->nr;
+    int i;
+    AddressSpace *as;
+
+    p(f, "FlatView #%d\n", fvi->counter);
+    ++fvi->counter;
+
+    for (i = 0; i < fv_address_spaces->len; ++i) {
+        as = g_array_index(fv_address_spaces, AddressSpace*, i);
+        p(f, " AS \"%s\", root: %s", as->name, memory_region_name(as->root));
+        if (as->root->alias) {
+            p(f, ", alias %s", memory_region_name(as->root->alias));
+        }
+        p(f, "\n");
+    }
+
+    p(f, " Root memory region: %s\n",
+      view->root ? memory_region_name(view->root) : "(none)");
 
     if (n <= 0) {
-        p(f, MTREE_INDENT "No rendered FlatView for "
-          "address space '%s'\n", as->name);
-        flatview_unref(view);
+        p(f, MTREE_INDENT "No rendered FlatView\n\n");
         return;
     }
 
@@ -2941,21 +2967,65 @@ static void mtree_print_flatview(fprintf_function p, void *f,
         range++;
     }
 
+#if !defined(CONFIG_USER_ONLY)
+    if (fvi->dispatch_tree && view->root) {
+        mtree_print_dispatch(p, f, view->dispatch, view->root);
+    }
+#endif
+
+    p(f, "\n");
+}
+
+static gboolean mtree_info_flatview_free(gpointer key, gpointer value,
+                                      gpointer user_data)
+{
+    FlatView *view = key;
+    GArray *fv_address_spaces = value;
+
+    g_array_unref(fv_address_spaces);
     flatview_unref(view);
+
+    return true;
 }
 
-void mtree_info(fprintf_function mon_printf, void *f, bool flatview)
+void mtree_info(fprintf_function mon_printf, void *f, bool flatview,
+                bool dispatch_tree)
 {
     MemoryRegionListHead ml_head;
     MemoryRegionList *ml, *ml2;
     AddressSpace *as;
 
     if (flatview) {
+        FlatView *view;
+        struct FlatViewInfo fvi = {
+            .mon_printf = mon_printf,
+            .f = f,
+            .counter = 0,
+            .dispatch_tree = dispatch_tree
+        };
+        GArray *fv_address_spaces;
+        GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal);
+
+        /* Gather all FVs in one table */
         QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
-            mon_printf(f, "address-space (flat view): %s\n", as->name);
-            mtree_print_flatview(mon_printf, f, as);
-            mon_printf(f, "\n");
+            view = address_space_get_flatview(as);
+
+            fv_address_spaces = g_hash_table_lookup(views, view);
+            if (!fv_address_spaces) {
+                fv_address_spaces = g_array_new(false, false, sizeof(as));
+                g_hash_table_insert(views, view, fv_address_spaces);
+            }
+
+            g_array_append_val(fv_address_spaces, as);
         }
+
+        /* Print */
+        g_hash_table_foreach(views, mtree_print_flatview, &fvi);
+
+        /* Free */
+        g_hash_table_foreach_remove(views, mtree_info_flatview_free, 0);
+        g_hash_table_unref(views);
+
         return;
     }
 
diff --git a/monitor.c b/monitor.c
index 058045b3cb..f4856b9268 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1703,8 +1703,9 @@ static void hmp_boot_set(Monitor *mon, const QDict *qdict)
 static void hmp_info_mtree(Monitor *mon, const QDict *qdict)
 {
     bool flatview = qdict_get_try_bool(qdict, "flatview", false);
+    bool dispatch_tree = qdict_get_try_bool(qdict, "dispatch_tree", false);
 
-    mtree_info((fprintf_function)monitor_printf, mon, flatview);
+    mtree_info((fprintf_function)monitor_printf, mon, flatview, dispatch_tree);
 }
 
 static void hmp_info_numa(Monitor *mon, const QDict *qdict)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 1c6772597d..4f1ece93e5 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -250,9 +250,10 @@ ETEXI
 
     {
         .name       = "mtree",
-        .args_type  = "flatview:-f",
-        .params     = "[-f]",
-        .help       = "show memory tree (-f: dump flat view for address spaces)",
+        .args_type  = "flatview:-f,dispatch_tree:-d",
+        .params     = "[-f][-d]",
+        .help       = "show memory tree (-f: dump flat view for address spaces;"
+                      "-d: dump dispatch tree, valid with -f only)",
         .cmd        = hmp_info_mtree,
     },
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 15/18] memory: Share special empty FlatView
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
                   ` (13 preceding siblings ...)
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 14/18] memory: Add flat views to HMP "info mtree" Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 17:13   ` Paolo Bonzini
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 16/18] memory: Get rid of address_space_init_shareable Alexey Kardashevskiy
                   ` (2 subsequent siblings)
  17 siblings, 1 reply; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

This shares an cached empty FlatView among address spaces. The empty
FV is used every time when a root MR renders into a FV without memory
sections which happens when MR or its children are not enabled or
zero-sized. The empty_view is not NULL to keep the rest of memory
API intact; it also has a dispatch tree for the same reason.

On POWER8 with 255 CPUs, 255 virtio-net, 40 PCI bridges guest this halves
the amount of FlatView's in use (557 -> 260) and dispatch tables
(~800000 -> ~370000), however the total memory footprint is pretty much
the same as RCU is holding all these temporary FVs which are created
(and then released) to make sure that they are empty and can be replaced
with @empty_view.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 memory.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/memory.c b/memory.c
index 4add0fd030..92b1304a20 100644
--- a/memory.c
+++ b/memory.c
@@ -48,6 +48,7 @@ static QTAILQ_HEAD(, AddressSpace) address_spaces
     = QTAILQ_HEAD_INITIALIZER(address_spaces);
 
 static GHashTable *flat_views;
+static FlatView *empty_view;
 
 typedef struct AddrRange AddrRange;
 
@@ -755,6 +756,19 @@ static FlatView *generate_memory_topology(MemoryRegion *mr)
     }
     flatview_simplify(view);
 
+    if (!view->nr) {
+        flatview_unref(view);
+        if (!empty_view) {
+            empty_view = flatview_new(NULL);
+        }
+        view = empty_view;
+        flatview_ref(view);
+    }
+
+    if (view->dispatch) {
+        return view;
+    }
+
     view->dispatch = address_space_dispatch_new(view);
     for (i = 0; i < view->nr; i++) {
         MemoryRegionSection mrs =
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 16/18] memory: Get rid of address_space_init_shareable
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
                   ` (14 preceding siblings ...)
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 15/18] memory: Share special empty FlatView Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 17/18] memory: Create FlatView directly Alexey Kardashevskiy
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 18/18] memory: Give memory_region_transaction_commit a hint Alexey Kardashevskiy
  17 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

Since FlatViews are shared now and ASes not, this gets rid of
address_space_init_shareable().

This should cause no behavioural change.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* now removes @malloced and @ref_count, used to be in the previos patch
---
 include/exec/memory.h   | 19 -------------------
 include/hw/arm/armv7m.h |  2 +-
 cpus.c                  |  5 +++--
 hw/arm/armv7m.c         |  9 ++++-----
 memory.c                | 22 +---------------------
 target/arm/cpu.c        | 16 ++++++++--------
 target/i386/cpu.c       |  5 +++--
 7 files changed, 20 insertions(+), 58 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2f4f56cf40..402824c6f2 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -309,8 +309,6 @@ struct AddressSpace {
     struct rcu_head rcu;
     char *name;
     MemoryRegion *root;
-    int ref_count;
-    bool malloced;
 
     /* Accessed via RCU.  */
     struct FlatView *current_map;
@@ -1586,23 +1584,6 @@ MemTxResult memory_region_dispatch_write(MemoryRegion *mr,
 void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name);
 
 /**
- * address_space_init_shareable: return an address space for a memory region,
- *                               creating it if it does not already exist
- *
- * @root: a #MemoryRegion that routes addresses for the address space
- * @name: an address space name.  The name is only used for debugging
- *        output.
- *
- * This function will return a pointer to an existing AddressSpace
- * which was initialized with the specified MemoryRegion, or it will
- * create and initialize one if it does not already exist. The ASes
- * are reference-counted, so the memory will be freed automatically
- * when the AddressSpace is destroyed via address_space_destroy.
- */
-AddressSpace *address_space_init_shareable(MemoryRegion *root,
-                                           const char *name);
-
-/**
  * address_space_destroy: destroy an address space
  *
  * Releases all resources associated with an address space.  After an address space
diff --git a/include/hw/arm/armv7m.h b/include/hw/arm/armv7m.h
index 10eb058027..008000fe11 100644
--- a/include/hw/arm/armv7m.h
+++ b/include/hw/arm/armv7m.h
@@ -21,7 +21,7 @@ typedef struct {
     SysBusDevice parent_obj;
     /*< public >*/
 
-    AddressSpace *source_as;
+    AddressSpace source_as;
     MemoryRegion iomem;
     uint32_t base;
     MemoryRegion *source_memory;
diff --git a/cpus.c b/cpus.c
index 9bed61eefc..c9a624003a 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1764,8 +1764,9 @@ void qemu_init_vcpu(CPUState *cpu)
         /* If the target cpu hasn't set up any address spaces itself,
          * give it the default one.
          */
-        AddressSpace *as = address_space_init_shareable(cpu->memory,
-                                                        "cpu-memory");
+        AddressSpace *as = g_new0(AddressSpace, 1);
+
+        address_space_init(as, cpu->memory, "cpu-memory");
         cpu->num_ases = 1;
         cpu_address_space_init(cpu, as, 0);
     }
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index b64a409b40..4900339646 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -41,7 +41,7 @@ static MemTxResult bitband_read(void *opaque, hwaddr offset,
 
     /* Find address in underlying memory and round down to multiple of size */
     addr = bitband_addr(s, offset) & (-size);
-    res = address_space_read(s->source_as, addr, attrs, buf, size);
+    res = address_space_read(&s->source_as, addr, attrs, buf, size);
     if (res) {
         return res;
     }
@@ -66,7 +66,7 @@ static MemTxResult bitband_write(void *opaque, hwaddr offset, uint64_t value,
 
     /* Find address in underlying memory and round down to multiple of size */
     addr = bitband_addr(s, offset) & (-size);
-    res = address_space_read(s->source_as, addr, attrs, buf, size);
+    res = address_space_read(&s->source_as, addr, attrs, buf, size);
     if (res) {
         return res;
     }
@@ -79,7 +79,7 @@ static MemTxResult bitband_write(void *opaque, hwaddr offset, uint64_t value,
     } else {
         buf[bitpos >> 3] &= ~bit;
     }
-    return address_space_write(s->source_as, addr, attrs, buf, size);
+    return address_space_write(&s->source_as, addr, attrs, buf, size);
 }
 
 static const MemoryRegionOps bitband_ops = {
@@ -111,8 +111,7 @@ static void bitband_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    s->source_as = address_space_init_shareable(s->source_memory,
-                                                "bitband-source");
+    address_space_init(&s->source_as, s->source_memory, "bitband-source");
 }
 
 /* Board init.  */
diff --git a/memory.c b/memory.c
index 92b1304a20..25000d1035 100644
--- a/memory.c
+++ b/memory.c
@@ -2732,9 +2732,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
 {
     memory_region_ref(root);
     memory_region_transaction_begin();
-    as->ref_count = 1;
     as->root = root;
-    as->malloced = false;
     as->current_map = NULL;
     as->ioeventfd_nb = 0;
     as->ioeventfds = NULL;
@@ -2747,37 +2745,19 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
 
 static void do_address_space_destroy(AddressSpace *as)
 {
-    bool do_free = as->malloced;
-
     assert(QTAILQ_EMPTY(&as->listeners));
 
     flatview_unref(as->current_map);
     g_free(as->name);
     g_free(as->ioeventfds);
     memory_region_unref(as->root);
-    if (do_free) {
-        g_free(as);
-    }
-}
-
-AddressSpace *address_space_init_shareable(MemoryRegion *root, const char *name)
-{
-    AddressSpace *as;
-
-    as = g_malloc0(sizeof *as);
-    address_space_init(as, root, name);
-    as->malloced = true;
-    return as;
+    g_free(as);
 }
 
 void address_space_destroy(AddressSpace *as)
 {
     MemoryRegion *root = as->root;
 
-    as->ref_count--;
-    if (as->ref_count) {
-        return;
-    }
     /* Flush out anything from MemoryListeners listening in on this */
     memory_region_transaction_begin();
     as->root = NULL;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 412e94c7ad..bba24f4590 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -684,6 +684,9 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
     CPUARMState *env = &cpu->env;
     int pagebits;
     Error *local_err = NULL;
+#ifndef CONFIG_USER_ONLY
+    AddressSpace *as;
+#endif
 
     cpu_exec_realizefn(cs, &local_err);
     if (local_err != NULL) {
@@ -874,24 +877,21 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
 
 #ifndef CONFIG_USER_ONLY
     if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) {
-        AddressSpace *as;
+        as = g_new0(AddressSpace, 1);
 
         cs->num_ases = 2;
 
         if (!cpu->secure_memory) {
             cpu->secure_memory = cs->memory;
         }
-        as = address_space_init_shareable(cpu->secure_memory,
-                                          "cpu-secure-memory");
+        address_space_init(as, cpu->secure_memory, "cpu-secure-memory");
         cpu_address_space_init(cs, as, ARMASIdx_S);
     } else {
         cs->num_ases = 1;
     }
-
-    cpu_address_space_init(cs,
-                           address_space_init_shareable(cs->memory,
-                                                        "cpu-memory"),
-                           ARMASIdx_NS);
+    as = g_new0(AddressSpace, 1);
+    address_space_init(as, cs->memory, "cpu-memory");
+    cpu_address_space_init(cs, as, ARMASIdx_NS);
 #endif
 
     qemu_init_vcpu(cs);
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 4b0fa0613b..b0b123a571 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3741,10 +3741,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
 
 #ifndef CONFIG_USER_ONLY
     if (tcg_enabled()) {
-        AddressSpace *as_normal = address_space_init_shareable(cs->memory,
-                                                               "cpu-memory");
+        AddressSpace *as_normal = g_new0(AddressSpace, 1);
         AddressSpace *as_smm = g_new(AddressSpace, 1);
 
+        address_space_init(as_normal, cs->memory, "cpu-memory");
+
         cpu->cpu_as_mem = g_new(MemoryRegion, 1);
         cpu->cpu_as_root = g_new(MemoryRegion, 1);
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 17/18] memory: Create FlatView directly
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
                   ` (15 preceding siblings ...)
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 16/18] memory: Get rid of address_space_init_shareable Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 17:18   ` Paolo Bonzini
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 18/18] memory: Give memory_region_transaction_commit a hint Alexey Kardashevskiy
  17 siblings, 1 reply; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

This avoids usual memory_region_transaction_commit() which rebuilds
all FVs.

On POWER8 with 255 CPUs, 255 virtio-net, 40 PCI bridges guest this brings
down the boot time from 25s to 20s and reduces the amount of temporary FVs
allocated during machine constructon (~800000 -> ~640000) and amount of
temporary dispatch trees (~370000 -> ~300000), the total memory footprint
goes down (18G -> 17G).

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 memory.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/memory.c b/memory.c
index 25000d1035..d3b3581990 100644
--- a/memory.c
+++ b/memory.c
@@ -1020,6 +1020,20 @@ static void flatview_set_to_address_space(AddressSpace *as)
     }
 }
 
+static void flatview_update_topology_single(AddressSpace *as)
+{
+    MemoryRegion *physmr = memory_region_get_flatview_root(as->root);
+    FlatView *new_view;
+
+    flatviews_init();
+    new_view = g_hash_table_lookup(flat_views, physmr);
+    if (!new_view) {
+        new_view = generate_memory_topology(physmr);
+        g_hash_table_insert(flat_views, physmr, new_view);
+    }
+    flatview_set_to_address_space(as);
+}
+
 void memory_region_transaction_begin(void)
 {
     qemu_flush_coalesced_mmio_buffer();
@@ -2731,7 +2745,6 @@ void memory_region_invalidate_mmio_ptr(MemoryRegion *mr, hwaddr offset,
 void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
 {
     memory_region_ref(root);
-    memory_region_transaction_begin();
     as->root = root;
     as->current_map = NULL;
     as->ioeventfd_nb = 0;
@@ -2739,8 +2752,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
     QTAILQ_INIT(&as->listeners);
     QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
     as->name = g_strdup(name ? name : "anonymous");
-    memory_region_update_pending |= root->enabled;
-    memory_region_transaction_commit();
+    flatview_update_topology_single(as);
 }
 
 static void do_address_space_destroy(AddressSpace *as)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [Qemu-devel] [PATCH qemu v4 18/18] memory: Give memory_region_transaction_commit a hint
  2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
                   ` (16 preceding siblings ...)
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 17/18] memory: Create FlatView directly Alexey Kardashevskiy
@ 2017-09-20 11:46 ` Alexey Kardashevskiy
  2017-09-20 17:14   ` Paolo Bonzini
  17 siblings, 1 reply; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 11:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Paolo Bonzini

This extends memory_region_transaction_commit() to receive a MR as
if it is a root MR or its topmost parent is, then we can only rebuild
its FlatView and update it for address spaces sharing it.

The optimization gets disabled though if there is full update about to
commit.

memory_region_set_enabled() is a special case here, it does not use
a hint when MR is being disabled.

On POWER8 with 255 CPUs, 255 virtio-net, 40 PCI bridges guest this brings
down the boot time from 20s to 12s, the total memory footprint
goes down (17G -> 8G).

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---

This is an RFC patch, I suppose.

If memory_region_set_enabled() uses the hint all the time (like others),
then 12s become 9s and memory use is about 4.4G but
make check-qtest fails (no idea why):

  GTESTER check-qtest-ppc64
**
ERROR:/home/aik/p/qemu-kvm/tests/boot-order-test.c:40:test_a_boot_order: assertion failed (actual == expected_boot): (0x0
0000000 == 0x00000063)
---
 memory.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 51 insertions(+), 16 deletions(-)

diff --git a/memory.c b/memory.c
index d3b3581990..81a0962844 100644
--- a/memory.c
+++ b/memory.c
@@ -38,6 +38,7 @@
 
 static unsigned memory_region_transaction_depth;
 static bool memory_region_update_pending;
+static bool memory_region_full_update_pending;
 static bool ioeventfd_update_pending;
 static bool global_dirty_log = false;
 
@@ -1040,7 +1041,30 @@ void memory_region_transaction_begin(void)
     ++memory_region_transaction_depth;
 }
 
-void memory_region_transaction_commit(void)
+static bool memory_region_transaction_fast_commit(MemoryRegion *mr)
+{
+    FlatView *old_view, *new_view;
+    MemoryRegion *physmr;
+
+    while (mr->container) {
+        mr = mr->container;
+    }
+
+    physmr = memory_region_get_flatview_root(mr);
+
+    flatviews_init();
+    old_view = g_hash_table_lookup(flat_views, physmr);
+    if (!old_view) {
+        return false;
+    }
+
+    new_view = generate_memory_topology(physmr);
+    g_hash_table_replace(flat_views, physmr, new_view);
+
+    return true;
+}
+
+static void memory_region_transaction_commit_mr(MemoryRegion *mr)
 {
     AddressSpace *as;
 
@@ -1050,7 +1074,10 @@ void memory_region_transaction_commit(void)
     --memory_region_transaction_depth;
     if (!memory_region_transaction_depth) {
         if (memory_region_update_pending) {
-            flatviews_reset();
+            if (memory_region_full_update_pending || !mr ||
+                !memory_region_transaction_fast_commit(mr)) {
+                flatviews_reset();
+            }
 
             MEMORY_LISTENER_CALL_GLOBAL(begin, Forward);
 
@@ -1060,6 +1087,8 @@ void memory_region_transaction_commit(void)
             }
             memory_region_update_pending = false;
             MEMORY_LISTENER_CALL_GLOBAL(commit, Forward);
+            memory_region_full_update_pending = false;
+
         } else if (ioeventfd_update_pending) {
             QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
                 address_space_update_ioeventfds(as);
@@ -1069,6 +1098,12 @@ void memory_region_transaction_commit(void)
    }
 }
 
+void memory_region_transaction_commit(void)
+{
+    memory_region_full_update_pending = true;
+    memory_region_transaction_commit_mr(NULL);
+}
+
 static void memory_region_destructor_none(MemoryRegion *mr)
 {
 }
@@ -1678,7 +1713,7 @@ static void memory_region_finalize(Object *obj)
         MemoryRegion *subregion = QTAILQ_FIRST(&mr->subregions);
         memory_region_del_subregion(mr, subregion);
     }
-    memory_region_transaction_commit();
+    memory_region_transaction_commit_mr(mr);
 
     mr->destructor(mr);
     memory_region_clear_coalescing(mr);
@@ -1903,7 +1938,7 @@ void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
     memory_region_transaction_begin();
     mr->dirty_log_mask = (mr->dirty_log_mask & ~mask) | (log * mask);
     memory_region_update_pending |= mr->enabled;
-    memory_region_transaction_commit();
+    memory_region_transaction_commit_mr(mr);
 }
 
 bool memory_region_get_dirty(MemoryRegion *mr, hwaddr addr,
@@ -1983,7 +2018,7 @@ void memory_region_set_readonly(MemoryRegion *mr, bool readonly)
         memory_region_transaction_begin();
         mr->readonly = readonly;
         memory_region_update_pending |= mr->enabled;
-        memory_region_transaction_commit();
+        memory_region_transaction_commit_mr(mr);
     }
 }
 
@@ -1993,7 +2028,7 @@ void memory_region_rom_device_set_romd(MemoryRegion *mr, bool romd_mode)
         memory_region_transaction_begin();
         mr->romd_mode = romd_mode;
         memory_region_update_pending |= mr->enabled;
-        memory_region_transaction_commit();
+        memory_region_transaction_commit_mr(mr);
     }
 }
 
@@ -2208,7 +2243,7 @@ void memory_region_add_eventfd(MemoryRegion *mr,
             sizeof(*mr->ioeventfds) * (mr->ioeventfd_nb-1 - i));
     mr->ioeventfds[i] = mrfd;
     ioeventfd_update_pending |= mr->enabled;
-    memory_region_transaction_commit();
+    memory_region_transaction_commit_mr(mr);
 }
 
 void memory_region_del_eventfd(MemoryRegion *mr,
@@ -2243,7 +2278,7 @@ void memory_region_del_eventfd(MemoryRegion *mr,
     mr->ioeventfds = g_realloc(mr->ioeventfds,
                                   sizeof(*mr->ioeventfds)*mr->ioeventfd_nb + 1);
     ioeventfd_update_pending |= mr->enabled;
-    memory_region_transaction_commit();
+    memory_region_transaction_commit_mr(mr);
 }
 
 static void memory_region_update_container_subregions(MemoryRegion *subregion)
@@ -2263,7 +2298,7 @@ static void memory_region_update_container_subregions(MemoryRegion *subregion)
     QTAILQ_INSERT_TAIL(&mr->subregions, subregion, subregions_link);
 done:
     memory_region_update_pending |= mr->enabled && subregion->enabled;
-    memory_region_transaction_commit();
+    memory_region_transaction_commit_mr(mr);
 }
 
 static void memory_region_add_subregion_common(MemoryRegion *mr,
@@ -2302,7 +2337,7 @@ void memory_region_del_subregion(MemoryRegion *mr,
     QTAILQ_REMOVE(&mr->subregions, subregion, subregions_link);
     memory_region_unref(subregion);
     memory_region_update_pending |= mr->enabled && subregion->enabled;
-    memory_region_transaction_commit();
+    memory_region_transaction_commit_mr(mr);
 }
 
 void memory_region_set_enabled(MemoryRegion *mr, bool enabled)
@@ -2313,7 +2348,7 @@ void memory_region_set_enabled(MemoryRegion *mr, bool enabled)
     memory_region_transaction_begin();
     mr->enabled = enabled;
     memory_region_update_pending = true;
-    memory_region_transaction_commit();
+    memory_region_transaction_commit_mr(enabled ? mr : NULL);
 }
 
 void memory_region_set_size(MemoryRegion *mr, uint64_t size)
@@ -2329,7 +2364,7 @@ void memory_region_set_size(MemoryRegion *mr, uint64_t size)
     memory_region_transaction_begin();
     mr->size = s;
     memory_region_update_pending = true;
-    memory_region_transaction_commit();
+    memory_region_transaction_commit_mr(mr);
 }
 
 static void memory_region_readd_subregion(MemoryRegion *mr)
@@ -2343,7 +2378,7 @@ static void memory_region_readd_subregion(MemoryRegion *mr)
         mr->container = container;
         memory_region_update_container_subregions(mr);
         memory_region_unref(mr);
-        memory_region_transaction_commit();
+        memory_region_transaction_commit_mr(mr);
     }
 }
 
@@ -2366,7 +2401,7 @@ void memory_region_set_alias_offset(MemoryRegion *mr, hwaddr offset)
     memory_region_transaction_begin();
     mr->alias_offset = offset;
     memory_region_update_pending |= mr->enabled;
-    memory_region_transaction_commit();
+    memory_region_transaction_commit_mr(mr);
 }
 
 uint64_t memory_region_get_alignment(const MemoryRegion *mr)
@@ -2654,7 +2689,7 @@ bool memory_region_request_mmio_ptr(MemoryRegion *mr, hwaddr addr)
     host = mr->ops->request_ptr(mr->opaque, addr - mr->addr, &size, &offset);
 
     if (!host || !size) {
-        memory_region_transaction_commit();
+        memory_region_transaction_commit_mr(mr);
         return false;
     }
 
@@ -2666,7 +2701,7 @@ bool memory_region_request_mmio_ptr(MemoryRegion *mr, hwaddr addr)
     qdev_prop_set_ptr(DEVICE(new_interface), "subregion", mr);
     object_property_set_bool(OBJECT(new_interface), true, "realized", NULL);
 
-    memory_region_transaction_commit();
+    memory_region_transaction_commit_mr(mr);
     return true;
 }
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH qemu v4 15/18] memory: Share special empty FlatView
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 15/18] memory: Share special empty FlatView Alexey Kardashevskiy
@ 2017-09-20 17:13   ` Paolo Bonzini
  2017-09-20 23:48     ` Alexey Kardashevskiy
  0 siblings, 1 reply; 31+ messages in thread
From: Paolo Bonzini @ 2017-09-20 17:13 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel

On 20/09/2017 13:46, Alexey Kardashevskiy wrote:
> This shares an cached empty FlatView among address spaces. The empty
> FV is used every time when a root MR renders into a FV without memory
> sections which happens when MR or its children are not enabled or
> zero-sized. The empty_view is not NULL to keep the rest of memory
> API intact; it also has a dispatch tree for the same reason.
> 
> On POWER8 with 255 CPUs, 255 virtio-net, 40 PCI bridges guest this halves
> the amount of FlatView's in use (557 -> 260) and dispatch tables
> (~800000 -> ~370000), however the total memory footprint is pretty much
> the same as RCU is holding all these temporary FVs which are created
> (and then released) to make sure that they are empty and can be replaced
> with @empty_view.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  memory.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/memory.c b/memory.c
> index 4add0fd030..92b1304a20 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -48,6 +48,7 @@ static QTAILQ_HEAD(, AddressSpace) address_spaces
>      = QTAILQ_HEAD_INITIALIZER(address_spaces);
>  
>  static GHashTable *flat_views;
> +static FlatView *empty_view;
>  
>  typedef struct AddrRange AddrRange;
>  
> @@ -755,6 +756,19 @@ static FlatView *generate_memory_topology(MemoryRegion *mr)
>      }
>      flatview_simplify(view);
>  
> +    if (!view->nr) {
> +        flatview_unref(view);

This can be changed to flatview_destroy directly to avoid overloading
RCU with all these temporary FlatViews.

Paolo

> +        if (!empty_view) {
> +            empty_view = flatview_new(NULL);
> +        }
> +        view = empty_view;
> +        flatview_ref(view);
> +    }
> +
> +    if (view->dispatch) {
> +        return view;
> +    }
> +
>      view->dispatch = address_space_dispatch_new(view);
>      for (i = 0; i < view->nr; i++) {
>          MemoryRegionSection mrs =
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH qemu v4 18/18] memory: Give memory_region_transaction_commit a hint
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 18/18] memory: Give memory_region_transaction_commit a hint Alexey Kardashevskiy
@ 2017-09-20 17:14   ` Paolo Bonzini
  2017-09-21  0:02     ` Alexey Kardashevskiy
  0 siblings, 1 reply; 31+ messages in thread
From: Paolo Bonzini @ 2017-09-20 17:14 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel

On 20/09/2017 13:46, Alexey Kardashevskiy wrote:
> This extends memory_region_transaction_commit() to receive a MR as
> if it is a root MR or its topmost parent is, then we can only rebuild
> its FlatView and update it for address spaces sharing it.
> 
> The optimization gets disabled though if there is full update about to
> commit.
> 
> memory_region_set_enabled() is a special case here, it does not use
> a hint when MR is being disabled.
> 
> On POWER8 with 255 CPUs, 255 virtio-net, 40 PCI bridges guest this brings
> down the boot time from 20s to 12s, the total memory footprint
> goes down (17G -> 8G).

I think this is incorrect if MR has an alias (no matter if enabling or
disabling)?

Paolo

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH qemu v4 09/18] memory: Store physical root MR in FlatView
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 09/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
@ 2017-09-20 17:15   ` Paolo Bonzini
  2017-09-21  0:02     ` Alexey Kardashevskiy
  0 siblings, 1 reply; 31+ messages in thread
From: Paolo Bonzini @ 2017-09-20 17:15 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel

On 20/09/2017 13:46, Alexey Kardashevskiy wrote:
> Address spaces get to keep a root MR (alias or not) but FlatView stores
> the actual MR as this is going to be used later on to decide whether to
> share a particular FlatView or not.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v4:
> * s/memory_region_unalias_entire/memory_region_get_flatview_root/

Did you try the idea of checking for single-child regions too?

Paolo

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH qemu v4 12/18] memory: Share FlatView's and dispatch trees between address spaces
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 12/18] memory: Share FlatView's and dispatch trees between address spaces Alexey Kardashevskiy
@ 2017-09-20 17:18   ` Paolo Bonzini
  2017-09-20 23:53     ` Alexey Kardashevskiy
  0 siblings, 1 reply; 31+ messages in thread
From: Paolo Bonzini @ 2017-09-20 17:18 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel

On 20/09/2017 13:46, Alexey Kardashevskiy wrote:
> +    QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> +        MemoryRegion *physmr = memory_region_get_flatview_root(as->root);
> +        FlatView *new_view = g_hash_table_lookup(flat_views, physmr);
> +
> +        if (new_view) {
> +            continue;
> +        }
> +
> +        new_view = generate_memory_topology(physmr);
> +        g_hash_table_insert(flat_views, physmr, new_view);

generate_memory_topology can do the g_hash_table_lookup + insert I think?

>  static void flatview_set_to_address_space(AddressSpace *as)
>  {
> -    FlatView *old_view = address_space_get_flatview(as);
> +    FlatView *old_view = address_space_to_flatview(as);
>      MemoryRegion *physmr = memory_region_get_flatview_root(as->root);
>      FlatView *new_view = g_hash_table_lookup(flat_views, physmr);

Rename to address_space_set_flatview?

Thanks,

Paolo

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH qemu v4 17/18] memory: Create FlatView directly
  2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 17/18] memory: Create FlatView directly Alexey Kardashevskiy
@ 2017-09-20 17:18   ` Paolo Bonzini
  0 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2017-09-20 17:18 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel

On 20/09/2017 13:46, Alexey Kardashevskiy wrote:
> +static void flatview_update_topology_single(AddressSpace *as)

address_space_update_topology?

Thanks,

Paolo

> +{
> +    MemoryRegion *physmr = memory_region_get_flatview_root(as->root);
> +    FlatView *new_view;
> +
> +    flatviews_init();
> +    new_view = g_hash_table_lookup(flat_views, physmr);
> +    if (!new_view) {
> +        new_view = generate_memory_topology(physmr);
> +        g_hash_table_insert(flat_views, physmr, new_view);
> +    }
> +    flatview_set_to_address_space(as);
> +}
> +
>  void memory_region_transaction_begin(void)
>  {
>      qemu_flush_coalesced_mmio_buffer();
> @@ -2731,7 +2745,6 @@ void memory_region_invalidate_mmio_ptr(MemoryRegion *mr, hwaddr offset,
>  void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
>  {
>      memory_region_ref(root);
> -    memory_region_transaction_begin();
>      as->root = root;
>      as->current_map = NULL;
>      as->ioeventfd_nb = 0;
> @@ -2739,8 +2752,7 @@ void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
>      QTAILQ_INIT(&as->listeners);
>      QTAILQ_INSERT_TAIL(&address_spaces, as, address_spaces_link);
>      as->name = g_strdup(name ? name : "anonymous");
> -    memory_region_update_pending |= root->enabled;
> -    memory_region_transaction_commit();
> +    flatview_update_topology_single(as);

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH qemu v4 15/18] memory: Share special empty FlatView
  2017-09-20 17:13   ` Paolo Bonzini
@ 2017-09-20 23:48     ` Alexey Kardashevskiy
  0 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 23:48 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

On 21/09/17 03:13, Paolo Bonzini wrote:
> On 20/09/2017 13:46, Alexey Kardashevskiy wrote:
>> This shares an cached empty FlatView among address spaces. The empty
>> FV is used every time when a root MR renders into a FV without memory
>> sections which happens when MR or its children are not enabled or
>> zero-sized. The empty_view is not NULL to keep the rest of memory
>> API intact; it also has a dispatch tree for the same reason.
>>
>> On POWER8 with 255 CPUs, 255 virtio-net, 40 PCI bridges guest this halves
>> the amount of FlatView's in use (557 -> 260) and dispatch tables
>> (~800000 -> ~370000), however the total memory footprint is pretty much
>> the same as RCU is holding all these temporary FVs which are created
>> (and then released) to make sure that they are empty and can be replaced
>> with @empty_view.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>  memory.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/memory.c b/memory.c
>> index 4add0fd030..92b1304a20 100644
>> --- a/memory.c
>> +++ b/memory.c
>> @@ -48,6 +48,7 @@ static QTAILQ_HEAD(, AddressSpace) address_spaces
>>      = QTAILQ_HEAD_INITIALIZER(address_spaces);
>>  
>>  static GHashTable *flat_views;
>> +static FlatView *empty_view;
>>  
>>  typedef struct AddrRange AddrRange;
>>  
>> @@ -755,6 +756,19 @@ static FlatView *generate_memory_topology(MemoryRegion *mr)
>>      }
>>      flatview_simplify(view);
>>  
>> +    if (!view->nr) {
>> +        flatview_unref(view);
> 
> This can be changed to flatview_destroy directly to avoid overloading
> RCU with all these temporary FlatViews.


Yeah, this or just allocate every new FlatView on the stack first, I
thought about this a second after I posted this.


> 
> Paolo
> 
>> +        if (!empty_view) {
>> +            empty_view = flatview_new(NULL);
>> +        }
>> +        view = empty_view;
>> +        flatview_ref(view);
>> +    }
>> +
>> +    if (view->dispatch) {
>> +        return view;
>> +    }
>> +
>>      view->dispatch = address_space_dispatch_new(view);
>>      for (i = 0; i < view->nr; i++) {
>>          MemoryRegionSection mrs =
>>
> 


-- 
Alexey

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH qemu v4 12/18] memory: Share FlatView's and dispatch trees between address spaces
  2017-09-20 17:18   ` Paolo Bonzini
@ 2017-09-20 23:53     ` Alexey Kardashevskiy
  0 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-20 23:53 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

On 21/09/17 03:18, Paolo Bonzini wrote:
> On 20/09/2017 13:46, Alexey Kardashevskiy wrote:
>> +    QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
>> +        MemoryRegion *physmr = memory_region_get_flatview_root(as->root);
>> +        FlatView *new_view = g_hash_table_lookup(flat_views, physmr);
>> +
>> +        if (new_view) {
>> +            continue;
>> +        }
>> +
>> +        new_view = generate_memory_topology(physmr);
>> +        g_hash_table_insert(flat_views, physmr, new_view);
> 
> generate_memory_topology can do the g_hash_table_lookup + insert I think?

Yeah, I suppose. But rather g_hash_table_replace() if we decide to proceed
with 18/18 (or even if we do not - _replace() simply inserts if there was
no such element).


>>  static void flatview_set_to_address_space(AddressSpace *as)
>>  {
>> -    FlatView *old_view = address_space_get_flatview(as);
>> +    FlatView *old_view = address_space_to_flatview(as);
>>      MemoryRegion *physmr = memory_region_get_flatview_root(as->root);
>>      FlatView *new_view = g_hash_table_lookup(flat_views, physmr);
> 
> Rename to address_space_set_flatview?

Sure, why not :)


-- 
Alexey

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH qemu v4 18/18] memory: Give memory_region_transaction_commit a hint
  2017-09-20 17:14   ` Paolo Bonzini
@ 2017-09-21  0:02     ` Alexey Kardashevskiy
  2017-09-21  7:34       ` Paolo Bonzini
  0 siblings, 1 reply; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21  0:02 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

On 21/09/17 03:14, Paolo Bonzini wrote:
> On 20/09/2017 13:46, Alexey Kardashevskiy wrote:
>> This extends memory_region_transaction_commit() to receive a MR as
>> if it is a root MR or its topmost parent is, then we can only rebuild
>> its FlatView and update it for address spaces sharing it.
>>
>> The optimization gets disabled though if there is full update about to
>> commit.
>>
>> memory_region_set_enabled() is a special case here, it does not use
>> a hint when MR is being disabled.
>>
>> On POWER8 with 255 CPUs, 255 virtio-net, 40 PCI bridges guest this brings
>> down the boot time from 20s to 12s, the total memory footprint
>> goes down (17G -> 8G).
> 
> I think this is incorrect if MR has an alias (no matter if enabling or
> disabling)?

Hmmm. Right. I can add a aliases_nr counter to an MR (it does not even have
to go down as aliases are not creared/destroyed often) and take the slow
path if it is not zero, does it make sense?


-- 
Alexey

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH qemu v4 09/18] memory: Store physical root MR in FlatView
  2017-09-20 17:15   ` Paolo Bonzini
@ 2017-09-21  0:02     ` Alexey Kardashevskiy
  2017-09-21  5:22       ` Alexey Kardashevskiy
  0 siblings, 1 reply; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21  0:02 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

On 21/09/17 03:15, Paolo Bonzini wrote:
> On 20/09/2017 13:46, Alexey Kardashevskiy wrote:
>> Address spaces get to keep a root MR (alias or not) but FlatView stores
>> the actual MR as this is going to be used later on to decide whether to
>> share a particular FlatView or not.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> Changes:
>> v4:
>> * s/memory_region_unalias_entire/memory_region_get_flatview_root/
> 
> Did you try the idea of checking for single-child regions too?

No, I did not, I do not see how I can actually measure the difference - the
PCI and virtio root MRs or single child MRs are unique anyway, I can save
some time by just checking for 2 @enabled flags instead of rendering a
FlatView but rendering such cases itself is fast as well. I'll give a try
though.


-- 
Alexey

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH qemu v4 09/18] memory: Store physical root MR in FlatView
  2017-09-21  0:02     ` Alexey Kardashevskiy
@ 2017-09-21  5:22       ` Alexey Kardashevskiy
  2017-09-21  6:28         ` Alexey Kardashevskiy
  0 siblings, 1 reply; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21  5:22 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

On 21/09/17 10:02, Alexey Kardashevskiy wrote:
> On 21/09/17 03:15, Paolo Bonzini wrote:
>> On 20/09/2017 13:46, Alexey Kardashevskiy wrote:
>>> Address spaces get to keep a root MR (alias or not) but FlatView stores
>>> the actual MR as this is going to be used later on to decide whether to
>>> share a particular FlatView or not.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>> Changes:
>>> v4:
>>> * s/memory_region_unalias_entire/memory_region_get_flatview_root/
>>
>> Did you try the idea of checking for single-child regions too?
> 
> No, I did not, I do not see how I can actually measure the difference - the
> PCI and virtio root MRs or single child MRs are unique anyway, I can save
> some time by just checking for 2 @enabled flags instead of rendering a
> FlatView but rendering such cases itself is fast as well. I'll give a try
> though.

I tried. memory_region_get_flatview_root() returns a last child which still
covers the same space as the root; generate_memory_topology() checks for
@enabled first and only if it is enabled - renders a new FV (this solves
PCI busmater).

With 256 CPUs and 256 virtio devices this saves 0.1s (20.4s -> 20.3s) and
100MB of RAM (14.38G -> 14.28G) :) I'll push it out anyway.


-- 
Alexey

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH qemu v4 09/18] memory: Store physical root MR in FlatView
  2017-09-21  5:22       ` Alexey Kardashevskiy
@ 2017-09-21  6:28         ` Alexey Kardashevskiy
  0 siblings, 0 replies; 31+ messages in thread
From: Alexey Kardashevskiy @ 2017-09-21  6:28 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

On 21/09/17 15:22, Alexey Kardashevskiy wrote:
> On 21/09/17 10:02, Alexey Kardashevskiy wrote:
>> On 21/09/17 03:15, Paolo Bonzini wrote:
>>> On 20/09/2017 13:46, Alexey Kardashevskiy wrote:
>>>> Address spaces get to keep a root MR (alias or not) but FlatView stores
>>>> the actual MR as this is going to be used later on to decide whether to
>>>> share a particular FlatView or not.
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>> ---
>>>> Changes:
>>>> v4:
>>>> * s/memory_region_unalias_entire/memory_region_get_flatview_root/
>>>
>>> Did you try the idea of checking for single-child regions too?
>>
>> No, I did not, I do not see how I can actually measure the difference - the
>> PCI and virtio root MRs or single child MRs are unique anyway, I can save
>> some time by just checking for 2 @enabled flags instead of rendering a
>> FlatView but rendering such cases itself is fast as well. I'll give a try
>> though.
> 
> I tried. memory_region_get_flatview_root() returns a last child which still
> covers the same space as the root; generate_memory_topology() checks for
> @enabled first and only if it is enabled - renders a new FV (this solves
> PCI busmater).
> 
> With 256 CPUs and 256 virtio devices this saves 0.1s (20.4s -> 20.3s) and
> 100MB of RAM (14.38G -> 14.28G) :) I'll push it out anyway.


Hm. Actually using that child as a root for FV in 09/18 increases memory
use from 18G to 20G. If I just check the nested MR if it is not enabled,
this does not change a thing - time and memory use is the same. Well, it is
beyond accuracy of my measurements :)


-- 
Alexey

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Qemu-devel] [PATCH qemu v4 18/18] memory: Give memory_region_transaction_commit a hint
  2017-09-21  0:02     ` Alexey Kardashevskiy
@ 2017-09-21  7:34       ` Paolo Bonzini
  0 siblings, 0 replies; 31+ messages in thread
From: Paolo Bonzini @ 2017-09-21  7:34 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: qemu-devel



----- Original Message -----
> From: "Alexey Kardashevskiy" <aik@ozlabs.ru>
> To: "Paolo Bonzini" <pbonzini@redhat.com>, qemu-devel@nongnu.org
> Sent: Thursday, September 21, 2017 2:02:02 AM
> Subject: Re: [PATCH qemu v4 18/18] memory: Give memory_region_transaction_commit a hint
> 
> On 21/09/17 03:14, Paolo Bonzini wrote:
> > On 20/09/2017 13:46, Alexey Kardashevskiy wrote:
> >> This extends memory_region_transaction_commit() to receive a MR as
> >> if it is a root MR or its topmost parent is, then we can only rebuild
> >> its FlatView and update it for address spaces sharing it.
> >>
> >> The optimization gets disabled though if there is full update about to
> >> commit.
> >>
> >> memory_region_set_enabled() is a special case here, it does not use
> >> a hint when MR is being disabled.
> >>
> >> On POWER8 with 255 CPUs, 255 virtio-net, 40 PCI bridges guest this brings
> >> down the boot time from 20s to 12s, the total memory footprint
> >> goes down (17G -> 8G).
> > 
> > I think this is incorrect if MR has an alias (no matter if enabling or
> > disabling)?
> 
> Hmmm. Right. I can add a aliases_nr counter to an MR (it does not even have
> to go down as aliases are not creared/destroyed often) and take the slow
> path if it is not zero, does it make sense?

It should be a separate series anyway.  Let's get the infrastructure in
so it's easier to experiment.

Paolo

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2017-09-21  7:34 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-20 11:46 [Qemu-devel] [PATCH qemu v4 00/18] memory: Reduce memory use Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 01/18] exec: Explicitly export target AS from address_space_translate_internal Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 02/18] memory: Open code FlatView rendering Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 03/18] memory: Move FlatView allocation to a helper Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 04/18] memory: Move AddressSpaceDispatch from AddressSpace to FlatView Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 05/18] memory: Remove AddressSpace pointer from AddressSpaceDispatch Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 06/18] memory: Switch memory from using AddressSpace to FlatView Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 07/18] memory: Cleanup after switching " Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 08/18] memory: Rename mem_begin/mem_commit/mem_add helpers Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 09/18] memory: Store physical root MR in FlatView Alexey Kardashevskiy
2017-09-20 17:15   ` Paolo Bonzini
2017-09-21  0:02     ` Alexey Kardashevskiy
2017-09-21  5:22       ` Alexey Kardashevskiy
2017-09-21  6:28         ` Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 10/18] memory: Alloc dispatch tree where topology is generared Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 11/18] memory: Move address_space_update_ioeventfds Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 12/18] memory: Share FlatView's and dispatch trees between address spaces Alexey Kardashevskiy
2017-09-20 17:18   ` Paolo Bonzini
2017-09-20 23:53     ` Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 13/18] memory: Do not allocate FlatView in address_space_init Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 14/18] memory: Add flat views to HMP "info mtree" Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 15/18] memory: Share special empty FlatView Alexey Kardashevskiy
2017-09-20 17:13   ` Paolo Bonzini
2017-09-20 23:48     ` Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 16/18] memory: Get rid of address_space_init_shareable Alexey Kardashevskiy
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 17/18] memory: Create FlatView directly Alexey Kardashevskiy
2017-09-20 17:18   ` Paolo Bonzini
2017-09-20 11:46 ` [Qemu-devel] [PATCH qemu v4 18/18] memory: Give memory_region_transaction_commit a hint Alexey Kardashevskiy
2017-09-20 17:14   ` Paolo Bonzini
2017-09-21  0:02     ` Alexey Kardashevskiy
2017-09-21  7:34       ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.