Re: [Qemu-devel] [PATCH v1 1/4] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_VQ

[Wang, Wei W]

On Wednesday, January 17, 2018 7:41 PM, Juan Quintela wrote:
> Wei Wang <address@hidden> wrote:
> > +void skip_free_pages_from_dirty_bitmap(RAMBlock *block, ram_addr_t
> offset,
> > +                                       size_t len) {
> > +    long start = offset >> TARGET_PAGE_BITS,
> > +         nr = len >> TARGET_PAGE_BITS;
> > +
> > +    bitmap_clear(block->bmap, start, nr);
> 
> But what assures us that all the nr pages are dirty?

Actually the free page optimization is used for the bulk stage, where all the 
pages are treated as dirty. In patch 2, we have:

+            if (rs->free_page_support) {
+                bitmap_set(block->bmap, 1, pages);
+            } else {
+                bitmap_set(block->bmap, 0, pages);
+            }


> 
> > +    ram_state->migration_dirty_pages -= nr;
> 
> This should be
> ram_state->migration_dirty_pages -=
>      count_ones(block->bmap, start, nr);
> 
> For a count_ones function, no?

Not really. "nr" is the number of bits to clear from the bitmap, so 
"migration_dirty_pages -= nr" shows how many dirty bits left in the bitmap.

One cornercase I'm thinking about is when we do 
bitmap_clear(block->bmap, start, nr);
would it be possible that "start + nr" exceeds the bitmap size? that is, would 
it be possible that the free page block (e.g. 2MB) from the guest crosses the 
QEMU ram block boundary?


> Furthermore, we have one "optimization" and this only works for the second
> stages onward:
> 
> static inline
> unsigned long migration_bitmap_find_dirty(RAMState *rs, RAMBlock *rb,
>                                           unsigned long start) {
>     unsigned long size = rb->used_length >> TARGET_PAGE_BITS;
>     unsigned long *bitmap = rb->bmap;
>     unsigned long next;
> 
>     if (rs->ram_bulk_stage && start > 0) {
>         next = start + 1;
>     } else {
>         next = find_next_bit(bitmap, size, start);
>     }
> 
>     return next;
> }
> 
> So, for making this really work, we have more work to do.
> 
> Actually, what I think we should do was to _ask_ the guest which pages are
> used at the beggining, instead of just setting all pages as dirty, but that
> requires kernel changes and lot of search of corner cases.
> 

This series optimizes the bulk stage memory transfer. Do you mean you have 
another idea to optimize the second round and onward? How would you let the 
guest track pages that are written?

Best,
Wei