diff options
author | Vasu Dev <vasu.dev@intel.com> | 2009-02-27 13:54:57 -0500 |
---|---|---|
committer | James Bottomley <James.Bottomley@HansenPartnership.com> | 2009-03-06 16:37:49 -0500 |
commit | bc0e17f691085315ae9303eb5b0883fe16dfe6b1 (patch) | |
tree | 53138e80aec810604d4eca4626e4f8df65376ccc /drivers/scsi/libfc | |
parent | a7e84f2b83f17f8f11da34ccef3ba5a862dc0182 (diff) |
[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 <vasu.dev@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Diffstat (limited to 'drivers/scsi/libfc')
-rw-r--r-- | drivers/scsi/libfc/fc_fcp.c | 4 | ||||
-rw-r--r-- | drivers/scsi/libfc/fc_lport.c | 36 |
2 files changed, 8 insertions, 32 deletions
diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c index 404e63ff46b8..f440aaca39c2 100644 --- a/drivers/scsi/libfc/fc_fcp.c +++ b/drivers/scsi/libfc/fc_fcp.c | |||
@@ -1621,7 +1621,7 @@ out: | |||
1621 | static inline int fc_fcp_lport_queue_ready(struct fc_lport *lp) | 1621 | static inline int fc_fcp_lport_queue_ready(struct fc_lport *lp) |
1622 | { | 1622 | { |
1623 | /* lock ? */ | 1623 | /* lock ? */ |
1624 | return (lp->state == LPORT_ST_READY) && (lp->link_status & FC_LINK_UP); | 1624 | return (lp->state == LPORT_ST_READY) && lp->link_up && !lp->qfull; |
1625 | } | 1625 | } |
1626 | 1626 | ||
1627 | /** | 1627 | /** |
@@ -1890,7 +1890,7 @@ int fc_eh_abort(struct scsi_cmnd *sc_cmd) | |||
1890 | lp = shost_priv(sc_cmd->device->host); | 1890 | lp = shost_priv(sc_cmd->device->host); |
1891 | if (lp->state != LPORT_ST_READY) | 1891 | if (lp->state != LPORT_ST_READY) |
1892 | return rc; | 1892 | return rc; |
1893 | else if (!(lp->link_status & FC_LINK_UP)) | 1893 | else if (!lp->link_up) |
1894 | return rc; | 1894 | return rc; |
1895 | 1895 | ||
1896 | spin_lock_irqsave(lp->host->host_lock, flags); | 1896 | 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 5db223ce3b25..a6ab692f5f51 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) | |||
250 | { | 250 | { |
251 | struct fc_lport *lp = shost_priv(shost); | 251 | struct fc_lport *lp = shost_priv(shost); |
252 | 252 | ||
253 | if ((lp->link_status & FC_LINK_UP) == FC_LINK_UP) | 253 | if (lp->link_up) |
254 | fc_host_port_state(shost) = FC_PORTSTATE_ONLINE; | 254 | fc_host_port_state(shost) = FC_PORTSTATE_ONLINE; |
255 | else | 255 | else |
256 | fc_host_port_state(shost) = FC_PORTSTATE_OFFLINE; | 256 | fc_host_port_state(shost) = FC_PORTSTATE_OFFLINE; |
@@ -577,8 +577,8 @@ void fc_linkup(struct fc_lport *lport) | |||
577 | fc_host_port_id(lport->host)); | 577 | fc_host_port_id(lport->host)); |
578 | 578 | ||
579 | mutex_lock(&lport->lp_mutex); | 579 | mutex_lock(&lport->lp_mutex); |
580 | if ((lport->link_status & FC_LINK_UP) != FC_LINK_UP) { | 580 | if (!lport->link_up) { |
581 | lport->link_status |= FC_LINK_UP; | 581 | lport->link_up = 1; |
582 | 582 | ||
583 | if (lport->state == LPORT_ST_RESET) | 583 | if (lport->state == LPORT_ST_RESET) |
584 | fc_lport_enter_flogi(lport); | 584 | fc_lport_enter_flogi(lport); |
@@ -597,8 +597,8 @@ void fc_linkdown(struct fc_lport *lport) | |||
597 | FC_DEBUG_LPORT("Link is down for port (%6x)\n", | 597 | FC_DEBUG_LPORT("Link is down for port (%6x)\n", |
598 | fc_host_port_id(lport->host)); | 598 | fc_host_port_id(lport->host)); |
599 | 599 | ||
600 | if ((lport->link_status & FC_LINK_UP) == FC_LINK_UP) { | 600 | if (lport->link_up) { |
601 | lport->link_status &= ~(FC_LINK_UP); | 601 | lport->link_up = 0; |
602 | fc_lport_enter_reset(lport); | 602 | fc_lport_enter_reset(lport); |
603 | lport->tt.fcp_cleanup(lport); | 603 | lport->tt.fcp_cleanup(lport); |
604 | } | 604 | } |
@@ -607,30 +607,6 @@ void fc_linkdown(struct fc_lport *lport) | |||
607 | EXPORT_SYMBOL(fc_linkdown); | 607 | EXPORT_SYMBOL(fc_linkdown); |
608 | 608 | ||
609 | /** | 609 | /** |
610 | * fc_pause - Pause the flow of frames | ||
611 | * @lport: The lport to be paused | ||
612 | */ | ||
613 | void fc_pause(struct fc_lport *lport) | ||
614 | { | ||
615 | mutex_lock(&lport->lp_mutex); | ||
616 | lport->link_status |= FC_PAUSE; | ||
617 | mutex_unlock(&lport->lp_mutex); | ||
618 | } | ||
619 | EXPORT_SYMBOL(fc_pause); | ||
620 | |||
621 | /** | ||
622 | * fc_unpause - Unpause the flow of frames | ||
623 | * @lport: The lport to be unpaused | ||
624 | */ | ||
625 | void fc_unpause(struct fc_lport *lport) | ||
626 | { | ||
627 | mutex_lock(&lport->lp_mutex); | ||
628 | lport->link_status &= ~(FC_PAUSE); | ||
629 | mutex_unlock(&lport->lp_mutex); | ||
630 | } | ||
631 | EXPORT_SYMBOL(fc_unpause); | ||
632 | |||
633 | /** | ||
634 | * fc_fabric_logoff - Logout of the fabric | 610 | * fc_fabric_logoff - Logout of the fabric |
635 | * @lport: fc_lport pointer to logoff the fabric | 611 | * @lport: fc_lport pointer to logoff the fabric |
636 | * | 612 | * |
@@ -977,7 +953,7 @@ static void fc_lport_enter_reset(struct fc_lport *lport) | |||
977 | fc_host_fabric_name(lport->host) = 0; | 953 | fc_host_fabric_name(lport->host) = 0; |
978 | fc_host_port_id(lport->host) = 0; | 954 | fc_host_port_id(lport->host) = 0; |
979 | 955 | ||
980 | if ((lport->link_status & FC_LINK_UP) == FC_LINK_UP) | 956 | if (lport->link_up) |
981 | fc_lport_enter_flogi(lport); | 957 | fc_lport_enter_flogi(lport); |
982 | } | 958 | } |
983 | 959 | ||