[Open-FCoE] [PATCH 12/28] libfc: Rework the rport locking

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


There wasn't a clear locking strategy for rports. This patch attempts to
classify functions so that we can have an rport locking strategy that is
easy to understand. This patch re-does the locking according to the following
categorization, comments all relevant functions and converts irq safe spin
locking to mutex locking.

I do not beleive that this patch resolves all issues with rport managment.
There are still issues related to the interaction with the scsi_transport_fc
that must be resolved at another time.

These are the groupings used to determine when locks should be held.

Action Funtions (called without lock, will lock, call enter_* function and unlock)
 fc_rport_gpn_id_resp
 fc_rport_gnn_id_resp
 fc_rport_plogi_resp
 fc_rport_prli_resp
 fc_rport_rtv_resp
 fc_rport_logo_resp

 fc_rport_login
 fc_rport_logout
 fc_rport_reset
 fc_rport_timeout

 fc_rport_recv_req
        - Calls one of the request handlers with the lock held

Request Handlers (need lock held before calling)
 fc_rport_recv_plogi_req
 fc_rport_recv_prli_req
 fc_rport_recv_prlo_req
 fc_rport_recv_logo_req

Enter Funtions (need lock held before calling)
 fc_rport_enter_gpn_id
 fc_rport_enter_gnn_id
 fc_rport_enter_plogi
 fc_rport_enter_prli
 fc_rport_enter_rtv
 fc_rport_enter_logo

Helpers (Lock required)
 fc_rport_state_enter
        - Changing the state so the lock must be held
 fc_rport_error
        - Always called by an action or enter function
        - Does the lock need to be held or is it just becuase it's only callers
          already have the lock held?
 fc_rport_reject
        - Only call from retry handler (fc_rport_error)

Helpers (Lock not required)
 fc_rport_create_dummy
 fc_rport_destroy_dummy
 fc_rport_lookup
 fc_remote_port_create
 fc_rport_lock
 fc_rport_unlock
 fc_plogi_get_maxframe
 fc_lport_plogi_fill
 fc_rport_init

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

 drivers/scsi/libfc/fc_lport.c |   12 +
 drivers/scsi/libfc/fc_ns.c    |    3 
 drivers/scsi/libfc/fc_rport.c |  453 ++++++++++++++++++++++++++---------------
 include/scsi/libfc/libfc.h    |   14 +
 4 files changed, 305 insertions(+), 177 deletions(-)

diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
index 4fd4369..629c5d1 100644
--- a/drivers/scsi/libfc/fc_lport.c
+++ b/drivers/scsi/libfc/fc_lport.c
@@ -58,19 +58,25 @@ static int fc_frame_drop(struct fc_lport *lp, struct fc_frame *fp)
 	return 0;
 }
 
