linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/2] auto-ballooning prototype (guest part)
@ 2012-12-18 20:17 Luiz Capitulino
  2012-12-18 20:17 ` [RFC 1/2] virtio_balloon: move locking to the balloon thread Luiz Capitulino
  2012-12-18 20:17 ` [RFC 2/2] virtio_balloon: add auto-ballooning support Luiz Capitulino
  0 siblings, 2 replies; 8+ messages in thread
From: Luiz Capitulino @ 2012-12-18 20:17 UTC (permalink / raw)
  To: linux-mm; +Cc: linux-kernel, riel, aquini, mst, amit.shah, agl

Hi,

This series implements an early protoype of a new feature called
automatic ballooning. This is based on ideas by Rik van Riel and
I also got some help from Rafael Aquini (misconceptions and bugs
are all mine, though).

The auto-ballooning feature automatically performs balloon inflate
and deflate based on host and guest memory pressure. This can help
to avoid swapping or worse in both, host and guest.

This series implements the guest part. Full details on the design
and implementation can be found in patch 2/2.

To test this you will also need the host part, which can be found
here (please, never mind the repo name):

 git://repo.or.cz/qemu/qmp-unstable.git balloon/auto-ballooning/rfc

Any feedback is appreciated!

Luiz Capitulino (2):
  virtio_balloon: move locking to the balloon thread
  virtio_balloon: add auto-ballooning support

 drivers/virtio/virtio_balloon.c     | 60 ++++++++++++++++++++++++++++++++++---
 include/uapi/linux/virtio_balloon.h |  1 +
 2 files changed, 57 insertions(+), 4 deletions(-)

-- 
1.8.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RFC 1/2] virtio_balloon: move locking to the balloon thread
  2012-12-18 20:17 [RFC 0/2] auto-ballooning prototype (guest part) Luiz Capitulino
@ 2012-12-18 20:17 ` Luiz Capitulino
  2012-12-19 11:55   ` Rafael Aquini
  2012-12-18 20:17 ` [RFC 2/2] virtio_balloon: add auto-ballooning support Luiz Capitulino
  1 sibling, 1 reply; 8+ messages in thread
From: Luiz Capitulino @ 2012-12-18 20:17 UTC (permalink / raw)
  To: linux-mm; +Cc: linux-kernel, riel, aquini, mst, amit.shah, agl

Today, the balloon_lock mutex is taken and released by fill_balloon()
and leak_balloon() when both functions are entered and when they
return.

This commit moves the locking to the caller instead, which is
the balloon() thread. The balloon thread is the sole caller of those
functions today.

The reason for this move is that the next commit will introduce
a shrinker callback for the balloon driver, which will also call
leak_balloon() but will require different locking semantics.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 drivers/virtio/virtio_balloon.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 2a70558..877e695 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -133,7 +133,6 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num)
 	/* We can only do one array worth at a time. */
 	num = min(num, ARRAY_SIZE(vb->pfns));
 
-	mutex_lock(&vb->balloon_lock);
 	for (vb->num_pfns = 0; vb->num_pfns < num;
 	     vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
 		struct page *page = balloon_page_enqueue(vb_dev_info);
@@ -155,7 +154,6 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num)
 	/* Did we get any? */
 	if (vb->num_pfns != 0)
 		tell_host(vb, vb->inflate_vq);
-	mutex_unlock(&vb->balloon_lock);
 }
 
 static void release_pages_by_pfn(const u32 pfns[], unsigned int num)
@@ -177,7 +175,6 @@ static void leak_balloon(struct virtio_balloon *vb, size_t num)
 	/* We can only do one array worth at a time. */
 	num = min(num, ARRAY_SIZE(vb->pfns));
 
-	mutex_lock(&vb->balloon_lock);
 	for (vb->num_pfns = 0; vb->num_pfns < num;
 	     vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
 		page = balloon_page_dequeue(vb_dev_info);
@@ -193,7 +190,6 @@ static void leak_balloon(struct virtio_balloon *vb, size_t num)
 	 * is true, we *have* to do it in this order
 	 */
 	tell_host(vb, vb->deflate_vq);
-	mutex_unlock(&vb->balloon_lock);
 	release_pages_by_pfn(vb->pfns, vb->num_pfns);
 }
 
@@ -306,11 +302,13 @@ static int balloon(void *_vballoon)
 					 || freezing(current));
 		if (vb->need_stats_update)
 			stats_handle_request(vb);
+		mutex_lock(&vb->balloon_lock);
 		if (diff > 0)
 			fill_balloon(vb, diff);
 		else if (diff < 0)
 			leak_balloon(vb, -diff);
 		update_balloon_size(vb);
+		mutex_unlock(&vb->balloon_lock);
 	}
 	return 0;
 }
-- 
1.8.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC 2/2] virtio_balloon: add auto-ballooning support
  2012-12-18 20:17 [RFC 0/2] auto-ballooning prototype (guest part) Luiz Capitulino
  2012-12-18 20:17 ` [RFC 1/2] virtio_balloon: move locking to the balloon thread Luiz Capitulino
