Virtualization - Oct 2014 - [PATCH 2/2] virtio_balloon: free some memory from baloon on OOM

On Mon, Oct 13, 2014 at 04:02:52PM +1030, Rusty Russell
wrote:
> "Denis V. Lunev" <den at parallels.com> writes:
> > From: Raushaniya Maksudova <rmaksudova at parallels.com>
> >
> > Excessive virtio_balloon inflation can cause invocation of OOM-killer,
> > when Linux is under severe memory pressure. Various mechanisms are
> > responsible for correct virtio_balloon memory management. Nevertheless
> > it is often the case that these control tools does not have enough
time
> > to react on fast changing memory load. As a result OS runs out of
memory
> > and invokes OOM-killer. The balancing of memory by use of the virtio
> > balloon should not cause the termination of processes while there are
> > pages in the balloon. Now there is no way for virtio balloon driver to
> > free some memory at the last moment before some process will be get
> > killed by OOM-killer.
> 
> This makes some amount of sense.
This reminds me of the balloon fs that Google once proposed.
This really needs to be controlled from host though.
At the moment host does not expect guest to deflate before
requests.
So as a minimum, add a feature bit for this.  what if you want a mix of
mandatory and optional balooning? I guess we can use multiple balloons,
is that the idea?

> But I suggest a few minor changes:
> 
> > +static int oom_vballoon_pages = OOM_VBALLOON_DEFAULT_PAGES;
> > +module_param(oom_vballoon_pages, int, S_IRUSR | S_IWUSR);
> > +MODULE_PARM_DESC(oom_vballoon_pages, "pages to free on
OOM");
> 
> Since this is already prefixed with "virtio_balloon." I suggest
just
> calling it "oom_pages".
> 
> > +static int virtballoon_oom_notify(struct notifier_block *self,
> > +				  unsigned long dummy, void *parm)
> > +{
> > +	unsigned int num_freed_pages;
> > +	unsigned long *freed = (unsigned long *)parm;
> > +	struct virtio_balloon *vb = container_of((struct notifier_block
*)self,
> > +						 struct virtio_balloon, nb);
> 
> Why cast self here?
> 
> > +	num_freed_pages = leak_balloon(vb, oom_vballoon_pages);
> > +	update_balloon_size(vb);
> > +	*freed += num_freed_pages;
> > +
> > +	return NOTIFY_OK;
> > +}
> 
> Cheers,
> Rusty.

"Michael S. Tsirkin" <mst at redhat.com> writes:
> On Mon, Oct 13, 2014 at 04:02:52PM +1030, Rusty Russell wrote:
>> "Denis V. Lunev" <den at parallels.com> writes:
>> > From: Raushaniya Maksudova <rmaksudova at parallels.com>
>> >
>> > Excessive virtio_balloon inflation can cause invocation of
OOM-killer,
>> > when Linux is under severe memory pressure. Various mechanisms are
>> > responsible for correct virtio_balloon memory management.
Nevertheless
>> > it is often the case that these control tools does not have enough
time
>> > to react on fast changing memory load. As a result OS runs out of
memory
>> > and invokes OOM-killer. The balancing of memory by use of the
virtio
>> > balloon should not cause the termination of processes while there
are
>> > pages in the balloon. Now there is no way for virtio balloon
driver to
>> > free some memory at the last moment before some process will be
get
>> > killed by OOM-killer.
>> 
>> This makes some amount of sense.
>
> This reminds me of the balloon fs that Google once proposed.
> This really needs to be controlled from host though.
> At the moment host does not expect guest to deflate before
> requests.
> So as a minimum, add a feature bit for this.  what if you want a mix of
> mandatory and optional balooning? I guess we can use multiple balloons,
> is that the idea?
Trying to claw back some pages on OOM is almost certainly correct,
even if the host doesn't expect it.  It's roughly equivalent to not
giving up pages in the first place.

Cheers,
Rusty.
PS.  Yes, a real guest-driven balloon is preferable, but that's a much
     larger task.

On Tue, Oct 14, 2014 at 10:14:05AM +1030, Rusty Russell
wrote:
> "Michael S. Tsirkin" <mst at redhat.com> writes:
> 
> > On Mon, Oct 13, 2014 at 04:02:52PM +1030, Rusty Russell wrote:
> >> "Denis V. Lunev" <den at parallels.com> writes:
> >> > From: Raushaniya Maksudova <rmaksudova at
parallels.com>
> >> >
> >> > Excessive virtio_balloon inflation can cause invocation of
OOM-killer,
> >> > when Linux is under severe memory pressure. Various
mechanisms are
> >> > responsible for correct virtio_balloon memory management.
Nevertheless
> >> > it is often the case that these control tools does not have
enough time
> >> > to react on fast changing memory load. As a result OS runs
out of memory
> >> > and invokes OOM-killer. The balancing of memory by use of the
virtio
> >> > balloon should not cause the termination of processes while
there are
> >> > pages in the balloon. Now there is no way for virtio balloon
driver to
> >> > free some memory at the last moment before some process will
be get
> >> > killed by OOM-killer.
> >> 
> >> This makes some amount of sense.
> >
> > This reminds me of the balloon fs that Google once proposed.
> > This really needs to be controlled from host though.
> > At the moment host does not expect guest to deflate before
> > requests.
> > So as a minimum, add a feature bit for this.  what if you want a mix
of
> > mandatory and optional balooning? I guess we can use multiple
balloons,
> > is that the idea?
> 
> Trying to claw back some pages on OOM is almost certainly correct,
> even if the host doesn't expect it.  It's roughly equivalent to not
> giving up pages in the first place.
Well the difference is that there are management tools that
poll balloon in host until they see balloon size reaches
the expected value.

They don't expect balloon to shrink below num_pages and will respond in
various
unexpected ways like e.g. killing the VM if it does.
Killing a userspace process within the guest might be better
for VM health.

Besides the fact that we always did it like this, these tools seem to have
basis in the spec.
Specifically, this is based on this text from the spec:
	the device asks for a certain amount of memory, and the driver
	supplies it (or withdraws it, if the device has more than it asks for).
	This allows the guest to adapt to changes in allowance of underlying
	physical memory.

and

	The device is driven by the receipt of a configuration change interrupt.


> Cheers,
> Rusty.
> PS.  Yes, a real guest-driven balloon is preferable, but that's a much
>      larger task.

Any objection to making the feature depend on a feature flag?

-- 
MST