Re: [PATCH v3 3/8] ibmvfc: make ibmvfc login to fabric

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] A recursive spinlock deadlock occurs if `ibmvfc_send_event()` fails to send the newly allocated Fabric Login MAD event.
--

commit f65f56e7064cfb403146e98d083f3a2365344ff8
Author: Dave Marquardt <davemarq@linux.ibm.com>

ibmvfc: make ibmvfc login to fabric

Add support for fabric login in order to support the asynchronous
event queue with its own interrupt as required by NPIV specification
to support the asynchronous sub-queue and interrupt and full and
extended FPIN messages.

> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index 110d77b9aac32..6dd1e726d9b94 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c

[ ... ]

> @@ -5261,6 +5261,88 @@ static void ibmvfc_discover_targets(struct ibmvfc_host *vhost)

[ ... ]

> +static void ibmvfc_fabric_login(struct ibmvfc_host *vhost)
> +{
> +	struct ibmvfc_fabric_login *mad;
> +	struct ibmvfc_event *evt;
> +	int level = IBMVFC_DEFAULT_LOG_LEVEL;

[ ... ]

> +	ibmvfc_init_event(evt, ibmvfc_fabric_login_done, IBMVFC_MAD_FORMAT);
> +	mad = &evt->iu.fabric_login;
> +	memset(mad, 0, sizeof(*mad));
> +	mad->common.opcode = cpu_to_be32(IBMVFC_FABRIC_LOGIN);
> +	mad->common.version = cpu_to_be32(1);
> +	mad->common.length = cpu_to_be16(sizeof(*mad));
> +
> +	ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_INIT_WAIT);
> +
> +	if (ibmvfc_send_event(evt, vhost, default_timeout))
[Severity: High]
This is a pre-existing issue, but does this replicate a recursive spinlock
deadlock pattern if the event transmission fails?

If ibmvfc_send_event() encounters a failure (such as a hypervisor error), its
error path synchronously calls evt->done(evt).

Because this is initialized as a MAD event, the callback is handled by
ibmvfc_locked_done(), which unconditionally attempts to acquire
vhost->host->host_lock.

When called from an event completion handler like ibmvfc_channel_setup_done()
which already holds the host lock, could this sequence occur:

ibmvfc_channel_setup_done() (holds host_lock)
  -> ibmvfc_fabric_login()
    -> ibmvfc_send_event()
      -> evt->done(evt) (which is ibmvfc_locked_done)
        -> spin_lock_irqsave(evt->vhost->host->host_lock, flags)

and cause the CPU to deadlock?

> +		ibmvfc_link_down(vhost, IBMVFC_LINK_DOWN);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702-ibmvfc-fpin-support-v3-0-d95b9547cf88@linux.ibm.com?part=3