Re: [PATCH] KVM: selftests: hyperv_tlb_flush: replace NOP loop with udelay()

[Vitaly Kuznetsov]

Piotr Zarycki <piotr.zarycki@xxxxxxxxx> writes:

> Replace the open-coded NOP loop with udelay() which was added to KVM
> selftests in commit 6b878cbb87bf ("KVM: selftests: Add guest udelay()
> utility for x86"). The NOP loop is CPU speed dependent while udelay()
> provides a deterministic delay regardless of host CPU frequency.
>
> Signed-off-by: Piotr Zarycki <piotr.zarycki@xxxxxxxxx>
> ---
>  tools/testing/selftests/kvm/x86/hyperv_tlb_flush.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/x86/hyperv_tlb_flush.c b/tools/testing/selftests/kvm/x86/hyperv_tlb_flush.c
> index c542cc4762b1..68ebd790ff41 100644
> --- a/tools/testing/selftests/kvm/x86/hyperv_tlb_flush.c
> +++ b/tools/testing/selftests/kvm/x86/hyperv_tlb_flush.c
> @@ -141,15 +141,9 @@ static void swap_two_test_pages(vm_paddr_t pte_gva1, vm_paddr_t pte_gva2)
>  	*(uint64_t *)pte_gva2 = tmp;
>  }
>  
> -/*
> - * TODO: replace the silly NOP loop with a proper udelay() implementation.
> - */
>  static inline void do_delay(void)
>  {
> -	int i;
> -
> -	for (i = 0; i < 1000000; i++)
> -		asm volatile("nop");
> +	udelay(100);
>  }
>  
>  /*

Oh, I completely forgot I added the TODO :-) Thanks for the patch! I
also gave it a spin and it looks like rdtsc() from udelay() doesn't
hurt.

Personally, I belive we should now just drop the whole do_delay()
function and use udelay(100) instead. We have only two call sites and
there's no strict requirement that the delay time should match. 

In any case,

Reviewed-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>

-- 
Vitaly