From 8798a694da59486e4a3ff0abeec183202fb34c20 Mon Sep 17 00:00:00 2001 From: Michael Reed Date: Fri, 9 Oct 2009 14:15:59 -0500 Subject: [SCSI] scsi_transport_fc: remove invalid BUG_ON I was doing some large lun count testing with 2.6.31 and hit a BUG_ON() in fc_timeout_deleted_rport(), and it seems like it should have been just a matter of time before someone did. It seems invalid to set port_state under lock, then expect it to remain set after releasing the lock. Another thread called fc_remote_port_add() when the lock was released, changing the port_state. This patch removes the BUG_ON and moves the test of the port_state to inside the host_lock. It's been running for several weeks now with no ill effect. Signed-off-by: Michael Reed Acked-by: James Smart Signed-off-by: James Bottomley --- drivers/scsi/scsi_transport_fc.c | 68 +++++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 26 deletions(-) (limited to 'drivers/scsi/scsi_transport_fc.c') diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c index a67fed10598..f436e033ada 100644 --- a/drivers/scsi/scsi_transport_fc.c +++ b/drivers/scsi/scsi_transport_fc.c @@ -2384,6 +2384,7 @@ fc_rport_final_delete(struct work_struct *work) struct Scsi_Host *shost = rport_to_shost(rport); struct fc_internal *i = to_fc_internal(shost->transportt); unsigned long flags; + int do_callback = 0; /* * if a scan is pending, flush the SCSI Host work_q so that @@ -2422,8 +2423,15 @@ fc_rport_final_delete(struct work_struct *work) * Avoid this call if we already called it when we preserved the * rport for the binding. */ + spin_lock_irqsave(shost->host_lock, flags); if (!(rport->flags & FC_RPORT_DEVLOSS_CALLBK_DONE) && - (i->f->dev_loss_tmo_callbk)) + (i->f->dev_loss_tmo_callbk)) { + rport->flags |= FC_RPORT_DEVLOSS_CALLBK_DONE; + do_callback = 1; + } + spin_unlock_irqrestore(shost->host_lock, flags); + + if (do_callback) i->f->dev_loss_tmo_callbk(rport); fc_bsg_remove(rport->rqst_q); @@ -2970,6 +2978,7 @@ fc_timeout_deleted_rport(struct work_struct *work) struct fc_internal *i = to_fc_internal(shost->transportt); struct fc_host_attrs *fc_host = shost_to_fc_host(shost); unsigned long flags; + int do_callback = 0; spin_lock_irqsave(shost->host_lock, flags); @@ -3035,7 +3044,6 @@ fc_timeout_deleted_rport(struct work_struct *work) rport->roles = FC_PORT_ROLE_UNKNOWN; rport->port_state = FC_PORTSTATE_NOTPRESENT; rport->flags &= ~FC_RPORT_FAST_FAIL_TIMEDOUT; - rport->flags |= FC_RPORT_DEVLOSS_CALLBK_DONE; /* * Pre-emptively kill I/O rather than waiting for the work queue @@ -3045,32 +3053,40 @@ fc_timeout_deleted_rport(struct work_struct *work) spin_unlock_irqrestore(shost->host_lock, flags); fc_terminate_rport_io(rport); - BUG_ON(rport->port_state != FC_PORTSTATE_NOTPRESENT); + spin_lock_irqsave(shost->host_lock, flags); - /* remove the identifiers that aren't used in the consisting binding */ - switch (fc_host->tgtid_bind_type) { - case FC_TGTID_BIND_BY_WWPN: - rport->node_name = -1; - rport->port_id = -1; - break; - case FC_TGTID_BIND_BY_WWNN: - rport->port_name = -1; - rport->port_id = -1; - break; - case FC_TGTID_BIND_BY_ID: - rport->node_name = -1; - rport->port_name = -1; - break; - case FC_TGTID_BIND_NONE: /* to keep compiler happy */ - break; + if (rport->port_state == FC_PORTSTATE_NOTPRESENT) { /* still missing */ + + /* remove the identifiers that aren't used in the consisting binding */ + switch (fc_host->tgtid_bind_type) { + case FC_TGTID_BIND_BY_WWPN: + rport->node_name = -1; + rport->port_id = -1; + break; + case FC_TGTID_BIND_BY_WWNN: + rport->port_name = -1; + rport->port_id = -1; + break; + case FC_TGTID_BIND_BY_ID: + rport->node_name = -1; + rport->port_name = -1; + break; + case FC_TGTID_BIND_NONE: /* to keep compiler happy */ + break; + } + + /* + * As this only occurs if the remote port (scsi target) + * went away and didn't come back - we'll remove + * all attached scsi devices. + */ + rport->flags |= FC_RPORT_DEVLOSS_CALLBK_DONE; + fc_queue_work(shost, &rport->stgt_delete_work); + + do_callback = 1; } - /* - * As this only occurs if the remote port (scsi target) - * went away and didn't come back - we'll remove - * all attached scsi devices. - */ - fc_queue_work(shost, &rport->stgt_delete_work); + spin_unlock_irqrestore(shost->host_lock, flags); /* * Notify the driver that the rport is now dead. The LLDD will @@ -3078,7 +3094,7 @@ fc_timeout_deleted_rport(struct work_struct *work) * * Note: we set the CALLBK_DONE flag above to correspond */ - if (i->f->dev_loss_tmo_callbk) + if (do_callback && i->f->dev_loss_tmo_callbk) i->f->dev_loss_tmo_callbk(rport); } -- cgit v1.2.3 From 65d430fa99cbd0e88d09a3343f697c51fc8a7009 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Fri, 30 Oct 2009 17:59:29 +0100 Subject: [SCSI] scsi_transport_fc: Introduce helper function for blocking scsi_eh Move the duplicated code from FC LLDs to SCSI FC transport class. Acked-by: James Smart Acked-by: Giridhar Malavali Acked-by: Abhijeet Joglekar Signed-off-by: Christof Schmitt Signed-off-by: James Bottomley --- drivers/scsi/scsi_transport_fc.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'drivers/scsi/scsi_transport_fc.c') diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c index f436e033ada..3ce56b3b2cd 100644 --- a/drivers/scsi/scsi_transport_fc.c +++ b/drivers/scsi/scsi_transport_fc.c @@ -27,6 +27,7 @@ */ #include #include +#include #include #include #include @@ -3144,6 +3145,31 @@ fc_scsi_scan_rport(struct work_struct *work) spin_unlock_irqrestore(shost->host_lock, flags); } +/** + * fc_block_scsi_eh - Block SCSI eh thread for blocked fc_rport + * @cmnd: SCSI command that scsi_eh is trying to recover + * + * This routine can be called from a FC LLD scsi_eh callback. It + * blocks the scsi_eh thread until the fc_rport leaves the + * FC_PORTSTATE_BLOCKED. This is necessary to avoid the scsi_eh + * failing recovery actions for blocked rports which would lead to + * offlined SCSI devices. + */ +void fc_block_scsi_eh(struct scsi_cmnd *cmnd) +{ + struct Scsi_Host *shost = cmnd->device->host; + struct fc_rport *rport = starget_to_rport(scsi_target(cmnd->device)); + unsigned long flags; + + spin_lock_irqsave(shost->host_lock, flags); + while (rport->port_state == FC_PORTSTATE_BLOCKED) { + spin_unlock_irqrestore(shost->host_lock, flags); + msleep(1000); + spin_lock_irqsave(shost->host_lock, flags); + } + spin_unlock_irqrestore(shost->host_lock, flags); +} +EXPORT_SYMBOL(fc_block_scsi_eh); /** * fc_vport_setup - allocates and creates a FC virtual port. -- cgit v1.2.3 From 2bc1c59dbdefdb6f9767e06efb86bbdb2923a8be Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Thu, 5 Nov 2009 11:18:09 -0600 Subject: [SCSI] fc class: fail fast bsg requests If the port state is blocked and the fast io fail tmo has fired then this patch will fail bsg requests immediately. This is needed if userspace is sending IOs to test the transport like with fcping, so it will not have to wait for the dev loss tmo. With this patch he bsg req fast io fail code behaves like the normal and sg io/passthrough fast io fail. Signed-off-by: Mike Christie Acked-By: James Smart Signed-off-by: James Bottomley --- drivers/scsi/scsi_transport_fc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/scsi/scsi_transport_fc.c') diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c index 3ce56b3b2cd..600502aa3b6 100644 --- a/drivers/scsi/scsi_transport_fc.c +++ b/drivers/scsi/scsi_transport_fc.c @@ -3809,8 +3809,9 @@ fc_bsg_request_handler(struct request_queue *q, struct Scsi_Host *shost, return; while (!blk_queue_plugged(q)) { - if (rport && (rport->port_state == FC_PORTSTATE_BLOCKED)) - break; + if (rport && (rport->port_state == FC_PORTSTATE_BLOCKED) && + !(rport->flags & FC_RPORT_FAST_FAIL_TIMEDOUT)) + break; req = blk_fetch_request(q); if (!req) -- cgit v1.2.3