Re: [PATCH 0/1] KVM: make get dirty log ioctl return the first dirty page's position

[Avi Kivity <avi@redhat.com>]

On 02/24/2010 10:43 AM, Takuya Yoshikawa wrote:
> Some weeks ago, OHMURA Kei revised the qemu-kvm's
> dirty bitmap scan by accessing the bitmap as an
> unsigned long array.
>
> By reviewing this work more, we notice that kernel
> side is doing a similar thing to check the bitmap is
> all clean or not.
>
> So I made a patch which makes the get dirty log ioctl
> return the first dirty page position found by this check.
>
> Though my test is not enough to show the effect of this
> patch, the fact that this patch has no bad effect to both
> performance and implementation logic and we can skip some
> extra memory accesses and comparisons in userspace seems
> to be suggesting this patch is promising, right?
>    

Well, if 10% of the pages are dirty, the new ioctl will statistically 
return something within the first 20% of the slot, so we can skip 10% 
and have to do the next 90%.  Given that we already walked the bitmap 
once in the kernel and the saving is only in userspace, the average 
saving in bitmap-walking is only 5%.

The patch's greatest benefit is if all pages are clean (100% saved) or 
if just one page is dirty (50% saved) but that will be very rare.  So I 
think the return-on-churn here is too low.

>
> Below is a simple test result that compares the newly
> obtained ioctl's return value to the slot len.
> ---
> Host: AMD Phenom II
> Guest memory size: 512M
> Explanation: during migration, dumped the ioctl's return
>    value(r) in kvm_get_map(), with no specific workload.
>
> static int kvm_get_map(kvm_context_t kvm, int ioctl_num, int slot, void *buf)
> {
> ...
>      r = kvm_vm_ioctl(kvm_state, ioctl_num,&log);
>      /* test: compare the return value and the slot's length */
>      fprintf(stderr, "kvm_get_map(slot%2d): r=%5d, slot.len=%10lu(%lu)\n",
>              slot, r, slots[slot].len,
>              slots[slot].len / (4*1024) / (8*sizeof(unsigned long)));
> ...
> }
>
> Result:
> ...
> kvm_get_map(slot 0): r=    3, slot.len=    655360(2)
> kvm_get_map(slot 1): r= 2044, slot.len= 535822336(2044)
> kvm_get_map(slot 2): r=    1, slot.len=    131072(0)
> kvm_get_map(slot 3): r=    1, slot.len=    131072(0)
> kvm_get_map(slot 4): r=    1, slot.len=    131072(0)
> kvm_get_map(slot 5): r=   64, slot.len=  16777216(64)
> kvm_get_map(slot 6): r=    1, slot.len=     32768(0)
> kvm_get_map(slot 7): r=    1, slot.len=     32768(0)
> kvm_get_map(slot 0): r=    3, slot.len=    655360(2)
> kvm_get_map(slot 1): r= 2044, slot.len= 535822336(2044)
> kvm_get_map(slot 2): r=    1, slot.len=    131072(0)
> kvm_get_map(slot 3): r=    1, slot.len=    131072(0)
> kvm_get_map(slot 4): r=    1, slot.len=    131072(0)
> kvm_get_map(slot 5): r=   64, slot.len=  16777216(64)
> kvm_get_map(slot 6): r=    1, slot.len=     32768(0)
> kvm_get_map(slot 7): r=    1, slot.len=     32768(0)
> kvm_get_map(slot 0): r=    3, slot.len=    655360(2)
> kvm_get_map(slot 1): r= 2044, slot.len= 535822336(2044)
> kvm_get_map(slot 2): r=    1, slot.len=    131072(0)
> kvm_get_map(slot 3): r=    1, slot.len=    131072(0)
> kvm_get_map(slot 4): r=    1, slot.len=    131072(0)
> kvm_get_map(slot 5): r=   64, slot.len=  16777216(64)
> kvm_get_map(slot 6): r=    1, slot.len=     32768(0)
> kvm_get_map(slot 7): r=    1, slot.len=     32768(0)
> kvm_get_map(slot 0): r=    3, slot.len=    655360(2)
> kvm_get_map(slot 1): r= 2044, slot.len= 535822336(2044)
> kvm_get_map(slot 2): r=    1, slot.len=    131072(0)
> kvm_get_map(slot 3): r=    1, slot.len=    131072(0)
> kvm_get_map(slot 4): r=    1, slot.len=    131072(0)
> kvm_get_map(slot 5): r=   64, slot.len=  16777216(64)
> kvm_get_map(slot 6): r=    1, slot.len=     32768(0)
> kvm_get_map(slot 7): r=    1, slot.len=     32768(0)
> ...
>    

Seems to confirm - not much can be skipped.



-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.