Re: [Qemu-devel] [PATCH V6 07/10] migration: add bitmap for copied page

[Alexey Perevalov]

On 05/25/2017 02:30 AM, Peter Xu wrote:
> On Wed, May 24, 2017 at 03:16:23PM +0300, Alexey Perevalov wrote:
> > On 05/24/2017 03:01 PM, Peter Xu wrote:
> > > On Wed, May 24, 2017 at 10:56:37AM +0300, Alexey wrote:
> > > > On Wed, May 24, 2017 at 02:57:36PM +0800, Peter Xu wrote:
> > > > > On Tue, May 23, 2017 at 02:31:08PM +0300, Alexey Perevalov wrote:
> > > > > > This patch adds ability to track down already copied
> > > > > > pages, it's necessary for calculation vCPU block time in
> > > > > > postcopy migration feature and maybe for restore after
> > > > > > postcopy migration failure.
> > > > > >
> > > > > > Functions which work with RAMBlock are placed into ram.c,
> > > > > > due to TARGET_WORDS_BIGENDIAN is poisoned int postcopy-ram.c -
> > > > > > hardware independed code.
> > > > > >
> > > > > > Signed-off-by: Alexey Perevalov <address@hidden>
> > > > > > ---
> > > > > >   include/migration/migration.h | 16 +++++++++++
> > > > > >   migration/postcopy-ram.c      | 22 ++++++++++++---
> > > > > >   migration/ram.c               | 63 +++++++++++++++++++++++++++++++++++++++++++
> > > > > >   3 files changed, 97 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/include/migration/migration.h b/include/migration/migration.h
> > > > > > index 449cb07..4e05c83 100644
> > > > > > --- a/include/migration/migration.h
> > > > > > +++ b/include/migration/migration.h
> > > > > > @@ -101,6 +101,20 @@ struct MigrationIncomingState {
> > > > > >       LoadStateEntry_Head loadvm_handlers;
> > > > > >       /*
> > > > > > +     * bitmap indicates whether page copied,
> > > > > > +     * based on ramblock offset
> > > > > > +     * now it is using only for blocktime calculation in
> > > > > > +     * postcopy migration, so livetime of this entry:
> > > > > > +     * since user requested blocktime calculation,
> > > > > > +     * till the end of postcopy migration
> > > > > > +     * as an example it could represend following memory map
> > > > > > +     * ___________________________________
> > > > > > +     * |4k pages | hugepages | 4k pages
> > > > > Can we really do this?
> > > > > The problem is AFAIU migration stream is sending pages only in target
> > > > > page size, even for huge pages... so even for huge pages we should
> > > > > still need per TARGET_PAGE_SIZE bitmap?
> > > > sending maybe, but copying to userfault fd is doing in hugepage size
> > > Yes you are right. :)
> > >
> > > > in case of hugetlbfs memory backend.
> > > > > (Please refer to ram_state_init() on init of RAMBlock.bmap)
> > > > I thought to use bitmap per ramblock, but I decided to not do it,
> > > > because of following reasons:
> > > >         1. There is only 4k ramblocks, for such ramblock it's not
> > > >         optimal
> > > Could you explain what do you mean by "there is only 4k ramblocks"?
> > sorry, could be ramblocks with 4k size,
> > as example, your's qemu hmp info ramblock shows
> >   Block Name    PSize              Offset Used              Total
> >              /objects/mem    2 MiB  0x0000000000000000 0x0000000020000000
> > 0x0000000020000000
> >                  vga.vram    4 KiB  0x0000000020060000 0x0000000001000000
> > 0x0000000001000000
> >      /address@hidden/acpi/tables    4 KiB  0x0000000021130000 0x0000000000020000
> > 0x0000000000200000
> >                   pc.bios    4 KiB  0x0000000020000000 0x0000000000040000
> > 0x0000000000040000
> >    0000:00:03.0/e1000.rom    4 KiB  0x0000000021070000 0x0000000000040000
> > 0x0000000000040000
> >    0000:00:04.0/e1000.rom    4 KiB  0x00000000210b0000 0x0000000000040000
> > 0x0000000000040000
> >    0000:00:05.0/e1000.rom    4 KiB  0x00000000210f0000 0x0000000000040000
> > 0x0000000000040000
> >                    pc.rom    4 KiB  0x0000000020040000 0x0000000000020000
> > 0x0000000000020000
> >      0000:00:02.0/vga.rom    4 KiB  0x0000000021060000 0x0000000000010000
> > 0x0000000000010000
> >     /address@hidden/table-loader    4 KiB  0x0000000021330000 0x0000000000001000
> > 0x0000000000001000
> >        /address@hidden/acpi/rsdp    4 KiB  0x0000000021331000 0x0000000000001000
> > 0x0000000000001000
> >
> > /address@hidden/table-loader and /address@hidden/acpi/rsdp has only one 4K page.
> I see. Thanks for explaining this.
>
> A 4k sized ramblock means the bitmap would be only one unsigned long
> width (via bitmap_new(1)). I still don't see why it's not good... :-)
Ok, I'll take into account your comment, and I'll place copied

bitmap into RAMBlock.


--
Best regards,
Alexey Perevalov