Re: [PATCH v2] PCI: set flag IRQCHIP_ONESHOT_SAFE for PCI-MSI irqchip's

[Bjorn Helgaas <helgaas@kernel.org>]

On Sun, Aug 05, 2018 at 10:31:03PM +0200, Heiner Kallweit wrote:
> If flag IRQCHIP_ONESHOT_SAFE isn't set for an irqchip and we have a
> threaded interrupt with no primary handler, then flag IRQF_ONESHOT
> needs to be set for the interrupt, causing some overhead in the
> threaded interrupt handler. For more detailed explanation also check
> following comment in __setup_irq():
> 
> The interrupt was requested with handler = NULL, so we use the default
> primary handler for it. But it does not have the oneshot flag set.
> In combination with level interrupts this is deadly, because the
> default primary handler just wakes the thread, then the irq lines is
> reenabled, but the device still has the level irq asserted.
> Rinse and repeat....
> While this works for edge type interrupts, we play it safe and reject
> unconditionally because we can't say for sure which type this interrupt
> really has. The type flags are unreliable as the underlying chip
> implementation can override them.
> 
> Another comment in __setup_irq() gives a hint already that this
> overhead can be avoided for PCI-MSI:
> 
> Some irq chips like MSI based interrupts are per se one shot safe.
> Check the chip flags, so we can avoid the unmask dance at the end
> of the threaded handler for those.
> 
> Following this let's mark all PCI-MSI irqchip's as oneshot-safe.
> 
> See also discussion here:
> https://marc.info/?l=linux-pci&m=153332526101128&w=2
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Applied to pci/msi for v4.19, thanks!

> ---
> v2:
> - extended commit message
> ---
>  drivers/pci/msi.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 4d88afdf..f2ef8964 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -1446,6 +1446,9 @@ struct irq_domain *pci_msi_create_irq_domain(struct fwnode_handle *fwnode,
>  	if (IS_ENABLED(CONFIG_GENERIC_IRQ_RESERVATION_MODE))
>  		info->flags |= MSI_FLAG_MUST_REACTIVATE;
>  
> +	/* PCI-MSI is oneshot-safe */
> +	info->chip->flags |= IRQCHIP_ONESHOT_SAFE;
> +
>  	domain = msi_create_irq_domain(fwnode, info, parent);
>  	if (!domain)
>  		return NULL;
> -- 
> 2.18.0
> 
>