Re: [PATCH v2] scsi: be2iscsi: Replace irq_poll with threaded IRQ handler.

On 2021-10-29 14:41:53 [+0100], John Garry wrote:

quoted

@@ -713,9 +752,22 @@ static irqreturn_t be_isr_msix(int irq, void *dev_id)
  	phba = pbe_eq->phba;
  	/* disable interrupt till iopoll completes */
  	hwi_ring_eq_db(phba, eq->id, 1,	0, 0, 1);
-	irq_poll_sched(&pbe_eq->iopoll);
-	return IRQ_HANDLED;
+	return IRQ_WAKE_THREAD;
+}
+

@@ -819,9 +873,10 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
  				goto free_msix_irqs;
  			}
-			ret = request_irq(pci_irq_vector(pcidev, i),
-					  be_isr_msix, 0, phba->msi_name[i],
-					  &phwi_context->be_eq[i]);
+			ret = request_threaded_irq(pci_irq_vector(pcidev, i),
+						   be_isr_msix, be_isr_misx_th,
+						   0, phba->msi_name[i],
+						   &phwi_context->be_eq[i]);

Would it be sensible to set ONESHOT flag here? I assume that we don't want
the hard irq handler firing continuously while we poll completions in the
threaded handler. That's my understanding of how ONESHOT should or would
work... they currently seem to manually disable in be_isr_msix() ->
hwi_ring_eq_db() for the same purpose, I guess.

We could. My understanding of hwi_ring_eq_db() of is that it disables
the interrupt from on HW level. This is needed _already_ in order to
continue to process the request later in softirq without the interrupt
constantly firing.
For the MSI interrupts this could be replaced "easily" and leaving the
primary handler empty. However the non-MSI interrupts may be shared and
here all handlers need the ONESHOT which may not be the case.

Thanks,
John

Sebastian