Re: [PATCH v1 2/6] kvm/x86: Hyper-V unify stimer_start() and stimer_restart()

[Paolo Bonzini <pbonzini@redhat.com>]

On 23/12/2015 12:28, Andrey Smetanin wrote:
> This will be used in future to start Hyper-V SynIC timer
> in several places by one logic in one function.
> 
> Signed-off-by: Andrey Smetanin <asmetanin@virtuozzo.com>
> Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
> CC: Gleb Natapov <gleb@kernel.org>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> CC: Roman Kagan <rkagan@virtuozzo.com>
> CC: Denis V. Lunev <den@openvz.org>
> CC: qemu-devel@nongnu.org
> ---
>  arch/x86/kvm/hyperv.c | 37 ++++++++++++++++---------------------
>  1 file changed, 16 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index ec3a900..8623aa6 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -408,6 +408,7 @@ static void stimer_cleanup(struct kvm_vcpu_hv_stimer *stimer)
>  	clear_bit(stimer->index,
>  		  vcpu_to_hv_vcpu(vcpu)->stimer_pending_bitmap);
>  	stimer->msg_pending = false;
> +	stimer->exp_time = 0;
>  }
>  
>  static enum hrtimer_restart stimer_timer_callback(struct hrtimer *timer)
> @@ -420,24 +421,6 @@ static enum hrtimer_restart stimer_timer_callback(struct hrtimer *timer)
>  	return HRTIMER_NORESTART;
>  }
>  
> -static void stimer_restart(struct kvm_vcpu_hv_stimer *stimer)
> -{
> -	u64 time_now;
> -	ktime_t ktime_now;
> -	u64 remainder;
> -
> -	time_now = get_time_ref_counter(stimer_to_vcpu(stimer)->kvm);
> -	ktime_now = ktime_get();
> -
> -	div64_u64_rem(time_now - stimer->exp_time, stimer->count, &remainder);
> -	stimer->exp_time = time_now + (stimer->count - remainder);
> -
> -	hrtimer_start(&stimer->timer,
> -		      ktime_add_ns(ktime_now,
> -				   100 * (stimer->exp_time - time_now)),
> -		      HRTIMER_MODE_ABS);
> -}
> -
>  static int stimer_start(struct kvm_vcpu_hv_stimer *stimer)
>  {
>  	u64 time_now;
> @@ -450,9 +433,21 @@ static int stimer_start(struct kvm_vcpu_hv_stimer *stimer)
>  		if (stimer->count == 0)
>  			return -EINVAL;
>  
> -		stimer->exp_time = time_now + stimer->count;
> +		if (stimer->exp_time) {
> +			if (time_now >= stimer->exp_time) {

Just for my education, is it possible to have this function called with
stimer->exp_time != 0 && time_now < stimer->exp_time?

Paolo

> +				u64 remainder;
> +
> +				div64_u64_rem(time_now - stimer->exp_time,
> +					      stimer->count, &remainder);
> +				stimer->exp_time =
> +					time_now + (stimer->count - remainder);
> +			}
> +		} else
> +			stimer->exp_time = time_now + stimer->count;
> +
>  		hrtimer_start(&stimer->timer,
> -			      ktime_add_ns(ktime_now, 100 * stimer->count),
> +			      ktime_add_ns(ktime_now,
> +					   100 * (stimer->exp_time - time_now)),
>  			      HRTIMER_MODE_ABS);
>  		return 0;
>  	}
> @@ -580,7 +575,7 @@ static void stimer_expiration(struct kvm_vcpu_hv_stimer *stimer)
>  	if (!(stimer->config & HV_STIMER_PERIODIC))
>  		stimer->config |= ~HV_STIMER_ENABLE;
>  	else
> -		stimer_restart(stimer);
> +		stimer_start(stimer);
>  }
>  
>  void kvm_hv_process_stimers(struct kvm_vcpu *vcpu)
>