On Tue, Sep 28, 2010 at 9:49 PM, Dave Young <hidave.darkstar@xxxxxxxxx> wrote:
> On Tue, Sep 28, 2010 at 9:34 PM, Anthony Liguori <anthony@xxxxxxxxxxxxx> wrote:
>> On 09/28/2010 08:19 AM, Dave Young wrote:
>>>
>>> Balloon could cause guest memory oom killing and panic. If we disable the
>>> oom killer it will be better at least avoid guest panic.
>>>
>>> If alloc failed we can just adjust the balloon target to be equal to
>>> current number by call vdev->config->set
>>>
>>> But during test I found the config->set num_pages does not change the
>>> config actually, Should I do hacks in userspace as well? If so where should
>>> I start to hack?
>>>
>>
>
> Hi,
>
> Thanks your comments.
>
>> The guest is not supposed to set the target field in it's config. ÂThis is a
>> host read/write, guest read-only field.
>
> Could you tell where to set it? If so, IMHO set config api should
> fail, isn't it?
Get it, in hw/virtio-balloon.c function virtio_balloon_set_config
>
>>
>> The problem with your approach generally speaking is that it's unclear
>> whether this is the right policy. ÂFor instance, what if another application
>> held a very large allocation which caused the fill request to stop but then
>> shortly afterwards, the aforementioned application released that allocation.
>> ÂIf instead of just stopping, we paused and tried again later, we could
>> potentially succeed.
>
> Yes, it is possible. But maybe better to do balloon from qemu monitor later?
>
>>
>> I think a better approach might be a graceful back off. ÂFor instance, when
>> you hit this condition, deflate the balloon by 10% based on the assumption
>> that you probably already gone too far. ÂBefore you attempt to allocate to
>> the target again, set a timeout that increases in duration exponentially
>> until you reach some maximum (say 10s).
>
> I'm afraid most times it will keep doing inflate/deflate circularly.
>
>>
>> This isn't the only approach, but hopefully it conveys the idea of
>> gracefully backing off without really giving up.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>> Thanks
>>>
>>> --- linux-2.6.orig/drivers/virtio/virtio_balloon.c   Â2010-09-25
>>> 20:58:14.190000001 +0800
>>> +++ linux-2.6/drivers/virtio/virtio_balloon.c  2010-09-28
>>> 21:05:42.203333675 +0800
>>> @@ -25,6 +25,7 @@
>>> Â#include<linux/freezer.h>
>>> Â#include<linux/delay.h>
>>> Â#include<linux/slab.h>
>>> +#include<linux/oom.h>
>>>
>>> Âstruct virtio_balloon
>>> Â{
>>> @@ -97,8 +98,22 @@ static void tell_host(struct virtio_ball
>>> Â Â Â Âwait_for_completion(&vb->acked);
>>> Â}
>>>
>>> -static void fill_balloon(struct virtio_balloon *vb, size_t num)
>>> +static int cblimit(int times)
>>> Â{
>>> + Â Â Â static int t;
>>> +
>>> + Â Â Â if (t< Âtimes)
>>> + Â Â Â Â Â Â Â t++;
>>> + Â Â Â else
>>> + Â Â Â Â Â Â Â t = 0;
>>> +
>>> + Â Â Â return !t;
>>> +}
>>> +
>>> +static int fill_balloon(struct virtio_balloon *vb, size_t num)
>>> +{
>>> + Â Â Â int ret = 0;
>>> +
>>> Â Â Â Â/* We can only do one array worth at a time. */
>>> Â Â Â Ânum = min(num, ARRAY_SIZE(vb->pfns));
>>>
>>> @@ -106,10 +121,13 @@ static void fill_balloon(struct virtio_b
>>> Â Â Â Â Â Â Â Âstruct page *page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY
>>> |
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â__GFP_NOMEMALLOC | __GFP_NOWARN);
>>> Â Â Â Â Â Â Â Âif (!page) {
>>> - Â Â Â Â Â Â Â Â Â Â Â if (printk_ratelimit())
>>> + Â Â Â Â Â Â Â Â Â Â Â if (cblimit(5)) {
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âdev_printk(KERN_INFO,&vb->vdev->dev,
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â "Out of puff! Can't get %zu
>>> pages\n",
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â num);
>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ret = -ENOMEM;
>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto out;
>>> + Â Â Â Â Â Â Â Â Â Â Â }
>>> Â Â Â Â Â Â Â Â Â Â Â Â/* Sleep for at least 1/5 of a second before retry.
>>> */
>>> Â Â Â Â Â Â Â Â Â Â Â Âmsleep(200);
>>> Â Â Â Â Â Â Â Â Â Â Â Âbreak;
>>> @@ -120,11 +138,11 @@ static void fill_balloon(struct virtio_b
>>> Â Â Â Â Â Â Â Âlist_add(&page->lru,&vb->pages);
>>> Â Â Â Â}
>>>
>>> - Â Â Â /* Didn't get any? ÂOh well. */
>>> - Â Â Â if (vb->num_pfns == 0)
>>> - Â Â Â Â Â Â Â return;
>>> +out:
>>> + Â Â Â if (vb->num_pfns)
>>> + Â Â Â Â Â Â Â tell_host(vb, vb->inflate_vq);
>>>
>>> - Â Â Â tell_host(vb, vb->inflate_vq);
>>> + Â Â Â return ret;
>>> Â}
>>>
>>> Âstatic void release_pages_by_pfn(const u32 pfns[], unsigned int num)
>>> @@ -251,6 +269,14 @@ static void update_balloon_size(struct v
>>> Â Â Â Â Â Â Â Â Â Â Â Â&actual, sizeof(actual));
>>> Â}
>>>
>>> +static void update_balloon_target(struct virtio_balloon *vb)
>>> +{
>>> + Â Â Â __le32 num_pages = cpu_to_le32(vb->num_pages);
>>> + Â Â Â vb->vdev->config->set(vb->vdev,
>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â offsetof(struct virtio_balloon_config,
>>> num_pages),
>>> + Â Â Â Â Â Â Â Â Â Â Â &num_pages, sizeof(num_pages));
>>> +}
>>> +
>>> Âstatic int balloon(void *_vballoon)
>>> Â{
>>> Â Â Â Âstruct virtio_balloon *vb = _vballoon;
>>> @@ -267,9 +293,14 @@ static int balloon(void *_vballoon)
>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â || freezing(current));
>>> Â Â Â Â Â Â Â Âif (vb->need_stats_update)
>>> Â Â Â Â Â Â Â Â Â Â Â Âstats_handle_request(vb);
>>> - Â Â Â Â Â Â Â if (diff> Â0)
>>> - Â Â Â Â Â Â Â Â Â Â Â fill_balloon(vb, diff);
>>> - Â Â Â Â Â Â Â else if (diff< Â0)
>>> + Â Â Â Â Â Â Â if (diff> Â0) {
>>> + Â Â Â Â Â Â Â Â Â Â Â int oom;
>>> + Â Â Â Â Â Â Â Â Â Â Â oom_killer_disable();
>>> + Â Â Â Â Â Â Â Â Â Â Â oom = fill_balloon(vb, diff);
>>> + Â Â Â Â Â Â Â Â Â Â Â oom_killer_enable();
>>> + Â Â Â Â Â Â Â Â Â Â Â if (oom)
>>> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â update_balloon_target(vb);
>>> + Â Â Â Â Â Â Â } else if (diff< Â0)
>>> Â Â Â Â Â Â Â Â Â Â Â Âleak_balloon(vb, -diff);
>>> Â Â Â Â Â Â Â Âupdate_balloon_size(vb);
>>> Â Â Â Â}
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>> More majordomo info at Âhttp://vger.kernel.org/majordomo-info.html
>>>
>>
>>
>
>
>
> --
> Regards
> dave
>
--
Regards
dave
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
