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