From 73b4376477672a4336cba5f26ede2e277511ce70 Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Fri, 8 Oct 2010 17:12:10 -0700 Subject: [SCSI] libfc: fix setting of rport dev loss There does not seem to be a reason why libfc adds a 5 second delay to the user requested value for the dev loss tmo. There also does not seem to be a reason to allow setting it to 0 (or really close). This patch removes the extra 5 sec delay, and for 0 it sets it to 1 like other fc drivers. We should actually be able to set it to 0 since the queue_delayed_work API will just call queue_work, but other drivers set it to 1 in that case. Signed-off-by: Mike Christie Signed-off-by: Robert Love Signed-off-by: James Bottomley --- drivers/scsi/libfc/fc_rport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c index b9f2286fe0cb..a84ef13ed74a 100644 --- a/drivers/scsi/libfc/fc_rport.c +++ b/drivers/scsi/libfc/fc_rport.c @@ -196,9 +196,9 @@ static const char *fc_rport_state(struct fc_rport_priv *rdata) void fc_set_rport_loss_tmo(struct fc_rport *rport, u32 timeout) { if (timeout) - rport->dev_loss_tmo = timeout + 5; + rport->dev_loss_tmo = timeout; else - rport->dev_loss_tmo = 30; + rport->dev_loss_tmo = 1; } EXPORT_SYMBOL(fc_set_rport_loss_tmo); -- cgit v1.2.2 From 75eee7252466342e0dfa6675106bcd3955090f67 Mon Sep 17 00:00:00 2001 From: Vasu Dev Date: Fri, 8 Oct 2010 17:12:15 -0700 Subject: [SCSI] libfc: use DID_TRANSPORT_DISRUPTED while lport not ready This is per Mile Christie feedback since in this case IO could get retried for tape devices and therefore DID_REQUEUE cannot be used, more details in this thread. http://marc.info/?l=linux-scsi&m=127970522630136&w=2 Signed-off-by: Vasu Dev Signed-off-by: Robert Love Signed-off-by: James Bottomley --- drivers/scsi/libfc/fc_fcp.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c index c797f6b48f05..43866e6d0bf2 100644 --- a/drivers/scsi/libfc/fc_fcp.c +++ b/drivers/scsi/libfc/fc_fcp.c @@ -1971,10 +1971,8 @@ static void fc_io_compl(struct fc_fcp_pkt *fsp) break; } - if (lport->state != LPORT_ST_READY && fsp->status_code != FC_COMPLETE) { - sc_cmd->result = (DID_REQUEUE << 16); - FC_FCP_DBG(fsp, "Returning DID_REQUEUE to scsi-ml\n"); - } + if (lport->state != LPORT_ST_READY && fsp->status_code != FC_COMPLETE) + sc_cmd->result = (DID_TRANSPORT_DISRUPTED << 16); spin_lock_irqsave(&si->scsi_queue_lock, flags); list_del(&fsp->list); -- cgit v1.2.2 From 60a3c4dfb7830845a97a20e73b7d938dc2fde6c1 Mon Sep 17 00:00:00 2001 From: Vasu Dev Date: Fri, 8 Oct 2010 17:12:20 -0700 Subject: [SCSI] libfc: adds flogi retry in case DID is zero in RJT Sometimes switch in NPV mode rejects flogi request with DID zero and in that case flogi is not tried again and port remains offline, so this patch validates DID for non zero along with only ACC response to allow flogi retry for RJT with DID=0 also succeed FLOGI in next try. Signed-off-by: Vasu Dev Signed-off-by: Robert Love Signed-off-by: James Bottomley --- drivers/scsi/libfc/fc_lport.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c index d9b6e11b0e88..9be63edbf8fb 100644 --- a/drivers/scsi/libfc/fc_lport.c +++ b/drivers/scsi/libfc/fc_lport.c @@ -1447,13 +1447,7 @@ void fc_lport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp, } did = fc_frame_did(fp); - - if (!did) { - FC_LPORT_DBG(lport, "Bad FLOGI response\n"); - goto out; - } - - if (fc_frame_payload_op(fp) == ELS_LS_ACC) { + if (fc_frame_payload_op(fp) == ELS_LS_ACC && did) { flp = fc_frame_payload_get(fp, sizeof(*flp)); if (flp) { mfs = ntohs(flp->fl_csp.sp_bb_data) & @@ -1492,8 +1486,10 @@ void fc_lport_flogi_resp(struct fc_seq *sp, struct fc_frame *fp, fc_lport_enter_dns(lport); } } - } else + } else { + FC_LPORT_DBG(lport, "FLOGI RJT or bad response\n"); fc_lport_error(lport, fp); + } out: fc_frame_free(fp); -- cgit v1.2.2 From 8b7ac2bb07bbadb0636f21f51564e6d363bb6d20 Mon Sep 17 00:00:00 2001 From: Vasu Dev Date: Fri, 8 Oct 2010 17:12:31 -0700 Subject: [SCSI] libfc: possible race could panic system due to NULL fsp->cmd It is unlikely but in case if it hits then it would cause panic due to null cmd ptr, so far only one instance seen recently with ESX though this was introduced long ago with this commit:- commit c1ecb90a66c5afc7cc5c9349f9c3714eef4a5cfb Author: Chris Leech Date: Thu Dec 10 09:59:26 2009 -0800 [SCSI] libfc: reduce hold time on SCSI host lock Currently fsp->cmd is set to NULL w/o scsi_queue_lock before dequeuing from scsi_pkt_queue and that could cause NULL fsp->cmd in fc_fcp_cleanup_each_cmd for cmd completing with fsp->cmd = NULL after fc_fcp_cleanup_each_cmd taken reference. No need to set fsp->cmd to NULL as this is also protected by fc_fcp_lock_pkt(), for above race the fc_fcp_lock_pkt() in fc_fcp_cleanup_each_cmd() will fail as that cmd is already done. Mike mentioned same issue at http://www.open-fcoe.org/pipermail/devel/2010-September/010533.html Similarly moved sc_cmd->SCp.ptr = NULL under scsi_queue_lock so that scsi abort error handler won't abort on completed cmds. Signed-off-by: Vasu Dev Signed-off-by: Robert Love Signed-off-by: James Bottomley --- drivers/scsi/libfc/fc_fcp.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c index 43866e6d0bf2..e340373b509b 100644 --- a/drivers/scsi/libfc/fc_fcp.c +++ b/drivers/scsi/libfc/fc_fcp.c @@ -58,8 +58,7 @@ struct kmem_cache *scsi_pkt_cachep; #define FC_SRB_WRITE (1 << 0) /* - * The SCp.ptr should be tested and set under the host lock. NULL indicates - * that the command has been retruned to the scsi layer. + * The SCp.ptr should be tested and set under the scsi_pkt_queue lock */ #define CMD_SP(Cmnd) ((struct fc_fcp_pkt *)(Cmnd)->SCp.ptr) #define CMD_ENTRY_STATUS(Cmnd) ((Cmnd)->SCp.have_data_in) @@ -1880,8 +1879,6 @@ static void fc_io_compl(struct fc_fcp_pkt *fsp) lport = fsp->lp; si = fc_get_scsi_internal(lport); - if (!fsp->cmd) - return; /* * if can_queue ramp down is done then try can_queue ramp up @@ -1891,11 +1888,6 @@ static void fc_io_compl(struct fc_fcp_pkt *fsp) fc_fcp_can_queue_ramp_up(lport); sc_cmd = fsp->cmd; - fsp->cmd = NULL; - - if (!sc_cmd->SCp.ptr) - return; - CMD_SCSI_STATUS(sc_cmd) = fsp->cdb_status; switch (fsp->status_code) { case FC_COMPLETE: @@ -1976,8 +1968,8 @@ static void fc_io_compl(struct fc_fcp_pkt *fsp) spin_lock_irqsave(&si->scsi_queue_lock, flags); list_del(&fsp->list); - spin_unlock_irqrestore(&si->scsi_queue_lock, flags); sc_cmd->SCp.ptr = NULL; + spin_unlock_irqrestore(&si->scsi_queue_lock, flags); sc_cmd->scsi_done(sc_cmd); /* release ref from initial allocation in queue command */ @@ -1995,6 +1987,7 @@ int fc_eh_abort(struct scsi_cmnd *sc_cmd) { struct fc_fcp_pkt *fsp; struct fc_lport *lport; + struct fc_fcp_internal *si; int rc = FAILED; unsigned long flags; @@ -2004,7 +1997,8 @@ int fc_eh_abort(struct scsi_cmnd *sc_cmd) else if (!lport->link_up) return rc; - spin_lock_irqsave(lport->host->host_lock, flags); + si = fc_get_scsi_internal(lport); + spin_lock_irqsave(&si->scsi_queue_lock, flags); fsp = CMD_SP(sc_cmd); if (!fsp) { /* command completed while scsi eh was setting up */ @@ -2013,7 +2007,7 @@ int fc_eh_abort(struct scsi_cmnd *sc_cmd) } /* grab a ref so the fsp and sc_cmd cannot be relased from under us */ fc_fcp_pkt_hold(fsp); - spin_unlock_irqrestore(lport->host->host_lock, flags); + spin_unlock_irqrestore(&si->scsi_queue_lock, flags); if (fc_fcp_lock_pkt(fsp)) { /* completed while we were waiting for timer to be deleted */ -- cgit v1.2.2 From c531b9b49b146e1535dbed006d15e58f4f528f7e Mon Sep 17 00:00:00 2001 From: Bhanu Prakash Gollapudi Date: Fri, 8 Oct 2010 17:12:36 -0700 Subject: [SCSI] libfc: Do not let disc work cancel itself When number of NPIV ports created are greater than the xids allocated per pool -- for eg., creating 255 NPIV ports on a system with nr_cpu_ids of 32, with each pool containing 128 xids -- and then generating a link event - for eg., shutdown/no shutdown -- on the switch port causes the hang with the following stack trace. Call Trace: schedule_timeout+0x19d/0x230 wait_for_common+0xc0/0x170 __cancel_work_timer+0xcf/0x1b0 fc_disc_stop+0x16/0x30 [libfc] fc_lport_reset_locked+0x47/0x90 [libfc] fc_lport_enter_reset+0x67/0xe0 [libfc] fc_lport_disc_callback+0xbc/0xe0 [libfc] fc_disc_done+0xa8/0xf0 [libfc] fc_disc_timeout+0x29/0x40 [libfc] run_workqueue+0xb8/0x140 worker_thread+0x96/0x110 kthread+0x96/0xa0 child_rip+0xa/0x20 Fix is to not cancel the disc_work if discovery is already stopped, thus allowing lport state machine to restart and try discovery again. Signed-off-by: Bhanu Prakash Gollapudi Acked-by: Joe Eykholt Signed-off-by: Robert Love Signed-off-by: James Bottomley --- drivers/scsi/libfc/fc_disc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_disc.c b/drivers/scsi/libfc/fc_disc.c index 32f67c4b03fc..911b2736cafa 100644 --- a/drivers/scsi/libfc/fc_disc.c +++ b/drivers/scsi/libfc/fc_disc.c @@ -684,10 +684,9 @@ void fc_disc_stop(struct fc_lport *lport) { struct fc_disc *disc = &lport->disc; - if (disc) { + if (disc->pending) cancel_delayed_work_sync(&disc->disc_work); - fc_disc_stop_rports(disc); - } + fc_disc_stop_rports(disc); } /** -- cgit v1.2.2 From f281233d3eba15fb225d21ae2e228fd4553d824a Mon Sep 17 00:00:00 2001 From: Jeff Garzik Date: Tue, 16 Nov 2010 02:10:29 -0500 Subject: SCSI host lock push-down Move the mid-layer's ->queuecommand() invocation from being locked with the host lock to being unlocked to facilitate speeding up the critical path for drivers who don't need this lock taken anyway. The patch below presents a simple SCSI host lock push-down as an equivalent transformation. No locking or other behavior should change with this patch. All existing bugs and locking orders are preserved. Additionally, add one parameter to queuecommand, struct Scsi_Host * and remove one parameter from queuecommand, void (*done)(struct scsi_cmnd *) Scsi_Host* is a convenient pointer that most host drivers need anyway, and 'done' is redundant to struct scsi_cmnd->scsi_done. Minimal code disturbance was attempted with this change. Most drivers needed only two one-line modifications for their host lock push-down. Signed-off-by: Jeff Garzik Acked-by: James Bottomley Signed-off-by: Linus Torvalds --- drivers/scsi/libfc/fc_fcp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/scsi/libfc') diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c index e340373b509b..2924363d142b 100644 --- a/drivers/scsi/libfc/fc_fcp.c +++ b/drivers/scsi/libfc/fc_fcp.c @@ -1753,7 +1753,7 @@ static inline int fc_fcp_lport_queue_ready(struct fc_lport *lport) * This is the i/o strategy routine, called by the SCSI layer. This routine * is called with the host_lock held. */ -int fc_queuecommand(struct scsi_cmnd *sc_cmd, void (*done)(struct scsi_cmnd *)) +static int fc_queuecommand_lck(struct scsi_cmnd *sc_cmd, void (*done)(struct scsi_cmnd *)) { struct fc_lport *lport; struct fc_rport *rport = starget_to_rport(scsi_target(sc_cmd->device)); @@ -1851,6 +1851,8 @@ out: spin_lock_irq(lport->host->host_lock); return rc; } + +DEF_SCSI_QCMD(fc_queuecommand) EXPORT_SYMBOL(fc_queuecommand); /** -- cgit v1.2.2