-static void fc_lport_rport_event(struct fc_lport *lport, struct fc_rport *rport,
+static void fc_lport_rport_event(struct fc_lport *lport, u32 port_id,
 				 enum fc_lport_event event)
 {
+	struct fc_rport *rport = lport->tt.rport_lookup(lport, port_id);
 	fc_lport_lock(lport);
-	if (rport->port_id == FC_FID_DIR_SERV) {
+	if (port_id == FC_FID_DIR_SERV) {
 		switch (event) {
 		case LPORT_EV_RPORT_CREATED:
-			lport->tt.dns_register(lport);
+			if (rport) {
+				lport->dns_rp = rport;
+				lport->tt.dns_register(lport);
+			}
 			break;
 		case LPORT_EV_RPORT_LOGO:
 		case LPORT_EV_RPORT_FAILED:
 			fc_lport_enter_reset(lport);
 			break;
+		case LPORT_EV_RPORT_NONE:
+			break;
 		}
 	}
 	fc_lport_unlock(lport);
diff --git a/drivers/scsi/libfc/fc_ns.c b/drivers/scsi/libfc/fc_ns.c
index cdef2ad..cac98a4 100644
--- a/drivers/scsi/libfc/fc_ns.c
+++ b/drivers/scsi/libfc/fc_ns.c
@@ -76,9 +76,10 @@ struct fc_rport *fc_ns_create_dummy_rport(struct fc_ns_port *dp)
 	rp->node_name = dp->ids.node_name;
 	rp->roles = dp->ids.roles;
 
-	spin_lock_init(&rpp->rp_lock);
+	mutex_init(&rpp->rp_mutex);
 	rpp->local_port = dp->lp;
 	rpp->rp_state = RPORT_ST_INIT;
+	rpp->event = LPORT_EV_RPORT_NONE;
 	rpp->flags = FC_RP_FLAGS_REC_SUPPORTED;
 	INIT_DELAYED_WORK(&rpp->retry_work, fc_ns_timeout);
 
diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
index b596b3e..f3d838b 100644
--- a/drivers/scsi/libfc/fc_rport.c
+++ b/drivers/scsi/libfc/fc_rport.c
@@ -53,6 +53,22 @@ 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",
@@ -125,8 +141,10 @@ static struct fc_rport *fc_remote_port_create(struct fc_lport *lport,
 	/* default value until service parameters are exchanged in PLOGI */
 	rport->maxframe_size = FC_MIN_MAX_PAYLOAD;
 
-	spin_lock_init(&rdata->rp_lock);
+	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;
@@ -150,18 +168,6 @@ void fc_set_rport_loss_tmo(struct fc_rport *rport, u32 timeout)
 }
 EXPORT_SYMBOL(fc_set_rport_loss_tmo);
 
-static inline void fc_rport_lock(struct fc_rport *rport)
-{
-	struct fc_rport_libfc_priv *rdata = rport->dd_data;
-	spin_lock_bh(&rdata->rp_lock);
-}
-
-static inline void fc_rport_unlock(struct fc_rport *rport)
-{
-	struct fc_rport_libfc_priv *rdata = rport->dd_data;
-	spin_unlock_bh(&rdata->rp_lock);
-}
-
 /**
  * fc_plogi_get_maxframe - Get max payload from the common service parameters
  * @flp: FLOGI payload structure
@@ -222,6 +228,13 @@ fc_lport_plogi_fill(struct fc_lport *lport,
 	}
 }
 
+/**
+ * fc_rport_state_enter - Change the rport's state
+ * @rport: The rport whose state should change
+ * @new: The new state of the rport
+ *
+ * Locking Note: Called with the rport lock held
+ */
 static void fc_rport_state_enter(struct fc_rport *rport,
 				 enum fc_rport_state new)
 {
@@ -234,93 +247,79 @@ static void fc_rport_state_enter(struct fc_rport *rport,
 /**
  * fc_rport_login - Start the remote port login state machine
  * @rport: Fibre Channel remote port
+ *
+ * Locking Note: Called without the rport lock held. This
+ * function will hold the rport lock, call an _enter_*
+ * function and then unlock the rport.
  */
 int fc_rport_login(struct fc_rport *rport)
 {
 	struct fc_rport_libfc_priv *rdata = rport->dd_data;
-	struct fc_lport *lport = rdata->local_port;
 
-	fc_rport_lock(rport);
-	if (rdata->rp_state == RPORT_ST_INIT) {
-		fc_rport_unlock(rport);
-		fc_rport_enter_plogi(rport);
-	} else if (rdata->rp_state == RPORT_ST_ERROR) {
-		fc_rport_state_enter(rport, RPORT_ST_INIT);
-		fc_rport_unlock(rport);
-		if (fc_rp_debug)
-			FC_DBG("remote %6x closed\n", rport->port_id);
+	mutex_lock(&rdata->rp_mutex);
 
-		if (rdata->event_callback)
-			rdata->event_callback(lport, rport,
-					      LPORT_EV_RPORT_FAILED);
+	if (fc_rp_debug)
+		FC_DBG("Login to port (%6x)\n", rport->port_id);
 
-		fc_remote_port_delete(rport);
-	} else
-		fc_rport_unlock(rport);
+	fc_rport_enter_plogi(rport);
+
+	fc_rport_unlock(rport);
 
 	return 0;
 }
 
-/*
- * Stop the session - log it off.
+/**
+ * fc_rport_logout - Logout of the remote port and delete it
+ * @rport: Fibre Channel remote port
+ *
+ * Locking Note: Called without the rport lock held. This
+ * function will hold the rport lock, call an _enter_*
+ * function and then unlock the rport.
  */
 int fc_rport_logout(struct fc_rport *rport)
 {
 	struct fc_rport_libfc_priv *rdata = rport->dd_data;
-	struct fc_lport *lport = rdata->local_port;
 
-	fc_rport_lock(rport);
-	switch (rdata->rp_state) {
-	case RPORT_ST_PRLI:
-	case RPORT_ST_RTV:
-	case RPORT_ST_READY:
-		fc_rport_enter_logo(rport);
-		fc_rport_unlock(rport);
-		break;
-	default:
-		fc_rport_state_enter(rport, RPORT_ST_INIT);
-		fc_rport_unlock(rport);
-		if (fc_rp_debug)
-			FC_DBG("remote %6x closed\n", rport->port_id);
+	mutex_lock(&rdata->rp_mutex);
 
-		if (rdata->event_callback)
-			rdata->event_callback(lport, rport,
-					      LPORT_EV_RPORT_FAILED);
+	if (fc_rp_debug)
+		FC_DBG("Logout of port (%6x)\n", rport->port_id);
 
-			fc_remote_port_delete(rport);
-		break;
-	}
+	fc_rport_enter_logo(rport);
+	fc_rport_unlock(rport);
 
 	return 0;
 }
 
-/*
- * Reset the session - assume it is logged off.	 Used after fabric logoff.
- * The local port code takes care of resetting the exchange manager.
+/**
+ * fc_rport_reset - Reset the remote port
+ * @rport: Fibre Channel remote port
+ *
+ * XXX - This functionality is currently broken
+ *
+ * Locking Note: Called without the rport lock held. This
+ * function will hold the rport lock, call an _enter_*
+ * function and then unlock the rport.
  */
 void fc_rport_reset(struct fc_rport *rport)
 {
 	struct fc_rport_libfc_priv *rdata = rport->dd_data;
-	struct fc_lport *lport = rdata->local_port;
 
-	if (fc_rp_debug)
-		FC_DBG("sess to %6x reset\n", rport->port_id);
-
-	fc_rport_lock(rport);
-	fc_rport_state_enter(rport, RPORT_ST_INIT);
-	fc_rport_unlock(rport);
+	mutex_lock(&rdata->rp_mutex);
 
 	if (fc_rp_debug)
-		FC_DBG("remote %6x closed\n", rport->port_id);
+		FC_DBG("Reset port (%6x)\n", rport->port_id);
 
-	if (rdata->event_callback)
-		rdata->event_callback(lport, rport, LPORT_EV_RPORT_FAILED);
+	fc_rport_enter_plogi(rport);
 
-	fc_remote_port_delete(rport);
+	fc_rport_unlock(rport);
 }
 
-/*
- * Reset all sessions for a local port session list.
+/**
+ * fc_rport_reset_list - Reset all sessions for a local port session list.
+ * @lport: The lport whose rports should be reset
+ *
+ * Locking Note: TBD
  */
 void fc_rport_reset_list(struct fc_lport *lport)
 {
@@ -336,29 +335,33 @@ void fc_rport_reset_list(struct fc_lport *lport)
 	spin_unlock_irqrestore(shost->host_lock, flags);
 }
 
+/**
+ * fc_rport_enter_ready - The rport is ready
+ * @rport: Fibre Channel remote port that is ready
+ *
+ * Locking Note: The rport lock is expected to be held before calling
+ * this routine.
+ */
 static void fc_rport_enter_ready(struct fc_rport *rport)
 {
 	struct fc_rport_libfc_priv *rdata = rport->dd_data;
-	struct fc_lport *lport = rdata->local_port;
 
 	fc_rport_state_enter(rport, RPORT_ST_READY);
 
 	if (fc_rp_debug)
-		FC_DBG("remote %6x ready\n", rport->port_id);
+		FC_DBG("Port (%6x) is Ready\n", rport->port_id);
 
 	fc_remote_port_rolechg(rport, rdata->roles);
-
-	if (rdata->event_callback)
-		rdata->event_callback(lport, rport, LPORT_EV_RPORT_CREATED);
+	rdata->event = LPORT_EV_RPORT_CREATED;
 }
 
 /**
  * fc_rport_timeout - Handler for the retry_work timer.
- *  Simply determine what should be done next.
  * @work: The work struct of the fc_rport_libfc_priv
  *
- * Locking Note: This is an action function so we grab the
- * lock, call an _enter_* state and then unlock.
+ * Locking Note: Called without the rport lock held. This
+ * function will hold the rport lock, call an _enter_*
+ * function and then unlock the rport.
  */
 static void fc_rport_timeout(struct work_struct *work)
 {
@@ -366,6 +369,8 @@ static void fc_rport_timeout(struct work_struct *work)
 		container_of(work, struct fc_rport_libfc_priv, retry_work.work);
 	struct fc_rport *rport = (((void *)rdata) - sizeof(struct fc_rport));
 
+	mutex_lock(&rdata->rp_mutex);
+
 	switch (rdata->rp_state) {
 	case RPORT_ST_PLOGI:
 		fc_rport_enter_plogi(rport);
@@ -389,6 +394,8 @@ static void fc_rport_timeout(struct work_struct *work)
 		break;
 	}
 	put_device(&rport->dev);
+
+	fc_rport_unlock(rport);
 }
 
 /**
@@ -400,8 +407,8 @@ static void fc_rport_timeout(struct work_struct *work)
  * then wait for half a second and retry, otherwise retry
  * immediately.
  *
- * Locking Note: The lock is expect to be held before calling
- * this routine
+ * Locking Note: The rport lock is expected to be held before
+ * calling this routine
  */
 static void fc_rport_error(struct fc_rport *rport, struct fc_frame *fp)
 {
@@ -431,7 +438,7 @@ static void fc_rport_error(struct fc_rport *rport, struct fc_frame *fp)
 			fc_remote_port_delete(rport);
 
 			if (rdata->event_callback)
-				rdata->event_callback(lport, rport,
+				rdata->event_callback(lport, rport->port_id,
 						      LPORT_EV_RPORT_FAILED);
 			break;
 		case RPORT_ST_RTV:
@@ -449,35 +456,53 @@ static void fc_rport_error(struct fc_rport *rport, struct fc_frame *fp)
 }
 
 /**
- * fc_rport_plpogi_recv_resp - Handle incoming ELS PLOGI response
+ * fc_rport_plogi_recv_resp - Handle incoming ELS PLOGI response
  * @sp: current sequence in the PLOGI exchange
  * @fp: response frame
  * @rp_arg: Fibre Channel remote port
+ *
+ * Locking Note: This function will be called without the rport lock
+ * held, but it will lock, call an _enter_* function or fc_rport_error
+ * and then unlock the rport.
  */
 static void fc_rport_plogi_resp(struct fc_seq *sp, struct fc_frame *fp,
 				void *rp_arg)
 {
+	struct fc_rport *rport = rp_arg;
+	struct fc_rport_libfc_priv *rdata = rport->dd_data;
 	struct fc_els_flogi *plp;
 	u64 wwpn, wwnn;
 	unsigned int tov;
 	u16 csp_seq;
 	u16 cssp_seq;
 	u8 op;
-	struct fc_rport *rport = rp_arg;
-	struct fc_rport_libfc_priv *rdata = rport->dd_data;
 
-	if (!IS_ERR(fp)) {
-		op = fc_frame_payload_op(fp);
-		fc_rport_lock(rport);
-		if (op == ELS_LS_ACC &&
-		    (plp = fc_frame_payload_get(fp, sizeof(*plp))) != NULL) {
-			wwpn = get_unaligned_be64(&plp->fl_wwpn);
-			wwnn = get_unaligned_be64(&plp->fl_wwnn);
+	mutex_lock(&rdata->rp_mutex);
 
-			fc_rport_set_name(rport, wwpn, wwnn);
-			tov = ntohl(plp->fl_csp.sp_e_d_tov);
-			if (ntohs(plp->fl_csp.sp_features) & FC_SP_FT_EDTR)
-				tov /= 1000;
+	if (fc_rp_debug)
+		FC_DBG("Received a PLOGI response\n");
+
+	if (rdata->rp_state != RPORT_ST_PLOGI) {
+		FC_DBG("Received a PLOGI response, but in state %s\n",
+		       fc_rport_state(rport));
+		goto out;
+	}
+
+	if (IS_ERR(fp)) {
+		fc_rport_error(rport, fp);
+		goto out;
+	}
+
+	op = fc_frame_payload_op(fp);
+	if (op == ELS_LS_ACC &&
+	    (plp = fc_frame_payload_get(fp, sizeof(*plp))) != NULL) {
+		wwpn = get_unaligned_be64(&plp->fl_wwpn);
+		wwnn = get_unaligned_be64(&plp->fl_wwnn);
+
+		fc_rport_set_name(rport, wwpn, wwnn);
+		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)
 				rdata->e_d_tov = tov;
 			csp_seq = ntohs(plp->fl_csp.sp_tot_seq);
@@ -497,37 +522,42 @@ static void fc_rport_plogi_resp(struct fc_seq *sp, struct fc_frame *fp,
 				fc_rport_enter_ready(rport);
 			else
 				fc_rport_enter_prli(rport);
-		} else {
-			if (fc_rp_debug)
-				FC_DBG("bad PLOGI response\n");
-
-			fc_rport_error(rport, fp);
-		}
-		fc_rport_unlock(rport);
-		fc_frame_free(fp);
-	} else {
+	} else
 		fc_rport_error(rport, fp);
-	}
+
+out:
+	fc_rport_unlock(rport);
+	fc_frame_free(fp);
 }
 
 /**
  * fc_rport_enter_plogi - Send Port Login (PLOGI) request to peer
  * @rport: Fibre Channel remote port to send PLOGI to
+ *
+ * Locking Note: The rport lock is expected to be held before calling
+ * this routine.
  */
 static void fc_rport_enter_plogi(struct fc_rport *rport)
 {
-	struct fc_frame *fp;
-	struct fc_els_flogi *plogi;
 	struct fc_rport_libfc_priv *rdata = rport->dd_data;
 	struct fc_lport *lport = rdata->local_port;
+	struct fc_frame *fp;
+	struct fc_els_flogi *plogi;
+
+	if (fc_rp_debug)
+		FC_DBG("Port (%6x) entered PLOGI state from %s state\n",
+		       rport->port_id, fc_rport_state(rport));
 
 	fc_rport_state_enter(rport, RPORT_ST_PLOGI);
+
 	rport->maxframe_size = FC_MIN_MAX_PAYLOAD;
 	fp = fc_frame_alloc(lport, sizeof(*plogi));
-	if (!fp)
-		return fc_rport_error(rport, fp);
+	if (!fp) {
+		fc_rport_error(rport, fp);
+		return;
+	}
+
 	plogi = fc_frame_payload_get(fp, sizeof(*plogi));
-	WARN_ON(!plogi);
 	fc_lport_plogi_fill(rdata->local_port, plogi, ELS_PLOGI);
 	rdata->e_d_tov = lport->e_d_tov;
 	fc_frame_setup(fp, FC_RCTL_ELS_REQ, FC_TYPE_ELS);
@@ -546,13 +576,16 @@ static void fc_rport_enter_plogi(struct fc_rport *rport)
  * @sp: current sequence in the PRLI exchange
  * @fp: response frame
  * @rp_arg: Fibre Channel remote port
+ *
+ * Locking Note: This function will be called without the rport lock
+ * held, but it will lock, call an _enter_* function or fc_rport_error
+ * and then unlock the rport.
  */
 static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp,
 			       void *rp_arg)
 {
 	struct fc_rport *rport = rp_arg;
 	struct fc_rport_libfc_priv *rdata = rport->dd_data;
-	struct fc_lport *lport = rdata->local_port;
 	struct {
 		struct fc_els_prli prli;
 		struct fc_els_spp spp;
@@ -561,12 +594,22 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp,
 	u32 fcp_parm = 0;
 	u8 op;
 
+	mutex_lock(&rdata->rp_mutex);
+
+	if (fc_rp_debug)
+		FC_DBG("Received a PRLI response\n");
+
+	if (rdata->rp_state != RPORT_ST_PRLI) {
+		FC_DBG("Received a PRLI response, but in state %s\n",
+		       fc_rport_state(rport));
+		goto out;
+	}
+
 	if (IS_ERR(fp)) {
 		fc_rport_error(rport, fp);
-		return;
+		goto out;
 	}
 
-	fc_rport_lock(rport);
 	op = fc_frame_payload_op(fp);
 	if (op == ELS_LS_ACC) {
 		pp = fc_frame_payload_get(fp, sizeof(*pp));
@@ -584,19 +627,16 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp,
 
 		rdata->roles = roles;
 		fc_rport_enter_rtv(rport);
-		fc_rport_unlock(rport);
+
 	} else {
 		FC_DBG("bad ELS response\n");
 		fc_rport_state_enter(rport, RPORT_ST_ERROR);
-		fc_rport_unlock(rport);
-
-		if (rdata->event_callback)
-			rdata->event_callback(lport, rport,
-					      LPORT_EV_RPORT_FAILED);
-
+		rdata->event = LPORT_EV_RPORT_FAILED;
 		fc_remote_port_delete(rport);
 	}
 
+out:
+	fc_rport_unlock(rport);
 	fc_frame_free(fp);
 }
 
@@ -605,61 +645,79 @@ static void fc_rport_prli_resp(struct fc_seq *sp, struct fc_frame *fp,
  * @sp: current sequence in the LOGO exchange
  * @fp: response frame
  * @rp_arg: Fibre Channel remote port
+ *
+ * Locking Note: This function will be called without the rport lock
+ * held, but it will lock, call an _enter_* function or fc_rport_error
+ * and then unlock the rport.
  */
 static void fc_rport_logo_resp(struct fc_seq *sp, struct fc_frame *fp,
 			       void *rp_arg)
 {
 	struct fc_rport *rport = rp_arg;
 	struct fc_rport_libfc_priv *rdata = rport->dd_data;
-	struct fc_lport *lport = rdata->local_port;
 	u8 op;
 
+	mutex_lock(&rdata->rp_mutex);
+
+	if (fc_rp_debug)
+		FC_DBG("Received a LOGO response\n");
+
+	if (rdata->rp_state != RPORT_ST_LOGO) {
+		FC_DBG("Received a LOGO response, but in state %s\n",
+		       fc_rport_state(rport));
+		goto out;
+	}
+
 	if (IS_ERR(fp)) {
 		fc_rport_error(rport, fp);
-		return;
+		goto out;
 	}
 
-	fc_rport_lock(rport);
 	op = fc_frame_payload_op(fp);
 	if (op == ELS_LS_ACC) {
 		fc_rport_enter_rtv(rport);
-		fc_rport_unlock(rport);
-	} else {
-		FC_DBG("bad ELS response\n");
-		fc_rport_state_enter(rport, RPORT_ST_ERROR);
-		fc_rport_unlock(rport);
-
-		if (rdata->event_callback)
-			rdata->event_callback(lport, rport,
-					      LPORT_EV_RPORT_LOGO);
 
+	} else {
+		FC_DBG("Bad ELS response\n");
+		rdata->event = LPORT_EV_RPORT_LOGO;
 		fc_remote_port_delete(rport);
 	}
 
+out:
+	fc_rport_unlock(rport);
 	fc_frame_free(fp);
 }
 
 /**
  * fc_rport_enter_prli - Send Process Login (PRLI) request to peer
  * @rport: Fibre Channel remote port to send PRLI to
+ *
+ * Locking Note: The rport lock is expected to be held before calling
+ * this routine.
  */
 static void fc_rport_enter_prli(struct fc_rport *rport)
 {
+	struct fc_rport_libfc_priv *rdata = rport->dd_data;
+	struct fc_lport *lport = rdata->local_port;
 	struct {
 		struct fc_els_prli prli;
 		struct fc_els_spp spp;
 	} *pp;
 	struct fc_frame *fp;
-	struct fc_rport_libfc_priv *rdata = rport->dd_data;
-	struct fc_lport *lport = rdata->local_port;
+
+	if (fc_rp_debug)
+		FC_DBG("Port (%6x) entered PRLI state from %s state\n",
+		       rport->port_id, fc_rport_state(rport));
 
 	fc_rport_state_enter(rport, RPORT_ST_PRLI);
 
 	fp = fc_frame_alloc(lport, sizeof(*pp));
-	if (!fp)
-		return fc_rport_error(rport, fp);
+	if (!fp) {
+		fc_rport_error(rport, fp);
+		return;
+	}
+
 	pp = fc_frame_payload_get(fp, sizeof(*pp));
-	WARN_ON(!pp);
 	memset(pp, 0, sizeof(*pp));
 	pp->prli.prli_cmd = ELS_PRLI;
 	pp->prli.prli_spp_len = sizeof(struct fc_els_spp);
@@ -684,6 +742,10 @@ static void fc_rport_enter_prli(struct fc_rport *rport)
  * @rp_arg: Fibre Channel remote port
  *
  * Many targets don't seem to support this.
+ *
+ * Locking Note: This function will be called without the rport lock
+ * held, but it will lock, call an _enter_* function or fc_rport_error
+ * and then unlock the rport.
  */
 static void fc_rport_rtv_resp(struct fc_seq *sp, struct fc_frame *fp,
 			      void *rp_arg)
@@ -692,12 +754,22 @@ static void fc_rport_rtv_resp(struct fc_seq *sp, struct fc_frame *fp,
 	struct fc_rport_libfc_priv *rdata = rport->dd_data;
 	u8 op;
 
+	mutex_lock(&rdata->rp_mutex);
+
+	if (fc_rp_debug)
+		FC_DBG("Received a RTV response\n");
+
+	if (rdata->rp_state != RPORT_ST_RTV) {
+		FC_DBG("Received a RTV response, but in state %s\n",
+		       fc_rport_state(rport));
+		goto out;
+	}
+
 	if (IS_ERR(fp)) {
 		fc_rport_error(rport, fp);
-		return;
+		goto out;
 	}
 
-	fc_rport_lock(rport);
 	op = fc_frame_payload_op(fp);
 	if (op == ELS_LS_ACC) {
 		struct fc_els_rtv_acc *rtv;
@@ -721,6 +793,8 @@ static void fc_rport_rtv_resp(struct fc_seq *sp, struct fc_frame *fp,
 	}
 
 	fc_rport_enter_ready(rport);
+
+out:
 	fc_rport_unlock(rport);
 	fc_frame_free(fp);
 }
@@ -728,6 +802,9 @@ static void fc_rport_rtv_resp(struct fc_seq *sp, struct fc_frame *fp,
 /**
  * fc_rport_enter_rtv - Send Request Timeout Value (RTV) request to peer
  * @rport: Fibre Channel remote port to send RTV to
+ *
+ * Locking Note: The rport lock is expected to be held before calling
+ * this routine.
  */
 static void fc_rport_enter_rtv(struct fc_rport *rport)
 {
@@ -736,13 +813,19 @@ static void fc_rport_enter_rtv(struct fc_rport *rport)
 	struct fc_rport_libfc_priv *rdata = rport->dd_data;
 	struct fc_lport *lport = rdata->local_port;
 
+	if (fc_rp_debug)
+		FC_DBG("Port (%6x) entered RTV state from %s state\n",
+		       rport->port_id, fc_rport_state(rport));
+
 	fc_rport_state_enter(rport, RPORT_ST_RTV);
 
 	fp = fc_frame_alloc(lport, sizeof(*rtv));
-	if (!fp)
-		return fc_rport_error(rport, fp);
+	if (!fp) {
+		fc_rport_error(rport, fp);
+		return;
+	}
+
 	rtv = fc_frame_payload_get(fp, sizeof(*rtv));
-	WARN_ON(!rtv);
 	memset(rtv, 0, sizeof(*rtv));
 	rtv->rtv_cmd = ELS_RTV;
 	fc_frame_setup(fp, FC_RCTL_ELS_REQ, FC_TYPE_ELS);
@@ -758,25 +841,34 @@ static void fc_rport_enter_rtv(struct fc_rport *rport)
 /**
  * fc_rport_enter_logo - Send Logout (LOGO) request to peer
  * @rport: Fibre Channel remote port to send LOGO to
+ *
+ * Locking Note: The rport lock is expected to be held before calling
+ * this routine.
  */
 static void fc_rport_enter_logo(struct fc_rport *rport)
 {
-	struct fc_frame *fp;
-	struct fc_els_logo *logo;
 	struct fc_rport_libfc_priv *rdata = rport->dd_data;
 	struct fc_lport *lport = rdata->local_port;
+	struct fc_frame *fp;
+	struct fc_els_logo *logo;
+
+	if (fc_rp_debug)
+		FC_DBG("Port (%6x) entered LOGO state from %s state\n",
+		       rport->port_id, fc_rport_state(rport));
 
 	fc_rport_state_enter(rport, RPORT_ST_LOGO);
 
 	fp = fc_frame_alloc(lport, sizeof(*logo));
-	if (!fp)
-		return fc_rport_error(rport, fp);
+	if (!fp) {
+		fc_rport_error(rport, fp);
+		return;
+	}
+
 	logo = fc_frame_payload_get(fp, sizeof(*logo));
 	memset(logo, 0, sizeof(*logo));
 	logo->fl_cmd = ELS_LOGO;
 	hton24(logo->fl_n_port_id, lport->fid);
 	logo->fl_n_port_wwn = htonll(lport->wwpn);
-
 	fc_frame_setup(fp, FC_RCTL_ELS_REQ, FC_TYPE_ELS);
 
 	if (!lport->tt.exch_seq_send(lport, fp,
@@ -787,10 +879,16 @@ static void fc_rport_enter_logo(struct fc_rport *rport)
 		fc_rport_error(rport, fp);
 }
 
-/*
- * Handle a request received by the exchange manager for the session.
- * This may be an entirely new session, or a PLOGI or LOGO for an existing one.
- * This will free the frame.
+
+/**
+ * fc_rport_recv_req - Receive a request from a rport
+ * @sp: current sequence in the PLOGI exchange
+ * @fp: response frame
+ * @rp_arg: Fibre Channel remote port
+ *
+ * Locking Note: Called without the rport lock held. This
+ * function will hold the rport lock, call an _enter_*
+ * function and then unlock the rport.
  */
 void fc_rport_recv_req(struct fc_seq *sp, struct fc_frame *fp,
 		       struct fc_rport *rport)
@@ -802,6 +900,8 @@ void fc_rport_recv_req(struct fc_seq *sp, struct fc_frame *fp,
 	struct fc_seq_els_data els_data;
 	u8 op;
 
+	mutex_lock(&rdata->rp_mutex);
+
 	els_data.fp = NULL;
 	els_data.explan = ELS_EXPL_NONE;
 	els_data.reason = ELS_RJT_NONE;
@@ -834,12 +934,12 @@ void fc_rport_recv_req(struct fc_seq *sp, struct fc_frame *fp,
 		default:
 			els_data.reason = ELS_RJT_UNSUP;
 			lport->tt.seq_els_rsp_send(sp, ELS_LS_RJT, &els_data);
-			fc_frame_free(fp);
 			break;
 		}
-	} else {
-		fc_frame_free(fp);
 	}
+
+	fc_rport_unlock(rport);
+	fc_frame_free(fp);
 }
 
 /**
@@ -847,6 +947,9 @@ void fc_rport_recv_req(struct fc_seq *sp, struct fc_frame *fp,
  * @rport: Fibre Channel remote port that initiated PLOGI
  * @sp: current sequence in the PLOGI exchange
  * @fp: PLOGI request frame
+ *
+ * Locking Note: The rport lock is exected to be held before calling
+ * this function.
  */
 static void fc_rport_recv_plogi_req(struct fc_rport *rport,
 				    struct fc_seq *sp, struct fc_frame *rx_fp)
@@ -863,9 +966,15 @@ static void fc_rport_recv_plogi_req(struct fc_rport *rport,
 	u64 wwnn;
 	enum fc_els_rjt_reason reject = 0;
 	u32 f_ctl;
-
 	rjt_data.fp = NULL;
+
 	fh = fc_frame_header_get(fp);
+
+	if (fc_rp_debug)
+		FC_DBG("Received PLOGI request from port (%6x) "
+		       "while in state %s\n", ntoh24(fh->fh_s_id),
+		       fc_rport_state(rport));
+
 	sid = ntoh24(fh->fh_s_id);
 	pl = fc_frame_payload_get(fp, sizeof(*pl));
 	if (!pl) {
@@ -877,7 +986,6 @@ static void fc_rport_recv_plogi_req(struct fc_rport *rport,
 	}
 	wwpn = get_unaligned_be64(&pl->fl_wwpn);
 	wwnn = get_unaligned_be64(&pl->fl_wwnn);
-	fc_rport_lock(rport);
 
 	/*
 	 * If the session was just created, possibly due to the incoming PLOGI,
@@ -962,7 +1070,6 @@ static void fc_rport_recv_plogi_req(struct fc_rport *rport,
 						     RPORT_ST_PLOGI_RECV);
 		}
 	}
-	fc_rport_unlock(rport);
 }
 
 /**
@@ -970,6 +1077,9 @@ static void fc_rport_recv_plogi_req(struct fc_rport *rport,
  * @rport: Fibre Channel remote port that initiated PRLI
  * @sp: current sequence in the PRLI exchange
  * @fp: PRLI request frame
+ *
+ * Locking Note: The rport lock is exected to be held before calling
+ * this function.
  */
 static void fc_rport_recv_prli_req(struct fc_rport *rport,
 				   struct fc_seq *sp, struct fc_frame *rx_fp)
@@ -994,10 +1104,15 @@ static void fc_rport_recv_prli_req(struct fc_rport *rport,
 	u32 f_ctl;
 	u32 fcp_parm;
 	u32 roles = FC_RPORT_ROLE_UNKNOWN;
-
 	rjt_data.fp = NULL;
+
 	fh = fc_frame_header_get(rx_fp);
 
+	if (fc_rp_debug)
+		FC_DBG("Received PRLI request from port (%6x) "
+		       "while in state %s\n", ntoh24(fh->fh_s_id),
+		       fc_rport_state(rport));
+
 	switch (rdata->rp_state) {
 	case RPORT_ST_PLOGI_RECV:
 	case RPORT_ST_PRLI:
@@ -1094,7 +1209,6 @@ static void fc_rport_recv_prli_req(struct fc_rport *rport,
 		/*
 		 * Get lock and re-check state.
 		 */
-		fc_rport_lock(rport);
 		switch (rdata->rp_state) {
 		case RPORT_ST_PLOGI_RECV:
 		case RPORT_ST_PRLI:
@@ -1105,7 +1219,6 @@ static void fc_rport_recv_prli_req(struct fc_rport *rport,
 		default:
 			break;
 		}
-		fc_rport_unlock(rport);
 	}
 	fc_frame_free(rx_fp);
 }
@@ -1115,6 +1228,9 @@ static void fc_rport_recv_prli_req(struct fc_rport *rport,
  * @rport: Fibre Channel remote port that initiated PRLO
  * @sp: current sequence in the PRLO exchange
  * @fp: PRLO request frame
+ *
+ * Locking Note: The rport lock is exected to be held before calling
+ * this function.
  */
 static void fc_rport_recv_prlo_req(struct fc_rport *rport, struct fc_seq *sp,
 				   struct fc_frame *fp)
@@ -1126,8 +1242,12 @@ static void fc_rport_recv_prlo_req(struct fc_rport *rport, struct fc_seq *sp,
 	struct fc_seq_els_data rjt_data;
 
 	fh = fc_frame_header_get(fp);
-	FC_DBG("incoming PRLO from %x state %d\n",
-	       ntoh24(fh->fh_s_id), rdata->rp_state);
+
+	if (fc_rp_debug)
+		FC_DBG("Received PRLO request from port (%6x) "
+		       "while in state %s\n", ntoh24(fh->fh_s_id),
+		       fc_rport_state(rport));
+
 	rjt_data.fp = NULL;
 	rjt_data.reason = ELS_RJT_UNAB;
 	rjt_data.explan = ELS_EXPL_NONE;
@@ -1140,6 +1260,9 @@ static void fc_rport_recv_prlo_req(struct fc_rport *rport, struct fc_seq *sp,
  * @rport: Fibre Channel remote port that initiated LOGO
  * @sp: current sequence in the LOGO exchange
  * @fp: LOGO request frame
+ *
+ * Locking Note: The rport lock is exected to be held before calling
+ * this function.
  */
 static void fc_rport_recv_logo_req(struct fc_rport *rport, struct fc_seq *sp,
 				   struct fc_frame *fp)
@@ -1149,15 +1272,13 @@ static void fc_rport_recv_logo_req(struct fc_rport *rport, struct fc_seq *sp,
 	struct fc_lport *lport = rdata->local_port;
 
 	fh = fc_frame_header_get(fp);
-	fc_rport_lock(rport);
-	fc_rport_state_enter(rport, RPORT_ST_INIT);
-	fc_rport_unlock(rport);
-	if (fc_rp_debug)
-		FC_DBG("remote %6x closed\n", rport->port_id);
 
-	if (rdata->event_callback)
-		rdata->event_callback(lport, rport, LPORT_EV_RPORT_LOGO);
+	if (fc_rp_debug)
+		FC_DBG("Received LOGO request from port (%6x) "
+		       "while in state %s\n", ntoh24(fh->fh_s_id),
+		       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);
diff --git a/include/scsi/libfc/libfc.h b/include/scsi/libfc/libfc.h
index 0c483f3..8eab130 100644
--- a/include/scsi/libfc/libfc.h
+++ b/include/scsi/libfc/libfc.h
@@ -102,7 +102,8 @@ enum fc_lport_state {
 };
 
 enum fc_lport_event {
-	LPORT_EV_RPORT_CREATED = 0,
+	LPORT_EV_RPORT_NONE = 0,
+	LPORT_EV_RPORT_CREATED,
 	LPORT_EV_RPORT_FAILED,
 	LPORT_EV_RPORT_LOGO
 };
@@ -142,7 +143,7 @@ struct fc_ns_port {
  * @retries: retry count in current state
  * @e_d_tov: error detect timeout value (in msec)
  * @r_a_tov: resource allocation timeout value (in msec)
- * @rp_lock: lock protects state
+ * @rp_mutex: mutex protects rport
  * @retry_work:
  * @event_callback: Callback for rport READY, FAILED or LOGO
  */
@@ -156,10 +157,10 @@ struct fc_rport_libfc_priv {
 	unsigned int	retries;
 	unsigned int	e_d_tov;
 	unsigned int	r_a_tov;
-	spinlock_t	rp_lock;
+	struct mutex    rp_mutex;
 	struct delayed_work	retry_work;
-	void (*event_callback)(struct fc_lport *,
-			       struct fc_rport *,
+	enum fc_lport_event     event;
+	void (*event_callback)(struct fc_lport *, u32,
 			       enum fc_lport_event);
 	u32 roles;
 };
@@ -353,8 +354,7 @@ struct libfc_function_template {
 	int (*lport_reset)(struct fc_lport *);
 	int (*lport_logout)(struct fc_lport *);
 
-	void (*event_callback)(struct fc_lport *,
-			       struct fc_rport *,
+	void (*event_callback)(struct fc_lport *, u32,
 			       enum fc_lport_event);
 
 	/**




More information about the devel mailing list