Re: [Qemu-devel] [QEMU 1/7] balloon: speed up inflating & deflating process

[Li, Liang Z]

> > On 13.06.2016 12:16, Liang Li wrote:
> > > The implementation of the current virtio-balloon is not very
> > > efficient, Bellow is test result of time spends on inflating the
> > > balloon to 3GB of a 4GB idle guest:
> > >
> > > a. allocating pages (6.5%, 103ms)
> > > b. sending PFNs to host (68.3%, 787ms) c. address translation (6.1%,
> > > 96ms) d. madvise (19%, 300ms)
> > >
> > > It takes about 1577ms for the whole inflating process to complete.
> > > The test shows that the bottle neck is the stage b and stage d.
> > >
> > > If using a bitmap to send the page info instead of the PFNs, we can
> > > reduce the overhead spends on stage b quite a lot. Furthermore, it's
> > > possible to do the address translation and do the madvise with a
> > > bulk of pages, instead of the current page per page way, so the
> > > overhead of stage c and stage d can also be reduced a lot.
> > >
> > > This patch is the QEMU side implementation which is intended to
> > > speed up the inflating & deflating process by adding a new feature
> > > to the virtio-balloon device. And now, inflating the balloon to 3GB
> > > of a 4GB idle guest only takes 210ms, it's about 8 times as fast as 
> > > before.
> > >
> > > TODO: optimize stage a by allocating/freeing a chunk of pages
> > > instead of a single page at a time.
> > >
> > > Signed-off-by: Liang Li <address@hidden>
> > > ---
> > >  hw/virtio/virtio-balloon.c                      | 159 
> > > ++++++++++++++++++++----
> > >  include/standard-headers/linux/virtio_balloon.h |   1 +
> > >  2 files changed, 139 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> > > index 8c15e09..8cf74c2 100644
> > > --- a/hw/virtio/virtio-balloon.c
> > > +++ b/hw/virtio/virtio-balloon.c
> > > @@ -47,6 +47,76 @@ static void balloon_page(void *addr, int deflate)
> > > #endif  }
> > >
> > > +static void do_balloon_bulk_pages(ram_addr_t base_pfn, int
> page_shift,
> > > +                                  unsigned long len, bool deflate) {
> > > +    ram_addr_t size, processed, chunk, base;
> > > +    void *addr;
> > > +    MemoryRegionSection section = {.mr = NULL};
> > > +
> > > +    size = (len << page_shift);
> > > +    base = (base_pfn << page_shift);
> > > +
> > > +    for (processed = 0; processed < size; processed += chunk) {
> > > +        chunk = size - processed;
> > > +        while (chunk >= TARGET_PAGE_SIZE) {
> > > +            section = memory_region_find(get_system_memory(),
> > > +                                         base + processed, chunk);
> > > +            if (!section.mr) {
> > > +                chunk = QEMU_ALIGN_DOWN(chunk / 2, TARGET_PAGE_SIZE);
> > > +            } else {
> > > +                break;
> > > +            }
> > > +        }
> > > +
> > > +        if (section.mr &&
> > > +            (int128_nz(section.size) && 
> > > memory_region_is_ram(section.mr)))
> {
> > > +            addr = section.offset_within_region +
> > > +                   memory_region_get_ram_ptr(section.mr);
> > > +            qemu_madvise(addr, chunk,
> > > +                         deflate ? QEMU_MADV_WILLNEED :
> > QEMU_MADV_DONTNEED);
> > > +        } else {
> > > +            fprintf(stderr, "can't find the chunk, skip\n");
> >
> > Please try to avoid new fprintf(stderr, ...) in the QEMU sources.
> > Use error_report(...) or in this case maybe rather
> > qemu_log_mask(LOG_GUEST_ERROR, ...) instead, and try to use a more
> > reasonable error message (e.g. that it is clear that the error
> > happened in the balloon code).
> >
> 
> Indeed, the error message is no good, will change in next version.
> 
> > > +            chunk = TARGET_PAGE_SIZE;
> > > +        }
> > > +    }
> > > +}
> > > +
> > > +static void balloon_bulk_pages(ram_addr_t base_pfn, unsigned long
> > *bitmap,
> > > +                               unsigned long len, int page_shift,
> > > +bool deflate) { #if defined(__linux__)
> >
> > Why do you need this #if here?
> >
> 
> Ooh,  it is wrong to add the '#if' here, will remove.
No, it is needed, just follow the code in balloon_page().
only Linux support the madvise().

Liang