From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 0/1] KVM: make get dirty log ioctl return the first dirty page's position Date: Wed, 24 Feb 2010 10:55:33 +0200 Message-ID: <4B84E985.8000508@redhat.com> References: <20100224174303.881da4f4.yoshikawa.takuya@oss.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: mtosatti@redhat.com, kvm@vger.kernel.org To: Takuya Yoshikawa Return-path: Received: from mx1.redhat.com ([209.132.183.28]:50741 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755706Ab0BXIzl (ORCPT ); Wed, 24 Feb 2010 03:55:41 -0500 In-Reply-To: <20100224174303.881da4f4.yoshikawa.takuya@oss.ntt.co.jp> Sender: kvm-owner@vger.kernel.org List-ID: 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.