On 2020-12-24 14:11:47 [+0100], Ahmed S. Darwish wrote:
> --- a/drivers/net/ethernet/chelsio/cxgb/cxgb2.c
> +++ b/drivers/net/ethernet/chelsio/cxgb/cxgb2.c
> @@ -211,9 +211,9 @@ static int cxgb_up(struct adapter *adapter)
> t1_interrupts_clear(adapter);
>
> adapter->params.has_msi = !disable_msi &&
> !pci_enable_msi(adapter->pdev);
> - err = request_irq(adapter->pdev->irq, t1_interrupt,
> - adapter->params.has_msi ? 0 : IRQF_SHARED,
> - adapter->name, adapter);
> + err = request_threaded_irq(adapter->pdev->irq, t1_irq, t1_irq_thread,
> + adapter->params.has_msi ? 0 : IRQF_SHARED,
> + adapter->name, adapter);
> if (err) {
> if (adapter->params.has_msi)
> pci_disable_msi(adapter->pdev);
> diff --git a/drivers/net/ethernet/chelsio/cxgb/sge.c
> b/drivers/net/ethernet/chelsio/cxgb/sge.c
> index d6df1a87db0b..f1c402f6b889 100644
> --- a/drivers/net/ethernet/chelsio/cxgb/sge.c
> +++ b/drivers/net/ethernet/chelsio/cxgb/sge.c
> @@ -1626,11 +1626,10 @@ int t1_poll(struct napi_struct *napi, int budget)
> return work_done;
> }
>
> -irqreturn_t t1_interrupt(int irq, void *data)
> +irqreturn_t t1_irq(int irq, void *data)
> {
> struct adapter *adapter = data;
> struct sge *sge = adapter->sge;
> - int handled;
>
> if (likely(responses_pending(adapter))) {
> writel(F_PL_INTR_SGE_DATA, adapter->regs + A_PL_CAUSE);
> @@ -1645,9 +1644,19 @@ irqreturn_t t1_interrupt(int irq, void *data)
> napi_enable(&adapter->napi);
> }
> }
> +
> return IRQ_HANDLED;
> }
>
> + return IRQ_WAKE_THREAD;
> +}
> +
> +irqreturn_t t1_irq_thread(int irq, void *data)
> +{
> + struct adapter *adapter = data;
> + struct sge *sge = adapter->sge;
> + int handled;
> +
> spin_lock(&adapter->async_lock);
> handled = t1_slow_intr_handler(adapter);
> spin_unlock(&adapter->async_lock);
This does not work in general, it might work in the MSI case but does
not work for the LEVEL interrupt case: The interrupt remains active
because it has not been ACKed. Chances are that the threaded handler
never gets scheduled because interrupt is still pending and t1_irq()
gets invoked right away.
For that reason, the primary must either mask the interrupt source or
use IRQF_ONESHOT to mask the interrupt line until the threaded handler
is done.
If you look at t1_elmer0_ext_intr() it disables F_PL_INTR_EXT before the
worker scheduled so the interrupt does not trigger again.
The worker then does what ever is needed (t1_elmer0_ext_intr_handler)
and then ACKs F_PL_INTR_EXT and enables F_PL_INTR_EXT again so it may
trigger an interrupt again.
Sebastian
