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 a67fed10598a..f436e033adaf 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/fnic/fnic_scsi.c | 20 ++------------------ drivers/scsi/lpfc/lpfc_scsi.c | 30 ++++-------------------------- drivers/scsi/qla2xxx/qla_os.c | 25 ++++--------------------- drivers/scsi/scsi_transport_fc.c | 26 ++++++++++++++++++++++++++ include/scsi/scsi_transport_fc.h | 1 + 5 files changed, 37 insertions(+), 65 deletions(-) (limited to 'drivers/scsi/scsi_transport_fc.c') diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c index b5d17385939b..8d26d7a9f01b 100644 --- a/drivers/scsi/fnic/fnic_scsi.c +++ b/drivers/scsi/fnic/fnic_scsi.c @@ -1225,22 +1225,6 @@ void fnic_terminate_rport_io(struct fc_rport *rport) } -static void fnic_block_error_handler(struct scsi_cmnd *sc) -{ - struct Scsi_Host *shost = sc->device->host; - struct fc_rport *rport = starget_to_rport(scsi_target(sc->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); - -} - /* * This function is exported to SCSI for sending abort cmnds. * A SCSI IO is represented by a io_req in the driver. @@ -1260,7 +1244,7 @@ int fnic_abort_cmd(struct scsi_cmnd *sc) DECLARE_COMPLETION_ONSTACK(tm_done); /* Wait for rport to unblock */ - fnic_block_error_handler(sc); + fc_block_scsi_eh(sc); /* Get local-port, check ready and link up */ lp = shost_priv(sc->device->host); @@ -1542,7 +1526,7 @@ int fnic_device_reset(struct scsi_cmnd *sc) DECLARE_COMPLETION_ONSTACK(tm_done); /* Wait for rport to unblock */ - fnic_block_error_handler(sc); + fc_block_scsi_eh(sc); /* Get local-port, check ready and link up */ lp = shost_priv(sc->device->host); diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c index c88f59f0ce30..e25179193a82 100644 --- a/drivers/scsi/lpfc/lpfc_scsi.c +++ b/drivers/scsi/lpfc/lpfc_scsi.c @@ -2916,28 +2916,6 @@ lpfc_queuecommand(struct scsi_cmnd *cmnd, void (*done) (struct scsi_cmnd *)) return 0; } -/** - * lpfc_block_error_handler - Routine to block error handler - * @cmnd: Pointer to scsi_cmnd data structure. - * - * This routine blocks execution till fc_rport state is not FC_PORSTAT_BLCOEKD. - **/ -static void -lpfc_block_error_handler(struct scsi_cmnd *cmnd) -{ - struct Scsi_Host *shost = cmnd->device->host; - struct fc_rport *rport = starget_to_rport(scsi_target(cmnd->device)); - - spin_lock_irq(shost->host_lock); - while (rport->port_state == FC_PORTSTATE_BLOCKED) { - spin_unlock_irq(shost->host_lock); - msleep(1000); - spin_lock_irq(shost->host_lock); - } - spin_unlock_irq(shost->host_lock); - return; -} - /** * lpfc_abort_handler - scsi_host_template eh_abort_handler entry point * @cmnd: Pointer to scsi_cmnd data structure. @@ -2961,7 +2939,7 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd) int ret = SUCCESS; DECLARE_WAIT_QUEUE_HEAD_ONSTACK(waitq); - lpfc_block_error_handler(cmnd); + fc_block_scsi_eh(cmnd); lpfc_cmd = (struct lpfc_scsi_buf *)cmnd->host_scribble; BUG_ON(!lpfc_cmd); @@ -3259,7 +3237,7 @@ lpfc_device_reset_handler(struct scsi_cmnd *cmnd) struct lpfc_scsi_event_header scsi_event; int status; - lpfc_block_error_handler(cmnd); + fc_block_scsi_eh(cmnd); status = lpfc_chk_tgt_mapped(vport, cmnd); if (status == FAILED) { @@ -3318,7 +3296,7 @@ lpfc_target_reset_handler(struct scsi_cmnd *cmnd) struct lpfc_scsi_event_header scsi_event; int status; - lpfc_block_error_handler(cmnd); + fc_block_scsi_eh(cmnd); status = lpfc_chk_tgt_mapped(vport, cmnd); if (status == FAILED) { @@ -3384,7 +3362,7 @@ lpfc_bus_reset_handler(struct scsi_cmnd *cmnd) fc_host_post_vendor_event(shost, fc_get_event_number(), sizeof(scsi_event), (char *)&scsi_event, LPFC_NL_VENDOR_ID); - lpfc_block_error_handler(cmnd); + fc_block_scsi_eh(cmnd); /* * Since the driver manages a single bus device, reset all diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c index d69744a62fe4..41669357b186 100644 --- a/drivers/scsi/qla2xxx/qla_os.c +++ b/drivers/scsi/qla2xxx/qla_os.c @@ -728,23 +728,6 @@ qla2x00_abort_fcport_cmds(fc_port_t *fcport) spin_unlock_irqrestore(&ha->hardware_lock, flags); } -static void -qla2x00_block_error_handler(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); - return; -} - /************************************************************************** * qla2xxx_eh_abort * @@ -774,7 +757,7 @@ qla2xxx_eh_abort(struct scsi_cmnd *cmd) struct req_que *req = vha->req; srb_t *spt; - qla2x00_block_error_handler(cmd); + fc_block_scsi_eh(cmd); if (!CMD_SP(cmd)) return SUCCESS; @@ -905,7 +888,7 @@ __qla2xxx_eh_generic_reset(char *name, enum nexus_wait_type type, fc_port_t *fcport = (struct fc_port *) cmd->device->hostdata; int err; - qla2x00_block_error_handler(cmd); + fc_block_scsi_eh(cmd); if (!fcport) return FAILED; @@ -985,7 +968,7 @@ qla2xxx_eh_bus_reset(struct scsi_cmnd *cmd) unsigned long serial; srb_t *sp = (srb_t *) CMD_SP(cmd); - qla2x00_block_error_handler(cmd); + fc_block_scsi_eh(cmd); id = cmd->device->id; lun = cmd->device->lun; @@ -1048,7 +1031,7 @@ qla2xxx_eh_host_reset(struct scsi_cmnd *cmd) srb_t *sp = (srb_t *) CMD_SP(cmd); scsi_qla_host_t *base_vha = pci_get_drvdata(ha->pdev); - qla2x00_block_error_handler(cmd); + fc_block_scsi_eh(cmd); id = cmd->device->id; lun = cmd->device->lun; diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c index f436e033adaf..3ce56b3b2cd7 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. diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h index fc50bd64aa4e..8e86a94faf06 100644 --- a/include/scsi/scsi_transport_fc.h +++ b/include/scsi/scsi_transport_fc.h @@ -807,5 +807,6 @@ void fc_host_post_vendor_event(struct Scsi_Host *shost, u32 event_number, struct fc_vport *fc_vport_create(struct Scsi_Host *shost, int channel, struct fc_vport_identifiers *); int fc_vport_terminate(struct fc_vport *vport); +void fc_block_scsi_eh(struct scsi_cmnd *cmnd); #endif /* SCSI_TRANSPORT_FC_H */ -- 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 3ce56b3b2cd7..600502aa3b61 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