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

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

[Takuya Yoshikawa]

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?


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)
...

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

[Takuya Yoshikawa]

This patch changes the get dirty log ioctl to inform the
already scanned clean bitmap area to userspace.

Without this patch, the information obtained by the scan in
kernel is just ignored. To fix this, we use the return value
of ioctl call as a hint of from where we should scan.

Be careful that this value have to be used as an index of an
unsigned long array.

No side effect: qemu currently just checks the return value is
negative(-1) or not, so you can use it without any change.
Needless to say, you'd better to use an improved qemu to get
more performance.


Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
 Documentation/kvm/api.txt |    8 +++++---
 arch/ia64/kvm/kvm-ia64.c  |    5 ++---
 arch/powerpc/kvm/book3s.c |    5 ++---
 arch/s390/kvm/kvm-s390.c  |    2 +-
 arch/x86/kvm/x86.c        |   15 ++++++++++-----
 virt/kvm/kvm_main.c       |   18 +++++++++---------
 6 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
index d170cb4..7ebfd7c 100644
--- a/Documentation/kvm/api.txt
+++ b/Documentation/kvm/api.txt
@@ -201,7 +201,7 @@ Capability: basic
 Architectures: x86
 Type: vm ioctl
 Parameters: struct kvm_dirty_log (in/out)
-Returns: 0 on success, -1 on error
+Returns: first dirty page position on success, -1 on error
 
 /* for KVM_GET_DIRTY_LOG */
 struct kvm_dirty_log {
@@ -215,8 +215,10 @@ struct kvm_dirty_log {
 
 Given a memory slot, return a bitmap containing any pages dirtied
 since the last call to this ioctl.  Bit 0 is the first page in the
-memory slot.  Ensure the entire structure is cleared to avoid padding
-issues.
+memory slot.  The return value ensures that the contents of the
+bitmap whose indexes, when you access as unsigned long array, are
+less than this are clean.  Ensure the entire structure is cleared
+to avoid padding issues.
 
 4.8 KVM_SET_MEMORY_ALIAS
 
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 26e0e08..ae78301 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -1828,7 +1828,7 @@ out:
 }
 
 int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
-		struct kvm_dirty_log *log)
+			       struct kvm_dirty_log *log)
 {
 	int r;
 	int n;
@@ -1843,7 +1843,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
 		goto out;
 
 	r = kvm_get_dirty_log(kvm, log, &is_dirty);
-	if (r)
+	if (r < 0)
 		goto out;
 
 	/* If nothing is dirty, don't bother messing with page tables. */
@@ -1853,7 +1853,6 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
 		n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
 		memset(memslot->dirty_bitmap, 0, n);
 	}
-	r = 0;
 out:
 	mutex_unlock(&kvm->slots_lock);
 	spin_unlock(&kvm->arch.dirty_log_lock);
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index 794c94b..6ffd6f2 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -1065,7 +1065,7 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
  * Get (and clear) the dirty memory log for a memory slot.
  */
 int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
-				      struct kvm_dirty_log *log)
+			       struct kvm_dirty_log *log)
 {
 	struct kvm_memory_slot *memslot;
 	struct kvm_vcpu *vcpu;
@@ -1076,7 +1076,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
 	mutex_lock(&kvm->slots_lock);
 
 	r = kvm_get_dirty_log(kvm, log, &is_dirty);
-	if (r)
+	if (r < 0)
 		goto out;
 
 	/* If nothing is dirty, don't bother messing with page tables. */
@@ -1093,7 +1093,6 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
 		memset(memslot->dirty_bitmap, 0, n);
 	}
 
