Re: [PATCH v2 09/33] ibmvfc: alloc/dealloc sub-queues for nvme channels

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] NVMe sub-queue registration failure unintentionally disables SCSI multiqueue support.
- [Medium] Incomplete integration of NVMe sub-CRQs and targets in host lifecycle and work scheduling routines.
--

--- Patch [9]: [PATCH v2 09/33] ibmvfc: alloc/dealloc sub-queues for nvme channels ---
commit bf86ac3ca0d20bb114b63c6bb20bb9e08b7ed641
Author: Tyrel Datwyler <tyreld@linux.ibm.com>

ibmvfc: alloc/dealloc sub-queues for nvme channels

This commit updates the ibmvfc driver to allocate, register, deregister, and
release NVMe subordinate CRQs alongside the existing SCSI sub-CRQs. It
extends the CRQ reset and re-enable paths to handle NVMe channels during
adapter setup and teardown.

> This keeps the NVMe queue lifecycle aligned with the existing SCSI queue
> lifecycle so both protocols are reset consistently across probe, remove,
> and connection recovery.

[Severity: Medium]
Is this alignment complete? Looking at the rest of the host lifecycle,
several routines like ibmvfc_purge_requests() only iterate over the SCSI
queues:

drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_purge_requests() {
    ...
	struct ibmvfc_queue *queues = vhost->scsi_scrqs.scrqs;
    ...
}

Similarly, ibmvfc_link_down() and ibmvfc_dev_init_to_do() appear to abandon
NVMe targets by only checking scsi_scrqs:

drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_link_down() {
    ...
	list_for_each_entry(tgt, &vhost->scsi_scrqs.targets, queue)
		ibmvfc_del_tgt(tgt);
    ...
}

Will this cause pending I/O or targets on nvme_scrqs to be missed during
connection recovery or adapter teardown events?

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index 5b6570232df36..d8c2b56258eeb 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c

[ ... ]

> @@ -6119,6 +6123,13 @@ static void ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
>  
>  	ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs);
>  
> +	if (vhost->nvme_enabled) {
> +		if (ibmvfc_alloc_channels(vhost, &vhost->nvme_scrqs))
> +			vhost->nvme_enabled = 0;
> +		else
> +			ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs);

[Severity: High]
If NVMe channel registration fails here, will it unintentionally disable SCSI
multiqueue support?

Looking at ibmvfc_reg_sub_crqs(), if channel registration fails, it clears
the global do_enquiry flag:

drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_reg_sub_crqs() {
    ...
	for (i = 0; i < channels->max_queues; i++) {
		if (ibmvfc_register_channel(vhost, channels, i)) {
			for (j = i; j > 0; j--)
				ibmvfc_deregister_channel(vhost, channels, j - 1);
			vhost->do_enquiry = 0;
			return;
		}
	}
    ...
}

Since ibmvfc_reg_sub_crqs() is called for the SCSI channels right before
the NVMe channels, setting do_enquiry to 0 upon an NVMe failure would
prevent the driver from sending IBMVFC_CHANNEL_ENQUIRY and
IBMVFC_CHANNEL_SETUP during NPIV login.

Does this cause the SCSI protocol to silently fall back to single-queue mode
if a user attempts to enable NVMe on a hypervisor that lacks support or
resources for additional sub-queues?

> +	}
> +
>  	LEAVE;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723000149.969416-1-tyreld@linux.ibm.com?part=9