[SCSI] update fc_transport for removal of block/unblock functions

We recently went back to implement a board reset. When we perform the
reset, we wanted to tear down the internal data structures and rebuild
them. Unfortunately, when it came to the rport structure, things were
odd. If we deleted them, the scsi targets and sdevs would be
torn down. Not a good thing for a temporary reset. We could block the
rports, but we either maintain the internal structures to keep the
rport reference (perhaps even replicating what's in the transport),
or we have to fatten the fc transport with new search routines to find
the rport (and deal with a case of a dangling rport that the driver
forgets).

It dawned on me that we had actually reached this state incorrectly.
When the fc transport first started, we did the block/unblock first, then
added the rport interface. The purpose of block/unblock is to hide the
temporary disappearance of the rport (e.g. being deleted, then readded).
Why are we making the driver do the block/unblock ? We should be making
the transport have only an rport add/delete, and the let the transport
handle the block/unblock.

So... This patch removes the existing fc_remote_port_block/unblock
functions. It moves the block/unblock functionality into the
fc_remote_port_add/delete functions.  Updates for the lpfc driver are
included. Qlogic driver updates are also enclosed, thanks to the
contributions of Andrew Vasquez. [Note: the qla2xxx changes are
relative to the scsi-misc-2.6 tree as of this morning - which does
not include the recent patches sent by Andrew]. The zfcp driver does
not use the block/unblock functions.

One last comment: The resulting behavior feels very clean. The LLDD is
concerned only with add/delete, which corresponds to the physical
disappearance.  However, the fact that the scsi target and sdevs are
not immediately torn down after the LLDD calls delete causes an
interesting scenario... the midlayer can call the xxx_slave_alloc and
xxx_queuecommand functions with a sdev that is at the location the
rport used to be. The driver must validate the device exists when it
first enters these functions. In thinking about it, this has always
been the case for the LLDD and these routines. The existing drivers
already check for existence. However, this highlights that simple
validation via data structure dereferencing needs to be watched.
To deal with this, a new transport function, fc_remote_port_chkready()
was created that LLDDs should call when they first enter these two
routines. It validates the rport state, and returns a scsi result
which could be returned. In addition to solving the above, it also
creates consistent behavior from the LLDD's when the block and deletes
are occuring.

Rejections fixed up and
Signed-off-by: NJames Bottomley <James.Bottomley@SteelEye.com>

drivers/scsi/lpfc/lpfc_hbadisc.c

@@ -1028,6 +1028,7 @@ lpfc_register_remote_port(struct lpfc_hba * phba,
 	if (ndlp->nlp_type & NLP_FCP_INITIATOR)
 		rport_ids.roles |= FC_RPORT_ROLE_FCP_INITIATOR;
 
+	scsi_block_requests(phba->host);
 	ndlp->rport = rport = fc_remote_port_add(phba->host, 0, &rport_ids);
 	if (!rport) {
 		dev_printk(KERN_WARNING, &phba->pcidev->dev,
@@ -1044,6 +1045,23 @@ lpfc_register_remote_port(struct lpfc_hba * phba,
 	}
 	rdata = rport->dd_data;
 	rdata->pnode = ndlp;
+	scsi_unblock_requests(phba->host);
+
+	return;
+}
+
+static void
+lpfc_unregister_remote_port(struct lpfc_hba * phba,
+			    struct lpfc_nodelist * ndlp)
+{
+	struct fc_rport *rport = ndlp->rport;
+	struct lpfc_rport_data *rdata = rport->dd_data;
+
+	ndlp->rport = NULL;
+	rdata->pnode = NULL;
+	scsi_block_requests(phba->host);
+	fc_remote_port_delete(rport);
+	scsi_unblock_requests(phba->host);
 
 	return;
 }
@@ -1260,7 +1278,7 @@ lpfc_nlp_list(struct lpfc_hba * phba, struct lpfc_nodelist * nlp, int list)
 		 * may have removed the remote port.
 		 */
 		if ((rport_del != none) && nlp->rport)
-			fc_remote_port_block(nlp->rport);
+			lpfc_unregister_remote_port(phba, nlp);
 
 		if (rport_add != none) {
 			/*
@@ -1270,8 +1288,6 @@ lpfc_nlp_list(struct lpfc_hba * phba, struct lpfc_nodelist * nlp, int list)
 			 */
 			if (!nlp->rport)
 				lpfc_register_remote_port(phba, nlp);
-			else
-				fc_remote_port_unblock(nlp->rport);
 
 			/*
 			 * if we added to Mapped list, but the remote port
@@ -1490,7 +1506,6 @@ lpfc_freenode(struct lpfc_hba * phba, struct lpfc_nodelist * ndlp)
 	LPFC_MBOXQ_t       *mb;
 	LPFC_MBOXQ_t       *nextmb;
 	struct lpfc_dmabuf *mp;
-	struct fc_rport *rport;
 
 	/* Cleanup node for NPort <nlp_DID> */
 	lpfc_printf_log(phba, KERN_INFO, LOG_NODE,
@@ -1507,10 +1522,7 @@ lpfc_freenode(struct lpfc_hba * phba, struct lpfc_nodelist * ndlp)
 	 * and flush cache's w/o generating flush errors.
 	 */
 	if ((ndlp->rport) && !(phba->fc_flag & FC_UNLOADING)) {
-		rport = ndlp->rport;
-		ndlp->rport = NULL;
-		fc_remote_port_unblock(rport);
-		fc_remote_port_delete(rport);
+		lpfc_unregister_remote_port(phba, ndlp);
 		ndlp->nlp_sid = NLP_NO_SID;
 	}
 

drivers/scsi/lpfc/lpfc_init.c

@@ -537,12 +537,6 @@ lpfc_handle_eratt(struct lpfc_hba * phba)
 
 		lpfc_offline(phba);
 
-		/*
-		 * Restart all traffic to this host.  Since the fc_transport
-		 * block functions (future) were not called in lpfc_offline,
-		 * don't call them here.
-		 */
-		scsi_unblock_requests(phba->host);
 	}
 }
 
@@ -1226,12 +1220,6 @@ lpfc_online(struct lpfc_hba * phba)
 	phba->fc_flag &= ~FC_OFFLINE_MODE;
 	spin_unlock_irq(phba->host->host_lock);
 
-	/*
-	 * Restart all traffic to this host.  Since the fc_transport block
-	 * functions (future) were not called in lpfc_offline, don't call them
-	 * here.
-	 */
-	scsi_unblock_requests(phba->host);
 	return 0;
 }
 
@@ -1249,13 +1237,6 @@ lpfc_offline(struct lpfc_hba * phba)
 	if (phba->fc_flag & FC_OFFLINE_MODE)
 		return 0;
 
-	/*
-	 * Don't call the fc_transport block api (future).  The device is
-	 * going offline and causing a timer to fire in the midlayer is
-	 * unproductive.  Just block all new requests until the driver
-	 * comes back online.
-	 */
-	scsi_block_requests(phba->host);
 	psli = &phba->sli;
 	pring = &psli->ring[psli->fcp_ring];
 

drivers/scsi/lpfc/lpfc_scsi.c

@@ -403,14 +403,9 @@ lpfc_scsi_cmd_iocb_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *pIocbIn,
 			break;
 		}
 
-		if (pnode) {
-			if (pnode->nlp_state != NLP_STE_MAPPED_NODE)
-				cmd->result = ScsiResult(DID_BUS_BUSY,
-					SAM_STAT_BUSY);
-		}
-		else {
-			cmd->result = ScsiResult(DID_NO_CONNECT, 0);
-		}
+		if ((pnode == NULL )
+		    || (pnode->nlp_state != NLP_STE_MAPPED_NODE))
+			cmd->result = ScsiResult(DID_BUS_BUSY, SAM_STAT_BUSY);
 	} else {
 		cmd->result = ScsiResult(DID_OK, 0);
 	}
