[2/3] powerpc/64s/idle: avoid sync for KVM state when waking from idle

Message ID 20171117140807.22105-3-npiggin@gmail.com
State Accepted
Commit 8c1c7fb0b5ec95c392e9b585a6cf8cde254308d3
Headers show
Series
  • one more try at idle improvements
Related show

Commit Message

Nicholas Piggin Nov. 17, 2017, 2:08 p.m.
When waking from a CPU idle instruction (e.g., nap or stop), the sync
for ordering the KVM secondary thread state can be avoided if there
wakeup is coming from a kernel context rather than KVM context.

This improves performance for ping-pong benchmark with the stop0 idle
state by 0.46% for 2 threads in the same core, and 1.02% for different
cores.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/idle_book3s.S | 3 +++
 1 file changed, 3 insertions(+)

Comments

Vaidyanathan Srinivasan Feb. 28, 2018, 6:16 p.m. | #1
* Nicholas Piggin <npiggin@gmail.com> [2017-11-18 00:08:06]:

> When waking from a CPU idle instruction (e.g., nap or stop), the sync
> for ordering the KVM secondary thread state can be avoided if there
> wakeup is coming from a kernel context rather than KVM context.
> 
> This improves performance for ping-pong benchmark with the stop0 idle
> state by 0.46% for 2 threads in the same core, and 1.02% for different
> cores.

Cool, the improvement comes from avoiding the "sync" alone?
 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/kernel/idle_book3s.S | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 2f8364e7b489..07a306173c5a 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -532,6 +532,9 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300)
>  	mr	r3,r12
> 
>  #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> +	lbz	r0,HSTATE_HWTHREAD_STATE(r13)
> +	cmpwi	r0,KVM_HWTHREAD_IN_KERNEL
> +	beq	1f
>  	li	r0,KVM_HWTHREAD_IN_KERNEL
>  	stb	r0,HSTATE_HWTHREAD_STATE(r13)
>  	/* Order setting hwthread_state vs. testing hwthread_req */

With this change, we will not check for HSTATE_HWTHREAD_REQ != 0 
condition but unconditionally goto host kernel if
HSTATE_HWTHREAD_STATE == KVM_HWTHREAD_IN_KERNEL at wakeup.

Host is in ST mode and sibling thread got a wakeup event (door bell) to execute
a new vcpu by calling kvm_start_guest, what will HSTATE_HWTHREAD_STATE be?

Just to clarify, what will the flags looks like for

(a) Host cpu sibling thread is offline and need to execute guest
(b) Host cpu sibling thread is idle and need to execute guest

--Vaidy
Nicholas Piggin March 1, 2018, 11:38 a.m. | #2
On Wed, 28 Feb 2018 23:46:23 +0530
Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> wrote:

> * Nicholas Piggin <npiggin@gmail.com> [2017-11-18 00:08:06]:
> 
> > When waking from a CPU idle instruction (e.g., nap or stop), the sync
> > for ordering the KVM secondary thread state can be avoided if there
> > wakeup is coming from a kernel context rather than KVM context.
> > 
> > This improves performance for ping-pong benchmark with the stop0 idle
> > state by 0.46% for 2 threads in the same core, and 1.02% for different
> > cores.  
> 
> Cool, the improvement comes from avoiding the "sync" alone?

Yes, they can be pretty costly.

>  
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >  arch/powerpc/kernel/idle_book3s.S | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> > index 2f8364e7b489..07a306173c5a 100644
> > --- a/arch/powerpc/kernel/idle_book3s.S
> > +++ b/arch/powerpc/kernel/idle_book3s.S
> > @@ -532,6 +532,9 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300)
> >  	mr	r3,r12
> > 
> >  #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> > +	lbz	r0,HSTATE_HWTHREAD_STATE(r13)
> > +	cmpwi	r0,KVM_HWTHREAD_IN_KERNEL
> > +	beq	1f
> >  	li	r0,KVM_HWTHREAD_IN_KERNEL
> >  	stb	r0,HSTATE_HWTHREAD_STATE(r13)
> >  	/* Order setting hwthread_state vs. testing hwthread_req */  
> 
> With this change, we will not check for HSTATE_HWTHREAD_REQ != 0 
> condition but unconditionally goto host kernel if
> HSTATE_HWTHREAD_STATE == KVM_HWTHREAD_IN_KERNEL at wakeup.

That's right.

> Host is in ST mode and sibling thread got a wakeup event (door bell) to execute
> a new vcpu by calling kvm_start_guest, what will HSTATE_HWTHREAD_STATE be?

It should be KVM_HWTHREAD_IN_IDLE.

> Just to clarify, what will the flags looks like for
> 
> (a) Host cpu sibling thread is offline and need to execute guest
> (b) Host cpu sibling thread is idle and need to execute guest

In the idle case we are running with independent threads mode and
siblings not unplugged, so we should not get KVM wake-up requests
come through this path. I'm not fluent in KVM though, so I could
be wrong.

Thanks,
Nick
Michael Ellerman April 3, 2018, 4:03 p.m. | #3
On Fri, 2017-11-17 at 14:08:06 UTC, Nicholas Piggin wrote:
> When waking from a CPU idle instruction (e.g., nap or stop), the sync
> for ordering the KVM secondary thread state can be avoided if there
> wakeup is coming from a kernel context rather than KVM context.
> 
> This improves performance for ping-pong benchmark with the stop0 idle
> state by 0.46% for 2 threads in the same core, and 1.02% for different
> cores.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/8c1c7fb0b5ec95c392e9b585a6cf8c

cheers

Patch

diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 2f8364e7b489..07a306173c5a 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -532,6 +532,9 @@  ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_300)
 	mr	r3,r12
 
 #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
+	lbz	r0,HSTATE_HWTHREAD_STATE(r13)
+	cmpwi	r0,KVM_HWTHREAD_IN_KERNEL
+	beq	1f
 	li	r0,KVM_HWTHREAD_IN_KERNEL
 	stb	r0,HSTATE_HWTHREAD_STATE(r13)
 	/* Order setting hwthread_state vs. testing hwthread_req */