virtio: balloon: cleanups and a fix

PATCH Applied Amit Shah 3 participants 8 messages Apr 26, 2012 – May 7, 2012

Subsystems (2)

virtio core virtio balloon

lore

Share & tools

lore

Amit Shah

author

2012-04-26 19:15 UTC #0 lore raw

cover virtio: balloon: cleanups and a fix

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(-)

Signature · 1 line

1.7.7.6

Amit Shah

author

2012-04-26 19:15 UTC #1 lore raw

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

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 <[email protected]>

---
 drivers/virtio/virtio_balloon.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

drivers/virtio/virtio_balloon.c +1 -6

index c2d05a8..6921326 100644
@@ -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[] = {

Signature · 1 line

1.7.7.6

Amit Shah

author

2012-04-26 19:15 UTC #2 lore raw

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

In reply to 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 <[email protected]>

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

drivers/virtio/virtio_balloon.c +1 -0

index 6921326..04baad6 100644
@@ -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);

Signature · 1 line

1.7.7.6

Michael S. Tsirkin

maintainer Applied

2012-04-26 20:50 UTC #3 lore raw

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

Quoted 9 lines · 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 <[email protected]>

applied,
thanks

the rest are 3.5 material imo

Quoted 18 lines

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

drivers/virtio/virtio_balloon.c +1 -0

index 6921326..04baad6 100644
@@ -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);

Signature · 1 line

1.7.7.6

Amit Shah

author

2012-04-26 19:15 UTC #4 lore raw

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

In reply to 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 <[email protected]>

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

drivers/virtio/virtio_balloon.c +12 ~~-14~~

index 04baad6..bfbc15c 100644
@@ -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;
 }

Signature · 1 line

1.7.7.6

Michael S. Tsirkin

maintainer

2012-04-26 20:07 UTC #5 lore raw

In reply to Amit Shah

Quoted 8 lines · 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.

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

Quoted 14 lines

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(-)
Signature · 1 line
1.7.7.6

Amit Shah

author

2012-04-26 20:15 UTC #6 lore raw

Quoted 8 lines · Michael S. Tsirkin

Quoted 7 lines · 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.

Quoted 4 lines

Quoted 1 line
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

Rusty Russell

2012-05-07 03:53 UTC #7 lore raw

Quoted 9 lines · Amit Shah

Quoted 8 lines · Michael S. Tsirkin
Quoted 7 lines · 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.

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
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.