[Open-FCoE] [PATCH 28/28] libfc: fc_rport_unlock interacts with scsi_transport_fc

Robert Love robert.w.love at intel.com
Tue Sep 30 18:27:19 UTC 2008


There were many locking issues when calling functions provided
by the FC transport class. We cannot call _add with a lock held
becuase it will malloc. We also can't call _delete with our lock
held becuase it will free the memory allocated for the lock.

This patch moves all interaction with the transport class into
the unlock function of the rport. It also makes the event
member of the rport private data very important. It is that
event that will determine what should be reported to the
transport class. So, when we unlock the rport we determine
what just happened in the state machine and report it to the
OS.

Signed-off-by: Robert Love <robert.w.love at intel.com>
---

 drivers/scsi/libfc/fc_disc.c  |   16 ++--
 drivers/scsi/libfc/fc_lport.c |   58 ++++++-------
 drivers/scsi/libfc/fc_rport.c |  182 ++++++++++++++++++-----------------------
 include/scsi/libfc/libfc.h    |    9 +-
 4 files changed, 125 insertions(+), 140 deletions(-)

diff --git a/drivers/scsi/libfc/fc_disc.c b/drivers/scsi/libfc/fc_disc.c
index fc0da8f..30403aa 100644
--- a/drivers/scsi/libfc/fc_disc.c
+++ b/drivers/scsi/libfc/fc_disc.c
@@ -207,7 +207,6 @@ int fc_disc_start(struct fc_lport *lp)
 		if (!error)
 			fc_disc_done(lp);
 	} else {
-
 		fc_block_rports(lp);
 		fc_disc_gpn_ft_req(lp);	/* get ports by FC-4 type */
 		error = 0;
@@ -243,8 +242,8 @@ static void fc_disc_retry(struct fc_lport *lp)
  *  FC_EV_READY, when remote port is rediscovered.
  */
 static int fc_disc_new_target(struct fc_lport *lp,
-			    struct fc_rport *rport,
-			    struct fc_rport_identifiers *ids)
+			      struct fc_rport *rport,
+			      struct fc_rport_identifiers *ids)
 {
 	struct fc_rport_libfc_priv *rp;
 	int error = 0;
@@ -272,8 +271,15 @@ static int fc_disc_new_target(struct fc_lport *lp,
 	    ids->port_id != lp->fid && ids->port_name != lp->wwpn) {
 		if (!rport) {
 			rport = lp->tt.rport_lookup(lp, ids->port_id);
-			if (rport == NULL)
-				rport = lp->tt.rport_create(lp, ids);
+			if (!rport) {
+				struct fc_disc_port dp;
+				dp.lp = lp;
+				dp.ids.port_id = ids->port_id;
+				dp.ids.port_name = ids->port_name;
+				dp.ids.node_name = ids->node_name;
+				dp.ids.roles = ids->roles;
+				rport = fc_rport_dummy_create(&dp);
+			}
 			if (!rport)
 				error = ENOMEM;
 		}
diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
index f5eac8b..1d39ef9 100644
--- a/drivers/scsi/libfc/fc_lport.c
+++ b/drivers/scsi/libfc/fc_lport.c
@@ -115,6 +115,18 @@ static const char *fc_lport_state(struct fc_lport *lport)
 }
 
 /**
+ * fc_lport_ptp_clear - Delete the ptp rport
+ * @lport: The lport whose ptp rport should be removed
+ */
+static void fc_lport_ptp_clear(struct fc_lport *lport)
+{
+	if (lport->ptp_rp) {
+		fc_remote_port_delete(lport->ptp_rp);
+		lport->ptp_rp = NULL;
+	}
+}
+
+/**
  * fc_lport_ptp_setup - Create an rport for point-to-point mode
  * @lport: The lport to attach the ptp rport to
  * @fid: The FID of the ptp rport
@@ -125,40 +137,21 @@ static void fc_lport_ptp_setup(struct fc_lport *lport,
 			       u32 remote_fid, u64 remote_wwpn,
 			       u64 remote_wwnn)
 {
-	struct fc_rport *rport;
-	struct fc_rport_identifiers ids = {
-		.port_id = remote_fid,
-		.port_name = remote_wwpn,
-		.node_name = remote_wwnn,
-	};
+	struct fc_disc_port dp;
 
-	/*
-	 * if we have to create a rport the fc class can sleep so we must
-	 * drop the lock here
-	 */
-	rport = lport->tt.rport_lookup(lport, ids.port_id);
+	dp.lp = lport;
+	dp.ids.port_id = remote_fid;
+	dp.ids.port_name = remote_wwpn;
+	dp.ids.node_name = remote_wwnn;
+	dp.ids.roles = FC_RPORT_ROLE_UNKNOWN;
 
-	if (!rport)
-		rport = lport->tt.rport_create(lport, &ids);
+	fc_lport_ptp_clear(lport);
 
-	if (rport) {
-		if (lport->ptp_rp)
-			fc_remote_port_delete(lport->ptp_rp);
-		lport->ptp_rp = rport;
-		fc_lport_enter_ready(lport);
-	}
-}
+	lport->ptp_rp = fc_rport_dummy_create(&dp);
 
