[Qemu-devel] [RFC v2 31/32] vhost: Don't break merged regions on small remove/non-adds

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <address@hidden>

The previous patch merges hugepage regions with small gaps
back into one; this patch stops some of the cases that split
them up again.

vhost_set_memory avoids adding small regions that are being
dirty-monitored (typically VGA regions), but vhost_set_memory
does remove these regions from any region they overlap.

Avoid doing that removal if we'll just merge them again.

The typical case is where the VGA regions dynamically change
at run time after we've done the merge; although the merge works, the
result is that the memory is marked as having changed and
a set_mem_table message is transmitted.  By avoiding the split
we avoid the join and we avoid marking the regions as having
changed, and thus avoid sending the set_mem_table.

Signed-off-by: Dr. David Alan Gilbert <address@hidden>
---
 hw/virtio/trace-events |  2 ++
 hw/virtio/vhost.c      | 42 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index f98efb39fd..e22d2055b0 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -10,6 +10,8 @@ vhost_huge_page_stretch_and_merge_can(void) ""
 vhost_huge_page_stretch_and_merge_size_align(int d, uint64_t gpa, uint64_t 
align) "%d: gpa: 0x%"PRIx64" align: 0x%"PRIx64
 vhost_huge_page_stretch_and_merge_start_align(int d, uint64_t gpa, uint64_t 
align) "%d: gpa: 0x%"PRIx64" align: 0x%"PRIx64
 vhost_section(const char *name, int r) "%s:%d"
+vhost_dev_would_remerge_no(uint64_t start_addr, uint64_t size) "0x%"PRIx64" + 
0x%"PRIx64
+vhost_dev_would_remerge_yes(uint64_t start_addr, uint64_t size) "0x%"PRIx64" + 
0x%"PRIx64
 
 # hw/virtio/vhost-user.c
 vhost_user_postcopy_end_entry(void) ""
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index fb506e747f..18714f6d03 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -564,6 +564,43 @@ static bool vhost_dev_cmp_memory(struct vhost_dev *dev,
     return uaddr != reg->userspace_addr + start_addr - reg->guest_phys_addr;
 }
 
+/* Called by vhost_set_memory on removal where start_addr/size have
+ * been found to correspond to an existing region (reg).
+ * Returns True iff removing the section would be undone later
+ * by merging (for hugepage) and thus there's no point in removing it.
+ */
+static bool vhost_dev_would_remerge(struct vhost_dev *dev,
+                                    struct vhost_memory_region *reg,
+                                    hwaddr start_addr, ram_addr_t size)
+{
+    uint64_t reglast = range_get_last(reg->guest_phys_addr, reg->memory_size);
+    uint64_t memlast = range_get_last(start_addr, size);
+    ram_addr_t offset;
+    RAMBlock *rb = qemu_ram_block_from_host((void *)reg->userspace_addr,
+                                            false, &offset);
+    size_t reg_pagesize = qemu_ram_pagesize(rb);
+
+    /* If the region being deleted hangs over the end of the existing
+     * region, it's not a case we're interested in.
+     * If it's just a normal sized page then there won't normally be any
+     * alignment merging going on.
+     * and if the hole being cut is larger than the pagesize of the
+     * region then assume it won't be remerged.
+     */
+    if (memlast > reglast || start_addr < reg->guest_phys_addr ||
+        reg_pagesize == getpagesize() ||
+        size >= reg_pagesize) {
+        trace_vhost_dev_would_remerge_no(start_addr, size);
+        return false;
+    }
+
+    /* This is a small chunk overlapping a big hugepage region,
+     * deleting it will get remerged
+     */
+    trace_vhost_dev_would_remerge_yes(start_addr, size);
+    return true;
+}
+
 static void vhost_set_memory(MemoryListener *listener,
                              MemoryRegionSection *section,
                              bool add)
@@ -594,7 +631,10 @@ static void vhost_set_memory(MemoryListener *listener,
             return;
         }
     } else {
-        if (!vhost_dev_find_reg(dev, start_addr, size)) {
+        struct vhost_memory_region *reg = vhost_dev_find_reg(dev,
+                                                             start_addr,
+                                                             size);
+        if (!reg || vhost_dev_would_remerge(dev, reg, start_addr, size)) {
             /* Removing region that we don't access. Nothing to do. */
             return;
         }
-- 
2.13.5