Thread (21 messages) flat view 21 messages, 1 author, 7d ago
COOLING7d

Revision v2 of 2 in this series.

Revisions (2)
  1. v1 [diff vs current]
  2. v2 current

[PATCH v2 20/20] scsi: ibmvfc: fix concurrent SCSI and NVMe discover-targets race dropping targets

From: Tyrel Datwyler <tyreld@linux.ibm.com>
Date: 2026-09-19 01:33:34
Also in: linux-scsi, lkml
Subsystem: ibm power virtual fc device drivers, linux for powerpc (32-bit and 64-bit), scsi subsystem, the rest · Maintainers: Tyrel Datwyler, Madhavan Srinivasan, "James E.J. Bottomley", "Martin K. Petersen", Linus Torvalds 

Commit 5e9dd03726ad ("scsi: ibmvfc: send NVMe target discovery MAD")
introduced concurrent SCSI and NVMe discover-target MADs.
ibmvfc_discover_targets_done() unconditionally set
IBMVFC_HOST_ACTION_ALLOC_TGTS on the first completion to arrive.
ibmvfc_alloc_targets() ran immediately and read both channels'
disc_buf and num_targets fields while the slower protocol's MAD was
still in flight. The slower protocol's num_targets remained zero from
the previous cycle, silently dropping all targets for that protocol.
The second completion attempted to transition to ALLOC_TGTS again but
the state machine had already advanced, so the transition was a no-op
and ibmvfc_alloc_targets() was never re-run.

Fix this by serializing discovery: send the SCSI discover-targets MAD
first and wait for its completion before sending the NVMe MAD.
ibmvfc_discover_scsi_targets_done() advances the job_step to
ibmvfc_discover_nvme_targets() when NVMe is active, or transitions
directly to IBMVFC_HOST_ACTION_ALLOC_TGTS when it is not.
ibmvfc_discover_nvme_targets_done() then transitions to ALLOC_TGTS
after the NVMe buffer is fully written. This guarantees that
ibmvfc_alloc_targets() is only called after both discovery buffers
are complete with no shared counter and no concurrent MADs in flight.

Fixes: 5e9dd03726ad ("scsi: ibmvfc: send NVMe target discovery MAD")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvfc-core.c | 115 +++++++++++++++++++---------
 1 file changed, 79 insertions(+), 36 deletions(-)

diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index ca9a0437fc0c..46d6b94cb96d 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -184,6 +184,7 @@ static void ibmvfc_tgt_send_prli(struct ibmvfc_target *);
 static void ibmvfc_tgt_send_plogi(struct ibmvfc_target *);
 static void ibmvfc_tgt_query_target(struct ibmvfc_target *);
 static void ibmvfc_npiv_logout(struct ibmvfc_host *);
+static void ibmvfc_discover_nvme_targets(struct ibmvfc_host *);
 static void ibmvfc_tgt_implicit_logout_and_del(struct ibmvfc_target *);
 static void ibmvfc_tgt_move_login(struct ibmvfc_target *);
 
@@ -5137,45 +5138,80 @@ static int ibmvfc_alloc_targets(struct ibmvfc_host *vhost)
 }
 
 /**
- * ibmvfc_discover_targets_done - Completion handler for discover targets MAD
+ * ibmvfc_discover_scsi_targets_done - Completion handler for SCSI discover targets MAD
  * @evt:	ibmvfc event struct
  *
  **/