-/**
- * fc_lport_ptp_clear - Delete the ptp rport
- * @lport: The lport whose ptp rport should be removed
- */
-static void fc_lport_ptp_clear(struct fc_lport *lport)
-{
-	if (lport->ptp_rp) {
-		fc_remote_port_delete(lport->ptp_rp);
-		lport->ptp_rp = NULL;
-	}
+	lport->tt.rport_login(lport->ptp_rp);
+
+	fc_lport_enter_ready(lport);
 }
 
 /**
@@ -1129,6 +1122,7 @@ static void fc_lport_enter_rpn_id(struct fc_lport *lport)
 				     FC_FID_DIR_SERV,
 				     FC_FC_SEQ_INIT | FC_FC_END_SEQ))
 		fc_lport_error(lport, fp);
+
 }
 
 /**
@@ -1148,6 +1142,7 @@ static void fc_lport_enter_dns(struct fc_lport *lport)
 	dp.ids.port_name = -1;
 	dp.ids.node_name = -1;
 	dp.ids.roles = FC_RPORT_ROLE_UNKNOWN;
+	dp.lp = lport;
 
 	if (fc_lport_debug)
 		FC_DBG("Port (%6x) entered DNS state from %s state\n",
@@ -1162,12 +1157,11 @@ static void fc_lport_enter_dns(struct fc_lport *lport)
 		if (!rport)
 			goto err;
 		lport->dns_rp = rport;
+		FC_DBG("created an rport for the NS\n");
 	}
 
 	rport = lport->dns_rp;
 	rdata = rport->dd_data;
-	rdata->local_port = lport;
-
 	rdata->event_callback = fc_lport_rport_event;
 	lport->tt.rport_login(rport);
 	return;
diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
index d720143..67c9149 100644
--- a/drivers/scsi/libfc/fc_rport.c
+++ b/drivers/scsi/libfc/fc_rport.c
@@ -53,22 +53,6 @@ static void fc_rport_recv_logo_req(struct fc_rport *,
 static void fc_rport_timeout(struct work_struct *);
 static void fc_rport_error(struct fc_rport *, struct fc_frame *);
 
-static void fc_rport_unlock(struct fc_rport *rport)
-{
-	struct fc_rport_libfc_priv *rdata = rport->dd_data;
-	enum fc_lport_event event = rdata->event;
-	struct fc_lport *lport = rdata->local_port;
-	u32 fid = rport->port_id;
-	void (*event_callback)(struct fc_lport *, u32,
-			       enum fc_lport_event) =
-		rdata->event_callback;
-
-	mutex_unlock(&rdata->rp_mutex);
-
-	if (event != LPORT_EV_RPORT_NONE && event_callback)
-		event_callback(lport, fid, event);
-}
-
 static const char *fc_rport_state_names[] = {
 	[RPORT_ST_NONE] = "None",
 	[RPORT_ST_INIT] = "Init",
@@ -95,12 +79,17 @@ struct fc_rport *fc_rport_dummy_create(struct fc_disc_port *dp)
 	rport->port_name = dp->ids.port_name;
 	rport->node_name = dp->ids.node_name;
 	rport->roles = dp->ids.roles;
+	rport->maxframe_size = FC_MIN_MAX_PAYLOAD;
 
 	mutex_init(&rdata->rp_mutex);
 	rdata->local_port = dp->lp;
+	rdata->trans_state = FC_PORTSTATE_ROGUE;
 	rdata->rp_state = RPORT_ST_INIT;
 	rdata->event = LPORT_EV_RPORT_NONE;
 	rdata->flags = FC_RP_FLAGS_REC_SUPPORTED;
+	rdata->event_callback = NULL;
+	rdata->e_d_tov = dp->lp->e_d_tov;
+	rdata->r_a_tov = dp->lp->r_a_tov;
 	INIT_DELAYED_WORK(&rdata->retry_work, fc_rport_timeout);
 
 	return rport;
@@ -151,40 +140,6 @@ struct fc_rport *fc_rport_lookup(const struct fc_lport *lp, u32 fid)
 }
 
 /**
- * fc_remote_port_create - create a remote port
- * @lp: Fibre Channel host port instance
- * @ids: remote port identifiers (port_id, port_name, and node_name must be set)
- */
-static struct fc_rport *fc_remote_port_create(struct fc_lport *lport,
-					      struct fc_rport_identifiers *ids)
-{
-	struct fc_rport_libfc_priv *rdata;
-	struct fc_rport *rport;
-
-	rport = fc_remote_port_add(lport->host, 0, ids);
-	if (!rport)
-		return NULL;
-
-	rdata = rport->dd_data;
-	rdata->local_port = lport;
-
-	/* default value until service parameters are exchanged in PLOGI */
-	rport->maxframe_size = FC_MIN_MAX_PAYLOAD;
-
-	mutex_init(&rdata->rp_mutex);
-	rdata->rp_state = RPORT_ST_INIT;
-	rdata->event = LPORT_EV_RPORT_NONE;
-	rdata->event_callback = NULL;
-	rdata->local_port = lport;
-	rdata->e_d_tov = lport->e_d_tov;
-	rdata->r_a_tov = lport->r_a_tov;
-	rdata->flags = FC_RP_FLAGS_REC_SUPPORTED;
-	INIT_DELAYED_WORK(&rdata->retry_work, fc_rport_timeout);
-
-	return rport;
-}
-
-/**
  * fc_set_rport_loss_tmo - Set the remote port loss timeout in seconds.
  * @rport: Pointer to Fibre Channel remote port structure
  * @timeout: timeout in seconds
@@ -274,6 +229,76 @@ static void fc_rport_state_enter(struct fc_rport *rport,
 	rdata->rp_state = new;
 }
 
+static void fc_rport_unlock(struct fc_rport *rport)
+{
+	struct fc_rport_libfc_priv *rdata = rport->dd_data;
+	enum fc_lport_event event = rdata->event;
+	struct fc_lport *lport = rdata->local_port;
+	u32 fid = rport->port_id;
+	void (*event_callback)(struct fc_lport *, u32,
+			       enum fc_lport_event) =
+		rdata->event_callback;
+
+	if (event == LPORT_EV_RPORT_CREATED) {
+		struct fc_rport *new_rport;
+		struct fc_rport_libfc_priv *new_rdata;
+		struct fc_rport_identifiers ids;
+
+		ids.port_id = rport->port_id;
+		ids.roles = rport->roles;
+		ids.port_name = rport->port_name;
+		ids.node_name = rport->node_name;
+
+		new_rport = fc_remote_port_add(lport->host, 0, &ids);
+		if (new_rport) {
+			/*
+			 * Switch from the dummy rport to the rport
+			 * returned by the FC class.
+			 */
+			new_rport->maxframe_size = rport->maxframe_size;
+
+			new_rdata = new_rport->dd_data;
+			new_rdata->e_d_tov = rdata->e_d_tov;
+			new_rdata->r_a_tov = rdata->r_a_tov;
+			new_rdata->event_callback = rdata->event_callback;
+			new_rdata->local_port = rdata->local_port;
+			new_rdata->flags = FC_RP_FLAGS_REC_SUPPORTED;
+			new_rdata->trans_state = FC_PORTSTATE_REAL;
+			mutex_init(&new_rdata->rp_mutex);
+			INIT_DELAYED_WORK(&new_rdata->retry_work,
+					  fc_rport_timeout);
+
+			fc_rport_state_enter(new_rport, RPORT_ST_READY);
+			fc_remote_port_rolechg(new_rport, rdata->roles);
+		} else {
+			FC_DBG("Failed to create the rport for port "
+			       "(%6x).\n", ids.port_id);
+			event = LPORT_EV_RPORT_FAILED;
+		}
+
+		mutex_unlock(&rdata->rp_mutex);
+		fc_rport_dummy_destroy(rport);
+		rport = new_rport;
+		rdata = new_rport->dd_data;
+	} else if ((event == LPORT_EV_RPORT_FAILED) ||
+		   (event == LPORT_EV_RPORT_LOGO)) {
+		if (rdata->trans_state == FC_PORTSTATE_ROGUE) {
+			mutex_unlock(&rdata->rp_mutex);
+			fc_rport_dummy_destroy(rport);
+		} else {
+			mutex_unlock(&rdata->rp_mutex);
+			fc_remote_port_delete(rport);
+		}
+	} else {
+		mutex_unlock(&rdata->rp_mutex);
+	}
+
+	if (event != LPORT_EV_RPORT_NONE && event_callback) {
+		event_callback(lport, fid, event);
+		rdata->event = LPORT_EV_RPORT_NONE;
+	}
+}
+
 /**
  * fc_rport_login - Start the remote port login state machine
  * @rport: Fibre Channel remote port
@@ -381,7 +406,6 @@ static void fc_rport_enter_ready(struct fc_rport *rport)
 	if (fc_rp_debug)
 		FC_DBG("Port (%6x) is Ready\n", rport->port_id);
 
-	fc_remote_port_rolechg(rport, rdata->roles);
 	rdata->event = LPORT_EV_RPORT_CREATED;
 }
 
@@ -519,65 +543,27 @@ static void fc_rport_plogi_resp(struct fc_seq *sp, struct fc_frame *fp,
 	op = fc_frame_payload_op(fp);
 	if (op == ELS_LS_ACC &&
 	    (plp = fc_frame_payload_get(fp, sizeof(*plp))) != NULL) {
-		struct fc_rport_identifiers ids;
-		struct fc_rport *new_rport;
-		struct fc_rport_libfc_priv *new_rdata;
-
-		ids.port_id = rport->port_id;
-		ids.roles = rport->roles;
-		ids.port_name = get_unaligned_be64(&plp->fl_wwpn);
-		ids.node_name = get_unaligned_be64(&plp->fl_wwnn);
-
-		new_rport = lport->tt.rport_lookup(lport, ids.port_id);
-		if (!new_rport)
-			new_rport = lport->tt.rport_create(lport, &ids);
-
-		if (!new_rport) {
-			FC_DBG("Failed to create the rport for port "
-			       "(%6x).\n", ids.port_id);
-			fc_rport_dummy_destroy(rport);
-			goto out;
-		}
-
-		/*
-		 * Switch from the dummy rport to the rport
-		 * returned by the FC class.
-		 */
-		new_rdata = new_rport->dd_data;
-
-		mutex_lock(&new_rdata->rp_mutex);
-
-		new_rdata->event_callback = rdata->event_callback;
-		new_rdata->local_port = rdata->local_port;
-		fc_rport_state_enter(new_rport, RPORT_ST_PLOGI);
-
-		mutex_unlock(&rdata->rp_mutex);
-		fc_rport_dummy_destroy(rport);
-
 		tov = ntohl(plp->fl_csp.sp_e_d_tov);
 		if (ntohs(plp->fl_csp.sp_features) & FC_SP_FT_EDTR)
 			tov /= 1000;
 		if (tov > rdata->e_d_tov)
