| Message ID | 20170920114637.42004-10-aik@ozlabs.ru |
|---|---|
| State | New |
| Headers | show |
| Series | memory: Reduce memory use | expand |
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
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.
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.
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 :)
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);
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(-)