[PATCH] kvm_vm_ioctl_get_dirty_log doesn't need any, probably doesn't want any

[PATCH] kvm_vm_ioctl_get_dirty_log doesn't need any, probably doesn't want any

[Rusty Russell]

I'm not sure what "any" was for: some optimization which got trimmed,
I imagine.  But neither "any" nor "i" are used later in the function,
so I've excised it.

Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>

diff -r a17d7fe2b240 drivers/kvm/kvm_main.c
--- a/drivers/kvm/kvm_main.c	Mon Jul 30 15:10:22 2007 +1000
+++ b/drivers/kvm/kvm_main.c	Mon Jul 30 15:20:33 2007 +1000
@@ -830,9 +830,7 @@ static int kvm_vm_ioctl_get_dirty_log(st
 				      struct kvm_dirty_log *log)
 {
 	struct kvm_memory_slot *memslot;
-	int r, i;
-	int n;
-	unsigned long any = 0;
+	int r, n;
 
 	spin_lock(&kvm->lock);
 
@@ -851,11 +849,7 @@ static int kvm_vm_ioctl_get_dirty_log(st
 	if (!memslot->dirty_bitmap)
 		goto out;
 
-	n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
-
-	for (i = 0; !any && i < n/sizeof(long); ++i)
-		any = memslot->dirty_bitmap[i];
-
+	n = BITS_TO_LONGS(memslot->npages) * sizeof(long);
 	r = -EFAULT;
 	if (copy_to_user(log->dirty_bitmap, memslot->dirty_bitmap, n))
 		goto out;



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH] kvm_vm_ioctl_get_dirty_log doesn't need any, probably doesn't want any

[Avi Kivity]

Rusty Russell wrote:
> I'm not sure what "any" was for: some optimization which got trimmed,
> I imagine.  But neither "any" nor "i" are used later in the function,
> so I've excised it.
>
>   

Looks like it was used to guard kvm_mmu_slot_remove_write_access(); 
optimizing the case where the guest just leaves the screen alone (which 
it usually does, especially in benchmarks).

I'd rather reinstate that optimization.  See 
66d8a4e4d4bd470216028daabb9d887b73259c96 where the damage was done.

> Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>
>
> diff -r a17d7fe2b240 drivers/kvm/kvm_main.c
> --- a/drivers/kvm/kvm_main.c	Mon Jul 30 15:10:22 2007 +1000
> +++ b/drivers/kvm/kvm_main.c	Mon Jul 30 15:20:33 2007 +1000
> @@ -830,9 +830,7 @@ static int kvm_vm_ioctl_get_dirty_log(st
>  				      struct kvm_dirty_log *log)
>  {
>  	struct kvm_memory_slot *memslot;
> -	int r, i;
> -	int n;
> -	unsigned long any = 0;
> +	int r, n;
>  
>  	spin_lock(&kvm->lock);
>  
> @@ -851,11 +849,7 @@ static int kvm_vm_ioctl_get_dirty_log(st
>  	if (!memslot->dirty_bitmap)
>  		goto out;
>  
> -	n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
> -
> -	for (i = 0; !any && i < n/sizeof(long); ++i)
> -		any = memslot->dirty_bitmap[i];
> -
> +	n = BITS_TO_LONGS(memslot->npages) * sizeof(long);
>  	r = -EFAULT;
>  	if (copy_to_user(log->dirty_bitmap, memslot->dirty_bitmap, n))
>  		goto out;
>
>
>   


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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[PATCH] kvm_vm_ioctl_get_dirty_log restore "nothing dirty" optimization

[Rusty Russell]

kvm_vm_ioctl_get_dirty_log scans bitmap to see it it's all zero, but
doesn't use that information.

Avi says:
	Looks like it was used to guard	kvm_mmu_slot_remove_write_access();
	optimizing the case where the guest just leaves the screen alone (which
	it usually does, especially in benchmarks).

	I'd rather reinstate that optimization.  See
	66d8a4e4d4bd470216028daabb9d887b73259c96 where the damage was done.

It's pretty simple: if the bitmap is all zero, we don't need to do anything to
clean it.

Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>

diff -r 66566cf6b576 drivers/kvm/kvm_main.c
--- a/drivers/kvm/kvm_main.c	Fri Jul 27 16:27:11 2007 +1000
+++ b/drivers/kvm/kvm_main.c	Mon Jul 30 23:15:47 2007 +1000
@@ -838,11 +838,13 @@ static int kvm_vm_ioctl_get_dirty_log(st
 	if (copy_to_user(log->dirty_bitmap, memslot->dirty_bitmap, n))
 		goto out;
 
-	spin_lock(&kvm->lock);
-	kvm_mmu_slot_remove_write_access(kvm, log->slot);
-	kvm_flush_remote_tlbs(kvm);
-	memset(memslot->dirty_bitmap, 0, n);
-	spin_unlock(&kvm->lock);
+	if (any) {
+		spin_lock(&kvm->lock);
+		kvm_mmu_slot_remove_write_access(kvm, log->slot);
+		kvm_flush_remote_tlbs(kvm);
+		memset(memslot->dirty_bitmap, 0, n);
+		spin_unlock(&kvm->lock);
+	}
 
 	r = 0;
 



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH] kvm_vm_ioctl_get_dirty_log restore "nothing dirty" optimization

[Avi Kivity]

Rusty Russell wrote:
> kvm_vm_ioctl_get_dirty_log scans bitmap to see it it's all zero, but
> doesn't use that information.
>
> Avi says:
> 	Looks like it was used to guard	kvm_mmu_slot_remove_write_access();
> 	optimizing the case where the guest just leaves the screen alone (which
> 	it usually does, especially in benchmarks).
>
> 	I'd rather reinstate that optimization.  See
> 	66d8a4e4d4bd470216028daabb9d887b73259c96 where the damage was done.
>
> It's pretty simple: if the bitmap is all zero, we don't need to do anything to
> clean it.
>
> Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>
>
> diff -r 66566cf6b576 drivers/kvm/kvm_main.c
> --- a/drivers/kvm/kvm_main.c	Fri Jul 27 16:27:11 2007 +1000
> +++ b/drivers/kvm/kvm_main.c	Mon Jul 30 23:15:47 2007 +1000
> @@ -838,11 +838,13 @@ static int kvm_vm_ioctl_get_dirty_log(st
>  	if (copy_to_user(log->dirty_bitmap, memslot->dirty_bitmap, n))
>  		goto out;
>  
> -	spin_lock(&kvm->lock);
> -	kvm_mmu_slot_remove_write_access(kvm, log->slot);
> -	kvm_flush_remote_tlbs(kvm);
> -	memset(memslot->dirty_bitmap, 0, n);
> -	spin_unlock(&kvm->lock);
> +	if (any) {
> +		spin_lock(&kvm->lock);
> +		kvm_mmu_slot_remove_write_access(kvm, log->slot);
> +		kvm_flush_remote_tlbs(kvm);
> +		memset(memslot->dirty_bitmap, 0, n);
> +		spin_unlock(&kvm->lock);
> +	}
>  

spin_lock()?  my, that's so last week.  kvm->lock is a mutex.


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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[PATCH] kvm_vm_ioctl_get_dirty_log restore "nothing dirty" optimization

[Rusty Russell]

kvm_vm_ioctl_get_dirty_log scans bitmap to see it it's all zero, but
doesn't use that information.

Avi says:
	Looks like it was used to guard	kvm_mmu_slot_remove_write_access();
	optimizing the case where the guest just leaves the screen alone (which
	it usually does, especially in benchmarks).

	I'd rather reinstate that optimization.  See
	66d8a4e4d4bd470216028daabb9d887b73259c96 where the damage was done.

It's pretty simple: if the bitmap is all zero, we don't need to do anything to
clean it.

Signed-off-by: Rusty Russell <rusty-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>

diff -r 37f0dd1eaa0d drivers/kvm/kvm_main.c
--- a/drivers/kvm/kvm_main.c	Tue Jul 31 19:30:22 2007 +1000
+++ b/drivers/kvm/kvm_main.c	Tue Jul 31 19:34:26 2007 +1000
@@ -803,11 +803,14 @@ static int kvm_vm_ioctl_get_dirty_log(st
 	if (copy_to_user(log->dirty_bitmap, memslot->dirty_bitmap, n))
 		goto out;
 
-	mutex_lock(&kvm->lock);
-	kvm_mmu_slot_remove_write_access(kvm, log->slot);
-	kvm_flush_remote_tlbs(kvm);
-	memset(memslot->dirty_bitmap, 0, n);
-	mutex_unlock(&kvm->lock);
+	/* If nothing is dirty, don't bother messing with page tables. */
+	if (any) {
+		mutex_lock(&kvm->lock);
+		kvm_mmu_slot_remove_write_access(kvm, log->slot);
+		kvm_flush_remote_tlbs(kvm);
+		memset(memslot->dirty_bitmap, 0, n);
+		mutex_unlock(&kvm->lock);
+	}
 
 	r = 0;
 



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: [PATCH] kvm_vm_ioctl_get_dirty_log restore "nothing dirty" optimization

[Avi Kivity]

Rusty Russell wrote:
> kvm_vm_ioctl_get_dirty_log scans bitmap to see it it's all zero, but
> doesn't use that information.
>
> Avi says:
> 	Looks like it was used to guard	kvm_mmu_slot_remove_write_access();
> 	optimizing the case where the guest just leaves the screen alone (which
> 	it usually does, especially in benchmarks).
>
> 	I'd rather reinstate that optimization.  See
> 	66d8a4e4d4bd470216028daabb9d887b73259c96 where the damage was done.
>
> It's pretty simple: if the bitmap is all zero, we don't need to do anything to
> clean it.
>
>   

Applied; thanks.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/