From bc0e17f691085315ae9303eb5b0883fe16dfe6b1 Mon Sep 17 00:00:00 2001 From: Vasu Dev Date: Fri, 27 Feb 2009 10:54:57 -0800 Subject: [SCSI] libfc, fcoe: fixed locking issues with lport->lp_mutex around lport->link_status 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 Signed-off-by: Robert Love Signed-off-by: James Bottomley --- drivers/scsi/libfc/fc_fcp.c | 4 ++-- drivers/scsi/libfc/fc_lport.c | 36 ++++++------------------------------ 2 files changed, 8 insertions(+), 32 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c index 404e63ff46b..f440aaca39c 100644 --- a/drivers/scsi/libfc/fc_fcp.c +++ b/drivers/scsi/libfc/fc_fcp.c @@ -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); diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c index 5db223ce3b2..a6ab692f5f5 100644 --- a/drivers/scsi/libfc/fc_lport.c +++ b/drivers/scsi/libfc/fc_lport.c @@ -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); } @@ -606,30 +606,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); } -- cgit v1.2.3