-static void ibmvfc_discover_targets_done(struct ibmvfc_event *evt)
+static void ibmvfc_discover_scsi_targets_done(struct ibmvfc_event *evt)
 {
 	struct ibmvfc_host *vhost = evt->vhost;
 	struct ibmvfc_discover_targets *rsp = &evt->xfer_iu->discover_targets;
-	struct ibmvfc_channels *channels;
 	u32 mad_status = be16_to_cpu(rsp->common.status);
-	u32 opcode = be32_to_cpu(rsp->common.opcode);
 	int level = IBMVFC_DEFAULT_LOG_LEVEL;
 
-	if (opcode == IBMVFC_DISC_TARGETS)
-		channels = &vhost->scsi_scrqs;
-	else
-		channels = &vhost->nvme_scrqs;
+	switch (mad_status) {
+	case IBMVFC_MAD_SUCCESS:
+		ibmvfc_dbg(vhost, "Discover SCSI Targets succeeded\n");
+		vhost->scsi_scrqs.num_targets = min_t(u32, be32_to_cpu(rsp->num_written),
+						      max_targets);
+		ibmvfc_dbg(vhost, "%d SCSI targets found\n",
+			   vhost->scsi_scrqs.num_targets);
+		if (ibmvfc_nvme_active(vhost)) {
+			ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_INIT);
+			vhost->job_step = ibmvfc_discover_nvme_targets;
+		} else {
+			ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_ALLOC_TGTS);
+		}
+		break;
+	case IBMVFC_MAD_FAILED:
+		level += ibmvfc_retry_host_init(vhost);
+		ibmvfc_log(vhost, level, "Discover SCSI Targets failed: %s (%x:%x)\n",
+			   ibmvfc_get_cmd_error(be16_to_cpu(rsp->status), be16_to_cpu(rsp->error)),
+			   be16_to_cpu(rsp->status), be16_to_cpu(rsp->error));
+		break;
+	case IBMVFC_MAD_DRIVER_FAILED:
+		break;
+	default:
+		dev_err(vhost->dev, "Invalid Discover SCSI Targets response: 0x%x\n", mad_status);
+		ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
+		break;
+	}
+
+	ibmvfc_free_event(evt);
+	wake_up(&vhost->work_wait_q);
+}
+
+/**
+ * ibmvfc_discover_nvme_targets_done - Completion handler for NVMe discover targets MAD
+ * @evt:	ibmvfc event struct
+ *
+ **/
+static void ibmvfc_discover_nvme_targets_done(struct ibmvfc_event *evt)
+{
+	struct ibmvfc_host *vhost = evt->vhost;
+	struct ibmvfc_discover_targets *rsp = &evt->xfer_iu->discover_targets;
+	u32 mad_status = be16_to_cpu(rsp->common.status);
+	int level = IBMVFC_DEFAULT_LOG_LEVEL;
 
 	switch (mad_status) {
 	case IBMVFC_MAD_SUCCESS:
-		ibmvfc_dbg(vhost, "Discover %s Targets succeeded\n",
-			   proto_type[channels->protocol]);
-		channels->num_targets = min_t(u32, be32_to_cpu(rsp->num_written),
-					      max_targets);
-		ibmvfc_dbg(vhost, "%d %s targets found\n", channels->num_targets,
-			   proto_type[channels->protocol]);
+		ibmvfc_dbg(vhost, "Discover NVMe Targets succeeded\n");
+		vhost->nvme_scrqs.num_targets = min_t(u32, be32_to_cpu(rsp->num_written),
+						      max_targets);
+		ibmvfc_dbg(vhost, "%d NVMe targets found\n",
+			   vhost->nvme_scrqs.num_targets);
 		ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_ALLOC_TGTS);
 		break;
 	case IBMVFC_MAD_FAILED:
 		level += ibmvfc_retry_host_init(vhost);
-		ibmvfc_log(vhost, level, "Discover %s Targets failed: %s (%x:%x)\n",
-			   proto_type[channels->protocol],
+		ibmvfc_log(vhost, level, "Discover NVMe Targets failed: %s (%x:%x)\n",
 			   ibmvfc_get_cmd_error(be16_to_cpu(rsp->status), be16_to_cpu(rsp->error)),
 			   be16_to_cpu(rsp->status), be16_to_cpu(rsp->error));
 		break;
 	case IBMVFC_MAD_DRIVER_FAILED:
 		break;
 	default:
-		dev_err(vhost->dev, "Invalid Discover Targets response: 0x%x\n", mad_status);
+		dev_err(vhost->dev, "Invalid Discover NVMe Targets response: 0x%x\n", mad_status);
 		ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
 		break;
 	}
@@ -5184,7 +5220,8 @@ static void ibmvfc_discover_targets_done(struct ibmvfc_event *evt)
 	wake_up(&vhost->work_wait_q);
 }
 
