libfc: Pass lport in exch_mgr_reset

PATCH New Robert Love 1 participant 6 messages Jan 21, 2009

Subsystems (2)

SCSI FCoE subsystem (libfc, libfcoe, fcoe)

lore

Share & tools

lore

Robert Love

author

2009-01-21 20:44 UTC #0 lore raw

PATCH 1/6 libfc: Pass lport in exch_mgr_reset

From: Abhijeet Joglekar <[email protected]>

fc_exch_mgr structure is private to fc_exch.c. To export exch_mgr_reset to transport, transport needs access to the exch manager. Change exch_mgr_reset to use lport param which is the shared structure between libFC and transport.

Alternatively, fc_exch_mgr definition can be moved to libfc.h so that lport can be accessed from mp*.

Signed-off-by Abhijeet Joglekar <[email protected]>

Signed-off-by Robert Love <[email protected]>

---

 drivers/scsi/libfc/fc_exch.c  |    3 ++-
 drivers/scsi/libfc/fc_lport.c |    4 ++--
 drivers/scsi/libfc/fc_rport.c |    4 ++--
 include/scsi/libfc.h          |    4 ++--
 4 files changed, 8 insertions(+), 7 deletions(-)

drivers/scsi/libfc/fc_exch.c +2 -1

index 66db08a..a09416f 100644
@@ -1480,10 +1480,11 @@ static void fc_exch_reset(struct fc_exch *ep)
  * If sid is non-zero, reset only exchanges we source from that FID.
  * If did is non-zero, reset only exchanges destined to that FID.
  */
-void fc_exch_mgr_reset(struct fc_exch_mgr *mp, u32 sid, u32 did)
+void fc_exch_mgr_reset(struct fc_lport *lp, u32 sid, u32 did)
 {
 	struct fc_exch *ep;
 	struct fc_exch *next;
+	struct fc_exch_mgr *mp = lp->emp;
 
 	spin_lock_bh(&mp->em_lock);
 restart:

drivers/scsi/libfc/fc_lport.c +2 -2

index 0b9bdb1..5db223c 100644
@@ -663,7 +663,7 @@ int fc_lport_destroy(struct fc_lport *lport)
 {
 	lport->tt.frame_send = fc_frame_drop;
 	lport->tt.fcp_abort_io(lport);
-	lport->tt.exch_mgr_reset(lport->emp, 0, 0);
+	lport->tt.exch_mgr_reset(lport, 0, 0);
 	return 0;
 }
 EXPORT_SYMBOL(fc_lport_destroy);

@@ -973,7 +973,7 @@ static void fc_lport_enter_reset(struct fc_lport *lport)
 
 	lport->tt.disc_stop(lport);
 
-	lport->tt.exch_mgr_reset(lport->emp, 0, 0);
+	lport->tt.exch_mgr_reset(lport, 0, 0);
 	fc_host_fabric_name(lport->host) = 0;
 	fc_host_port_id(lport->host) = 0;

drivers/scsi/libfc/fc_rport.c +2 -2

index e780d8c..dec7bae 100644
@@ -1285,7 +1285,7 @@ void fc_rport_terminate_io(struct fc_rport *rport)
 	struct fc_rport_libfc_priv *rdata = rport->dd_data;
 	struct fc_lport *lport = rdata->local_port;
 
-	lport->tt.exch_mgr_reset(lport->emp, 0, rport->port_id);
-	lport->tt.exch_mgr_reset(lport->emp, rport->port_id, 0);
+	lport->tt.exch_mgr_reset(lport, 0, rport->port_id);
+	lport->tt.exch_mgr_reset(lport, rport->port_id, 0);
 }
 EXPORT_SYMBOL(fc_rport_terminate_io);

include/scsi/libfc.h +2 -2

