diff mbox series

KVM: selftests: use unified time type for comparison

Message ID 20230731022405.854884-1-maobibo@loongson.cn
State New
Headers
Series KVM: selftests: use unified time type for comparison |

Commit Message

maobibo July 31, 2023, 2:24 a.m. UTC 
  With test case kvm_page_table_test, start time is acquired with
time type CLOCK_MONOTONIC_RAW, however end time in function timespec_elapsed
is acquired with time type CLOCK_MONOTONIC. This will cause
inaccurate elapsed time calculation on some platform such as LoongArch.

This patch modified test case kvm_page_table_test, and uses unified
time type CLOCK_MONOTONIC for start time.

Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 tools/testing/selftests/kvm/kvm_page_table_test.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Sean Christopherson Aug. 2, 2023, 7:30 p.m. UTC | #1 
+Oliver to get input on something beyond non-x86, and because I hate anything
clock related :-)

On Mon, Jul 31, 2023, Bibo Mao wrote:
> With test case kvm_page_table_test, start time is acquired with
> time type CLOCK_MONOTONIC_RAW, however end time in function timespec_elapsed
> is acquired with time type CLOCK_MONOTONIC. This will cause
> inaccurate elapsed time calculation on some platform such as LoongArch.
> 
> This patch modified test case kvm_page_table_test, and uses unified
> time type CLOCK_MONOTONIC for start time.

AFAICT, there's zero reason to use CLOCK_MONOTONIC_RAW instead of CLOCK_MONOTONIC.
If there are no objections, I'll take this through kvm-x86/selftests for 6.6.

> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  tools/testing/selftests/kvm/kvm_page_table_test.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/kvm_page_table_test.c b/tools/testing/selftests/kvm/kvm_page_table_test.c
> index b3b00be1ef82..69f26d80c821 100644
> --- a/tools/testing/selftests/kvm/kvm_page_table_test.c
> +++ b/tools/testing/selftests/kvm/kvm_page_table_test.c
> @@ -200,7 +200,7 @@ static void *vcpu_worker(void *data)
>  		if (READ_ONCE(host_quit))
>  			return NULL;
>  
> -		clock_gettime(CLOCK_MONOTONIC_RAW, &start);
> +		clock_gettime(CLOCK_MONOTONIC, &start);
>  		ret = _vcpu_run(vcpu);
>  		ts_diff = timespec_elapsed(start);
>  
> @@ -367,7 +367,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  	/* Test the stage of KVM creating mappings */
>  	*current_stage = KVM_CREATE_MAPPINGS;
>  
> -	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
> +	clock_gettime(CLOCK_MONOTONIC, &start);
>  	vcpus_complete_new_stage(*current_stage);
>  	ts_diff = timespec_elapsed(start);
>  
> @@ -380,7 +380,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  
>  	*current_stage = KVM_UPDATE_MAPPINGS;
>  
> -	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
> +	clock_gettime(CLOCK_MONOTONIC, &start);
>  	vcpus_complete_new_stage(*current_stage);
>  	ts_diff = timespec_elapsed(start);
>  
> @@ -392,7 +392,7 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>  
>  	*current_stage = KVM_ADJUST_MAPPINGS;
>  
> -	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
> +	clock_gettime(CLOCK_MONOTONIC, &start);
>  	vcpus_complete_new_stage(*current_stage);
>  	ts_diff = timespec_elapsed(start);
>  
> -- 
> 2.27.0
>
Oliver Upton Aug. 2, 2023, 7:52 p.m. UTC | #2 
Howdy,

On Wed, Aug 02, 2023 at 12:30:43PM -0700, Sean Christopherson wrote:
> On Mon, Jul 31, 2023, Bibo Mao wrote:
> > With test case kvm_page_table_test, start time is acquired with
> > time type CLOCK_MONOTONIC_RAW, however end time in function timespec_elapsed
> > is acquired with time type CLOCK_MONOTONIC. This will cause
> > inaccurate elapsed time calculation on some platform such as LoongArch.

Well, there's the fundamental issue of mixing to timebases, so this
really isn't platform specific. One is subject to NTP frequency
adjustments and the other is not.

> > This patch modified test case kvm_page_table_test, and uses unified
> > time type CLOCK_MONOTONIC for start time.
> 
> AFAICT, there's zero reason to use CLOCK_MONOTONIC_RAW instead of CLOCK_MONOTONIC.
> If there are no objections, I'll take this through kvm-x86/selftests for 6.6.

CLOCK_MONOTONIC is objectively the better choice if you want something
that accurately tracks wall time, which we certainly do. So yeah, fine
by me to take this through the x86 tree:

Reviewed-by: Oliver Upton <oliver.upton@linux.dev>
Sean Christopherson Aug. 2, 2023, 9:11 p.m. UTC | #3 
On Mon, 31 Jul 2023 10:24:05 +0800, Bibo Mao wrote:
> With test case kvm_page_table_test, start time is acquired with
> time type CLOCK_MONOTONIC_RAW, however end time in function timespec_elapsed
> is acquired with time type CLOCK_MONOTONIC. This will cause
> inaccurate elapsed time calculation on some platform such as LoongArch.
> 
> This patch modified test case kvm_page_table_test, and uses unified
> time type CLOCK_MONOTONIC for start time.
> 
> [...]

Applied to kvm-x86 selftests, thanks!

[1/1] KVM: selftests: use unified time type for comparison
      https://github.com/kvm-x86/linux/commit/b859b018aadf

--
https://github.com/kvm-x86/linux/tree/next
https://github.com/kvm-x86/linux/tree/fixes
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/kvm_page_table_test.c b/tools/testing/selftests/kvm/kvm_page_table_test.c
index b3b00be1ef82..69f26d80c821 100644
--- a/tools/testing/selftests/kvm/kvm_page_table_test.c
+++ b/tools/testing/selftests/kvm/kvm_page_table_test.c
@@ -200,7 +200,7 @@  static void *vcpu_worker(void *data)
 		if (READ_ONCE(host_quit))
 			return NULL;
 
-		clock_gettime(CLOCK_MONOTONIC_RAW, &start);
+		clock_gettime(CLOCK_MONOTONIC, &start);
 		ret = _vcpu_run(vcpu);
 		ts_diff = timespec_elapsed(start);
 
@@ -367,7 +367,7 @@  static void run_test(enum vm_guest_mode mode, void *arg)
 	/* Test the stage of KVM creating mappings */
 	*current_stage = KVM_CREATE_MAPPINGS;
 
-	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
+	clock_gettime(CLOCK_MONOTONIC, &start);
 	vcpus_complete_new_stage(*current_stage);
 	ts_diff = timespec_elapsed(start);
 
@@ -380,7 +380,7 @@  static void run_test(enum vm_guest_mode mode, void *arg)
 
 	*current_stage = KVM_UPDATE_MAPPINGS;
 
-	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
+	clock_gettime(CLOCK_MONOTONIC, &start);
 	vcpus_complete_new_stage(*current_stage);
 	ts_diff = timespec_elapsed(start);
 
@@ -392,7 +392,7 @@  static void run_test(enum vm_guest_mode mode, void *arg)
 
 	*current_stage = KVM_ADJUST_MAPPINGS;
 
-	clock_gettime(CLOCK_MONOTONIC_RAW, &start);
+	clock_gettime(CLOCK_MONOTONIC, &start);
 	vcpus_complete_new_stage(*current_stage);
 	ts_diff = timespec_elapsed(start);