Subject: Re: [PATCH] powerpc/mpic: supply a .disable callback

From: Benjamin Herrenschmidt <[email-masked]>
Date: Tue, 07 Jan 2014 16:49:36 +1100

On Tue, 2014-01-07 at 13:38 +0800, Dongsheng Wang wrote:
> From: Wang Dongsheng <[email-masked]>
> 
> Currently MPIC provides .mask, but not .disable.  This means that
> effectively disable_irq() soft-disables the interrupt, and you get
> a .mask call if an interrupt actually occurs.
> 
> I'm not sure if this was intended as a performance benefit (it seems common
> to omit .disable on powerpc interrupt controllers, but nowhere else), but it
> interacts badly with threaded/workqueue interrupts (including KVM
> reflection).  In such cases, where the real interrupt handler does a
> disable_irq_nosync(), schedules defered handling, and returns, we get two
> interrupts for every real interrupt.  The second interrupt does nothing
> but see that IRQ_DISABLED is set, and decide that it would be a good
> idea to actually call .mask.

We probably don't want to do that for edge, only level interrupts.

Cheers,
Ben.

> 
> Signed-off-by: Scott Wood <[email-masked]>
> Signed-off-by: Wang Dongsheng <[email-masked]>
> 
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 0e166ed..dd7564b 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -975,6 +975,7 @@ void mpic_set_destination(unsigned int virq, unsigned int cpuid)
>  }
>  
>  static struct irq_chip mpic_irq_chip = {
> +	.irq_disable	= mpic_mask_irq,
>  	.irq_mask	= mpic_mask_irq,
>  	.irq_unmask	= mpic_unmask_irq,
>  	.irq_eoi	= mpic_end_irq,
> @@ -984,6 +985,7 @@ static struct irq_chip mpic_irq_chip = {
>  
>  #ifdef CONFIG_SMP
>  static struct irq_chip mpic_ipi_chip = {
> +	.irq_disable	= mpic_mask_ipi,
>  	.irq_mask	= mpic_mask_ipi,
>  	.irq_unmask	= mpic_unmask_ipi,
>  	.irq_eoi	= mpic_end_ipi,
> @@ -991,6 +993,7 @@ static struct irq_chip mpic_ipi_chip = {
>  #endif /* CONFIG_SMP */
>  
>  static struct irq_chip mpic_tm_chip = {
> +	.irq_disable	= mpic_mask_tm,
>  	.irq_mask	= mpic_mask_tm,
>  	.irq_unmask	= mpic_unmask_tm,
>  	.irq_eoi	= mpic_end_irq,
> @@ -1001,6 +1004,7 @@ static struct irq_chip mpic_tm_chip = {
>  static struct irq_chip mpic_irq_ht_chip = {
>  	.irq_startup	= mpic_startup_ht_irq,
>  	.irq_shutdown	= mpic_shutdown_ht_irq,
> +	.irq_disable	= mpic_mask_irq,
>  	.irq_mask	= mpic_mask_irq,
>  	.irq_unmask	= mpic_unmask_ht_irq,
>  	.irq_eoi	= mpic_end_ht_irq,