Re: [PATCH 18/29] ibmvfc: update state machine to process NVMe/FC targets

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] NVMe targets leak memory and become permanently stranded because their cleanup logic is omitted from the target deletion phase in ibmvfc_do_work().
- [High] Asynchronous ELS events (LOGO, PRLO, PLOGI) from NVMe targets are ignored because the event handler only loops over the SCSI target list.
--

commit 2221ef8b59f01b903ca3e2cc56b9bd27be8a9dc1
Author: Tyrel Datwyler <tyreld@linux.ibm.com>

ibmvfc: update state machine to process NVMe/FC targets

Update the host work loop and target state-machine helpers to process
NVMe targets in addition to SCSI targets.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index 13e513bfd0a86..9a6a885aa57ed 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
[ ... ]
> @@ -5962,6 +5992,12 @@ static void ibmvfc_do_work(struct ibmvfc_host *vhost)
>  				break;
>  			}
>  		}
> +		list_for_each_entry(tgt, &vhost->nvme_scrqs.targets, queue) {
> +			if (tgt->action == IBMVFC_TGT_ACTION_LOGOUT_RPORT) {
> +				tgt->job_step(tgt);
> +				break;
> +			}
> +		}
>  
>  		if (ibmvfc_dev_logo_to_do(vhost)) {
>  			spin_unlock_irqrestore(vhost->host->host_lock, flags);

[Severity: High]
Does this code also need to loop over nvme_scrqs.targets for final target
deletion?

Looking further down in ibmvfc_do_work(), there is a loop handling
IBMVFC_TGT_ACTION_DEL_RPORT and IBMVFC_TGT_ACTION_DEL_AND_LOGOUT_RPORT:

		list_for_each_entry(tgt, &vhost->scsi_scrqs.targets, queue) {
			if (tgt->action == IBMVFC_TGT_ACTION_DEL_RPORT) {
				tgt_dbg(tgt, "Deleting rport\n");
				rport = tgt->rport;
				tgt->rport = NULL;
				list_del(&tgt->queue);
				ibmvfc_set_tgt_action(tgt, IBMVFC_TGT_ACTION_DELETED_RPORT);
				...

Since this loop only processes scsi_scrqs.targets, wouldn't NVMe targets in
the DEL_RPORT state be leaked and stranded?

[ ... ]

[Severity: High]
Are asynchronous ELS events from NVMe targets being ignored?

Looking at ibmvfc_handle_async(), which processes events like LOGO, PRLO,
and PLOGI, the loop only iterates over the SCSI targets:

	case IBMVFC_AE_ELS_LOGO:
	case IBMVFC_AE_ELS_PRLO:
	case IBMVFC_AE_ELS_PLOGI:
		list_for_each_entry(tgt, &vhost->scsi_scrqs.targets, queue) {

Since the NVMe targets are now in a separate nvme_scrqs.targets list,
wouldn't an explicit logout or state change from an NVMe target be dropped,
leaving the driver with stale state?

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