@ 2012-12-18 20:17 ` Luiz Capitulino
  2013-01-11 20:43   ` Amit Shah
  1 sibling, 1 reply; 8+ messages in thread
From: Luiz Capitulino @ 2012-12-18 20:17 UTC (permalink / raw)
  To: linux-mm; +Cc: linux-kernel, riel, aquini, mst, amit.shah, agl

The auto-ballooning feature automatically performs balloon inflate or
deflate based on host and guest memory pressure. This can help to
avoid swapping or worse in both, host and guest.

Auto-ballooning has a host and a guest part. The host performs
automatic inflate by requesting the guest to inflate its balloon
when the host is facing memory pressure. The guest performs
automatic deflate when it's facing memory pressure itself. It's
expected that auto-inflate and auto-deflate will balance each
other over time.

This commit implements the guest side of auto-ballooning.

To perform automatic deflate, the virtio_balloon driver registers
a shrinker callback, which will try to deflate the guest's balloon
on guest memory pressure just like if it were a cache. The shrinker
callback is only registered if the host supports the
VIRTIO_BALLOON_F_AUTO_BALLOON feature bit.

FIXMEs

 o the guest kernel seems to spin when the host is performing a long
   auto-inflate

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 drivers/virtio/virtio_balloon.c     | 54 +++++++++++++++++++++++++++++++++++++
 include/uapi/linux/virtio_balloon.h |  1 +
 2 files changed, 55 insertions(+)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 877e695..12fb70e 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -71,6 +71,9 @@ struct virtio_balloon
 	/* Memory statistics */
 	int need_stats_update;
 	struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
+
+	/* Memory shrinker */
+	struct shrinker shrinker;
 };
 
 static struct virtio_device_id id_table[] = {
@@ -286,6 +289,46 @@ static void update_balloon_size(struct virtio_balloon *vb)
 			      &actual, sizeof(actual));
 }
 
+static unsigned long balloon_get_nr_pages(const struct virtio_balloon *vb)
+{
+	return vb->num_pages / VIRTIO_BALLOON_PAGES_PER_PAGE;
+}
+
+static int balloon_shrinker(struct shrinker *shrinker,struct shrink_control *sc)
+{
+	unsigned int nr_pages, new_target;
+	struct virtio_balloon *vb;
+
+	vb = container_of(shrinker, struct virtio_balloon, shrinker);
+	if (!mutex_trylock(&vb->balloon_lock)) {
+		return -1;
+	}
+
+	nr_pages = balloon_get_nr_pages(vb);
+	if (!sc->nr_to_scan || !nr_pages) {
+		goto out;
+	}
+
+	/*
+	 * If the current balloon size is greater than the number of
+	 * pages being reclaimed by the kernel, deflate only the needed
+	 * amount. Otherwise deflate everything we have.
+	 */
+	if (nr_pages > sc->nr_to_scan) {
+		new_target = nr_pages - sc->nr_to_scan;
+	} else {
+		new_target = 0;
+	}
+
+	leak_balloon(vb, new_target);
+	update_balloon_size(vb);
+	nr_pages = balloon_get_nr_pages(vb);
+
+out:
+	mutex_unlock(&vb->balloon_lock);
+	return nr_pages;
+}
+
 static int balloon(void *_vballoon)
 {
 	struct virtio_balloon *vb = _vballoon;
@@ -472,6 +515,13 @@ static int virtballoon_probe(struct virtio_device *vdev)
 		goto out_del_vqs;
 	}
 
