Re: [RFC][QEMU Patch] KVM: Enable QEMU to free the pages hinted by the guest

[Alexander Duyck <alexander.duyck@xxxxxxxxx>]

Here are some changes I made to your patch in order to address the sizing
issue I called out. You may want to try testing with this patch applied to
your QEMU as I am finding it is making a signficant difference. It has cut
the test time for the 32G memhog test I called out earlier in half.

Signed-off-by: Alexander Duyck <alexander.h.duyck@xxxxxxxxxxxxxxx>
---
 hw/virtio/virtio-balloon.c |   28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index d2cf66ada3c0..3ca6b1c6d511 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -285,7 +285,7 @@ static void balloon_stats_set_poll_interval(Object *obj, Visitor *v,
     balloon_stats_change_timer(s, 0);
 }
 
-static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
+static void *gpa2hva(MemoryRegion **p_mr, unsigned long *size, hwaddr addr, Error **errp)
 {
     MemoryRegionSection mrs = memory_region_find(get_system_memory(),
                                                  addr, 1);
@@ -302,6 +302,7 @@ static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
     }
 
     *p_mr = mrs.mr;
+    *size = mrs.mr->size - mrs.offset_within_region;
     return qemu_map_ram_ptr(mrs.mr->ram_block, mrs.offset_within_region);
 }
 
@@ -313,30 +314,35 @@ void page_hinting_request(uint64_t addr, uint32_t len)
     struct guest_pages *guest_obj;
     int i = 0;
     void *hvaddr_to_free;
-    unsigned long pfn, pfn_end;
     uint64_t gpaddr_to_free;
-    void * temp_addr = gpa2hva(&mr, addr, &local_err);
+    unsigned long madv_size, size;
+    void * temp_addr = gpa2hva(&mr, &madv_size, addr, &local_err);
 
     if (local_err) {
         error_report_err(local_err);
         return;
     }
+    if (madv_size < sizeof(*guest_obj)) {
+	printf("\nBad guest object ptr\n");
+	return;
+    }
     guest_obj = temp_addr;
     while (i < len) {
-        pfn = guest_obj[i].pfn;
-	pfn_end = guest_obj[i].pfn + (1 << guest_obj[i].order) - 1;
-	trace_virtio_balloon_hinting_request(pfn,(1 << guest_obj[i].order));
-	while (pfn <= pfn_end) {
-	        gpaddr_to_free = pfn << VIRTIO_BALLOON_PFN_SHIFT;
-	        hvaddr_to_free = gpa2hva(&mr, gpaddr_to_free, &local_err);
+        gpaddr_to_free = guest_obj[i].pfn << VIRTIO_BALLOON_PFN_SHIFT;
+	size = (1 << VIRTIO_BALLOON_PFN_SHIFT) << guest_obj[i].order;
+	while (size) {
+	        hvaddr_to_free = gpa2hva(&mr, &madv_size, gpaddr_to_free, &local_err);
 	        if (local_err) {
 			error_report_err(local_err);
 		        return;
 		}
-		ret = qemu_madvise((void *)hvaddr_to_free, 4096, QEMU_MADV_DONTNEED);
+		if (size < madv_size)
+			madv_size = size;
+		ret = qemu_madvise((void *)hvaddr_to_free, madv_size, QEMU_MADV_DONTNEED);
 		if (ret == -1)
 		    printf("\n%d:%s Error: Madvise failed with error:%d\n", __LINE__, __func__, ret);
-		pfn++;
+		gpaddr_to_free += madv_size;
+		size -= madv_size;
 	}
 	i++;
     }