[PATCH 0/3] virtio: balloon: cleanups and a fix

[PATCH 0/3] virtio: balloon: cleanups and a fix

[Amit Shah]

Hello,

The main fix is to update the host with the current balloon value on
module removal after deflating the balloon.  Without the fix, the host
has the wrong idea of the ballooned memory in the guest.  This is
patch 2.

Patches 1 and 3 are cleanups with no effective code change.

Please apply,

Amit Shah (3):
  virtio: balloon: drop restore_common()
  virtio: balloon: let host know of updated balloon size before module
    removal
  virtio: balloon: separate out common code between remove and freeze
    functions

 drivers/virtio/virtio_balloon.c |   34 ++++++++++++++--------------------
 1 files changed, 14 insertions(+), 20 deletions(-)

-- 
1.7.7.6

[PATCH 1/3] virtio: balloon: drop restore_common()

[Amit Shah]

restore_common() was used when there were different thaw and freeze PM
callbacks implemented.  We removed thaw in commit
f38f8387cbdc4138a492ce9f2a5f04fd3cd3cf33.

restore_common() can be removed and virtballoon_restore() can itself do
the restore ops.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/virtio/virtio_balloon.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index c2d05a8..6921326 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -418,7 +418,7 @@ static int virtballoon_freeze(struct virtio_device *vdev)
 	return 0;
 }
 
-static int restore_common(struct virtio_device *vdev)
+static int virtballoon_restore(struct virtio_device *vdev)
 {
 	struct virtio_balloon *vb = vdev->priv;
 	int ret;
@@ -431,11 +431,6 @@ static int restore_common(struct virtio_device *vdev)
 	update_balloon_size(vb);
 	return 0;
 }
-
-static int virtballoon_restore(struct virtio_device *vdev)
-{
-	return restore_common(vdev);
-}
 #endif
 
 static unsigned int features[] = {
-- 
1.7.7.6

[PATCH 2/3] virtio: balloon: let host know of updated balloon size before module removal

[Amit Shah]

When the balloon module is removed, we deflate the balloon, reclaiming
all the pages that were given to the host.  However, we don't update the
config values for the new balloon size, resulting in the host showing
outdated balloon values.

The size update is done after each leak and fill operation, only the
module removal case was left out.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/virtio/virtio_balloon.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 6921326..04baad6 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -390,6 +390,7 @@ static void __devexit virtballoon_remove(struct virtio_device *vdev)
 	/* There might be pages left in the balloon: free them. */
 	while (vb->num_pages)
 		leak_balloon(vb, vb->num_pages);
+	update_balloon_size(vb);
 
 	/* Now we reset the device so we can clean up the queues. */
 	vdev->config->reset(vdev);
-- 
1.7.7.6

[PATCH 3/3] virtio: balloon: separate out common code between remove and freeze functions

[Amit Shah]

The remove and freeze functions have a lot of shared code; put it into a
common function that gets called by both.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/virtio/virtio_balloon.c |   26 ++++++++++++--------------
 1 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 04baad6..bfbc15c 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -381,21 +381,25 @@ out:
 	return err;
 }
 
-static void __devexit virtballoon_remove(struct virtio_device *vdev)
+static void remove_common(struct virtio_balloon *vb)
 {
-	struct virtio_balloon *vb = vdev->priv;
-
-	kthread_stop(vb->thread);
-
 	/* There might be pages left in the balloon: free them. */
 	while (vb->num_pages)
 		leak_balloon(vb, vb->num_pages);
 	update_balloon_size(vb);
 
 	/* Now we reset the device so we can clean up the queues. */
-	vdev->config->reset(vdev);
+	vb->vdev->config->reset(vb->vdev);
 
-	vdev->config->del_vqs(vdev);
+	vb->vdev->config->del_vqs(vb->vdev);
+}
+
+static void __devexit virtballoon_remove(struct virtio_device *vdev)
+{
+	struct virtio_balloon *vb = vdev->priv;
+
+	kthread_stop(vb->thread);
+	remove_common(vb);
 	kfree(vb);
 }
 
@@ -409,13 +413,7 @@ static int virtballoon_freeze(struct virtio_device *vdev)
 	 * function is called.
 	 */
 