-static struct ibmvfc_event *ibmvfc_get_disc_event(struct ibmvfc_channels *channels)
+static struct ibmvfc_event *ibmvfc_get_disc_event(struct ibmvfc_channels *channels,
+						  void (*done)(struct ibmvfc_event *))
 {
 	struct ibmvfc_discover_targets *mad;
 	struct ibmvfc_host *vhost = ibmvfc_channels_to_vhost(channels);
@@ -5193,7 +5230,7 @@ static struct ibmvfc_event *ibmvfc_get_disc_event(struct ibmvfc_channels *channe
 	if (!evt)
 		return NULL;
 
-	ibmvfc_init_event(evt, ibmvfc_discover_targets_done, IBMVFC_MAD_FORMAT);
+	ibmvfc_init_event(evt, done, IBMVFC_MAD_FORMAT);
 	mad = &evt->iu.discover_targets;
 	memset(mad, 0, sizeof(*mad));
 	mad->common.version = cpu_to_be32(1);
@@ -5211,17 +5248,18 @@ static struct ibmvfc_event *ibmvfc_get_disc_event(struct ibmvfc_channels *channe
 }
 
 /**
- * ibmvfc_discover_targets - Send Discover Targets MAD
+ * ibmvfc_discover_nvme_targets - Send Discover NVMe Targets MAD
  * @vhost:	ibmvfc host struct
  *
  **/
-static void ibmvfc_discover_targets(struct ibmvfc_host *vhost)
+static void ibmvfc_discover_nvme_targets(struct ibmvfc_host *vhost)
 {
-	struct ibmvfc_event *evt = ibmvfc_get_disc_event(&vhost->scsi_scrqs);
+	struct ibmvfc_event *evt = ibmvfc_get_disc_event(&vhost->nvme_scrqs,
+							 ibmvfc_discover_nvme_targets_done);
 	int level = IBMVFC_DEFAULT_LOG_LEVEL;
 
 	if (!evt) {
-		ibmvfc_log(vhost, level, "Discover SCSI Targets failed: no available events\n");
+		ibmvfc_log(vhost, level, "Discover NVMe Targets failed: no available events\n");
 		ibmvfc_hard_reset_host(vhost);
 		return;
 	}
@@ -5229,29 +5267,34 @@ static void ibmvfc_discover_targets(struct ibmvfc_host *vhost)
 	ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_INIT_WAIT);
 
 	if (!ibmvfc_send_event(evt, vhost, default_timeout))
-		ibmvfc_dbg(vhost, "Sent discover SCSI targets\n");
+		ibmvfc_dbg(vhost, "Sent discover NVMe targets\n");
 	else
-		goto link_down;
+		ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
+}
 
-	if (!ibmvfc_nvme_active(vhost))
-		return;
+/**
+ * ibmvfc_discover_targets - Send Discover SCSI Targets MAD
+ * @vhost:	ibmvfc host struct
+ *
+ **/
+static void ibmvfc_discover_targets(struct ibmvfc_host *vhost)
+{
+	struct ibmvfc_event *evt = ibmvfc_get_disc_event(&vhost->scsi_scrqs,
+							 ibmvfc_discover_scsi_targets_done);
+	int level = IBMVFC_DEFAULT_LOG_LEVEL;
 
-	evt = ibmvfc_get_disc_event(&vhost->nvme_scrqs);
 	if (!evt) {
-		ibmvfc_log(vhost, level, "Discover NVMe Targets failed: no available events\n");
+		ibmvfc_log(vhost, level, "Discover SCSI Targets failed: no available events\n");
 		ibmvfc_hard_reset_host(vhost);
 		return;
 	}
 
+	ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_INIT_WAIT);
+
 	if (!ibmvfc_send_event(evt, vhost, default_timeout))
-		ibmvfc_dbg(vhost, "Sent discover NVMe targets\n");
+		ibmvfc_dbg(vhost, "Sent discover SCSI targets\n");
 	else
-		goto link_down;
-
-	return;
-
-link_down:
-	ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
+		ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
 }
 
 static void ibmvfc_fabric_login_nvme_done(struct ibmvfc_event *evt)
-- 
2.55.0
Other recent patches touching these files
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help