diff mbox series

[RFC,1/3] KVM: Cap vcpu->halt_poll_ns before halting rather than after

Message ID 20221117001657.1067231-2-dmatlack@google.com (mailing list archive)
State New, archived
Headers show
Series KVM: Restore original behavior of kvm.halt_poll_ns | expand

Commit Message

David Matlack Nov. 17, 2022, 12:16 a.m. UTC
Cap vcpu->halt_poll_ns based on the max halt polling time just before
halting, rather than after the last halt. This arguably provides better
accuracy if an admin disables halt polling in between halts, although
the improvement is nominal.

A side-effect of this change is that grow_halt_poll_ns() no longer needs
to access vcpu->kvm->max_halt_poll_ns, which will be useful in a future
commit where the max halt polling time can come from the module parameter
halt_poll_ns instead.

Signed-off-by: David Matlack <dmatlack@google.com>
---
 virt/kvm/kvm_main.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Christian Borntraeger Nov. 17, 2022, 3:38 p.m. UTC | #1
Am 17.11.22 um 01:16 schrieb David Matlack:
> Cap vcpu->halt_poll_ns based on the max halt polling time just before
> halting, rather than after the last halt. This arguably provides better
> accuracy if an admin disables halt polling in between halts, although
> the improvement is nominal.
> 
> A side-effect of this change is that grow_halt_poll_ns() no longer needs
> to access vcpu->kvm->max_halt_poll_ns, which will be useful in a future
> commit where the max halt polling time can come from the module parameter
> halt_poll_ns instead.
> 
> Signed-off-by: David Matlack <dmatlack@google.com>

Looks sane

Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>


> ---
>   virt/kvm/kvm_main.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 43bbe4fde078..4b868f33c45d 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3385,9 +3385,6 @@ static void grow_halt_poll_ns(struct kvm_vcpu *vcpu)
>   	if (val < grow_start)
>   		val = grow_start;
>   
> -	if (val > vcpu->kvm->max_halt_poll_ns)
> -		val = vcpu->kvm->max_halt_poll_ns;
> -
>   	vcpu->halt_poll_ns = val;
>   out:
>   	trace_kvm_halt_poll_ns_grow(vcpu->vcpu_id, val, old);
> @@ -3500,11 +3497,16 @@ static inline void update_halt_poll_stats(struct kvm_vcpu *vcpu, ktime_t start,
>   void kvm_vcpu_halt(struct kvm_vcpu *vcpu)
>   {
>   	bool halt_poll_allowed = !kvm_arch_no_poll(vcpu);
> -	bool do_halt_poll = halt_poll_allowed && vcpu->halt_poll_ns;
>   	ktime_t start, cur, poll_end;
>   	bool waited = false;
> +	bool do_halt_poll;
>   	u64 halt_ns;
>   
> +	if (vcpu->halt_poll_ns > vcpu->kvm->max_halt_poll_ns)
> +		vcpu->halt_poll_ns = vcpu->kvm->max_halt_poll_ns;
> +
> +	do_halt_poll = halt_poll_allowed && vcpu->halt_poll_ns;
> +
>   	start = cur = poll_end = ktime_get();
>   	if (do_halt_poll) {
>   		ktime_t stop = ktime_add_ns(start, vcpu->halt_poll_ns);
Yanan Wang Nov. 18, 2022, 7:34 a.m. UTC | #2
On 2022/11/17 8:16, David Matlack wrote:
> Cap vcpu->halt_poll_ns based on the max halt polling time just before
> halting, rather than after the last halt. This arguably provides better
> accuracy if an admin disables halt polling in between halts, although
> the improvement is nominal.
>
> A side-effect of this change is that grow_halt_poll_ns() no longer needs
> to access vcpu->kvm->max_halt_poll_ns, which will be useful in a future
> commit where the max halt polling time can come from the module parameter
> halt_poll_ns instead.
>
> Signed-off-by: David Matlack <dmatlack@google.com>
> ---
>   virt/kvm/kvm_main.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 43bbe4fde078..4b868f33c45d 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3385,9 +3385,6 @@ static void grow_halt_poll_ns(struct kvm_vcpu *vcpu)
>   	if (val < grow_start)
>   		val = grow_start;
>   
> -	if (val > vcpu->kvm->max_halt_poll_ns)
> -		val = vcpu->kvm->max_halt_poll_ns;
> -
>   	vcpu->halt_poll_ns = val;
>   out:
>   	trace_kvm_halt_poll_ns_grow(vcpu->vcpu_id, val, old);
> @@ -3500,11 +3497,16 @@ static inline void update_halt_poll_stats(struct kvm_vcpu *vcpu, ktime_t start,
>   void kvm_vcpu_halt(struct kvm_vcpu *vcpu)
>   {
>   	bool halt_poll_allowed = !kvm_arch_no_poll(vcpu);
> -	bool do_halt_poll = halt_poll_allowed && vcpu->halt_poll_ns;
>   	ktime_t start, cur, poll_end;
>   	bool waited = false;
> +	bool do_halt_poll;
>   	u64 halt_ns;
>   
> +	if (vcpu->halt_poll_ns > vcpu->kvm->max_halt_poll_ns)
> +		vcpu->halt_poll_ns = vcpu->kvm->max_halt_poll_ns;
> +
> +	do_halt_poll = halt_poll_allowed && vcpu->halt_poll_ns;
> +
>   	start = cur = poll_end = ktime_get();
>   	if (do_halt_poll) {
>   		ktime_t stop = ktime_add_ns(start, vcpu->halt_poll_ns);
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>

Thanks,
Yanan
diff mbox series

Patch

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 43bbe4fde078..4b868f33c45d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3385,9 +3385,6 @@  static void grow_halt_poll_ns(struct kvm_vcpu *vcpu)
 	if (val < grow_start)
 		val = grow_start;
 
-	if (val > vcpu->kvm->max_halt_poll_ns)
-		val = vcpu->kvm->max_halt_poll_ns;
-
 	vcpu->halt_poll_ns = val;
 out:
 	trace_kvm_halt_poll_ns_grow(vcpu->vcpu_id, val, old);
@@ -3500,11 +3497,16 @@  static inline void update_halt_poll_stats(struct kvm_vcpu *vcpu, ktime_t start,
 void kvm_vcpu_halt(struct kvm_vcpu *vcpu)
 {
 	bool halt_poll_allowed = !kvm_arch_no_poll(vcpu);
-	bool do_halt_poll = halt_poll_allowed && vcpu->halt_poll_ns;
 	ktime_t start, cur, poll_end;
 	bool waited = false;
+	bool do_halt_poll;
 	u64 halt_ns;
 
+	if (vcpu->halt_poll_ns > vcpu->kvm->max_halt_poll_ns)
+		vcpu->halt_poll_ns = vcpu->kvm->max_halt_poll_ns;
+
+	do_halt_poll = halt_poll_allowed && vcpu->halt_poll_ns;
+
 	start = cur = poll_end = ktime_get();
 	if (do_halt_poll) {
 		ktime_t stop = ktime_add_ns(start, vcpu->halt_poll_ns);