* [PATCH][v2] KVM: use cpu_relax when halt polling
@ 2021-07-27 11:12 Li RongQing
2021-07-27 18:15 ` Pankaj Gupta
0 siblings, 1 reply; 5+ messages in thread
From: Li RongQing @ 2021-07-27 11:12 UTC (permalink / raw)
To: kvm, pbonzini, seanjc
SMT siblings share caches and other hardware, and busy halt polling
will degrade its sibling performance if its sibling is working
Sean Christopherson suggested as below:
"Rather than disallowing halt-polling entirely, on x86 it should be
sufficient to simply have the hardware thread yield to its sibling(s)
via PAUSE. It probably won't get back all performance, but I would
expect it to be close.
This compiles on all KVM architectures, and AFAICT the intended usage
of cpu_relax() is identical for all architectures."
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
diff v1: using cpu_relax, rather that stop halt-polling
virt/kvm/kvm_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 7d95126..1679728 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3110,6 +3110,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
++vcpu->stat.generic.halt_poll_invalid;
goto out;
}
+ cpu_relax();
poll_end = cur = ktime_get();
} while (kvm_vcpu_can_poll(cur, stop));
}
--
2.9.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH][v2] KVM: use cpu_relax when halt polling
2021-07-27 11:12 [PATCH][v2] KVM: use cpu_relax when halt polling Li RongQing
@ 2021-07-27 18:15 ` Pankaj Gupta
2021-07-28 2:57 ` 答复: " Li,Rongqing
0 siblings, 1 reply; 5+ messages in thread
From: Pankaj Gupta @ 2021-07-27 18:15 UTC (permalink / raw)
To: Li RongQing; +Cc: kvm, Paolo Bonzini, seanjc
> SMT siblings share caches and other hardware, and busy halt polling
> will degrade its sibling performance if its sibling is working
>
> Sean Christopherson suggested as below:
>
> "Rather than disallowing halt-polling entirely, on x86 it should be
> sufficient to simply have the hardware thread yield to its sibling(s)
> via PAUSE. It probably won't get back all performance, but I would
> expect it to be close.
> This compiles on all KVM architectures, and AFAICT the intended usage
> of cpu_relax() is identical for all architectures."
For sure change to cpu_relax() is better.
Was just curious to know if you got descent performance improvement compared
to previously reported with Unixbench.
Thanks,
Pankaj
>
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
> diff v1: using cpu_relax, rather that stop halt-polling
>
> virt/kvm/kvm_main.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 7d95126..1679728 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -3110,6 +3110,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
> ++vcpu->stat.generic.halt_poll_invalid;
> goto out;
> }
> + cpu_relax();
> poll_end = cur = ktime_get();
> } while (kvm_vcpu_can_poll(cur, stop));
> }
> --
> 2.9.4
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* 答复: [PATCH][v2] KVM: use cpu_relax when halt polling
2021-07-27 18:15 ` Pankaj Gupta
@ 2021-07-28 2:57 ` Li,Rongqing
2021-07-28 6:12 ` Pankaj Gupta
0 siblings, 1 reply; 5+ messages in thread
From: Li,Rongqing @ 2021-07-28 2:57 UTC (permalink / raw)
To: Pankaj Gupta; +Cc: kvm, Paolo Bonzini, seanjc
> -----邮件原件-----
> 发件人: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
> 发送时间: 2021年7月28日 2:16
> 收件人: Li,Rongqing <lirongqing@baidu.com>
> 抄送: kvm@vger.kernel.org; Paolo Bonzini <pbonzini@redhat.com>;
> seanjc@google.com
> 主题: Re: [PATCH][v2] KVM: use cpu_relax when halt polling
>
> > SMT siblings share caches and other hardware, and busy halt polling
> > will degrade its sibling performance if its sibling is working
> >
> > Sean Christopherson suggested as below:
> >
> > "Rather than disallowing halt-polling entirely, on x86 it should be
> > sufficient to simply have the hardware thread yield to its sibling(s)
> > via PAUSE. It probably won't get back all performance, but I would
> > expect it to be close.
> > This compiles on all KVM architectures, and AFAICT the intended usage
> > of cpu_relax() is identical for all architectures."
>
> For sure change to cpu_relax() is better.
> Was just curious to know if you got descent performance improvement
> compared to previously reported with Unixbench.
>
> Thanks,
> Pankaj
The test as below:
1. run unixbench dhry2reg: ./Run -c 1 dhry2reg -i 1
without SMT disturbance, the score is 3172
with a {while(1)i++} SMT disturbance, the score is 1583
with a {while(1)(rep nop/pause)} SMT disturbance, the score is 1729.4
seems cpu_relax can not get back all performance , what wrong?
2. back to haltpoll
run unixbench dhry2reg ./Run -c 1 dhry2reg -i 1
without SMT disturbance, the score is 3172
with redis-benchmark SMT disturbance, redis-benchmark takes 90%cpu:
without patch, the score is 1776.9
with my first patch, the score is 1782.3
with cpu_relax patch, the score is 1778
with redis-benchmark SMT disturbance, redis-benchmark takes 33%cpu:
without patch, the score is 1929.9
with my first patch, the score is 2294.6
with cpu_relax patch, the score is 2005.3
cpu_relax give less than stop halt polling, but it should have little effect for redis-benchmark which get benefit from halt polling
-Li
> >
> > Suggested-by: Sean Christopherson <seanjc@google.com>
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> > diff v1: using cpu_relax, rather that stop halt-polling
> >
> > virt/kvm/kvm_main.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index
> > 7d95126..1679728 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -3110,6 +3110,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
> >
> ++vcpu->stat.generic.halt_poll_invalid;
> > goto out;
> > }
> > + cpu_relax();
> > poll_end = cur = ktime_get();
> > } while (kvm_vcpu_can_poll(cur, stop));
> > }
> > --
> > 2.9.4
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH][v2] KVM: use cpu_relax when halt polling
2021-07-28 2:57 ` 答复: " Li,Rongqing
@ 2021-07-28 6:12 ` Pankaj Gupta
2021-07-28 6:46 ` 答复: " Li,Rongqing
0 siblings, 1 reply; 5+ messages in thread
From: Pankaj Gupta @ 2021-07-28 6:12 UTC (permalink / raw)
To: Li,Rongqing; +Cc: kvm, Paolo Bonzini, seanjc
'
> > > "Rather than disallowing halt-polling entirely, on x86 it should be
> > > sufficient to simply have the hardware thread yield to its sibling(s)
> > > via PAUSE. It probably won't get back all performance, but I would
> > > expect it to be close.
> > > This compiles on all KVM architectures, and AFAICT the intended usage
> > > of cpu_relax() is identical for all architectures."
> >
> > For sure change to cpu_relax() is better.
> > Was just curious to know if you got descent performance improvement
> > compared to previously reported with Unixbench.
> >
> > Thanks,
> > Pankaj
>
> The test as below:
>
> 1. run unixbench dhry2reg: ./Run -c 1 dhry2reg -i 1
> without SMT disturbance, the score is 3172
> with a {while(1)i++} SMT disturbance, the score is 1583
> with a {while(1)(rep nop/pause)} SMT disturbance, the score is 1729.4
>
> seems cpu_relax can not get back all performance , what wrong?
Maybe because of pause intercept filtering, comparatively Mayless VM Exits?
>
>
> 2. back to haltpoll
> run unixbench dhry2reg ./Run -c 1 dhry2reg -i 1
> without SMT disturbance, the score is 3172
>
> with redis-benchmark SMT disturbance, redis-benchmark takes 90%cpu:
> without patch, the score is 1776.9
> with my first patch, the score is 1782.3
> with cpu_relax patch, the score is 1778
>
> with redis-benchmark SMT disturbance, redis-benchmark takes 33%cpu:
> without patch, the score is 1929.9
> with my first patch, the score is 2294.6
> with cpu_relax patch, the score is 2005.3
>
>
> cpu_relax give less than stop halt polling, but it should have little effect for redis-benchmark which get benefit from halt polling
We are seeing improvement with cpu_relax() though not to the level of
stopping the halt polling when sibling
CPU running redis workload. For 90% case I think its expected to have
similar performance.
For 33% stopping halt poll gives better result because of the
workload. Overall I think this patch helps and not impact
performance in normal cases.
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
Best regards,
Pankaj
>
>
> -Li
>
> > >
> > > Suggested-by: Sean Christopherson <seanjc@google.com>
> > > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > > ---
> > > diff v1: using cpu_relax, rather that stop halt-polling
> > >
> > > virt/kvm/kvm_main.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index
> > > 7d95126..1679728 100644
> > > --- a/virt/kvm/kvm_main.c
> > > +++ b/virt/kvm/kvm_main.c
> > > @@ -3110,6 +3110,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
> > >
> > ++vcpu->stat.generic.halt_poll_invalid;
> > > goto out;
> > > }
> > > + cpu_relax();
> > > poll_end = cur = ktime_get();
> > > } while (kvm_vcpu_can_poll(cur, stop));
> > > }
> > > --
> > > 2.9.4
> > >
^ permalink raw reply [flat|nested] 5+ messages in thread
* 答复: [PATCH][v2] KVM: use cpu_relax when halt polling
2021-07-28 6:12 ` Pankaj Gupta
@ 2021-07-28 6:46 ` Li,Rongqing
0 siblings, 0 replies; 5+ messages in thread
From: Li,Rongqing @ 2021-07-28 6:46 UTC (permalink / raw)
To: Pankaj Gupta; +Cc: kvm, Paolo Bonzini, seanjc
> -----邮件原件-----
> 发件人: Pankaj Gupta <pankaj.gupta.linux@gmail.com>
> 发送时间: 2021年7月28日 14:12
> 收件人: Li,Rongqing <lirongqing@baidu.com>
> 抄送: kvm@vger.kernel.org; Paolo Bonzini <pbonzini@redhat.com>;
> seanjc@google.com
> 主题: Re: [PATCH][v2] KVM: use cpu_relax when halt polling
>
> '
> > > > "Rather than disallowing halt-polling entirely, on x86 it should
> > > > be sufficient to simply have the hardware thread yield to its
> > > > sibling(s) via PAUSE. It probably won't get back all performance,
> > > > but I would expect it to be close.
> > > > This compiles on all KVM architectures, and AFAICT the intended
> > > > usage of cpu_relax() is identical for all architectures."
> > >
> > > For sure change to cpu_relax() is better.
> > > Was just curious to know if you got descent performance improvement
> > > compared to previously reported with Unixbench.
> > >
> > > Thanks,
> > > Pankaj
> >
> > The test as below:
> >
> > 1. run unixbench dhry2reg: ./Run -c 1 dhry2reg -i 1 without SMT
> > disturbance, the score is 3172 with a {while(1)i++} SMT disturbance,
> > the score is 1583 with a {while(1)(rep nop/pause)} SMT disturbance,
> > the score is 1729.4
> >
> > seems cpu_relax can not get back all performance , what wrong?
>
> Maybe because of pause intercept filtering, comparatively Mayless VM Exits?
>
In vm;
I retest it in bare metal, pause instruction works as expect, the score with "pause loop" disturbance is 2886; about 90% of no disturbance
-Li
> >
> >
> > 2. back to haltpoll
> > run unixbench dhry2reg ./Run -c 1 dhry2reg -i 1 without SMT
> > disturbance, the score is 3172
> >
> > with redis-benchmark SMT disturbance, redis-benchmark takes 90%cpu:
> > without patch, the score is 1776.9
> > with my first patch, the score is 1782.3 with cpu_relax patch, the
> > score is 1778
> >
> > with redis-benchmark SMT disturbance, redis-benchmark takes 33%cpu:
> > without patch, the score is 1929.9
> > with my first patch, the score is 2294.6 with cpu_relax patch, the
> > score is 2005.3
> >
> >
> > cpu_relax give less than stop halt polling, but it should have little
> > effect for redis-benchmark which get benefit from halt polling
>
> We are seeing improvement with cpu_relax() though not to the level of stopping
> the halt polling when sibling CPU running redis workload. For 90% case I think its
> expected to have similar performance.
>
> For 33% stopping halt poll gives better result because of the workload. Overall I
> think this patch helps and not impact performance in normal cases.
>
> Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
>
> Best regards,
> Pankaj
>
>
> >
> >
> > -Li
> >
> > > >
> > > > Suggested-by: Sean Christopherson <seanjc@google.com>
> > > > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > > > ---
> > > > diff v1: using cpu_relax, rather that stop halt-polling
> > > >
> > > > virt/kvm/kvm_main.c | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index
> > > > 7d95126..1679728 100644
> > > > --- a/virt/kvm/kvm_main.c
> > > > +++ b/virt/kvm/kvm_main.c
> > > > @@ -3110,6 +3110,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
> > > >
> > > ++vcpu->stat.generic.halt_poll_invalid;
> > > > goto out;
> > > > }
> > > > + cpu_relax();
> > > > poll_end = cur = ktime_get();
> > > > } while (kvm_vcpu_can_poll(cur, stop));
> > > > }
> > > > --
> > > > 2.9.4
> > > >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-07-28 6:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-27 11:12 [PATCH][v2] KVM: use cpu_relax when halt polling Li RongQing
2021-07-27 18:15 ` Pankaj Gupta
2021-07-28 2:57 ` 答复: " Li,Rongqing
2021-07-28 6:12 ` Pankaj Gupta
2021-07-28 6:46 ` 答复: " Li,Rongqing
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.