Re: [PATCH v2 1/4] powerpc/interrupt: Interchange prep_irq_for_{kernel_enabled/user}_exit()

[Nicholas Piggin]

Excerpts from Christophe Leroy's message of June 5, 2021 12:56 am:
> prep_irq_for_user_exit() is a superset of
> prep_irq_for_kernel_enabled_exit(). In order to allow refactoring in
> following patch, interchange the two as prep_irq_for_user_exit() will
> call prep_irq_for_kernel_enabled_exit().
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
> ---
> This series applies on top of Nic's series to speed up interrupt return on 64s

Thanks for rebasing it.

Reviewed-by: Nicholas Piggin <npiggin@xxxxxxxxx>

> 
>  arch/powerpc/kernel/interrupt.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 74c995a42399..539455c62c5b 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -40,33 +40,27 @@ static inline bool exit_must_hard_disable(void)
>  #endif
>  
>  /*
> - * local irqs must be disabled. Returns false if the caller must re-enable
> - * them, check for new work, and try again.
> - *
> - * This should be called with local irqs disabled, but if they were previously
> - * enabled when the interrupt handler returns (indicating a process-context /
> - * synchronous interrupt) then irqs_enabled should be true.
> + * restartable is true then EE/RI can be left on because interrupts are handled
> + * with a restart sequence.
>   */
> -static notrace __always_inline bool prep_irq_for_user_exit(void)
> +static notrace __always_inline bool prep_irq_for_kernel_enabled_exit(bool restartable)
>  {
> -	user_enter_irqoff();
>  	/* This must be done with RI=1 because tracing may touch vmaps */
>  	trace_hardirqs_on();
>  
>  #ifdef CONFIG_PPC32
>  	__hard_EE_RI_disable();
>  #else
> -	if (exit_must_hard_disable())
> +	if (exit_must_hard_disable() || !restartable)
>  		__hard_EE_RI_disable();
>  
>  	/* This pattern matches prep_irq_for_idle */
>  	if (unlikely(lazy_irq_pending_nocheck())) {
> -		if (exit_must_hard_disable()) {
> +		if (exit_must_hard_disable() || !restartable) {
>  			local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
>  			__hard_RI_enable();
>  		}
>  		trace_hardirqs_off();
> -		user_exit_irqoff();
>  
>  		return false;
>  	}
> @@ -75,27 +69,33 @@ static notrace __always_inline bool prep_irq_for_user_exit(void)
>  }
>  
>  /*
> - * restartable is true then EE/RI can be left on because interrupts are handled
> - * with a restart sequence.
> + * local irqs must be disabled. Returns false if the caller must re-enable
> + * them, check for new work, and try again.
> + *
> + * This should be called with local irqs disabled, but if they were previously
> + * enabled when the interrupt handler returns (indicating a process-context /
> + * synchronous interrupt) then irqs_enabled should be true.
>   */
> -static notrace __always_inline bool prep_irq_for_kernel_enabled_exit(bool restartable)
> +static notrace __always_inline bool prep_irq_for_user_exit(void)
>  {
> +	user_enter_irqoff();
>  	/* This must be done with RI=1 because tracing may touch vmaps */
>  	trace_hardirqs_on();
>  
>  #ifdef CONFIG_PPC32
>  	__hard_EE_RI_disable();
>  #else
> -	if (exit_must_hard_disable() || !restartable)
> +	if (exit_must_hard_disable())
>  		__hard_EE_RI_disable();
>  
>  	/* This pattern matches prep_irq_for_idle */
>  	if (unlikely(lazy_irq_pending_nocheck())) {
> -		if (exit_must_hard_disable() || !restartable) {
> +		if (exit_must_hard_disable()) {
>  			local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
>  			__hard_RI_enable();
>  		}
>  		trace_hardirqs_off();
> +		user_exit_irqoff();
>  
>  		return false;
>  	}
> -- 
> 2.25.0
> 
>