diff mbox

[v2] virtio_balloon: disable oom killer when fill balloon

Message ID 20110426085953.GA12389@darkstar (mailing list archive)
State New, archived
Headers show

Commit Message

Dave Young April 26, 2011, 8:59 a.m. UTC
When memory pressure is high, virtio ballooning will probably cause oom killing.
Even if alloc_page with GFP_NORETRY itself does not directly trigger oom it
will make memory becoming low then memory alloc of other processes will trigger
oom killing. It is not desired behaviour.

Here disable oom killer in fill_balloon to address this issue.
Add code comment as KOSAKI Motohiro's suggestion.

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

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Minchan Kim April 26, 2011, 9:28 a.m. UTC | #1
Please resend this with [2/2] to linux-mm.

On Tue, Apr 26, 2011 at 5:59 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
> When memory pressure is high, virtio ballooning will probably cause oom killing.
> Even if alloc_page with GFP_NORETRY itself does not directly trigger oom it
> will make memory becoming low then memory alloc of other processes will trigger
> oom killing. It is not desired behaviour.

I can't understand why it is undesirable.
Why do we have to handle it specially?


>
> Here disable oom killer in fill_balloon to address this issue.
> Add code comment as KOSAKI Motohiro's suggestion.
>
> Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
> ---
>  drivers/virtio/virtio_balloon.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
>
> --- linux-2.6.orig/drivers/virtio/virtio_balloon.c      2011-04-26 11:39:14.053118406 +0800
> +++ linux-2.6/drivers/virtio/virtio_balloon.c   2011-04-26 16:54:56.419741542 +0800
> @@ -25,6 +25,7 @@
>  #include <linux/freezer.h>
>  #include <linux/delay.h>
>  #include <linux/slab.h>
> +#include <linux/oom.h>
>
>  struct virtio_balloon
>  {
> @@ -102,6 +103,12 @@ static void fill_balloon(struct virtio_b
>        /* We can only do one array worth at a time. */
>        num = min(num, ARRAY_SIZE(vb->pfns));
>
> +       /* Disable oom killer for indirect oom due to our memory consuming
> +        * Currently only hibernation code use oom_killer_disable,

Hmm, Please look at current mmotm. Now oom_killer_disabled is used by
do_try_to_free_pages in mmotm so it could make unnecessary oom kill.

BTW, I can't understand why we need to handle virtio by special.
Could you explain it in detail? :)
Dave Young April 26, 2011, 9:39 a.m. UTC | #2
On Tue, Apr 26, 2011 at 5:28 PM, Minchan Kim <minchan.kim@gmail.com> wrote:
> Please resend this with [2/2] to linux-mm.
>
> On Tue, Apr 26, 2011 at 5:59 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
>> When memory pressure is high, virtio ballooning will probably cause oom killing.
>> Even if alloc_page with GFP_NORETRY itself does not directly trigger oom it
>> will make memory becoming low then memory alloc of other processes will trigger
>> oom killing. It is not desired behaviour.
>
> I can't understand why it is undesirable.
> Why do we have to handle it specially?
>

Suppose user run some random memory hogging process while ballooning
it will be undesirable.

>
>>
>> Here disable oom killer in fill_balloon to address this issue.
>> Add code comment as KOSAKI Motohiro's suggestion.
>>
>> Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
>> ---
>>  drivers/virtio/virtio_balloon.c |    8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> --- linux-2.6.orig/drivers/virtio/virtio_balloon.c      2011-04-26 11:39:14.053118406 +0800
>> +++ linux-2.6/drivers/virtio/virtio_balloon.c   2011-04-26 16:54:56.419741542 +0800
>> @@ -25,6 +25,7 @@
>>  #include <linux/freezer.h>
>>  #include <linux/delay.h>
>>  #include <linux/slab.h>
>> +#include <linux/oom.h>
>>
>>  struct virtio_balloon
>>  {
>> @@ -102,6 +103,12 @@ static void fill_balloon(struct virtio_b
>>        /* We can only do one array worth at a time. */
>>        num = min(num, ARRAY_SIZE(vb->pfns));
>>
>> +       /* Disable oom killer for indirect oom due to our memory consuming
>> +        * Currently only hibernation code use oom_killer_disable,
>
> Hmm, Please look at current mmotm. Now oom_killer_disabled is used by
> do_try_to_free_pages in mmotm so it could make unnecessary oom kill.
>
> BTW, I can't understand why we need to handle virtio by special.
> Could you explain it in detail? :)
>
>
>
> --
> Kind regards,
> Minchan Kim
>
Minchan Kim April 26, 2011, 11:33 p.m. UTC | #3
On Tue, Apr 26, 2011 at 6:39 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
> On Tue, Apr 26, 2011 at 5:28 PM, Minchan Kim <minchan.kim@gmail.com> wrote:
>> Please resend this with [2/2] to linux-mm.
>>
>> On Tue, Apr 26, 2011 at 5:59 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
>>> When memory pressure is high, virtio ballooning will probably cause oom killing.
>>> Even if alloc_page with GFP_NORETRY itself does not directly trigger oom it
>>> will make memory becoming low then memory alloc of other processes will trigger
>>> oom killing. It is not desired behaviour.
>>
>> I can't understand why it is undesirable.
>> Why do we have to handle it specially?
>>
>
> Suppose user run some random memory hogging process while ballooning
> it will be undesirable.


In VM POV, kvm and random memory hogging processes are customers.
If we handle ballooning specially with disable OOM, what happens other
processes requires memory at same time? Should they wait for balloon
driver to release memory?

I don't know your point. Sorry.
Could you explain your scenario in detail for justify your idea?
And as I previous said, we have to solve oom_killer_disabled issue in
do_try_to_free_pages.

Thanks, Dave.
Dave Young April 27, 2011, 1:37 a.m. UTC | #4
On Wed, Apr 27, 2011 at 7:33 AM, Minchan Kim <minchan.kim@gmail.com> wrote:
> On Tue, Apr 26, 2011 at 6:39 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
>> On Tue, Apr 26, 2011 at 5:28 PM, Minchan Kim <minchan.kim@gmail.com> wrote:
>>> Please resend this with [2/2] to linux-mm.
>>>
>>> On Tue, Apr 26, 2011 at 5:59 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
>>>> When memory pressure is high, virtio ballooning will probably cause oom killing.
>>>> Even if alloc_page with GFP_NORETRY itself does not directly trigger oom it
>>>> will make memory becoming low then memory alloc of other processes will trigger
>>>> oom killing. It is not desired behaviour.
>>>
>>> I can't understand why it is undesirable.
>>> Why do we have to handle it specially?
>>>
>>
>> Suppose user run some random memory hogging process while ballooning
>> it will be undesirable.
>
>
> In VM POV, kvm and random memory hogging processes are customers.
> If we handle ballooning specially with disable OOM, what happens other
> processes requires memory at same time? Should they wait for balloon
> driver to release memory?
>
> I don't know your point. Sorry.
> Could you explain your scenario in detail for justify your idea?

What you said make sense I understand what you said now. Lets ignore
my above argue and see what I'm actually doing.

I'm hacking with balloon driver to fit to short the vm migration time.

while migrating host tell guest to balloon as much memory as it can, then start
migrate, just skip the ballooned pages, after migration done tell
guest to release the memory.

In migration case oom is not I want to see and disable oom will be good.

> And as I previous said, we have to solve oom_killer_disabled issue in
> do_try_to_free_pages.
>
> Thanks, Dave.
> --
> Kind regards,
> Minchan Kim
>
Dave Young April 27, 2011, 1:48 a.m. UTC | #5
On Wed, Apr 27, 2011 at 9:37 AM, Dave Young <hidave.darkstar@gmail.com> wrote:
> On Wed, Apr 27, 2011 at 7:33 AM, Minchan Kim <minchan.kim@gmail.com> wrote:
>> On Tue, Apr 26, 2011 at 6:39 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
>>> On Tue, Apr 26, 2011 at 5:28 PM, Minchan Kim <minchan.kim@gmail.com> wrote:
>>>> Please resend this with [2/2] to linux-mm.
>>>>
>>>> On Tue, Apr 26, 2011 at 5:59 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
>>>>> When memory pressure is high, virtio ballooning will probably cause oom killing.
>>>>> Even if alloc_page with GFP_NORETRY itself does not directly trigger oom it
>>>>> will make memory becoming low then memory alloc of other processes will trigger
>>>>> oom killing. It is not desired behaviour.
>>>>
>>>> I can't understand why it is undesirable.
>>>> Why do we have to handle it specially?
>>>>
>>>
>>> Suppose user run some random memory hogging process while ballooning
>>> it will be undesirable.
>>
>>
>> In VM POV, kvm and random memory hogging processes are customers.
>> If we handle ballooning specially with disable OOM, what happens other
>> processes requires memory at same time? Should they wait for balloon
>> driver to release memory?
>>
>> I don't know your point. Sorry.
>> Could you explain your scenario in detail for justify your idea?
>
> What you said make sense I understand what you said now. Lets ignore
> my above argue and see what I'm actually doing.
>
> I'm hacking with balloon driver to fit to short the vm migration time.
>
> while migrating host tell guest to balloon as much memory as it can, then start
> migrate, just skip the ballooned pages, after migration done tell
> guest to release the memory.
>
> In migration case oom is not I want to see and disable oom will be good.

BTW, if oom_killer_disabled is really not recommended to use I can
switch back to oom_notifier way.

>
>> And as I previous said, we have to solve oom_killer_disabled issue in
>> do_try_to_free_pages.
>>
>> Thanks, Dave.
>> --
>> Kind regards,
>> Minchan Kim
>>
>
>
>
> --
> Regards
> dave
>
Motohiro KOSAKI April 27, 2011, 2:06 a.m. UTC | #6
> On Wed, Apr 27, 2011 at 9:37 AM, Dave Young <hidave.darkstar@gmail.com> wrote:
> > On Wed, Apr 27, 2011 at 7:33 AM, Minchan Kim <minchan.kim@gmail.com> wrote:
> >> On Tue, Apr 26, 2011 at 6:39 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
> >>> On Tue, Apr 26, 2011 at 5:28 PM, Minchan Kim <minchan.kim@gmail.com> wrote:
> >>>> Please resend this with [2/2] to linux-mm.
> >>>>
> >>>> On Tue, Apr 26, 2011 at 5:59 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
> >>>>> When memory pressure is high, virtio ballooning will probably cause oom killing.
> >>>>> Even if alloc_page with GFP_NORETRY itself does not directly trigger oom it
> >>>>> will make memory becoming low then memory alloc of other processes will trigger
> >>>>> oom killing. It is not desired behaviour.
> >>>>
> >>>> I can't understand why it is undesirable.
> >>>> Why do we have to handle it specially?
> >>>>
> >>>
> >>> Suppose user run some random memory hogging process while ballooning
> >>> it will be undesirable.
> >>
> >>
> >> In VM POV, kvm and random memory hogging processes are customers.
> >> If we handle ballooning specially with disable OOM, what happens other
> >> processes requires memory at same time? Should they wait for balloon
> >> driver to release memory?
> >>
> >> I don't know your point. Sorry.
> >> Could you explain your scenario in detail for justify your idea?
> >
> > What you said make sense I understand what you said now. Lets ignore
> > my above argue and see what I'm actually doing.
> >
> > I'm hacking with balloon driver to fit to short the vm migration time.
> >
> > while migrating host tell guest to balloon as much memory as it can, then start
> > migrate, just skip the ballooned pages, after migration done tell
> > guest to release the memory.
> >
> > In migration case oom is not I want to see and disable oom will be good.
> 
> BTW, if oom_killer_disabled is really not recommended to use I can
> switch back to oom_notifier way.

Could you please explain why you dislike oom_notifier and what problem
you faced? I haven't understand why oom_notifier is bad. probably my
less knowledge of balloon is a reason.



--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dave Young April 27, 2011, 2:22 a.m. UTC | #7
On Wed, Apr 27, 2011 at 10:06 AM, KOSAKI Motohiro
<kosaki.motohiro@jp.fujitsu.com> wrote:
>> On Wed, Apr 27, 2011 at 9:37 AM, Dave Young <hidave.darkstar@gmail.com> wrote:
>> > On Wed, Apr 27, 2011 at 7:33 AM, Minchan Kim <minchan.kim@gmail.com> wrote:
>> >> On Tue, Apr 26, 2011 at 6:39 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
>> >>> On Tue, Apr 26, 2011 at 5:28 PM, Minchan Kim <minchan.kim@gmail.com> wrote:
>> >>>> Please resend this with [2/2] to linux-mm.
>> >>>>
>> >>>> On Tue, Apr 26, 2011 at 5:59 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
>> >>>>> When memory pressure is high, virtio ballooning will probably cause oom killing.
>> >>>>> Even if alloc_page with GFP_NORETRY itself does not directly trigger oom it
>> >>>>> will make memory becoming low then memory alloc of other processes will trigger
>> >>>>> oom killing. It is not desired behaviour.
>> >>>>
>> >>>> I can't understand why it is undesirable.
>> >>>> Why do we have to handle it specially?
>> >>>>
>> >>>
>> >>> Suppose user run some random memory hogging process while ballooning
>> >>> it will be undesirable.
>> >>
>> >>
>> >> In VM POV, kvm and random memory hogging processes are customers.
>> >> If we handle ballooning specially with disable OOM, what happens other
>> >> processes requires memory at same time? Should they wait for balloon
>> >> driver to release memory?
>> >>
>> >> I don't know your point. Sorry.
>> >> Could you explain your scenario in detail for justify your idea?
>> >
>> > What you said make sense I understand what you said now. Lets ignore
>> > my above argue and see what I'm actually doing.
>> >
>> > I'm hacking with balloon driver to fit to short the vm migration time.
>> >
>> > while migrating host tell guest to balloon as much memory as it can, then start
>> > migrate, just skip the ballooned pages, after migration done tell
>> > guest to release the memory.
>> >
>> > In migration case oom is not I want to see and disable oom will be good.
>>
>> BTW, if oom_killer_disabled is really not recommended to use I can
>> switch back to oom_notifier way.
>
> Could you please explain why you dislike oom_notifier and what problem
> you faced? I haven't understand why oom_notifier is bad. probably my
> less knowledge of balloon is a reason.
>

Both is fine for me indeed, oom_killer_disable is more simple to use
instead. I ever sent a oom_notifier patch last year and did not get
much intention, I can refresh and resend it.
diff mbox

Patch

--- linux-2.6.orig/drivers/virtio/virtio_balloon.c	2011-04-26 11:39:14.053118406 +0800
+++ linux-2.6/drivers/virtio/virtio_balloon.c	2011-04-26 16:54:56.419741542 +0800
@@ -25,6 +25,7 @@ 
 #include <linux/freezer.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
+#include <linux/oom.h>
 
 struct virtio_balloon
 {
@@ -102,6 +103,12 @@  static void fill_balloon(struct virtio_b
 	/* We can only do one array worth at a time. */
 	num = min(num, ARRAY_SIZE(vb->pfns));
 
+	/* Disable oom killer for indirect oom due to our memory consuming
+	 * Currently only hibernation code use oom_killer_disable,
+	 * hibernation will freeze us before disable oom killer, so
+	 * It's safe here without locks.
+	 */
+	oom_killer_disable();
 	for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) {
 		struct page *page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY |
 					__GFP_NOMEMALLOC | __GFP_NOWARN);
@@ -119,6 +126,7 @@  static void fill_balloon(struct virtio_b
 		vb->num_pages++;
 		list_add(&page->lru, &vb->pages);
 	}
+	oom_killer_enable();
 
 	/* Didn't get any?  Oh well. */
 	if (vb->num_pfns == 0)