qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Michael S . Tsirkin" <mst@redhat.com>,
	David Hildenbrand <david@redhat.com>,
	qemu-stable@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>,
	Igor Mammedov <imammedo@redhat.com>,
	David Gibson <david@gibson.dropbear.id.au>
Subject: [Qemu-devel] [PATCH-for-4.1 v3 6/6] virtio-balloon: Use temporary PBP only
Date: Mon, 22 Jul 2019 15:41:08 +0200	[thread overview]
Message-ID: <20190722134108.22151-7-david@redhat.com> (raw)
In-Reply-To: <20190722134108.22151-1-david@redhat.com>

We still have multiple issues in the current code
- The PBP is not freed during unrealize()
- The PBP is not reset on device resets: After a reset, the PBP is stale.
- We are not indicating VIRTIO_BALLOON_F_MUST_TELL_HOST, therefore
  guests (esp. legacy guests) will reuse pages without deflating,
  turning the PBP stale. Adding that would require compat handling.

Instead, let's use the PBP only temporarily, when processing one bulk of
inflation requests. This will keep guest_page_size > 4k working (with
Linux guests). There is nothing to do for deflation requests anymore.
The pbp is only used for a limited amount of time.

Fixes: ed48c59875b6 ("virtio-balloon: Safely handle BALLOON_PAGE_SIZE <
                     host page size")
Cc: qemu-stable@nongnu.org #v4.0.0
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/virtio/virtio-balloon.c         | 21 +++++++++------------
 include/hw/virtio/virtio-balloon.h |  3 ---
 2 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 40d493a31a..a6282d58d4 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -34,11 +34,11 @@
 
 #define BALLOON_PAGE_SIZE  (1 << VIRTIO_BALLOON_PFN_SHIFT)
 
-struct PartiallyBalloonedPage {
+typedef struct PartiallyBalloonedPage {
     ram_addr_t base_gpa;
     long subpages;
     unsigned long *bitmap;
-};
+} PartiallyBalloonedPage;
 
 static void virtio_balloon_pbp_free(PartiallyBalloonedPage *pbp)
 {
@@ -68,11 +68,11 @@ static bool virtio_balloon_pbp_matches(PartiallyBalloonedPage *pbp,
 }
 
 static void balloon_inflate_page(VirtIOBalloon *balloon,
-                                 MemoryRegion *mr, hwaddr mr_offset)
+                                 MemoryRegion *mr, hwaddr mr_offset,
+                                 PartiallyBalloonedPage **pbp)
 {
     void *addr = memory_region_get_ram_ptr(mr) + mr_offset;
     ram_addr_t rb_offset, rb_aligned_offset, base_gpa;
-    PartiallyBalloonedPage **pbp = &balloon->pbp;
     RAMBlock *rb;
     size_t rb_page_size;
     int subpages;
@@ -149,12 +149,6 @@ static void balloon_deflate_page(VirtIOBalloon *balloon,
     rb = qemu_ram_block_from_host(addr, false, &rb_offset);
     rb_page_size = qemu_ram_pagesize(rb);
 
-    if (balloon->pbp) {
-        /* Let's play safe and always reset the pbp on deflation requests. */
-        virtio_balloon_pbp_free(balloon->pbp);
-        balloon->pbp = NULL;
-    }
-
     host_addr = (void *)((uintptr_t)addr & ~(rb_page_size - 1));
 
     /* When a page is deflated, we hint the whole host page it lives
@@ -336,6 +330,7 @@ static void balloon_stats_set_poll_interval(Object *obj, Visitor *v,
 static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
+    PartiallyBalloonedPage *pbp = NULL;
     VirtQueueElement *elem;
     MemoryRegionSection section;
 
@@ -344,7 +339,7 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
         uint32_t pfn;
         elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
         if (!elem) {
-            return;
+            break;
         }
 
         while (iov_to_buf(elem->out_sg, elem->out_num, offset, &pfn, 4) == 4) {
@@ -373,7 +368,7 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
             if (!qemu_balloon_is_inhibited()) {
                 if (vq == s->ivq) {
                     balloon_inflate_page(s, section.mr,
-                                         section.offset_within_region);
+                                         section.offset_within_region, &pbp);
                 } else if (vq == s->dvq) {
                     balloon_deflate_page(s, section.mr, section.offset_within_region);
                 } else {
@@ -387,6 +382,8 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
         virtio_notify(vdev, vq);
         g_free(elem);
     }
+
+    virtio_balloon_pbp_free(pbp);
 }
 
 static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index 5a99293a45..d1c968d237 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -33,8 +33,6 @@ typedef struct virtio_balloon_stat_modern {
        uint64_t val;
 } VirtIOBalloonStatModern;
 
-typedef struct PartiallyBalloonedPage PartiallyBalloonedPage;
-
 enum virtio_balloon_free_page_report_status {
     FREE_PAGE_REPORT_S_STOP = 0,
     FREE_PAGE_REPORT_S_REQUESTED = 1,
@@ -70,7 +68,6 @@ typedef struct VirtIOBalloon {
     int64_t stats_last_update;
     int64_t stats_poll_interval;
     uint32_t host_features;
-    PartiallyBalloonedPage *pbp;
 
     bool qemu_4_0_config_size;
 } VirtIOBalloon;
-- 
2.21.0



WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
	David Gibson <david@gibson.dropbear.id.au>,
	qemu-stable@nongnu.org, David Hildenbrand <david@redhat.com>
Subject: [Qemu-devel] [PULL 10/12] virtio-balloon: Use temporary PBP only
Date: Thu, 25 Jul 2019 11:32:14 -0400	[thread overview]
Message-ID: <20190722134108.22151-7-david@redhat.com> (raw)
Message-ID: <20190725153214.kl2aXORRnuFNaEz9lyEW-KcUNfW6xfy_VHM2qLg3a5g@z> (raw)
In-Reply-To: <20190725153059.7313-1-mst@redhat.com>

From: David Hildenbrand <david@redhat.com>

We still have multiple issues in the current code
- The PBP is not freed during unrealize()
- The PBP is not reset on device resets: After a reset, the PBP is stale.
- We are not indicating VIRTIO_BALLOON_F_MUST_TELL_HOST, therefore
  guests (esp. legacy guests) will reuse pages without deflating,
  turning the PBP stale. Adding that would require compat handling.

Instead, let's use the PBP only temporarily, when processing one bulk of
inflation requests. This will keep guest_page_size > 4k working (with
Linux guests). There is nothing to do for deflation requests anymore.
The pbp is only used for a limited amount of time.

Fixes: ed48c59875b6 ("virtio-balloon: Safely handle BALLOON_PAGE_SIZE < host page size")
Cc: qemu-stable@nongnu.org #v4.0.0
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20190722134108.22151-7-david@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/virtio/virtio-balloon.c         | 21 +++++++++------------
 include/hw/virtio/virtio-balloon.h |  3 ---
 2 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 40d493a31a..a6282d58d4 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -34,11 +34,11 @@
 
 #define BALLOON_PAGE_SIZE  (1 << VIRTIO_BALLOON_PFN_SHIFT)
 
-struct PartiallyBalloonedPage {
+typedef struct PartiallyBalloonedPage {
     ram_addr_t base_gpa;
     long subpages;
     unsigned long *bitmap;
-};
+} PartiallyBalloonedPage;
 
 static void virtio_balloon_pbp_free(PartiallyBalloonedPage *pbp)
 {
@@ -68,11 +68,11 @@ static bool virtio_balloon_pbp_matches(PartiallyBalloonedPage *pbp,
 }
 
 static void balloon_inflate_page(VirtIOBalloon *balloon,
-                                 MemoryRegion *mr, hwaddr mr_offset)
+                                 MemoryRegion *mr, hwaddr mr_offset,
+                                 PartiallyBalloonedPage **pbp)
 {
     void *addr = memory_region_get_ram_ptr(mr) + mr_offset;
     ram_addr_t rb_offset, rb_aligned_offset, base_gpa;
-    PartiallyBalloonedPage **pbp = &balloon->pbp;
     RAMBlock *rb;
     size_t rb_page_size;
     int subpages;
@@ -149,12 +149,6 @@ static void balloon_deflate_page(VirtIOBalloon *balloon,
     rb = qemu_ram_block_from_host(addr, false, &rb_offset);
     rb_page_size = qemu_ram_pagesize(rb);
 
-    if (balloon->pbp) {
-        /* Let's play safe and always reset the pbp on deflation requests. */
-        virtio_balloon_pbp_free(balloon->pbp);
-        balloon->pbp = NULL;
-    }
-
     host_addr = (void *)((uintptr_t)addr & ~(rb_page_size - 1));
 
     /* When a page is deflated, we hint the whole host page it lives
@@ -336,6 +330,7 @@ static void balloon_stats_set_poll_interval(Object *obj, Visitor *v,
 static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
+    PartiallyBalloonedPage *pbp = NULL;
     VirtQueueElement *elem;
     MemoryRegionSection section;
 
@@ -344,7 +339,7 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
         uint32_t pfn;
         elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
         if (!elem) {
-            return;
+            break;
         }
 
         while (iov_to_buf(elem->out_sg, elem->out_num, offset, &pfn, 4) == 4) {
@@ -373,7 +368,7 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
             if (!qemu_balloon_is_inhibited()) {
                 if (vq == s->ivq) {
                     balloon_inflate_page(s, section.mr,
-                                         section.offset_within_region);
+                                         section.offset_within_region, &pbp);
                 } else if (vq == s->dvq) {
                     balloon_deflate_page(s, section.mr, section.offset_within_region);
                 } else {
@@ -387,6 +382,8 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
         virtio_notify(vdev, vq);
         g_free(elem);
     }
+
+    virtio_balloon_pbp_free(pbp);
 }
 
 static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index 5a99293a45..d1c968d237 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -33,8 +33,6 @@ typedef struct virtio_balloon_stat_modern {
        uint64_t val;
 } VirtIOBalloonStatModern;
 
-typedef struct PartiallyBalloonedPage PartiallyBalloonedPage;
-
 enum virtio_balloon_free_page_report_status {
     FREE_PAGE_REPORT_S_STOP = 0,
     FREE_PAGE_REPORT_S_REQUESTED = 1,
@@ -70,7 +68,6 @@ typedef struct VirtIOBalloon {
     int64_t stats_last_update;
     int64_t stats_poll_interval;
     uint32_t host_features;
-    PartiallyBalloonedPage *pbp;
 
     bool qemu_4_0_config_size;
 } VirtIOBalloon;
-- 
MST



  parent reply	other threads:[~2019-07-22 13:43 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-22 13:41 [Qemu-devel] [PATCH-for-4.1 v3 0/6] virtio-balloon: fixes David Hildenbrand
2019-07-22 13:41 ` [Qemu-devel] [PATCH-for-4.1 v3 1/6] virtio-balloon: Fix wrong sign extension of PFNs David Hildenbrand
2019-07-23  2:27   ` David Gibson
2019-07-25 15:31   ` [Qemu-devel] [PULL 05/12] " Michael S. Tsirkin
2019-07-22 13:41 ` [Qemu-devel] [PATCH-for-4.1 v3 2/6] virtio-balloon: Fix QEMU crashes on pagesize > BALLOON_PAGE_SIZE David Hildenbrand
2019-07-25 15:31   ` [Qemu-devel] [PULL 06/12] " Michael S. Tsirkin
2019-07-22 13:41 ` [Qemu-devel] [PATCH-for-4.1 v3 3/6] virtio-balloon: Simplify deflate with pbp David Hildenbrand
2019-07-25 15:32   ` [Qemu-devel] [PULL 07/12] " Michael S. Tsirkin
2019-07-22 13:41 ` [Qemu-devel] [PATCH-for-4.1 v3 4/6] virtio-balloon: Better names for offset variables in inflate/deflate code David Hildenbrand
2019-07-25 15:32   ` [Qemu-devel] [PULL 08/12] " Michael S. Tsirkin
2019-07-22 13:41 ` [Qemu-devel] [PATCH-for-4.1 v3 5/6] virtio-balloon: Rework pbp tracking data David Hildenbrand
2019-07-23  2:54   ` David Gibson
2019-07-23  7:38     ` David Hildenbrand
2019-07-25 15:32   ` [Qemu-devel] [PULL 09/12] " Michael S. Tsirkin
2019-07-22 13:41 ` David Hildenbrand [this message]
2019-07-23  3:22   ` [Qemu-devel] [PATCH-for-4.1 v3 6/6] virtio-balloon: Use temporary PBP only David Gibson
2019-07-25 15:32   ` [Qemu-devel] [PULL 10/12] " Michael S. Tsirkin
2019-07-25 11:36 [Qemu-devel] [PATCH-for-4.1 v4 0/7] virtio-balloon: fixes David Hildenbrand
2019-07-25 11:36 ` [Qemu-devel] [PATCH-for-4.1 v4 1/7] virtio-balloon: Fix wrong sign extension of PFNs David Hildenbrand
2019-07-25 12:36   ` Pankaj Gupta
2019-07-25 11:36 ` [Qemu-devel] [PATCH-for-4.1 v4 2/7] virtio-balloon: Fix QEMU crashes on pagesize > BALLOON_PAGE_SIZE David Hildenbrand
2019-07-25 11:36 ` [Qemu-devel] [PATCH-for-4.1 v4 3/7] virtio-balloon: Simplify deflate with pbp David Hildenbrand
2019-07-25 11:36 ` [Qemu-devel] [PATCH-for-4.1 v4 4/7] virtio-balloon: Better names for offset variables in inflate/deflate code David Hildenbrand
2019-07-25 11:36 ` [Qemu-devel] [PATCH-for-4.1 v4 5/7] virtio-balloon: Rework pbp tracking data David Hildenbrand
2019-07-26  8:08   ` David Gibson
2019-07-25 11:36 ` [Qemu-devel] [PATCH-for-4.1 v4 6/7] virtio-balloon: Use temporary PBP only David Hildenbrand
2019-07-25 11:53   ` Michael S. Tsirkin
2019-07-25 11:56     ` David Hildenbrand
2019-07-25 11:36 ` [Qemu-devel] [PATCH-for-4.1 v4 7/7] virtio-balloon: No need to track subpages for the PBP anymore David Hildenbrand
2019-07-25 15:32   ` [Qemu-devel] [PULL 11/12] virtio-balloon: don't track subpages for the PBP Michael S. Tsirkin
2019-07-26  8:10   ` [Qemu-devel] [PATCH-for-4.1 v4 7/7] virtio-balloon: No need to track subpages for the PBP anymore David Gibson
2019-07-25 15:31 [Qemu-devel] [PULL 00/12] virtio, pc: fixes, cleanups Michael S. Tsirkin
2019-06-02 11:42 ` [Qemu-devel] [PATCH] ioapic: kvm: Skip route updates for masked pins Jan Kiszka
2019-06-02 12:10   ` Peter Xu
2019-06-03  6:30     ` Jan Kiszka
2019-06-03  0:36   ` Michael S. Tsirkin
2019-07-21  8:58     ` Jan Kiszka
2019-07-21 10:04       ` Michael S. Tsirkin
2019-07-21 16:55         ` Paolo Bonzini
2019-07-25 15:31   ` [Qemu-devel] [PULL 03/12] " Michael S. Tsirkin
2019-06-24  9:13 ` [Qemu-devel] [PATCH] docs: clarify multiqueue vs multiple virtqueues Stefan Hajnoczi
2019-06-24 10:19   ` Marc-André Lureau
2019-07-17 10:14   ` Stefan Hajnoczi
2019-07-17 10:35     ` Michael S. Tsirkin
2019-07-25 15:31   ` [Qemu-devel] [PULL 01/12] " Michael S. Tsirkin
2019-07-18 16:14 ` [Qemu-devel] [PATCH v2] i386/acpi: fix gint overflow in crs_range_compare Evgeny Yakovlev
2019-07-18 20:30   ` Michael S. Tsirkin
2019-07-25 15:31   ` [Qemu-devel] [PULL 02/12] " Michael S. Tsirkin
2019-07-19  8:54 ` [Qemu-devel] [PATCH] i386/acpi: show PCI Express bus on pxb-pcie expanders Evgeny Yakovlev
2019-07-19 12:14   ` Igor Mammedov
2019-07-25 15:31   ` [Qemu-devel] [PULL 04/12] " Michael S. Tsirkin
2019-07-25 15:32 ` [Qemu-devel] [PULL 12/12] virtio-balloon: free pbp more aggressively Michael S. Tsirkin
2019-07-26  9:53 ` [Qemu-devel] [PULL 00/12] virtio, pc: fixes, cleanups Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190722134108.22151-7-david@redhat.com \
    --to=david@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=imammedo@redhat.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).