@@ -539,7 +534,7 @@ lpfc_scsi_prep_task_mgmt_cmd(struct lpfc_hba *phba,
 	struct lpfc_rport_data *rdata = scsi_dev->hostdata;
 	struct lpfc_nodelist *ndlp = rdata->pnode;
 
-	if ((ndlp == 0) || (ndlp->nlp_state != NLP_STE_MAPPED_NODE)) {
+	if ((ndlp == NULL) || (ndlp->nlp_state != NLP_STE_MAPPED_NODE)) {
 		return 0;
 	}
 
@@ -727,39 +722,23 @@ lpfc_queuecommand(struct scsi_cmnd *cmnd, void (*done) (struct scsi_cmnd *))
 	struct lpfc_rport_data *rdata = cmnd->device->hostdata;
 	struct lpfc_nodelist *ndlp = rdata->pnode;
 	struct lpfc_scsi_buf *lpfc_cmd = NULL;
+	struct fc_rport *rport = starget_to_rport(scsi_target(cmnd->device));
 	struct list_head *scsi_buf_list = &phba->lpfc_scsi_buf_list;
-	int err = 0;
+	int err;
 
-	/*
-	 * The target pointer is guaranteed not to be NULL because the driver
-	 * only clears the device->hostdata field in lpfc_slave_destroy.  This
-	 * approach guarantees no further IO calls on this target.
-	 */
-	if (!ndlp) {
-		cmnd->result = ScsiResult(DID_NO_CONNECT, 0);
+	err = fc_remote_port_chkready(rport);
+	if (err) {
+		cmnd->result = err;
 		goto out_fail_command;
 	}
 
 	/*
-	 * A Fibre Channel target is present and functioning only when the node
-	 * state is MAPPED.  Any other state is a failure.
+	 * Catch race where our node has transitioned, but the
+	 * transport is still transitioning.
 	 */
-	if (ndlp->nlp_state != NLP_STE_MAPPED_NODE) {
-		if ((ndlp->nlp_state == NLP_STE_UNMAPPED_NODE) ||
-		    (ndlp->nlp_state == NLP_STE_UNUSED_NODE)) {
-			cmnd->result = ScsiResult(DID_NO_CONNECT, 0);
-			goto out_fail_command;
-		}
-		else if (ndlp->nlp_state == NLP_STE_NPR_NODE) {
-			cmnd->result = ScsiResult(DID_BUS_BUSY, 0);
-			goto out_fail_command;
-		}
-		/*
-		 * The device is most likely recovered and the driver
-		 * needs a bit more time to finish.  Ask the midlayer
-		 * to retry.
-		 */
-		goto out_host_busy;
+	if (!ndlp) {
+		cmnd->result = ScsiResult(DID_BUS_BUSY, 0);
+		goto out_fail_command;
 	}
 
 	list_remove_head(scsi_buf_list, lpfc_cmd, struct lpfc_scsi_buf, list);
@@ -1163,44 +1142,16 @@ static int
 lpfc_slave_alloc(struct scsi_device *sdev)
 {
 	struct lpfc_hba *phba = (struct lpfc_hba *)sdev->host->hostdata[0];
-	struct lpfc_nodelist *ndlp = NULL;
-	int match = 0;
 	struct lpfc_scsi_buf *scsi_buf = NULL;
+	struct fc_rport *rport = starget_to_rport(scsi_target(sdev));
 	uint32_t total = 0, i;
 	uint32_t num_to_alloc = 0;
 	unsigned long flags;
-	struct list_head *listp;
-	struct list_head *node_list[6];
-
-	/*
-	 * Store the target pointer in the scsi_device hostdata pointer provided
-	 * the driver has already discovered the target id.
-	 */
-
-	/* Search the nlp lists other than unmap_list for this target ID */
-	node_list[0] = &phba->fc_npr_list;
-	node_list[1] = &phba->fc_nlpmap_list;
-	node_list[2] = &phba->fc_prli_list;
-	node_list[3] = &phba->fc_reglogin_list;
-	node_list[4] = &phba->fc_adisc_list;
-	node_list[5] = &phba->fc_plogi_list;
-
-	for (i = 0; i < 6 && !match; i++) {
-		listp = node_list[i];
-		if (list_empty(listp))
-			continue;
-		list_for_each_entry(ndlp, listp, nlp_listp) {
-			if ((sdev->id == ndlp->nlp_sid) && ndlp->rport) {
-				match = 1;
-				break;
-			}
-		}
-	}
 
-	if (!match)
+	if (!rport || fc_remote_port_chkready(rport))
 		return -ENXIO;
 
-	sdev->hostdata = ndlp->rport->dd_data;
+	sdev->hostdata = rport->dd_data;
 
 	/*
 	 * Populate the cmds_per_lun count scsi_bufs into this host's globally

drivers/scsi/qla2xxx/qla_def.h

@@ -21,6 +21,7 @@
 #include <linux/spinlock.h>
 #include <linux/completion.h>
 #include <linux/interrupt.h>
+#include <linux/workqueue.h>
 #include <asm/semaphore.h>
 
 #include <scsi/scsi.h>
@@ -1665,6 +1666,8 @@ typedef struct fc_port {
 
 	struct fc_rport *rport;
 	u32 supported_classes;
+	struct work_struct rport_add_work;
+	struct work_struct rport_del_work;
 } fc_port_t;
 
 /*

drivers/scsi/qla2xxx/qla_init.c

@@ -1668,6 +1668,24 @@ qla2x00_nvram_config(scsi_qla_host_t *ha)
 	return (rval);
 }
 
+static void
+qla2x00_rport_add(void *data)
+{
+	fc_port_t *fcport = data;
+
+	qla2x00_reg_remote_port(fcport->ha, fcport);
+}
+
+static void
+qla2x00_rport_del(void *data)
+{
+	fc_port_t *fcport = data;
+
+	if (fcport->rport)
+		fc_remote_port_delete(fcport->rport);
+	fcport->rport = NULL;
+}
+
 /**
  * qla2x00_alloc_fcport() - Allocate a generic fcport.
  * @ha: HA context
@@ -1693,6 +1711,8 @@ qla2x00_alloc_fcport(scsi_qla_host_t *ha, gfp_t flags)
 	atomic_set(&fcport->state, FCS_UNCONFIGURED);
 	fcport->flags = FCF_RLC_SUPPORT;
 	fcport->supported_classes = FC_COS_UNSPECIFIED;
+	INIT_WORK(&fcport->rport_add_work, qla2x00_rport_add, fcport);
+	INIT_WORK(&fcport->rport_del_work, qla2x00_rport_del, fcport);
 
 	return (fcport);
 }
@@ -2056,8 +2076,8 @@ qla2x00_reg_remote_port(scsi_qla_host_t *ha, fc_port_t *fcport)
 	struct fc_rport *rport;
 
 	if (fcport->rport) {
-		fc_remote_port_unblock(fcport->rport);
-		return;
+		fc_remote_port_delete(fcport->rport);
+		fcport->rport = NULL;
 	}
 
 	rport_ids.node_name = wwn_to_u64(fcport->node_name);
@@ -2071,7 +2091,7 @@ qla2x00_reg_remote_port(scsi_qla_host_t *ha, fc_port_t *fcport)
 		    "Unable to allocate fc remote port!\n");
 		return;
 	}
-	rport->dd_data = fcport;
+	*((fc_port_t **)rport->dd_data) = fcport;
 	rport->supported_classes = fcport->supported_classes;
 
 	rport_ids.roles = FC_RPORT_ROLE_UNKNOWN;

drivers/scsi/qla2xxx/qla_os.c

@@ -348,11 +348,13 @@ qla2x00_queuecommand(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *))
 {
 	scsi_qla_host_t *ha = to_qla_host(cmd->device->host);
 	fc_port_t *fcport = (struct fc_port *) cmd->device->hostdata;
+	struct fc_rport *rport = starget_to_rport(scsi_target(cmd->device));
 	srb_t *sp;
 	int rval;
 
-	if (!fcport) {
-		cmd->result = DID_NO_CONNECT << 16;
+	rval = fc_remote_port_chkready(rport);
+	if (rval) {
+		cmd->result = rval;
 		goto qc_fail_command;
 	}
 
@@ -401,11 +403,13 @@ qla24xx_queuecommand(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *))
 {
 	scsi_qla_host_t *ha = to_qla_host(cmd->device->host);
 	fc_port_t *fcport = (struct fc_port *) cmd->device->hostdata;
+	struct fc_rport *rport = starget_to_rport(scsi_target(cmd->device));
 	srb_t *sp;
 	int rval;
 
-	if (!fcport) {
-		cmd->result = DID_NO_CONNECT << 16;
+	rval = fc_remote_port_chkready(rport);
+	if (rval) {
+		cmd->result = rval;
 		goto qc24_fail_command;
 	}
 
@@ -1041,10 +1045,10 @@ qla2xxx_slave_alloc(struct scsi_device *sdev)
 {
 	struct fc_rport *rport = starget_to_rport(scsi_target(sdev));
 
-	if (!rport)
+	if (!rport || fc_remote_port_chkready(rport))
 		return -ENXIO;
 
-	sdev->hostdata = rport->dd_data;
+	sdev->hostdata = *(fc_port_t **)rport->dd_data;
 
 	return 0;
 }
@@ -1636,7 +1640,8 @@ void qla2x00_mark_device_lost(scsi_qla_host_t *ha, fc_port_t *fcport,
     int do_login)
 {
 	if (atomic_read(&fcport->state) == FCS_ONLINE && fcport->rport)
-		fc_remote_port_block(fcport->rport);
+		schedule_work(&fcport->rport_del_work);
+
 	/*
 	 * We may need to retry the login, so don't change the state of the
 	 * port but do the retries.
@@ -1697,7 +1702,7 @@ qla2x00_mark_all_devices_lost(scsi_qla_host_t *ha)
 		if (atomic_read(&fcport->state) == FCS_DEVICE_DEAD)
 			continue;
 		if (atomic_read(&fcport->state) == FCS_ONLINE && fcport->rport)
-			fc_remote_port_block(fcport->rport);
+			schedule_work(&fcport->rport_del_work);
 		atomic_set(&fcport->state, FCS_DEVICE_LOST);
 	}
 }

drivers/scsi/qla2xxx/qla_rscn.c

@@ -320,8 +320,7 @@ qla2x00_update_login_fcport(scsi_qla_host_t *ha, struct mbx_entry *mbxstat,
 	fcport->flags &= ~FCF_FAILOVER_NEEDED;
 	fcport->iodesc_idx_sent = IODESC_INVALID_INDEX;
 	atomic_set(&fcport->state, FCS_ONLINE);
-	if (fcport->rport)
-		fc_remote_port_unblock(fcport->rport);
+	schedule_work(&fcport->rport_add_work);
 }
 
 

include/scsi/scsi_transport_fc.h

@@ -28,6 +28,7 @@
 #define SCSI_TRANSPORT_FC_H
 
 #include <linux/config.h>
+#include <scsi/scsi.h>
 
 struct scsi_transport_template;
 
@@ -429,6 +430,34 @@ struct fc_function_template {
 };
 
 
+/**
+ * fc_remote_port_chkready - called to validate the remote port state
+ *   prior to initiating io to the port.
+ *
+ * Returns a scsi result code that can be returned by the LLDD.
+ *
+ * @rport:	remote port to be checked
+ **/
+static inline int
+fc_remote_port_chkready(struct fc_rport *rport)
+{
+	int result;
+
+	switch (rport->port_state) {
+	case FC_PORTSTATE_ONLINE:
+		result = 0;
+		break;
+	case FC_PORTSTATE_BLOCKED:
+		result = DID_BUS_BUSY << 16;
+		break;
+	default:
+		result = DID_NO_CONNECT << 16;
+		break;
+	}
+	return result;
+}
+
+
 struct scsi_transport_template *fc_attach_transport(
 			struct fc_function_template *);
 void fc_release_transport(struct scsi_transport_template *);
@@ -437,8 +466,6 @@ struct fc_rport *fc_remote_port_add(struct Scsi_Host *shost,
 			int channel, struct fc_rport_identifiers  *ids);
 void fc_remote_port_delete(struct fc_rport  *rport);
 void fc_remote_port_rolechg(struct fc_rport  *rport, u32 roles);
-int fc_remote_port_block(struct fc_rport *rport);
-void fc_remote_port_unblock(struct fc_rport *rport);
 int scsi_is_fc_rport(const struct device *);
 
 static inline u64 wwn_to_u64(u8 *wwn)