Re: [PATCH 2/2] virtio_balloon: free some memory from baloon on OOM

["Denis V. Lunev" <den@openvz.org>]

On 13/10/14 09:32, Rusty Russell wrote:
> "Denis V. Lunev" <den@parallels.com> writes:
>> From: Raushaniya Maksudova <rmaksudova@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.
>
> 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".
ok, will do

>> +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?
this is a piece from a previous version of the patch, I'll fix this and 
resend.

>> +	num_freed_pages = leak_balloon(vb, oom_vballoon_pages);
>> +	update_balloon_size(vb);
>> +	*freed += num_freed_pages;
>> +
>> +	return NOTIFY_OK;
>> +}
> Cheers,
> Rusty.