index 9f28763..042f4ad 100644
@@ -472,7 +472,7 @@ struct libfc_function_template {
 	 * If s_id is non-zero, reset only exchanges originating from that FID.
 	 * If d_id is non-zero, reset only exchanges sending to that FID.
 	 */
-	void (*exch_mgr_reset)(struct fc_exch_mgr *,
+	void (*exch_mgr_reset)(struct fc_lport *,
 			       u32 s_id, u32 d_id);
 
 	void (*rport_flush_queue)(void);

@@ -916,7 +916,7 @@ struct fc_seq *fc_seq_start_next(struct fc_seq *sp);
  * If s_id is non-zero, reset only exchanges originating from that FID.
  * If d_id is non-zero, reset only exchanges sending to that FID.
  */
-void fc_exch_mgr_reset(struct fc_exch_mgr *, u32 s_id, u32 d_id);
+void fc_exch_mgr_reset(struct fc_lport *, u32 s_id, u32 d_id);
 
 /*
  * Functions for fc_functions_template

Robert Love

author

2009-01-21 20:44 UTC #1 lore raw

PATCH 2/6 libfc: when rport goes away (re-plogi), clean up exchanges to/from rport

From: Abhijeet Joglekar <[email protected]>

When a rport goes away, libFC does a plogi which will reset exchanges
at the rport. Clean exchanges at our end, both in transport and libFC. If transport hooks into exch_mgr_reset, it will call back into fc_exch_mgr_reset() to clean up libFC exchanges.

Signed-off-by Abhijeet Joglekar <[email protected]>

Signed-off-by Robert Love <[email protected]>

---

 drivers/scsi/libfc/fc_rport.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

drivers/scsi/libfc/fc_rport.c +6 -1

index dec7bae..7175759 100644
@@ -214,6 +214,7 @@ static void fc_rport_state_enter(struct fc_rport *rport,
 
 static void fc_rport_work(struct work_struct *work)
 {
+	u32 port_id;
 	struct fc_rport_libfc_priv *rdata =
 		container_of(work, struct fc_rport_libfc_priv, event_work);
 	enum fc_rport_event event;

@@ -279,8 +280,12 @@ static void fc_rport_work(struct work_struct *work)
 			rport_ops->event_callback(lport, rport, event);
 		if (trans_state == FC_PORTSTATE_ROGUE)
 			put_device(&rport->dev);
-		else
+		else {
+			port_id = rport->port_id;
 			fc_remote_port_delete(rport);
+			lport->tt.exch_mgr_reset(lport, 0, port_id);
+			lport->tt.exch_mgr_reset(lport, port_id, 0);
+		}
 	} else
 		mutex_unlock(&rdata->rp_mutex);
 }

Robert Love

author

2009-01-21 20:44 UTC #2 lore raw

PATCH 3/6 libfc: handle RRQ exch timeout

In reply to Robert Love

From: Vasu Dev <[email protected]>

Cleanup exchange held due to RRQ when RRQ exch times out, in this case the ABTS is already done causing RRQ req therefore proceeding with cleanup in fc_exch_rrq_resp should be okay to restore exch resource.

Signed-off-by Vasu Dev <[email protected]>

Signed-off-by Robert Love <[email protected]>

---

 drivers/scsi/libfc/fc_exch.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/scsi/libfc/fc_exch.c +1 -1

index a09416f..e874e77 100644
@@ -1608,7 +1608,7 @@ static void fc_exch_rrq_resp(struct fc_seq *sp, struct fc_frame *fp, void *arg)
 	if (IS_ERR(fp)) {
 		int err = PTR_ERR(fp);
 
-		if (err == -FC_EX_CLOSED)
+		if (err == -FC_EX_CLOSED || err == -FC_EX_TIMEOUT)
 			goto cleanup;
 		FC_DBG("Cannot process RRQ, because of frame error %d\n", err);
 		return;

Robert Love

author

2009-01-21 20:45 UTC #3 lore raw

PATCH 4/6 libfc: fixed a soft lockup issue in fc_exch_recv_abts

In reply to Robert Love

From: Vasu Dev <[email protected]>

The fc_seq_start_next grabs ep->ex_lock but this lock was already held here, so instead called fc_seq_start_next_locked to avoid soft lockup.

Signed-off-by Vasu Dev <[email protected]>

Signed-off-by Robert Love <[email protected]>

---

 drivers/scsi/libfc/fc_exch.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/scsi/libfc/fc_exch.c +1 -1

index e874e77..dd269e5 100644
@@ -1096,7 +1096,7 @@ static void fc_exch_recv_abts(struct fc_exch *ep, struct fc_frame *rx_fp)
 		ap->ba_high_seq_cnt = fh->fh_seq_cnt;
 		ap->ba_low_seq_cnt = htons(sp->cnt);
 	}
-	sp = fc_seq_start_next(sp);
+	sp = fc_seq_start_next_locked(sp);
 	spin_unlock_bh(&ep->ex_lock);
 	fc_seq_send_last(sp, fp, FC_RCTL_BA_ACC, FC_TYPE_BLS);
 	fc_frame_free(rx_fp);

Robert Love

author

2009-01-21 20:45 UTC #4 lore raw

PATCH 5/6 libfc, fcoe: fixed locking issues with lport->lp_mutex around lport->link_status

In reply to Robert Love

From: Vasu Dev <[email protected]>

The fcoe_xmit could call fc_pause in case the pending skb queue len is larger than FCOE_MAX_QUEUE_DEPTH, the fc_pause was trying to grab lport->lp_muex to change lport->link_status and that had these issues :-

1. The fcoe_xmit was getting called with bh disabled, thus causing "BUG: scheduling while atomic" when grabbing lport->lp_muex with bh disabled.

2. fc_linkup and fc_linkdown function calls lport_enter function with lport->lp_mutex held and these enter function in turn calls fcoe_xmit to send lport related FC frame, e.g. fc_linkup => fc_lport_enter_flogi to send flogi req. In this case grabbing the same lport->lp_mutex again in fc_puase from fcoe_xmit would cause deadlock.

The lport->lp_mutex was used for setting FC_PAUSE in fcoe_xmit path but FC_PAUSE bit was not used anywhere beside just setting and clear this bit in lport->link_status, instead used a separate field qfull in fc_lport to eliminate need for lport->lp_mutex to track pending queue full condition and in turn avoid above described two locking issues.

Also added check for lp->qfull in fc_fcp_lport_queue_ready to trigger SCSI_MLQUEUE_HOST_BUSY when lp->qfull is set to prevent more scsi-ml cmds while lp->qfull is set.

This patch eliminated FC_LINK_UP and FC_PAUSE and instead used dedicated fields in fc_lport for this, this simplified all related conditional code.

Also removed fc_pause and fc_unpause functions and instead used newly added lport->qfull directly in fcoe.

Signed-off-by Vasu Dev <[email protected]>

Signed-off-by Robert Love <[email protected]>

---

 drivers/scsi/fcoe/fcoe_sw.c   |    6 +++---
 drivers/scsi/fcoe/libfcoe.c   |   41 +++++++++++++++++------------------------
 drivers/scsi/libfc/fc_fcp.c   |    4 ++--
 drivers/scsi/libfc/fc_lport.c |   36 ++++++------------------------------
 include/scsi/libfc.h          |   12 ++----------
 5 files changed, 30 insertions(+), 69 deletions(-)

drivers/scsi/fcoe/fcoe_sw.c +3 -3

index dc4cd5e..cf83675 100644
@@ -116,7 +116,8 @@ static int fcoe_sw_lport_config(struct fc_lport *lp)
 {
 	int i = 0;
 
-	lp->link_status = 0;
+	lp->link_up = 0;
+	lp->qfull = 0;
 	lp->max_retry_count = 3;
 	lp->e_d_tov = 2 * 1000;	/* FC-FS default */
 	lp->r_a_tov = 2 * 2 * 1000;

@@ -181,9 +182,8 @@ static int fcoe_sw_netdev_config(struct fc_lport *lp, struct net_device *netdev)
 	if (fc_set_mfs(lp, mfs))
 		return -EINVAL;
 
-	lp->link_status = ~FC_PAUSE & ~FC_LINK_UP;
 	if (!fcoe_link_ok(lp))
-		lp->link_status |= FC_LINK_UP;
+		lp->link_up = 1;
 
 	/* offload features support */
 	if (fc->real_dev->features & NETIF_F_SG)

drivers/scsi/fcoe/libfcoe.c +17 ~~-24~~

118 lines not on this page · raw message

drivers/scsi/libfc/fc_fcp.c +2 -2

index 404e63f..f440aac 100644
@@ -1621,7 +1621,7 @@ out:
 static inline int fc_fcp_lport_queue_ready(struct fc_lport *lp)
 {
 	/* lock ? */
-	return (lp->state == LPORT_ST_READY) && (lp->link_status & FC_LINK_UP);
+	return (lp->state == LPORT_ST_READY) && lp->link_up && !lp->qfull;
 }
 
 /**

@@ -1890,7 +1890,7 @@ int fc_eh_abort(struct scsi_cmnd *sc_cmd)
 	lp = shost_priv(sc_cmd->device->host);
 	if (lp->state != LPORT_ST_READY)
 		return rc;
-	else if (!(lp->link_status & FC_LINK_UP))
+	else if (!lp->link_up)
 		return rc;
 
 	spin_lock_irqsave(lp->host->host_lock, flags);

drivers/scsi/libfc/fc_lport.c +6 ~~-30~~

index 5db223c..a6ab692 100644
@@ -250,7 +250,7 @@ void fc_get_host_port_state(struct Scsi_Host *shost)
 {
 	struct fc_lport *lp = shost_priv(shost);
 
-	if ((lp->link_status & FC_LINK_UP) == FC_LINK_UP)
+	if (lp->link_up)
 		fc_host_port_state(shost) = FC_PORTSTATE_ONLINE;
 	else
 		fc_host_port_state(shost) = FC_PORTSTATE_OFFLINE;

@@ -577,8 +577,8 @@ void fc_linkup(struct fc_lport *lport)
 		       fc_host_port_id(lport->host));
 
 	mutex_lock(&lport->lp_mutex);
-	if ((lport->link_status & FC_LINK_UP) != FC_LINK_UP) {
-		lport->link_status |= FC_LINK_UP;
+	if (!lport->link_up) {
+		lport->link_up = 1;
 
 		if (lport->state == LPORT_ST_RESET)
 			fc_lport_enter_flogi(lport);

@@ -597,8 +597,8 @@ void fc_linkdown(struct fc_lport *lport)
 	FC_DEBUG_LPORT("Link is down for port (%6x)\n",
 		       fc_host_port_id(lport->host));
 
-	if ((lport->link_status & FC_LINK_UP) == FC_LINK_UP) {
-		lport->link_status &= ~(FC_LINK_UP);
+	if (lport->link_up) {
+		lport->link_up = 0;
 		fc_lport_enter_reset(lport);
 		lport->tt.fcp_cleanup(lport);
 	}

@@ -607,30 +607,6 @@ void fc_linkdown(struct fc_lport *lport)
 EXPORT_SYMBOL(fc_linkdown);
 
 /**
- * fc_pause - Pause the flow of frames
- * @lport: The lport to be paused
- */
-void fc_pause(struct fc_lport *lport)
-{
-	mutex_lock(&lport->lp_mutex);
-	lport->link_status |= FC_PAUSE;
-	mutex_unlock(&lport->lp_mutex);
-}
-EXPORT_SYMBOL(fc_pause);
-
-/**
- * fc_unpause - Unpause the flow of frames
- * @lport: The lport to be unpaused
- */
-void fc_unpause(struct fc_lport *lport)
-{
-	mutex_lock(&lport->lp_mutex);
-	lport->link_status &= ~(FC_PAUSE);
-	mutex_unlock(&lport->lp_mutex);
-}
-EXPORT_SYMBOL(fc_unpause);
-
-/**
  * fc_fabric_logoff - Logout of the fabric
  * @lport:	      fc_lport pointer to logoff the fabric
  *

@@ -977,7 +953,7 @@ static void fc_lport_enter_reset(struct fc_lport *lport)
 	fc_host_fabric_name(lport->host) = 0;
 	fc_host_port_id(lport->host) = 0;
 
-	if ((lport->link_status & FC_LINK_UP) == FC_LINK_UP)
+	if (lport->link_up)
 		fc_lport_enter_flogi(lport);
 }

include/scsi/libfc.h +2 ~~-10~~

index 042f4ad..b9e6c1c 100644
@@ -68,9 +68,6 @@
 /*
  * FC HBA status
  */
-#define FC_PAUSE		    (1 << 1)
-#define FC_LINK_UP		    (1 << 0)
-
 enum fc_lport_state {
 	LPORT_ST_NONE = 0,
 	LPORT_ST_FLOGI,

@@ -603,7 +600,8 @@ struct fc_lport {
 
 	/* Operational Information */
 	struct libfc_function_template tt;
-	u16			link_status;
+	u8			link_up;
+	u8			qfull;
 	enum fc_lport_state	state;
 	unsigned long		boot_time;

@@ -704,12 +702,6 @@ void fc_linkup(struct fc_lport *);
 void fc_linkdown(struct fc_lport *);
 
 /*
- * Pause and unpause traffic.
- */
-void fc_pause(struct fc_lport *);
-void fc_unpause(struct fc_lport *);
-
-/*
  * Configure the local port.
  */
 int fc_lport_config(struct fc_lport *);

Robert Love

author

2009-01-21 20:45 UTC #5 lore raw

PATCH 6/6 libfc: rport retry on LS_RJT from certain ELS

In reply to Robert Love

From: Chris Leech <[email protected]>

This allows any rport ELS to retry on LS_RJT.

The rport error handling would only retry on resource allocation failures and exchange timeouts. I have a target that will occasionally reject PLOGI when we do a quick LOGO/PLOGI. When a critical ELS was rejected, libfc would fail silently leaving the rport in a dead state.

The retry count and delay are managed by fc_rport_error_retry. If the retry count is exceeded fc_rport_error will be called. When retrying is not the correct course of action, fc_rport_error can be called directly.

Signed-off-by Chris Leech <[email protected]>

Signed-off-by Robert Love <[email protected]>

---

 drivers/scsi/libfc/fc_exch.c  |    2 -
 drivers/scsi/libfc/fc_rport.c |  111 ++++++++++++++++++++++++-----------------
 include/scsi/fc/fc_fs.h       |    5 ++
 3 files changed, 69 insertions(+), 49 deletions(-)

drivers/scsi/libfc/fc_exch.c +0 -2

index dd269e5..8c40189 100644
@@ -32,8 +32,6 @@
 #include <scsi/libfc.h>
 #include <scsi/fc_encode.h>
 
-#define	  FC_DEF_R_A_TOV      (10 * 1000) /* resource allocation timeout */
-
 /*
  * fc_exch_debug can be set in debugger or at compile time to get more logs.
  */

drivers/scsi/libfc/fc_rport.c +64 ~~-47~~

209 lines not on this page · raw message

include/scsi/fc/fc_fs.h +5 -0

index 3e4801d..1b7af3a 100644
@@ -337,4 +337,9 @@ enum fc_pf_rjt_reason {
 	FC_RJT_VENDOR =		0xff,	/* vendor specific reject */
 };
 
+/* default timeout values */
+
+#define FC_DEF_E_D_TOV	2000UL
+#define FC_DEF_R_A_TOV	10000UL
+
 #endif /* _FC_FS_H_ */