-	r = 0;
 out:
 	mutex_unlock(&kvm->slots_lock);
 	return r;
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 8f09959..640ce77 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -136,7 +136,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
 			       struct kvm_dirty_log *log)
 {
-	return 0;
+	return -ENOSYS;
 }
 
 long kvm_arch_vm_ioctl(struct file *filp,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e25a522..8068ee9 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2694,13 +2694,14 @@ static int kvm_vm_ioctl_reinject(struct kvm *kvm,
 
 /*
  * Get (and clear) the dirty memory log for a memory slot.
+ * Return the first dirty page position on success.
  */
 int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
-				      struct kvm_dirty_log *log)
+			       struct kvm_dirty_log *log)
 {
 	int r, n, i;
 	struct kvm_memory_slot *memslot;
-	unsigned long is_dirty = 0;
+	bool is_dirty;
 	unsigned long *dirty_bitmap = NULL;
 
 	mutex_lock(&kvm->slots_lock);
@@ -2722,8 +2723,12 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
 		goto out;
 	memset(dirty_bitmap, 0, n);
 
-	for (i = 0; !is_dirty && i < n/sizeof(long); i++)
-		is_dirty = memslot->dirty_bitmap[i];
+	for (i = 0, is_dirty = false; i < n/sizeof(long); i++) {
+		if (memslot->dirty_bitmap[i]) {
+			is_dirty = true;
+			break;
+		}
+	}
 
 	/* If nothing is dirty, don't bother messing with page tables. */
 	if (is_dirty) {
@@ -2747,7 +2752,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
 		kfree(old_slots);
 	}
 
-	r = 0;
+	r = i;
 	if (copy_to_user(log->dirty_bitmap, dirty_bitmap, n))
 		r = -EFAULT;
 out_free:
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 548f925..4b715c9 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -769,7 +769,6 @@ int kvm_get_dirty_log(struct kvm *kvm,
 	struct kvm_memory_slot *memslot;
 	int r, i;
 	int n;
-	unsigned long any = 0;
 
 	r = -EINVAL;
 	if (log->slot >= KVM_MEMORY_SLOTS)
@@ -782,17 +781,18 @@ int kvm_get_dirty_log(struct kvm *kvm,
 
 	n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
 
-	for (i = 0; !any && i < n/sizeof(long); ++i)
-		any = memslot->dirty_bitmap[i];
+	for (i = 0; i < n/sizeof(long); ++i) {
+		if (memslot->dirty_bitmap[i]) {
+			*is_dirty = 1;
+			break;
+		}
+	}
 
 	r = -EFAULT;
 	if (copy_to_user(log->dirty_bitmap, memslot->dirty_bitmap, n))
 		goto out;
 
-	if (any)
-		*is_dirty = 1;
-
-	r = 0;
+	r = i;
 out:
 	return r;
 }
@@ -1592,7 +1592,7 @@ static long kvm_vm_ioctl(struct file *filp,
 		if (copy_from_user(&log, argp, sizeof log))
 			goto out;
 		r = kvm_vm_ioctl_get_dirty_log(kvm, &log);
-		if (r)
+		if (r < 0)
 			goto out;
 		break;
 	}
@@ -1693,7 +1693,7 @@ static long kvm_vm_compat_ioctl(struct file *filp,
 		log.dirty_bitmap = compat_ptr(compat_log.dirty_bitmap);
 
 		r = kvm_vm_ioctl_get_dirty_log(kvm, &log);
-		if (r)
+		if (r < 0)
 			goto out;
 		break;
 	}
-- 
1.6.3.3

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

[Avi Kivity]

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.

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

[Avi Kivity]

On 02/24/2010 10:55 AM, Avi Kivity wrote:
> 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.

btw, one idea I had was to allocate the bitmap in userspace and let the 
kernel set bits directly.  This reduces the amount of unswappable memory 
the kernel allocates and reduces copying.

A problem with this is that userspace cannot just clear the bits, since 
the kernel has to write-protect the pages again.  I don't know how we 
can do this without copying the bitmap.

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

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

[Takuya Yoshikawa]

Avi Kivity wrote:
>>
>> 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.

Actually I do not know well about migration's use case in the actual
service system.

In our group, there is a Fault Tolerance project which uses migration's
functions for synchronization frequently. So I thought this may be helpful
to such cases, not confirmed yet.


Another issue I am thinking is the x86's bitmap allocation. Doing vmalloc()
every time is not nice, though I know it's needed.

> 
> btw, one idea I had was to allocate the bitmap in userspace and let the 
> kernel set bits directly.  This reduces the amount of unswappable memory 
> the kernel allocates and reduces copying.
> 
> A problem with this is that userspace cannot just clear the bits, since 
> the kernel has to write-protect the pages again.  I don't know how we 
> can do this without copying the bitmap.
> 

Yes, seems difficult.

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

[Avi Kivity]

On 02/24/2010 11:20 AM, Takuya Yoshikawa wrote:
>
> Another issue I am thinking is the x86's bitmap allocation. Doing 
> vmalloc()
> every time is not nice, though I know it's needed.
>

Right, that's why I want to move the allocation to userspace.

>>
>> btw, one idea I had was to allocate the bitmap in userspace and let 
>> the kernel set bits directly.  This reduces the amount of unswappable 
>> memory the kernel allocates and reduces copying.
>>
>> A problem with this is that userspace cannot just clear the bits, 
>> since the kernel has to write-protect the pages again.  I don't know 
>> how we can do this without copying the bitmap.
>>
>
> Yes, seems difficult.

Here's an idea: double buffering.  Instead of copying the dirty log to 
userspace, we switch the dirty log bitmap to another location 
atomically.  Then userspace can read the old bitmap.

Note the speed savings won't be huge, since the copying brings the 
bitmap into cache, so the second pass is fairly fast.  But if we combine 
it with dropping the qemu byte-based dirty log, then we eliminate the 
second pass as well.

-- 
error compiling committee.c: too many arguments to function

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

[Takuya Yoshikawa]

Avi Kivity wrote:
>>
>> 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.
> 


 >> kvm_get_map(slot 1): r= 2044, slot.len= 535822336(2044)
Am I wrong? in this case, the return value is suggesting
we can skip every 2044(the value in the bracket) bitmap check, right?



> 
> 

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

[Avi Kivity]

On 02/24/2010 11:45 AM, Takuya Yoshikawa wrote:
> Avi Kivity wrote:
>>>
>>> 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.
>>
>
>
> >> kvm_get_map(slot 1): r= 2044, slot.len= 535822336(2044)
> Am I wrong? in this case, the return value is suggesting
> we can skip every 2044(the value in the bracket) bitmap check, right?

Er, I was wrong, I thought the value in the brackets had the same units 
as slots.len.  But it's in units of ulongs.

In this case, the entire bitmap can be skipped, so the saving would be 
significant.  But this is for an idle load, where we'll converge quickly 
anyway.  For a busy server, things will look different.

So I recommend you look at the double buffer approach.  This can 
eliminate the kernel scan, the copying, and the userspace scan entirely, 
as well as remove the vmalloc().

-- 
error compiling committee.c: too many arguments to function

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

[Takuya Yoshikawa]

Avi Kivity wrote:
>>>
>>> Seems to confirm - not much can be skipped.
>>>
>>
>>
>> >> kvm_get_map(slot 1): r= 2044, slot.len= 535822336(2044)
>> Am I wrong? in this case, the return value is suggesting
>> we can skip every 2044(the value in the bracket) bitmap check, right?
> 
> Er, I was wrong, I thought the value in the brackets had the same units 
> as slots.len.  But it's in units of ulongs.
> 
> In this case, the entire bitmap can be skipped, so the saving would be 
> significant.  But this is for an idle load, where we'll converge quickly 
> anyway.  For a busy server, things will look different.
> 
> So I recommend you look at the double buffer approach.  This can 
> eliminate the kernel scan, the copying, and the userspace scan entirely, 
> as well as remove the vmalloc().
> 

OK, I want to try! Thanks for advice.

  Takuya