On Mon, Mar 22, 2010 at 03:12:40PM +0100, Christof Schmitt wrote: > On Wed, Mar 17, 2010 at 11:24:52PM -0500, Mike Christie wrote: > > On 03/16/2010 08:04 AM, Christof Schmitt wrote: > > >On Tue, Mar 09, 2010 at 09:14:37AM -0500, James Smart wrote: > > >>I don't ever expect to see large dev_loss_tmo values, but the patch is fine. > > >> > > >>Acked-by: James Smart<james.smart@xxxxxxxxxx> > > >> > > >>-- james s > > >> > > >> > > >>Hannes Reinecke wrote: > > >>>The rport structure defines dev_loss_tmo as u32, which is > > >>>later multiplied with HZ to get the actual timeout value. > > >>>This might overflow for large dev_loss_tmo values. So we > > >>>should be better using u64 as intermediate variables here > > >>>to protect against overflow. > > >>> > > >>>Signed-off-by: Hannes Reinecke<hare@xxxxxxx> > > >[...] > > > > > >I guess this is the intended use to prevent the dev_loss_tmo from > > >removing the SCSI devices: > > >http://git.kernel.org/?p=linux/kernel/git/hare/multipath-tools.git;a=commitdiff;h=b9903e2e8a6cdc5042897719fbae6c9346820bbf;hp=ed1dc6164fe530d146cfe65d4f99e44ec9b54b95 > > > > > >But does this raise the question again how to run SCSI EH with remote > > >port failures? > > > > > >The SCSI FC LLDs call fc_block_scsi_eh to wait until the fc_rport > > >leaves the state FC_PORTSTATE_BLOCKED. This effectively prevents SCSI > > >devices from being taken offline when there is a command timeout and > > >the fc_rport is BLOCKED. With the large dev_loss_tmo, the dev_loss_tmo > > >never expires and a problem with a single remote port can block the > > >host error handler. > > > > > > > I did the attached patch for a problem where we were supposed to be > > getting a RSCN but we were not, so the scsi eh ended up running. > > Then we sat in the scsi eh for a long time. multipath was being > > used, so we wanted to fast fail, but the the fc block scsi eh helper > > only waits for the rport state to change. > > > > It ended up the switch was busted and should have given a RSCN, but > > I think the patch is helpful for your problem where we fall into the > > long wait for legitimate reasons. > > > > This patch works nicely when using multipath to get IO failed and > > hosts unblocked, but the problem is that it could offline the > > devices more quickly than what we are prepared for. We have > > something similar for iscsi, but the iscsi userspace daemon has a > > way to set the device back to running when it recovers. FC does not, > > and dm-multipath does not do this, so I am not pushing the patch. > > > > I think if we can figure something out for that offline issue the > > patch could be helpful. I was thinking that if the the scsi eh > > failed because the fast io fail timer fired, then we could set the > > devices to a new state. The scsi_transport_fc and > > scsi_transport_iscsi code could use this new device state to > > indicate that the fast io fail timer has fired and we are failing > > IO, but the device is not in offline state and is not blocked. > > Thanks for providing the information and ideas. I think the main point > is to get the information that I/O has been failed to the scsi eh. > What about introducing a return value to indicate that I/O has been > failed with terminate_rport_io? This is a first shot to implement this > for the abort callback, but it needs to be extended for the other > callbacks as well. Or should a new SCSI device state be preferred? > > --- > drivers/scsi/scsi_error.c | 5 +++-- > drivers/scsi/scsi_transport_fc.c | 20 +++++++++++++++----- > include/scsi/scsi.h | 1 + > include/scsi/scsi_transport_fc.h | 2 +- > 4 files changed, 20 insertions(+), 8 deletions(-) > > --- a/drivers/scsi/scsi_error.c 2009-12-03 17:41:27.000000000 +0100 > +++ b/drivers/scsi/scsi_error.c 2010-03-22 14:58:07.000000000 +0100 > @@ -956,10 +956,11 @@ static int scsi_eh_abort_cmds(struct lis > "0x%p\n", current->comm, > scmd)); > rtn = scsi_try_to_abort_cmd(scmd); > - if (rtn == SUCCESS) { > + if (rtn == SUCCESS || rtn == FAST_IO_FAIL) { > scmd->eh_eflags &= ~SCSI_EH_CANCEL_CMD; > if (!scsi_device_online(scmd->device) || > - !scsi_eh_tur(scmd)) { > + !scsi_eh_tur(scmd) || > + rtn == FAST_IO_FAIL) { > scsi_eh_finish_cmd(scmd, done_q); > } > > --- a/drivers/scsi/scsi_transport_fc.c 2010-02-26 12:59:27.000000000 +0100 > +++ b/drivers/scsi/scsi_transport_fc.c 2010-03-22 14:56:28.000000000 +0100 > @@ -3162,23 +3162,33 @@ fc_scsi_scan_rport(struct work_struct *w > * > * 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. > + * FC_PORTSTATE_BLOCKED, or the fast_io_fail_tmo fires. This is > + * necessary to avoid the scsi_eh failing recovery actions for blocked > + * rports which would lead to offlined SCSI devices. > + * > + * Returns: 0 if the fc_rport left the state FC_PORTSTATE_BLOCKED. > + * FAST_IO_FAIL if the fast_io_fail_tmo fired, this should be > + * passed back to scsi_eh. > */ > -void fc_block_scsi_eh(struct scsi_cmnd *cmnd) > +int 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) { > + while (rport->port_state == FC_PORTSTATE_BLOCKED && > + !(rport->flags & FC_RPORT_FAST_FAIL_TIMEDOUT)) { > spin_unlock_irqrestore(shost->host_lock, flags); > msleep(1000); > spin_lock_irqsave(shost->host_lock, flags); > } > spin_unlock_irqrestore(shost->host_lock, flags); > + > + if (rport->flags & FC_RPORT_FAST_FAIL_TIMEDOUT) > + return FAST_IO_FAIL; > + > + return 0; > } > EXPORT_SYMBOL(fc_block_scsi_eh); > > --- a/include/scsi/scsi.h 2010-02-26 12:59:46.000000000 +0100 > +++ b/include/scsi/scsi.h 2010-03-22 14:56:48.000000000 +0100 > @@ -423,6 +423,7 @@ static inline int scsi_is_wlun(unsigned > #define ADD_TO_MLQUEUE 0x2006 > #define TIMEOUT_ERROR 0x2007 > #define SCSI_RETURN_NOT_HANDLED 0x2008 > +#define FAST_IO_FAIL 0x2009 > > /* > * Midlevel queue return values. > --- a/include/scsi/scsi_transport_fc.h 2009-11-05 16:59:33.000000000 +0100 > +++ b/include/scsi/scsi_transport_fc.h 2010-03-22 14:41:36.000000000 +0100 > @@ -807,6 +807,6 @@ void fc_host_post_vendor_event(struct Sc > 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); > +int fc_block_scsi_eh(struct scsi_cmnd *cmnd); > > #endif /* SCSI_TRANSPORT_FC_H */ > > The low-level driver simply passes the FAST_IO_FAIL from the eh callback > back to scsi eh: > > --- > drivers/s390/scsi/zfcp_scsi.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > --- a/drivers/s390/scsi/zfcp_scsi.c 2010-03-02 14:02:37.000000000 +0100 > +++ b/drivers/s390/scsi/zfcp_scsi.c 2010-03-22 14:50:31.000000000 +0100 > @@ -174,7 +174,7 @@ static int zfcp_scsi_eh_abort_handler(st > struct zfcp_fsf_req *old_req, *abrt_req; > unsigned long flags; > unsigned long old_reqid = (unsigned long) scpnt->host_scribble; > - int retval = SUCCESS; > + int retval = SUCCESS, ret; > int retry = 3; > char *dbf_tag; > > @@ -199,7 +199,9 @@ static int zfcp_scsi_eh_abort_handler(st > break; > > zfcp_erp_wait(adapter); > - fc_block_scsi_eh(scpnt); > + ret = fc_block_scsi_eh(scpnt); > + if (ret) > + return ret; > if (!(atomic_read(&adapter->status) & > ZFCP_STATUS_COMMON_RUNNING)) { > zfcp_dbf_scsi_abort("nres", adapter->dbf, scpnt, NULL, I just posted the complete version of the patch and the adaption of zfcp: http://marc.info/?l=linux-scsi&m=126944631211269&w=2 http://marc.info/?l=linux-scsi&m=126944631211272&w=2 http://marc.info/?l=linux-scsi&m=126944631611276&w=2 Any comments about this approach for unblocking the scsi eh thread? -- Christof -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
