[PATCH] kvm: add oom notifier for virtio balloon

[PATCH] kvm: add oom notifier for virtio balloon

[Dave Young]

Balloon could cause guest memory oom killing and panic.

Add oom notify to leak some memory and retry fill balloon after 5 minutes.

At the same time add a mutex to protect balloon operations
because we need leak balloon in oom notifier and give back freed value. 

Thanks Anthony Liguori for his sugestion about inflate retrying.
Sometimes it will cause endless inflate/oom/delay loop,
so I think next step is to add an option to do noretry-when-oom balloon.

Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
---
 drivers/virtio/virtio_balloon.c |   92 ++++++++++++++++++++++++++++++++--------
 1 file changed, 75 insertions(+), 17 deletions(-)

--- linux-2.6.orig/drivers/virtio/virtio_balloon.c	2010-10-02 10:35:44.723333335 +0800
+++ linux-2.6/drivers/virtio/virtio_balloon.c	2010-10-05 10:40:24.740001466 +0800
@@ -2,6 +2,7 @@
  * Tosatti's implementations.
  *
  *  Copyright 2008 Rusty Russell IBM Corporation
+ *  oom notify - Dave Young <hidave.darkstar@gmail.com>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -25,6 +26,14 @@
 #include <linux/freezer.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
+#include <linux/notifier.h>
+#include <linux/param.h>
+#include <linux/timer.h>
+#include <linux/jiffies.h>
+#include <linux/oom.h>
+
+#define BALLOON_OOM_DELAY_MINUTES	5
+#define BALLOON_OOM_PAGES		256
 
 struct virtio_balloon
 {
@@ -54,6 +63,10 @@ struct virtio_balloon
 	/* Memory statistics */
 	int need_stats_update;
 	struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
+
+	struct mutex mutex;
+	struct timer_list timer;
+	struct notifier_block oom_nb;
 };
 
 static struct virtio_device_id id_table[] = {
@@ -97,34 +110,37 @@ static void tell_host(struct virtio_ball
 	wait_for_completion(&vb->acked);
 }
 
+static void balloon_oom_timeout(unsigned long arg)
+{
+	struct virtio_balloon *v = (struct virtio_balloon *)arg;
+
+	wake_up(&v->config_change);
+}
+
 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));
 
 	for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) {
-		struct page *page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY |
+		struct page *page;
+
+		if (unlikely(timer_pending(&vb->timer)))
+			break;
+
+		page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY |
 					__GFP_NOMEMALLOC | __GFP_NOWARN);
-		if (!page) {
-			if (printk_ratelimit())
-				dev_printk(KERN_INFO, &vb->vdev->dev,
-					   "Out of puff! Can't get %zu pages\n",
-					   num);
-			/* Sleep for at least 1/5 of a second before retry. */
-			msleep(200);
+		if (!page)
 			break;
-		}
+
 		vb->pfns[vb->num_pfns] = page_to_balloon_pfn(page);
 		totalram_pages--;
 		vb->num_pages++;
 		list_add(&page->lru, &vb->pages);
 	}
 
-	/* Didn't get any?  Oh well. */
-	if (vb->num_pfns == 0)
-		return;
-
-	tell_host(vb, vb->inflate_vq);
+	if (vb->num_pfns)
+		tell_host(vb, vb->inflate_vq);
 }
 
 static void release_pages_by_pfn(const u32 pfns[], unsigned int num)
@@ -235,22 +251,53 @@ static void virtballoon_changed(struct v
 
 static inline s64 towards_target(struct virtio_balloon *vb)
 {
-	u32 v;
+	u32 v, ret;
 	vb->vdev->config->get(vb->vdev,
 			      offsetof(struct virtio_balloon_config, num_pages),
 			      &v, sizeof(v));
-	return (s64)v - vb->num_pages;
+	ret = (s64)v - vb->num_pages;
+
+	if (ret > 0 && (unlikely(timer_pending(&vb->timer)))) {
+		printk(KERN_INFO "balloon will delay inflate due to oom ...\n");
+		return 0;
+	}
+
+	return ret;
 }
 
 static void update_balloon_size(struct virtio_balloon *vb)
 {
-	__le32 actual = cpu_to_le32(vb->num_pages);
+	__le32 actual;
 
+	actual = cpu_to_le32(vb->num_pages);
 	vb->vdev->config->set(vb->vdev,
 			      offsetof(struct virtio_balloon_config, actual),
 			      &actual, sizeof(actual));
 }
 
+static int balloon_oom_notify(struct notifier_block *self,
+				unsigned long dummy, void *parm)
+{
+	struct virtio_balloon *vb;
+	unsigned long *freed = (unsigned long *)parm;
+	unsigned int nr;
+
+	vb = container_of(self, struct virtio_balloon, oom_nb);
+
+	mutex_lock(&vb->mutex);
+	nr = min_t(unsigned int, vb->num_pages, BALLOON_OOM_PAGES);
+	if (nr) {
+		printk(KERN_INFO "balloon oom notifier leak %d pages\n", nr);
+		leak_balloon(vb, nr);
+		update_balloon_size(vb);
+	}
+	*freed = nr;
+	mutex_unlock(&vb->mutex);
+	mod_timer(&vb->timer, jiffies + BALLOON_OOM_DELAY_MINUTES * 60 * HZ);
+
+	return NOTIFY_OK;
+}
+
 static int balloon(void *_vballoon)
 {
 	struct virtio_balloon *vb = _vballoon;
@@ -267,11 +314,14 @@ static int balloon(void *_vballoon)
 					 || freezing(current));
 		if (vb->need_stats_update)
 			stats_handle_request(vb);
+
+		mutex_lock(&vb->mutex);
 		if (diff > 0)
 			fill_balloon(vb, diff);
 		else if (diff < 0)
 			leak_balloon(vb, -diff);
 		update_balloon_size(vb);
+		mutex_unlock(&vb->mutex);
 	}
 	return 0;
 }
