public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: Marcelo Tosatti <marcelo@kvack.org>
Cc: kvm-devel <kvm-devel@lists.sourceforge.net>
Subject: Re: [PATCH] virtio-balloon: do not attempt to release more than available pages
Date: Tue, 11 Mar 2008 11:26:55 +1100	[thread overview]
Message-ID: <200803111126.55421.rusty@rustcorp.com.au> (raw)
In-Reply-To: <20080308190638.GA18152@dmt>

On Sunday 09 March 2008 06:06:38 Marcelo Tosatti wrote:
> But making the driver robust against it seems sensate. I agree that
> zeroing num_pages is hackish. What do you suggest?

OK, after more discussion on IRC, this seems like the correct thing to do.

1) Allow more than 2G of pages.  It doesn't cost us much to break the 8TB
   barrier.  This is an ABI change, so best to do this now.
2) Handle the case where we get nonsense from the host, which causes us to
   wrap around.

How's this (compile-tested only!):


diff -r fd0c80dbbd95 drivers/virtio/virtio_balloon.c
--- a/drivers/virtio/virtio_balloon.c	Tue Mar 11 09:21:00 2008 +1100
+++ b/drivers/virtio/virtio_balloon.c	Tue Mar 11 11:25:52 2008 +1100
@@ -43,12 +43,12 @@ struct virtio_balloon
 	bool tell_host_first;
 
 	/* The pages we've told the Host we're not using. */
-	unsigned int num_pages;
+	unsigned long num_pages;
 	struct list_head pages;
 
 	/* The array of pfns we tell the Host about. */
 	unsigned int num_pfns;
-	u32 pfns[256];
+	u64 pfns[256];
 };
 
 static struct virtio_device_id id_table[] = {
@@ -83,17 +83,17 @@ static void tell_host(struct virtio_ball
 	wait_for_completion(&vb->acked);
 }
 
-static void fill_balloon(struct virtio_balloon *vb, size_t num)
+static void fill_balloon(struct virtio_balloon *vb, u64 num)
 {
 	/* We can only do one array worth at a time. */
-	num = min(num, ARRAY_SIZE(vb->pfns));
+	num = min_t(u64, num, ARRAY_SIZE(vb->pfns));
 
 	for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) {
 		struct page *page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY);
 		if (!page) {
 			if (printk_ratelimit())
 				dev_printk(KERN_INFO, &vb->vdev->dev,
-					   "Out of puff! Can't get %zu pages\n",
+					   "Out of puff! Can't get %llu pages\n",
 					   num);
 			/* Sleep for at least 1/5 of a second before retry. */
 			msleep(200);
@@ -112,7 +112,7 @@ static void fill_balloon(struct virtio_b
 	tell_host(vb, vb->inflate_vq);
 }
 
-static void release_pages_by_pfn(const u32 pfns[], unsigned int num)
+static void release_pages_by_pfn(const u64 pfns[], unsigned int num)
 {
 	unsigned int i;
 
@@ -122,12 +122,12 @@ static void release_pages_by_pfn(const u
 	}
 }
 
-static void leak_balloon(struct virtio_balloon *vb, size_t num)
+static void leak_balloon(struct virtio_balloon *vb, u64 num)
 {
 	struct page *page;
 
 	/* We can only do one array worth at a time. */
-	num = min(num, ARRAY_SIZE(vb->pfns));
+	num = min_t(u64, num, ARRAY_SIZE(vb->pfns));
 
 	for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) {
 		page = list_first_entry(&vb->pages, struct page, lru);
@@ -152,12 +152,19 @@ static void virtballoon_changed(struct v
 	wake_up(&vb->config_change);
 }
 
-static inline int towards_target(struct virtio_balloon *vb)
+static inline s64 towards_target(struct virtio_balloon *vb)
 {
-	u32 v;
+	u64 v;
 	__virtio_config_val(vb->vdev,
 			    offsetof(struct virtio_balloon_config, num_pages),
 			    &v);
+
+	/* If they ask for something which will wrap, we ignore it. */
+	if (v & 0x8000000000000000ULL) {
+		dev_warn(&vb->vdev->dev, "Insane target 0x%llx\n", v);
+		return 0;
+	}
+
 	return v - vb->num_pages;
 }
 
@@ -176,7 +183,7 @@ static int balloon(void *_vballoon)
 
 	set_freezable();
 	while (!kthread_should_stop()) {
-		int diff;
+		s64 diff;
 
 		try_to_freeze();
 		wait_event_interruptible(vb->config_change,
diff -r fd0c80dbbd95 include/linux/virtio_balloon.h
--- a/include/linux/virtio_balloon.h	Tue Mar 11 09:21:00 2008 +1100
+++ b/include/linux/virtio_balloon.h	Tue Mar 11 11:25:52 2008 +1100
@@ -11,8 +11,8 @@ struct virtio_balloon_config
 struct virtio_balloon_config
 {
 	/* Number of pages host wants Guest to give up. */
-	__le32 num_pages;
+	__le64 num_pages;
 	/* Number of pages we've actually got in balloon. */
-	__le32 actual;
+	__le64 actual;
 };
 #endif /* _LINUX_VIRTIO_BALLOON_H */


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

  reply	other threads:[~2008-03-11  0:26 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-05 16:28 [PATCH] virtio-balloon: do not attempt to release more than available pages Marcelo Tosatti
2008-03-05 16:59 ` Avi Kivity
2008-03-05 18:12   ` Marcelo Tosatti
2008-03-05 18:13     ` Avi Kivity
2008-03-05 18:43     ` Anthony Liguori
2008-03-05 19:39       ` Marcelo Tosatti
2008-03-05 19:42         ` Anthony Liguori
2008-03-06  7:06           ` Avi Kivity
2008-03-05 18:42   ` Anthony Liguori
2008-03-05 22:39 ` Rusty Russell
2008-03-08 19:06   ` Marcelo Tosatti
2008-03-11  0:26     ` Rusty Russell [this message]
2008-03-11  0:52       ` Anthony Liguori
2008-03-11 11:54         ` Rusty Russell

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=200803111126.55421.rusty@rustcorp.com.au \
    --to=rusty@rustcorp.com.au \
    --cc=kvm-devel@lists.sourceforge.net \
    --cc=marcelo@kvack.org \
    /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