Re: [Qemu-devel] [PATCH v1] migration: skip sending ram pages released by virtio-balloon driver.

[Jitendra Kolhe]

On 3/11/2016 12:55 PM, Li, Liang Z wrote:
> > On 3/10/2016 3:19 PM, Roman Kagan wrote:
> > > On Fri, Mar 04, 2016 at 02:32:47PM +0530, Jitendra Kolhe wrote:
> > > > Even though the pages which are returned to the host by
> > > > virtio-balloon driver are zero pages, the migration algorithm will
> > > > still end up scanning the entire page ram_find_and_save_block() ->
> > > > ram_save_page/ ram_save_compressed_page -> save_zero_page() ->
> > > > is_zero_range().  We also end-up sending some control information
> > > > over network for these page during migration. This adds to total migration
> > time.
> > > I wonder if it is the scanning for zeros or sending the whiteout which
> > > affects the total migration time more.  If it is the former (as I
> > > would
> > > expect) then a rather local change to is_zero_range() to make use of
> > > the mapping information before scanning would get you all the speedups
> > > without protocol changes, interfering with postcopy etc.
> > >
> > > Roman.
> >
> > Localizing the solution to zero page scan check is a good idea. I too agree that
> > most of the time is send in scanning for zero page in which case we should be
> > able to localize solution to is_zero_range().
> > However in case of ballooned out pages (which can be seen as a subset of
> > guest zero pages) we also spend a very small portion of total migration time
> > in sending the control information, which can be also avoided.
> >   From my tests for 16GB idle guest of which 12GB was ballooned out, the
> > zero page scan time for 12GB ballooned out pages was ~1789 ms and
> > save_page_header + qemu_put_byte(f, 0); for same 12GB ballooned out
> > pages was ~556 ms. Total migration time was ~8000 ms
>
> How did you do the tests? ~ 556ms seems too long for putting several bytes to 
> the buffer.
> It's likely the time you measured contains the portion to processes the other 
> 4GB guest memory pages.
>
> Liang

I modified save_zero_page() as below and updated timers only for 
ballooned out pages so is_zero_page() should return true(also 
qemu_balloon_bitmap_test() from my patchset returned 1)
With below instrumentation, I got t1 = ~1789ms and t2 = ~556ms. Also the 
total migration time noted (~8000ms) is for unmodified qemu source.
It seems to addup to final migration time with proposed patchset.

Here is the last entry for “another round” of test, this time its ~547ms
JK: block=7f5417a345e0, offset=3ffe42020, zero_page_scan_time=1218 us, 
save_page_header_time=184 us, total_save_zero_page_time=1453 us
cumulated vals: zero_page_scan_time=1723920378 us, 
save_page_header_time=547514618 us, total_save_zero_page_time=2371059239 us

static int save_zero_page(QEMUFile *f, RAMBlock *block, ram_addr_t offset,
                          uint8_t *p, uint64_t *bytes_transferred)
{
    int pages = -1;
    int64_t time1, time2, time3, time4;
    static int64_t t1 = 0, t2 = 0, t3 = 0;

    time1 = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
    if (is_zero_range(p, TARGET_PAGE_SIZE)) {
        time2 = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
        acct_info.dup_pages++;
        *bytes_transferred += save_page_header(f, block,
                                               offset | 
RAM_SAVE_FLAG_COMPRESS);
        qemu_put_byte(f, 0);
        time3 = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
        *bytes_transferred += 1;
        pages = 1;
    }
    time4 = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);

    if (qemu_balloon_bitmap_test(block, offset) == 1) {
        t1 += (time2-time1);
        t2 += (time3-time2);
        t3 += (time4-time1);
        fprintf(stderr, "block=%lx, offset=%lx, zero_page_scan_time=%ld 
us, save_page_header_time=%ld us, total_save_zero_page_time=%ld us\n"
                        "cumulated vals: zero_page_scan_time=%ld us, 
save_page_header_time=%ld us, total_save_zero_page_time=%ld us\n",
                         (unsigned long)block, (unsigned long)offset,
                         (time2-time1), (time3-time2), (time4-time1), 
t1, t2, t3);
    }
    return pages;
}

Thanks,
- Jitendra

> >       if (is_zero_range(p, TARGET_PAGE_SIZE)) {
> >           acct_info.dup_pages++;
> >           *bytes_transferred += save_page_header(f, block,
> >                                                  offset | 
> > RAM_SAVE_FLAG_COMPRESS);
> >           qemu_put_byte(f, 0);
> >           *bytes_transferred += 1;
> >           pages = 1;
> >       }
> > Would moving the solution to save_zero_page() be good enough?
> >
> > Thanks,
> > - Jitendra