Re: [Qemu-devel] [PATCH v2] Add inst_dirty_pages_rate in 'info migrate'

[Daniel P. Berrange]

On Thu, Mar 09, 2017 at 06:05:38PM +0800, Chao Fan wrote:
> On Wed, Mar 08, 2017 at 01:45:59PM +0000, Daniel P. Berrange wrote:
> >On Wed, Mar 08, 2017 at 04:28:19PM +0800, Chao Fan wrote:
> >> Auto-converge aims to accelerate migration by slowing down the
> >> generation of dirty pages. But user doesn't know how to determine the
> >> throttle value, so, a new item "inst-dirty-pages-rate" in "info migrate"
> >> would be helpful for user's determination.
> Hi Daniel,
> 
> Thank you for your reply.
> >
> >The "info migrate" command already reports a "dirty-pages-rate" value.
> >
> >Maybe I'm mis-understanding what you're calculcating, this this proposal
> >looks the same, except reporting in bytes rather than page counts.
> >
> >QEMU in fact already records the bytes count internally too in the
> >'dirty_pages_bytes' parameter which is calculated from taking
> >'dirty_pages_size * TARGET_PAGE_SIZE'.
> >
> >So I wonder if we can just export the existing dirty-pages-bytes
> >value in info migrate, and avoid needing this new code here:
> >
> It's different, inst-dirty-pages-rate in this patch is greater than
> or equal to dirty-pages-bytes. Because in function
> cpu_physical_memory_sync_dirty_bitmap, file include/exec/ram_addr.h:
> 
> if (src[idx][offset]) {
>     unsigned long bits = atomic_xchg(&src[idx][offset], 0);
>     unsigned long new_dirty;
>     new_dirty = ~dest[k];
>     dest[k] |= bits;
>     new_dirty &= bits;
>     num_dirty += ctpopl(new_dirty);
> }
> 
> After these codes, only the pages not dirtied in bitmap(dest), but dirtied
> in dirty_memory[DIRTY_MEMORY_MIGRATION] will be calculated. For example:
> When ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION] = 0b00001111,
> and atomic_rcu_read(&migration_bitmap_rcu)->bmap = 0b00000011,
> the new_dirty will be 0b00001100, and this function will return 2 but not
> 4 which is expected.

Perhaps this is a reason to change how the current dirty-pages-rate and
dirty-pages-bytes values are calculated, rather than adding a new parameter
inst-dirty-pages-rate.

IMHO it will be pretty confusing to have two very similar parameters exposed
in the 'info migration' output, with subtley different calculations behind
them. You can only really understand the difference by looking at QEMU
code which is bad for apps consuming QEMU.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|