+	memset(&vb->shrinker, 0, sizeof(vb->shrinker));
+	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_AUTO_BALLOON)) {
+		vb->shrinker.shrink = balloon_shrinker;
+		vb->shrinker.seeks = DEFAULT_SEEKS;
+		register_shrinker(&vb->shrinker);
+	}
+
 	return 0;
 
 out_del_vqs:
@@ -488,6 +538,9 @@ out:
 
 static void remove_common(struct virtio_balloon *vb)
 {
+	if (vb->shrinker.shrink)
+		unregister_shrinker(&vb->shrinker);
+
 	/* There might be pages left in the balloon: free them. */
 	while (vb->num_pages)
 		leak_balloon(vb, vb->num_pages);
@@ -542,6 +595,7 @@ static int virtballoon_restore(struct virtio_device *vdev)
 static unsigned int features[] = {
 	VIRTIO_BALLOON_F_MUST_TELL_HOST,
 	VIRTIO_BALLOON_F_STATS_VQ,
+	VIRTIO_BALLOON_F_AUTO_BALLOON,
 };
 
 static struct virtio_driver virtio_balloon_driver = {
diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
index 652dc8b..3764cac 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -31,6 +31,7 @@
 /* The feature bitmap for virtio balloon */
 #define VIRTIO_BALLOON_F_MUST_TELL_HOST	0 /* Tell before reclaiming pages */
 #define VIRTIO_BALLOON_F_STATS_VQ	1 /* Memory Stats virtqueue */
+#define VIRTIO_BALLOON_F_AUTO_BALLOON	2 /* Automatic ballooning */
 
 /* Size of a PFN in the balloon interface. */
 #define VIRTIO_BALLOON_PFN_SHIFT 12
-- 
1.8.0

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RFC 1/2] virtio_balloon: move locking to the balloon thread
  2012-12-18 20:17 ` [RFC 1/2] virtio_balloon: move locking to the balloon thread Luiz Capitulino
@ 2012-12-19 11:55   ` Rafael Aquini
  2012-12-19 12:47     ` Luiz Capitulino
  0 siblings, 1 reply; 8+ messages in thread
From: Rafael Aquini @ 2012-12-19 11:55 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: linux-mm, linux-kernel, riel, mst, amit.shah, agl

On Tue, Dec 18, 2012 at 06:17:29PM -0200, Luiz Capitulino wrote:
> Today, the balloon_lock mutex is taken and released by fill_balloon()
> and leak_balloon() when both functions are entered and when they
> return.
> 
> This commit moves the locking to the caller instead, which is
> the balloon() thread. The balloon thread is the sole caller of those
> functions today.
> 
> The reason for this move is that the next commit will introduce
> a shrinker callback for the balloon driver, which will also call
> leak_balloon() but will require different locking semantics.
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  drivers/virtio/virtio_balloon.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 2a70558..877e695 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -133,7 +133,6 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num)
>  	/* We can only do one array worth at a time. */
>  	num = min(num, ARRAY_SIZE(vb->pfns));
>  
> -	mutex_lock(&vb->balloon_lock);
>  	for (vb->num_pfns = 0; vb->num_pfns < num;
>  	     vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
>  		struct page *page = balloon_page_enqueue(vb_dev_info);
> @@ -155,7 +154,6 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num)
>  	/* Did we get any? */
>  	if (vb->num_pfns != 0)
>  		tell_host(vb, vb->inflate_vq);
> -	mutex_unlock(&vb->balloon_lock);
>  }
>

Since you're removing the locking scheme from within this function, I think it
would be a good idea introduce a comment stating its caller must held the mutex
vb->balloon_lock.

  
>  static void release_pages_by_pfn(const u32 pfns[], unsigned int num)
> @@ -177,7 +175,6 @@ static void leak_balloon(struct virtio_balloon *vb, size_t num)
>  	/* We can only do one array worth at a time. */
>  	num = min(num, ARRAY_SIZE(vb->pfns));
>  
> -	mutex_lock(&vb->balloon_lock);
>  	for (vb->num_pfns = 0; vb->num_pfns < num;
>  	     vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
>  		page = balloon_page_dequeue(vb_dev_info);
> @@ -193,7 +190,6 @@ static void leak_balloon(struct virtio_balloon *vb, size_t num)
>  	 * is true, we *have* to do it in this order
>  	 */
>  	tell_host(vb, vb->deflate_vq);
> -	mutex_unlock(&vb->balloon_lock);
>  	release_pages_by_pfn(vb->pfns, vb->num_pfns);
>  }
>

ditto

  
> @@ -306,11 +302,13 @@ static int balloon(void *_vballoon)
>  					 || freezing(current));
>  		if (vb->need_stats_update)
>  			stats_handle_request(vb);
> +		mutex_lock(&vb->balloon_lock);
>  		if (diff > 0)
>  			fill_balloon(vb, diff);
>  		else if (diff < 0)
>  			leak_balloon(vb, -diff);
>  		update_balloon_size(vb);
> +		mutex_unlock(&vb->balloon_lock);
>  	}
>  	return 0;
>  }

Just a nitpick:
As leak_balloon() is also called at remove_common(), you'll need to introduce the
mutex there, similarly.


Thanks for move this forward.

Cheers!
-- Rafael

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC 1/2] virtio_balloon: move locking to the balloon thread
  2012-12-19 11:55   ` Rafael Aquini
@ 2012-12-19 12:47     ` Luiz Capitulino
  0 siblings, 0 replies; 8+ messages in thread
From: Luiz Capitulino @ 2012-12-19 12:47 UTC (permalink / raw)
  To: Rafael Aquini; +Cc: linux-mm, linux-kernel, riel, mst, amit.shah, agl

On Wed, 19 Dec 2012 09:55:58 -0200
Rafael Aquini <aquini@redhat.com> wrote:

> On Tue, Dec 18, 2012 at 06:17:29PM -0200, Luiz Capitulino wrote:
> > Today, the balloon_lock mutex is taken and released by fill_balloon()
> > and leak_balloon() when both functions are entered and when they
> > return.
> > 
> > This commit moves the locking to the caller instead, which is
> > the balloon() thread. The balloon thread is the sole caller of those
> > functions today.
> > 
> > The reason for this move is that the next commit will introduce
> > a shrinker callback for the balloon driver, which will also call
> > leak_balloon() but will require different locking semantics.
> > 
> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  drivers/virtio/virtio_balloon.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > index 2a70558..877e695 100644
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -133,7 +133,6 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num)
> >  	/* We can only do one array worth at a time. */
> >  	num = min(num, ARRAY_SIZE(vb->pfns));
> >  
> > -	mutex_lock(&vb->balloon_lock);
> >  	for (vb->num_pfns = 0; vb->num_pfns < num;
> >  	     vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
> >  		struct page *page = balloon_page_enqueue(vb_dev_info);
> > @@ -155,7 +154,6 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num)
> >  	/* Did we get any? */
> >  	if (vb->num_pfns != 0)
> >  		tell_host(vb, vb->inflate_vq);
> > -	mutex_unlock(&vb->balloon_lock);
> >  }
> >
> 
> Since you're removing the locking scheme from within this function, I think it
> would be a good idea introduce a comment stating its caller must held the mutex
> vb->balloon_lock.

Will address all comments for v1 (or rfc v2), thanks Rafael.

> 
>   
> >  static void release_pages_by_pfn(const u32 pfns[], unsigned int num)
> > @@ -177,7 +175,6 @@ static void leak_balloon(struct virtio_balloon *vb, size_t num)
> >  	/* We can only do one array worth at a time. */
> >  	num = min(num, ARRAY_SIZE(vb->pfns));
> >  
> > -	mutex_lock(&vb->balloon_lock);
> >  	for (vb->num_pfns = 0; vb->num_pfns < num;
> >  	     vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
> >  		page = balloon_page_dequeue(vb_dev_info);
> > @@ -193,7 +190,6 @@ static void leak_balloon(struct virtio_balloon *vb, size_t num)
> >  	 * is true, we *have* to do it in this order
> >  	 */
> >  	tell_host(vb, vb->deflate_vq);
> > -	mutex_unlock(&vb->balloon_lock);
> >  	release_pages_by_pfn(vb->pfns, vb->num_pfns);
> >  }
> >
> 
> ditto
> 
>   
> > @@ -306,11 +302,13 @@ static int balloon(void *_vballoon)
> >  					 || freezing(current));
> >  		if (vb->need_stats_update)
> >  			stats_handle_request(vb);
> > +		mutex_lock(&vb->balloon_lock);
> >  		if (diff > 0)
> >  			fill_balloon(vb, diff);
> >  		else if (diff < 0)
> >  			leak_balloon(vb, -diff);
> >  		update_balloon_size(vb);
> > +		mutex_unlock(&vb->balloon_lock);
> >  	}
> >  	return 0;
> >  }
> 
> Just a nitpick:
> As leak_balloon() is also called at remove_common(), you'll need to introduce the
> mutex there, similarly.
> 
> 
> Thanks for move this forward.
> 
> Cheers!
> -- Rafael
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC 2/2] virtio_balloon: add auto-ballooning support
  2012-12-18 20:17 ` [RFC 2/2] virtio_balloon: add auto-ballooning support Luiz Capitulino
@ 2013-01-11 20:43   ` Amit Shah
  2013-01-14 12:05     ` Luiz Capitulino
  0 siblings, 1 reply; 8+ messages in thread
From: Amit Shah @ 2013-01-11 20:43 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: linux-mm, linux-kernel, riel, aquini, mst, agl

On (Tue) 18 Dec 2012 [18:17:30], Luiz Capitulino wrote:
> The auto-ballooning feature automatically performs balloon inflate or
> deflate based on host and guest memory pressure. This can help to
> avoid swapping or worse in both, host and guest.
> 
> Auto-ballooning has a host and a guest part. The host performs
> automatic inflate by requesting the guest to inflate its balloon
> when the host is facing memory pressure. The guest performs
> automatic deflate when it's facing memory pressure itself. It's
> expected that auto-inflate and auto-deflate will balance each
> other over time.
> 
> This commit implements the guest side of auto-ballooning.
> 
> To perform automatic deflate, the virtio_balloon driver registers
> a shrinker callback, which will try to deflate the guest's balloon
> on guest memory pressure just like if it were a cache. The shrinker
> callback is only registered if the host supports the
> VIRTIO_BALLOON_F_AUTO_BALLOON feature bit.

I'm wondering if guest should auto-deflate even when the AUTO_BALLOON
feature isn't supported by the host: if a guest is under pressure,
there's no way for it to tell the host and wait for the host to
deflate the balloon, so it may be beneficial to just go ahead and
deflate the balloon for all hosts.

Similarly, on the host side, management can configure a VM to either
enable or disable auto-balloon (the auto-inflate part).  So even the
host can do away with the feature advertisement and negotiation.

Is there some use-case I'm missing where doing these actions after
feature negotiation is beneficial?

> FIXMEs
> 
>  o the guest kernel seems to spin when the host is performing a long
>    auto-inflate

Is this introduced by the current patches?  I'd assume it happens even
without it -- these patches just introduce some heuristics, the
mechanism has stayed the same.

> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  drivers/virtio/virtio_balloon.c     | 54 +++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/virtio_balloon.h |  1 +
>  2 files changed, 55 insertions(+)

Patch looks good, just one thing:

> +	/*
> +	 * If the current balloon size is greater than the number of
> +	 * pages being reclaimed by the kernel, deflate only the needed
> +	 * amount. Otherwise deflate everything we have.
> +	 */
> +	if (nr_pages > sc->nr_to_scan) {
> +		new_target = nr_pages - sc->nr_to_scan;
> +	} else {
> +		new_target = 0;
> +	}

This looks better:

	new_target = 0;
	if (nr_pages > sc->nr_to_scan) {
		new_target = nr_pages - sc->nr_to_scan;
	}


Thanks,
		Amit

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC 2/2] virtio_balloon: add auto-ballooning support
  2013-01-11 20:43   ` Amit Shah
@ 2013-01-14 12:05     ` Luiz Capitulino
  2013-01-25 10:14       ` Amit Shah
  0 siblings, 1 reply; 8+ messages in thread
From: Luiz Capitulino @ 2013-01-14 12:05 UTC (permalink / raw)
  To: Amit Shah; +Cc: linux-mm, linux-kernel, riel, aquini, mst, agl

On Sat, 12 Jan 2013 02:13:17 +0530
Amit Shah <amit.shah@redhat.com> wrote:

> On (Tue) 18 Dec 2012 [18:17:30], Luiz Capitulino wrote:
> > The auto-ballooning feature automatically performs balloon inflate or
> > deflate based on host and guest memory pressure. This can help to
> > avoid swapping or worse in both, host and guest.
> > 
> > Auto-ballooning has a host and a guest part. The host performs
> > automatic inflate by requesting the guest to inflate its balloon
> > when the host is facing memory pressure. The guest performs
> > automatic deflate when it's facing memory pressure itself. It's
> > expected that auto-inflate and auto-deflate will balance each
> > other over time.
> > 
> > This commit implements the guest side of auto-ballooning.
> > 
> > To perform automatic deflate, the virtio_balloon driver registers
> > a shrinker callback, which will try to deflate the guest's balloon
> > on guest memory pressure just like if it were a cache. The shrinker
> > callback is only registered if the host supports the
> > VIRTIO_BALLOON_F_AUTO_BALLOON feature bit.
> 
> I'm wondering if guest should auto-deflate even when the AUTO_BALLOON
> feature isn't supported by the host: if a guest is under pressure,
> there's no way for it to tell the host and wait for the host to
> deflate the balloon, so it may be beneficial to just go ahead and
> deflate the balloon for all hosts.

I see two problems with this. First, this will automagically override
balloon changes done by the user; and second, if we don't have the
auto-inflate part and if the host starts facing memory pressure, VMs
may start getting OOM.

> Similarly, on the host side, management can configure a VM to either
> enable or disable auto-balloon (the auto-inflate part).  So even the
> host can do away with the feature advertisement and negotiation.
> 
> Is there some use-case I'm missing where doing these actions after
> feature negotiation is beneficial?
> 
> > FIXMEs
> > 
> >  o the guest kernel seems to spin when the host is performing a long
> >    auto-inflate
> 
> Is this introduced by the current patches?  I'd assume it happens even
> without it -- these patches just introduce some heuristics, the
> mechanism has stayed the same.

Good point, I'll check that.

> > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> > ---
> >  drivers/virtio/virtio_balloon.c     | 54 +++++++++++++++++++++++++++++++++++++
> >  include/uapi/linux/virtio_balloon.h |  1 +
> >  2 files changed, 55 insertions(+)
> 
> Patch looks good, just one thing:
> 
> > +	/*
> > +	 * If the current balloon size is greater than the number of
> > +	 * pages being reclaimed by the kernel, deflate only the needed
> > +	 * amount. Otherwise deflate everything we have.
> > +	 */
> > +	if (nr_pages > sc->nr_to_scan) {
> > +		new_target = nr_pages - sc->nr_to_scan;
> > +	} else {
> > +		new_target = 0;
> > +	}
> 
> This looks better:
> 
> 	new_target = 0;
> 	if (nr_pages > sc->nr_to_scan) {
> 		new_target = nr_pages - sc->nr_to_scan;
> 	}

Ok.

> 
> 
> Thanks,
> 		Amit
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC 2/2] virtio_balloon: add auto-ballooning support
  2013-01-14 12:05     ` Luiz Capitulino