@@ -325,6 +375,13 @@ static int virtballoon_probe(struct virt
 
 	vb->tell_host_first
 		= virtio_has_feature(vdev, VIRTIO_BALLOON_F_MUST_TELL_HOST);
+	vb->oom_nb.notifier_call = balloon_oom_notify;
+
+	mutex_init(&vb->mutex);
+	err = register_oom_notifier(&vb->oom_nb);
+	if (err < 0)
+		goto out_del_vqs;
+	setup_timer(&vb->timer, balloon_oom_timeout, (unsigned long)vb);
 
 	return 0;
 
@@ -340,6 +397,7 @@ static void __devexit virtballoon_remove
 {
 	struct virtio_balloon *vb = vdev->priv;
 
+	unregister_oom_notifier(&vb->oom_nb);
 	kthread_stop(vb->thread);
 
 	/* There might be pages left in the balloon: free them. */

Re: [PATCH] kvm: add oom notifier for virtio balloon

[Rusty Russell]

On Tue, 5 Oct 2010 11:15:21 pm Dave Young wrote:
> Balloon could cause guest memory oom killing and panic.
> 
> Add oom notify to leak some memory and retry fill balloon after 5 minutes.

Have you tried registering a shrinker?  See mm.h.

Thanks,
Rusty.

Re: [PATCH] kvm: add oom notifier for virtio balloon

[Dave Young]

On Wed, Oct 6, 2010 at 5:05 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> On Tue, 5 Oct 2010 11:15:21 pm Dave Young wrote:
>> Balloon could cause guest memory oom killing and panic.
>>
>> Add oom notify to leak some memory and retry fill balloon after 5 minutes.
>
> Have you tried registering a shrinker?  See mm.h.

Hi, thanks. I didn't know shrinker can shrink mem beyond slab. Will try

>
> Thanks,
> Rusty.
>



-- 
Regards
dave

Re: [PATCH] kvm: add oom notifier for virtio balloon

[Dave Young]

On Wed, Oct 6, 2010 at 9:50 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
> On Wed, Oct 6, 2010 at 5:05 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> On Tue, 5 Oct 2010 11:15:21 pm Dave Young wrote:
>>> Balloon could cause guest memory oom killing and panic.
>>>
>>> Add oom notify to leak some memory and retry fill balloon after 5 minutes.
>>
>> Have you tried registering a shrinker?  See mm.h.
>
> Hi, thanks. I didn't know shrinker can shrink mem beyond slab. Will try
Hi, rusty

I did some test with shrinker, findings as following:

1. shrinker is for ageable cache shrinking, balloon pages is a different kind
2. oom notifier is a last minute shrink, but shrinker is more conservative.
after my tests of "balloon 30" for a slackware 13.0 guest, free -m in guest
oom notifier results 17M
shrinker results 77M

3. Aside of above use mutex locking in shrinker cause kernel hang, no
idea how to fix it


>
>>
>> Thanks,
>> Rusty.
>>
>
>
>
> --
> Regards
> dave
>



-- 
Regards
dave

Re: [PATCH] kvm: add oom notifier for virtio balloon

[Balbir Singh]

* Dave Young <hidave.darkstar@gmail.com> [2010-10-05 20:45:21]:

> Balloon could cause guest memory oom killing and panic.
> 
> Add oom notify to leak some memory and retry fill balloon after 5 minutes.
> 
> At the same time add a mutex to protect balloon operations
> because we need leak balloon in oom notifier and give back freed value. 
> 
> Thanks Anthony Liguori for his sugestion about inflate retrying.
> Sometimes it will cause endless inflate/oom/delay loop,
> so I think next step is to add an option to do noretry-when-oom balloon.
> 
> Signed-off-by: Dave Young <hidave.darkstar@gmail.com>

Won't __GFP_NORETRY prevent OOM? Could you please describe how you
tested the patch?

-- 
	Three Cheers,
	Balbir

Re: [PATCH] kvm: add oom notifier for virtio balloon

[Dave Young]

On Fri, Oct 8, 2010 at 9:09 PM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> * Dave Young <hidave.darkstar@gmail.com> [2010-10-05 20:45:21]:
>
>> Balloon could cause guest memory oom killing and panic.
>>
>> Add oom notify to leak some memory and retry fill balloon after 5 minutes.
>>
>> At the same time add a mutex to protect balloon operations
>> because we need leak balloon in oom notifier and give back freed value.
>>
>> Thanks Anthony Liguori for his sugestion about inflate retrying.
>> Sometimes it will cause endless inflate/oom/delay loop,
>> so I think next step is to add an option to do noretry-when-oom balloon.
>>
>> Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
>
> Won't __GFP_NORETRY prevent OOM? Could you please describe how you
> tested the patch?

I have not tried __GFP_NORETRY, it should work, but balloon thread
will keep wasting cpu resource to allocating.


To test the patch, just balloon to small than minimal memory.

I use "balloon 30" in qemu monitor to limit slackware guest memory
usage. The normal memory used is ~40M.

Actually we need to differentiate the process which caused oom. If it
is balloon thread we should just stop ballooning, if it is others we
can do something like this patch, e.g. retry ballooning after 5
minutes.
>
> --
>        Three Cheers,
>        Balbir
>



-- 
Regards
dave

Re: [PATCH] kvm: add oom notifier for virtio balloon

[Balbir Singh]

* Dave Young <hidave.darkstar@gmail.com> [2010-10-08 21:33:02]:

> On Fri, Oct 8, 2010 at 9:09 PM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> > * Dave Young <hidave.darkstar@gmail.com> [2010-10-05 20:45:21]:
> >
> >> Balloon could cause guest memory oom killing and panic.
> >>
> >> Add oom notify to leak some memory and retry fill balloon after 5 minutes.
> >>
> >> At the same time add a mutex to protect balloon operations
> >> because we need leak balloon in oom notifier and give back freed value.
> >>
> >> Thanks Anthony Liguori for his sugestion about inflate retrying.
> >> Sometimes it will cause endless inflate/oom/delay loop,
> >> so I think next step is to add an option to do noretry-when-oom balloon.
> >>
> >> Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
> >
> > Won't __GFP_NORETRY prevent OOM? Could you please describe how you
> > tested the patch?
> 
> I have not tried __GFP_NORETRY, it should work, but balloon thread
> will keep wasting cpu resource to allocating.
> 
> 
> To test the patch, just balloon to small than minimal memory.
> 
> I use "balloon 30" in qemu monitor to limit slackware guest memory
> usage. The normal memory used is ~40M.
> 
> Actually we need to differentiate the process which caused oom. If it
> is balloon thread we should just stop ballooning, if it is others we
> can do something like this patch, e.g. retry ballooning after 5
> minutes.

Ideally the balloon thread should never OOM with __GFP_NORETRY (IIRC).
The other situation should be dealt with, we should free up any pages
we have. I wonder if the timeout should be a sysctl tunable.

-- 
	Three Cheers,
	Balbir

Re: [PATCH] kvm: add oom notifier for virtio balloon

[Dave Young]

On Fri, Oct 8, 2010 at 11:53 PM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> * Dave Young <hidave.darkstar@gmail.com> [2010-10-08 21:33:02]:
>
>> On Fri, Oct 8, 2010 at 9:09 PM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
>> > * Dave Young <hidave.darkstar@gmail.com> [2010-10-05 20:45:21]:
>> >
>> >> Balloon could cause guest memory oom killing and panic.
>> >>
>> >> Add oom notify to leak some memory and retry fill balloon after 5 minutes.
>> >>
>> >> At the same time add a mutex to protect balloon operations
>> >> because we need leak balloon in oom notifier and give back freed value.
>> >>
>> >> Thanks Anthony Liguori for his sugestion about inflate retrying.
>> >> Sometimes it will cause endless inflate/oom/delay loop,
>> >> so I think next step is to add an option to do noretry-when-oom balloon.
>> >>
>> >> Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
>> >
>> > Won't __GFP_NORETRY prevent OOM? Could you please describe how you
>> > tested the patch?
>>
>> I have not tried __GFP_NORETRY, it should work, but balloon thread
>> will keep wasting cpu resource to allocating.
>>
>>
>> To test the patch, just balloon to small than minimal memory.
>>
>> I use "balloon 30" in qemu monitor to limit slackware guest memory
>> usage. The normal memory used is ~40M.
>>
>> Actually we need to differentiate the process which caused oom. If it
>> is balloon thread we should just stop ballooning, if it is others we
>> can do something like this patch, e.g. retry ballooning after 5
>> minutes.
>
> Ideally the balloon thread should never OOM with __GFP_NORETRY (IIRC).
> The other situation should be dealt with, we should free up any pages
> we have. I wonder if the timeout should be a sysctl tunable.

balbir, you are right,  the oom is not from balloon alloc_page.
balloon thread cause memory low, then afterwards vm readahead code
cause oom. With oom notifier patch oom killing does not happen in my
test

>
> --
>        Three Cheers,
>        Balbir
>



-- 
Regards
dave