-			new_rdata->e_d_tov = tov;
+			rdata->e_d_tov = tov;
 		csp_seq = ntohs(plp->fl_csp.sp_tot_seq);
 		cssp_seq = ntohs(plp->fl_cssp[3 - 1].cp_con_seq);
 		if (cssp_seq < csp_seq)
 			csp_seq = cssp_seq;
-		new_rdata->max_seq = csp_seq;
-		new_rport->maxframe_size =
-			fc_plogi_get_maxframe(plp, new_rdata->local_port->mfs);
+		rdata->max_seq = csp_seq;
+		rport->maxframe_size =
+			fc_plogi_get_maxframe(plp, lport->mfs);
 
 		/*
 		 * If the rport is one of the well known addresses
 		 * we skip PRLI and RTV and go straight to READY.
 		 */
-		if (new_rport->port_id >= FC_FID_DOM_MGR)
-			fc_rport_enter_ready(new_rport);
+		if (rport->port_id >= FC_FID_DOM_MGR)
+			fc_rport_enter_ready(rport);
 		else
-			fc_rport_enter_prli(new_rport);
-
-		rdata = new_rdata;
-		rport = new_rport;
+			fc_rport_enter_prli(rport);
 	} else
 		fc_rport_error(rport, fp);
 
@@ -1328,7 +1314,6 @@ static void fc_rport_recv_logo_req(struct fc_rport *rport, struct fc_seq *sp,
 		       fc_rport_state(rport));
 
 	rdata->event = LPORT_EV_RPORT_LOGO;