@ 2013-01-25 10:14       ` Amit Shah
  0 siblings, 0 replies; 8+ messages in thread
From: Amit Shah @ 2013-01-25 10:14 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: linux-mm, linux-kernel, riel, aquini, mst, agl

On (Mon) 14 Jan 2013 [10:05:01], Luiz Capitulino wrote:
> On Sat, 12 Jan 2013 02:13:17 +0530
> Amit Shah <amit.shah@redhat.com> wrote:
> 
> > On (Tue) 18 Dec 2012 [18:17:30], Luiz Capitulino wrote:
> > > The auto-ballooning feature automatically performs balloon inflate or
> > > deflate based on host and guest memory pressure. This can help to
> > > avoid swapping or worse in both, host and guest.
> > > 
> > > Auto-ballooning has a host and a guest part. The host performs
> > > automatic inflate by requesting the guest to inflate its balloon
> > > when the host is facing memory pressure. The guest performs
> > > automatic deflate when it's facing memory pressure itself. It's
> > > expected that auto-inflate and auto-deflate will balance each
> > > other over time.
> > > 
> > > This commit implements the guest side of auto-ballooning.
> > > 
> > > To perform automatic deflate, the virtio_balloon driver registers
> > > a shrinker callback, which will try to deflate the guest's balloon
> > > on guest memory pressure just like if it were a cache. The shrinker
> > > callback is only registered if the host supports the
> > > VIRTIO_BALLOON_F_AUTO_BALLOON feature bit.
> > 
> > I'm wondering if guest should auto-deflate even when the AUTO_BALLOON
> > feature isn't supported by the host: if a guest is under pressure,
> > there's no way for it to tell the host and wait for the host to
> > deflate the balloon, so it may be beneficial to just go ahead and
> > deflate the balloon for all hosts.
> 
> I see two problems with this. First, this will automagically override
> balloon changes done by the user; and second, if we don't have the
> auto-inflate part and if the host starts facing memory pressure, VMs
> may start getting OOM.

Practically, though, at least for hosts and VMs managed by libvirt,
guests will be confined by cgroups so they don't exceed some
pre-defined quota.  Guests should always be assumed to be malicious
and / or greedy, so I'm certain all host mgmt software will have some
checks in place.

		Amit

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-01-25 10:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-18 20:17 [RFC 0/2] auto-ballooning prototype (guest part) Luiz Capitulino
2012-12-18 20:17 ` [RFC 1/2] virtio_balloon: move locking to the balloon thread Luiz Capitulino
2012-12-19 11:55   ` Rafael Aquini
2012-12-19 12:47     ` Luiz Capitulino
2012-12-18 20:17 ` [RFC 2/2] virtio_balloon: add auto-ballooning support Luiz Capitulino
2013-01-11 20:43   ` Amit Shah
2013-01-14 12:05     ` Luiz Capitulino
2013-01-25 10:14       ` Amit Shah

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