FC transport : Avoid device offline cases by stalling aborts until device unblocked
PATCH 1/1 FC transport : Avoid device offline cases by stalling aborts until device unblocked
Following the recommendation from Christoph....
This moves the eh_timed_out functionality from the scsi_host_template to the transport_template. Given that this is now a transport function, the EH_RESET_TIMER case no longer caps the timer reschedulings. The transport guarantees that this is not an infinite condition.
This patch is dependent upon Christoph's change of:
http://marc.theaimsgroup.com/?l=linux-scsi&m=113923531412919&w=2
-- james s
Original Post:
http://marc.theaimsgroup.com/?l=linux-scsi&m=113924339701796&w=2
drivers/scsi/scsi_error.c +3 -6
@@ -29,6 +29,7 @@#include <scsi/scsi_dbg.h> #include <scsi/scsi_device.h> #include <scsi/scsi_eh.h> +#include <scsi/scsi_transport.h> #include <scsi/scsi_host.h> #include <scsi/scsi_ioctl.h> #include <scsi/scsi_request.h>
@@ -163,16 +164,12 @@ void scsi_times_out(struct scsi_cmnd *sc {scsi_log_completion (scmd,TIMEOUT_ERROR ); -if (scmd->device->host->hostt->eh_timed_out) -switch (scmd->device->host->hostt->eh_timed_out (scmd)) { +if (scmd->device->host->transportt->eh_timed_out) +switch (scmd->device->host->transportt->eh_timed_out (scmd)) {case EH_HANDLED :__scsi_done (scmd);return ;case EH_RESET_TIMER : -/* This allows a single retry even of a command -* with allowed == 0 */ -if (scmd->retries++ > scmd->allowed) -break ;scsi_add_timer (scmd, scmd->timeout_per_command, scsi_times_out);return ;
drivers/scsi/scsi_transport_fc.c +37 -0
@@ -31,6 +31,7 @@#include <scsi/scsi_host.h> #include <scsi/scsi_transport.h> #include <scsi/scsi_transport_fc.h> +#include <scsi/scsi_cmnd.h> #include "scsi_priv.h" /*
@@ -1090,6 +1091,40 @@ static int fc_rport_match(struct attribu } +/** +* fc_timed_out - FC Transport I/O timeout intercept handler +* +* @scmd: The SCSI command which timed out +* +* This routine protects against error handlers getting invoked while a +* rport is in a blocked state, typically due to a temporarily loss of +* connectivity. If the error handlers are allowed to proceed, requests +* to abort i/o, reset the target, etc will likely fail as there is no way +* to communicate with the device to perform the requested function. These +* failures may result in the midlayer taking the device offline, requiring +* manual intervention to restore operation. +* +* This routine, called whenever an i/o times out, validates the state of +* the underlying rport. If the rport is blocked, it returns +* EH_RESET_TIMER, which will continue to reschedule the timeout. +* Eventually, either the device will return, or devloss_tmo will fire, +* and when the timeout then fires, it will be handled normally. +* If the rport is not blocked, normal error handling continues. +* +* Notes: +* This routine assumes no locks are held on entry. +**/ +static enum scsi_eh_timer_return +fc_timed_out (struct scsi_cmnd *scmd) +{ +struct fc_rport *rport =starget_to_rport (scsi_target (scmd->device)); + +if (rport->port_state ==FC_PORTSTATE_BLOCKED ) +return EH_RESET_TIMER ; + +return EH_NOT_HANDLED ; +} +/* * Must be called with shost->host_lock held */
@@ -1146,6 +1181,8 @@ fc_attach_transport(struct fc_function_t/* Transport uses the shost workq for scsi scanning */ i->t.create_work_queue =1 ; + i->t.eh_timed_out = fc_timed_out; + i->t.user_scan = fc_user_scan;/*
include/scsi/scsi_host.h +0 -14
@@ -147,20 +147,6 @@ struct scsi_host_template {int (* eh_host_reset_handler)(struct scsi_cmnd *);/* -* This is an optional routine to notify the host that the scsi -* timer just fired. The returns tell the timer routine what to -* do about this: -* -* EH_HANDLED: I fixed the error, please complete the command -* EH_RESET_TIMER: I need more time, reset the timer and -* begin counting again -* EH_NOT_HANDLED Begin normal error recovery -* -* Status: OPTIONAL -*/ -enum scsi_eh_timer_return (* eh_timed_out)(struct scsi_cmnd *); - -/* * Before the mid layer attempts to scan for a new device where none * currently exists, it will call this entry in your driver. Should * your driver need to allocate any structs or perform any other init
include/scsi/scsi_transport.h +11 -0
@@ -48,6 +48,17 @@ struct scsi_transport_template {* True if the transport wants to use a host-based work-queue */ unsigned int create_work_queue : 1; + +/* +* This is an optional routine that allows the transport to become +* involved when a scsi io timer fires. The return value tells the +* timer routine how to finish the io timeout handling: +* EH_HANDLED: I fixed the error, please complete the command +* EH_RESET_TIMER: I need more time, reset the timer and +* begin counting again +* EH_NOT_HANDLED Begin normal error recovery +*/ +enum scsi_eh_timer_return (* eh_timed_out)(struct scsi_cmnd *); };#define transport_class_to_shost (tc) \
Quoted 2 lines · James Smart
This moves the
eh_timed_outfunctionality from thescsi_host_templateto thetransport_template.
Please take care to choose a proper Subject when posting patches like this. The patch changes the mid-low API and behaviour of the core, i.e. it concerns all transports.
Signature · 3 lines
Stefan Richter -=====-=-==- --=- -=--- http://arcgraph.de/sr/
Quoted 9 lines · James Smart
Following the recommendation from Christoph....
This moves the
eh_timed_outfunctionality from thescsi_host_templateto thetransport_template. Given that this is now a transport function, theEH_RESET_TIMERcase no longer caps the timer reschedulings. The transport guarantees that this is not an infinite condition.This patch is dependent upon Christoph's change of:
http://marc.theaimsgroup.com/?l=linux-scsi&m=113923531412919&w=2
James, could you put this into scsi-misc? I've just reposted the prerequisite megaraid_sas patch.
Quoted 9 lines · James Smart
Following the recommendation from Christoph....
This moves the
eh_timed_outfunctionality from thescsi_host_templateto thetransport_template. Given that this is now a transport function, theEH_RESET_TIMERcase no longer caps the timer reschedulings. The transport guarantees that this is not an infinite condition.This patch is dependent upon Christoph's change of:
http://marc.theaimsgroup.com/?l=linux-scsi&m=113923531412919&w=2
Your patch won't apply.
It looks like this:
drivers/scsi/scsi_error.c line 35
Quoted 8 lines
drivers/scsi/scsi_error.c +1
-0@@ -29,6 +29,7 @@#include <scsi/scsi_dbg.h> #include <scsi/scsi_device.h> #include <scsi/scsi_eh.h> +#include <scsi/scsi_transport.h> #include <scsi/scsi_host.h> #include <scsi/scsi_ioctl.h> #include <scsi/scsi_request.h>
is the problem ... for some reason there are two extra spaces in the lines surrounding the context diffs.
James