-	fc_remote_port_delete(rport);
 
 	lport->tt.seq_els_rsp_send(sp, ELS_LS_ACC, NULL);
 	fc_frame_free(fp);
@@ -1345,9 +1330,6 @@ int fc_rport_init(struct fc_lport *lport)
 	if (!lport->tt.rport_recv_req)
 		lport->tt.rport_recv_req = fc_rport_recv_req;
 
-	if (!lport->tt.rport_create)
-		lport->tt.rport_create = fc_remote_port_create;
-
 	if (!lport->tt.rport_lookup)
 		lport->tt.rport_lookup = fc_rport_lookup;
 
diff --git a/include/scsi/libfc/libfc.h b/include/scsi/libfc/libfc.h
index 8d55c3a..740a0e0 100644
--- a/include/scsi/libfc/libfc.h
+++ b/include/scsi/libfc/libfc.h
@@ -117,6 +117,11 @@ enum fc_rport_state {
 	RPORT_ST_LOGO,		/* port logout sent */
 };
 
+enum fc_rport_trans_state {
+	FC_PORTSTATE_ROGUE,
+	FC_PORTSTATE_REAL,
+};
+
 /**
  * struct fc_disc_port - temporary discovery port to hold rport identifiers
  * @lp: Fibre Channel host port instance
@@ -154,6 +159,7 @@ struct fc_rport_libfc_priv {
 	unsigned int	retries;
 	unsigned int	e_d_tov;
 	unsigned int	r_a_tov;
+	enum fc_rport_trans_state trans_state;
 	struct mutex    rp_mutex;
 	struct delayed_work	retry_work;
 	enum fc_lport_event     event;
@@ -378,9 +384,6 @@ struct libfc_function_template {
 
 	struct fc_rport *(*rport_lookup)(const struct fc_lport *, u32);
 
-	struct fc_rport *(*rport_create)(struct fc_lport *,
-					 struct fc_rport_identifiers *);
-
 	void (*rport_reset)(struct fc_rport *);
 
 	void (*rport_reset_list)(struct fc_lport *);




More information about the devel mailing list