Re: [PATCH] scsi: be2iscsi: use rollback index when freeing MSI-X vectors

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Flawed MSI-X fallback logic completely breaks the driver on MSI-X allocation failure and disables multiqueue on success.
--

--- Patch [1]: [PATCH] scsi: be2iscsi: use rollback index when freeing MSI-X vectors ---
commit 8e4e7a262620af574aa50d93aa71fefbc719c7d1
Author: Runyu Xiao <[email protected]>

scsi: be2iscsi: use rollback index when freeing MSI-X vectors
    
beiscsi_init_irqs() unwinds previously registered MSI-X handlers with
free_irq(pci_irq_vector(pcidev, i), &phwi_context->be_eq[j]). The IRQ
number should match the rollback slot j, not the current loop index i.
Using i passes the wrong IRQ/dev_id pair and leaves earlier handlers
registered on failure.

> diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
> index fd18d4d3d219b..9833ab06b299a 100644
> --- a/drivers/scsi/be2iscsi/be_main.c
> +++ b/drivers/scsi/be2iscsi/be_main.c
> @@ -878,7 +878,7 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
>  	return 0;
>  free_msix_irqs:
>  	for (j = i - 1; j >= 0; j--) {
> -		free_irq(pci_irq_vector(pcidev, i), &phwi_context->be_eq[j]);
> +		free_irq(pci_irq_vector(pcidev, j), &phwi_context->be_eq[j]);
>  		kfree(phba->msi_name[j]);
>  	}
>  	return ret;

[Severity: High]
This isn't a bug introduced by this patch, but the MSI-X fallback logic
completely breaks the driver on MSI-X allocation failure and disables
multiqueue on success.

In be2iscsi_enable_msix(), if pci_alloc_irq_vectors_affinity() fails (returns
< 0), the code incorrectly sets phba->num_cpus = nvec - 1:

drivers/scsi/be2iscsi/be_main.c:be2iscsi_enable_msix() {
    ...
		if (pci_alloc_irq_vectors_affinity(phba->pcidev, 2, nvec,
				PCI_IRQ_MSIX | PCI_IRQ_AFFINITY, &desc) < 0) {
			phba->num_cpus = nvec - 1;
			return;
		}
	}

	phba->num_cpus = 1;
}

The driver then falls back to INTx and provisions multiple event queues, but
the INTx handler be_isr() is hardcoded to only process be_eq[0]:

drivers/scsi/be2iscsi/be_main.c:be_isr() {
    ...
	pbe_eq = &phwi_context->be_eq[0];

	eq = &phwi_context->be_eq[0].q;
    ...
}

Any I/O submitted to other queues will time out and hang. Conversely, if
allocation succeeds (returns > 0), the code skips the error branch and
unconditionally executes phba->num_cpus = 1, artificially restricting the
driver to 1 I/O queue and effectively disabling multiqueue.

Is it possible to correct the logic so the driver does not hang on fallback
and properly utilizes multiqueue on success?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1