-	while (vb->num_pages)
-		leak_balloon(vb, vb->num_pages);
-	update_balloon_size(vb);
-
-	/* Ensure we don't get any more requests from the host */
-	vdev->config->reset(vdev);
-	vdev->config->del_vqs(vdev);
+	remove_common(vb);
 	return 0;
 }
 
-- 
1.7.7.6

Re: [PATCH 0/3] virtio: balloon: cleanups and a fix

[Michael S. Tsirkin]

On Fri, Apr 27, 2012 at 12:45:54AM +0530, Amit Shah wrote:
> Hello,
> 
> The main fix is to update the host with the current balloon value on
> module removal after deflating the balloon.  Without the fix, the host
> has the wrong idea of the ballooned memory in the guest.  This is
> patch 2.
> 
> Patches 1 and 3 are cleanups with no effective code change.


better to just do fixes for 3.4. can you reorder pls?

> Please apply,
> 
> Amit Shah (3):
>   virtio: balloon: drop restore_common()
>   virtio: balloon: let host know of updated balloon size before module
>     removal
>   virtio: balloon: separate out common code between remove and freeze
>     functions
> 
>  drivers/virtio/virtio_balloon.c |   34 ++++++++++++++--------------------
>  1 files changed, 14 insertions(+), 20 deletions(-)
> 
> -- 
> 1.7.7.6

Re: [PATCH 0/3] virtio: balloon: cleanups and a fix

[Amit Shah]

On (Thu) 26 Apr 2012 [23:07:47], Michael S. Tsirkin wrote:
> On Fri, Apr 27, 2012 at 12:45:54AM +0530, Amit Shah wrote:
> > Hello,
> > 
> > The main fix is to update the host with the current balloon value on
> > module removal after deflating the balloon.  Without the fix, the host
> > has the wrong idea of the ballooned memory in the guest.  This is
> > patch 2.
> > 
> > Patches 1 and 3 are cleanups with no effective code change.
> 
> 
> better to just do fixes for 3.4. can you reorder pls?

Well if you just pick patch 2, it'll work fine.  There's no context
change there.


		Amit

Re: [PATCH 2/3] virtio: balloon: let host know of updated balloon size before module removal

[Michael S. Tsirkin]

On Fri, Apr 27, 2012 at 12:45:56AM +0530, Amit Shah wrote:
> When the balloon module is removed, we deflate the balloon, reclaiming
> all the pages that were given to the host.  However, we don't update the
> config values for the new balloon size, resulting in the host showing
> outdated balloon values.
> 
> The size update is done after each leak and fill operation, only the
> module removal case was left out.
> 
> Signed-off-by: Amit Shah <amit.shah@redhat.com>


applied,
thanks

the rest are 3.5 material imo

> ---
>  drivers/virtio/virtio_balloon.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 6921326..04baad6 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -390,6 +390,7 @@ static void __devexit virtballoon_remove(struct virtio_device *vdev)
>  	/* There might be pages left in the balloon: free them. */
>  	while (vb->num_pages)
>  		leak_balloon(vb, vb->num_pages);
> +	update_balloon_size(vb);
>  
>  	/* Now we reset the device so we can clean up the queues. */
>  	vdev->config->reset(vdev);
> -- 
> 1.7.7.6

Re: [PATCH 0/3] virtio: balloon: cleanups and a fix

[Rusty Russell]

On Fri, 27 Apr 2012 01:45:54 +0530, Amit Shah <amit.shah@redhat.com> wrote:
> On (Thu) 26 Apr 2012 [23:07:47], Michael S. Tsirkin wrote:
> > On Fri, Apr 27, 2012 at 12:45:54AM +0530, Amit Shah wrote:
> > > Hello,
> > > 
> > > The main fix is to update the host with the current balloon value on
> > > module removal after deflating the balloon.  Without the fix, the host
> > > has the wrong idea of the ballooned memory in the guest.  This is
> > > patch 2.
> > > 
> > > Patches 1 and 3 are cleanups with no effective code change.
> > 
> > 
> > better to just do fixes for 3.4. can you reorder pls?
> 
> Well if you just pick patch 2, it'll work fine.  There's no context
> change there.

1 and 3 applied.

Thanks!
Rusty.