diff mbox

powerpc/64s: Use relon prolog for EXC_VIRT_OOL_MASKABLE_HV handlers

Message ID 20170413094548.10616-1-npiggin@gmail.com (mailing list archive)
State Accepted
Commit a050d20d024dc3e1b988e715e2d811aaa3d5c39c
Headers show

Commit Message

Nicholas Piggin April 13, 2017, 9:45 a.m. UTC
Hypervisor Virtualization and Directed Hypervisor Doorbell interrupt
handlers use the macro EXC_VIRT_OOL_MASKABLE_HV for their relocation-on
handlers, which calls MASKABLE_RELON_EXCEPTION_HV_OOL, which uses the
real mode interrupt prolog.

Use the RELON variant instead to reduce overhead.

Fixes: 1707dd1613 ("powerpc: Save CFAR before branching in interrupt entry paths")
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---

I was hesitant about using the Fixes: tag on this one because it's not
a correctness bug as far as I can see.

Thanks,
Nick

 arch/powerpc/include/asm/exception-64s.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael Ellerman April 13, 2017, 1:31 p.m. UTC | #1
Nicholas Piggin <npiggin@gmail.com> writes:

> Hypervisor Virtualization and Directed Hypervisor Doorbell interrupt
> handlers use the macro EXC_VIRT_OOL_MASKABLE_HV for their relocation-on
> handlers, which calls MASKABLE_RELON_EXCEPTION_HV_OOL, which uses the
> real mode interrupt prolog.
>
> Use the RELON variant instead to reduce overhead.
>
> Fixes: 1707dd1613 ("powerpc: Save CFAR before branching in interrupt entry paths")
> Cc: Paul Mackerras <paulus@samba.org>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>
> I was hesitant about using the Fixes: tag on this one because it's not
> a correctness bug as far as I can see.

Fixes can just mean "if you have this other commit you'll most likely
want this one too".

You can clarify the degree of Fixness in the change log, which then
allows a backporter to decide if they want it or not.

In this case I think it's entirely right, this is a fairly unfortunate
bug from a performance point of view if I'm reading it right.

cheers

> diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
> index 14752eee3d0c..b89c91e27dd0 100644
> --- a/arch/powerpc/include/asm/exception-64s.h
> +++ b/arch/powerpc/include/asm/exception-64s.h
> @@ -522,7 +522,7 @@ END_FTR_SECTION_NESTED(ftr,ftr,943)
>  
>  #define MASKABLE_RELON_EXCEPTION_HV_OOL(vec, label)			\
>  	EXCEPTION_PROLOG_1(PACA_EXGEN, SOFTEN_TEST_HV, vec);		\
> -	EXCEPTION_PROLOG_PSERIES_1(label, EXC_HV)
> +	EXCEPTION_RELON_PROLOG_PSERIES_1(label, EXC_HV)
>  
>  /*
>   * Our exception common code can be passed various "additions"
> -- 
> 2.11.0
Nicholas Piggin April 15, 2017, 10:31 a.m. UTC | #2
On Thu, 13 Apr 2017 23:31:23 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:

> Nicholas Piggin <npiggin@gmail.com> writes:
> 
> > Hypervisor Virtualization and Directed Hypervisor Doorbell interrupt
> > handlers use the macro EXC_VIRT_OOL_MASKABLE_HV for their relocation-on
> > handlers, which calls MASKABLE_RELON_EXCEPTION_HV_OOL, which uses the
> > real mode interrupt prolog.
> >
> > Use the RELON variant instead to reduce overhead.
> >
> > Fixes: 1707dd1613 ("powerpc: Save CFAR before branching in interrupt entry paths")
> > Cc: Paul Mackerras <paulus@samba.org>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >
> > I was hesitant about using the Fixes: tag on this one because it's not
> > a correctness bug as far as I can see.  
> 
> Fixes can just mean "if you have this other commit you'll most likely
> want this one too".
> 
> You can clarify the degree of Fixness in the change log, which then
> allows a backporter to decide if they want it or not.
> 
> In this case I think it's entirely right, this is a fairly unfortunate
> bug from a performance point of view if I'm reading it right.

Okay, makes sense. For POWER8 it only affects doorbell IPIs. Context switch
microbenchmark between threads with snooze disabled (which causes IPI) gets
about 3% faster, about 370 cycles. Should be more important on POWER9 with
global doorbells and HVI for host interrupts.

Thanks,
Nick
Michael Ellerman April 21, 2017, 4:09 a.m. UTC | #3
On Thu, 2017-04-13 at 09:45:48 UTC, Nicholas Piggin wrote:
> Hypervisor Virtualization and Directed Hypervisor Doorbell interrupt
> handlers use the macro EXC_VIRT_OOL_MASKABLE_HV for their relocation-on
> handlers, which calls MASKABLE_RELON_EXCEPTION_HV_OOL, which uses the
> real mode interrupt prolog.
> 
> Use the RELON variant instead to reduce overhead.
> 
> Fixes: 1707dd1613 ("powerpc: Save CFAR before branching in interrupt entry paths")
> Cc: Paul Mackerras <paulus@samba.org>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/a050d20d024dc3e1b988e715e2d811

cheers
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
index 14752eee3d0c..b89c91e27dd0 100644
--- a/arch/powerpc/include/asm/exception-64s.h
+++ b/arch/powerpc/include/asm/exception-64s.h
@@ -522,7 +522,7 @@  END_FTR_SECTION_NESTED(ftr,ftr,943)
 
 #define MASKABLE_RELON_EXCEPTION_HV_OOL(vec, label)			\
 	EXCEPTION_PROLOG_1(PACA_EXGEN, SOFTEN_TEST_HV, vec);		\
-	EXCEPTION_PROLOG_PSERIES_1(label, EXC_HV)
+	EXCEPTION_RELON_PROLOG_PSERIES_1(label, EXC_HV)
 
 /*
  * Our exception common code can be passed various "additions"