Hi, Ewan, Hannes:
On 10/09/2013 08:28 PM, Ewan Milne wrote:
On Wed, 2013-10-09 at 15:43 +0800, Ren Mingxin wrote:
The former minimum valid value of 'eh_deadline' is 1s, which means
the earliest occasion to shorten EH is 1 second later since a
command is failed or timed out. But if we want to skip EH steps
ASAP, we have to wait until the first EH step is finished. If the
duration of the first EH step is long, this waiting time is
excruciating. So, it is necessary to accept 0 as the minimum valid
value for 'eh_deadline'.
According to my test, with Hannes' patchset 'New EH command timeout
handler' as well, the minimum IO time is improved from 73s
(eh_deadline = 1) to 43s(eh_deadline = 0) when commands are timed
out by disabling RSCN and target port.
Another thing: scsi_finish_command() should be invoked if
scsi_eh_scmd_add() is returned on failure - let EH finish those
commands.
Signed-off-by: Ren Mingxin<renmx@xxxxxxxxxxxxxx>
---
drivers/scsi/hosts.c | 14 +++++++++++---
drivers/scsi/scsi_error.c | 40 +++++++++++++++++++++++++++-------------
drivers/scsi/scsi_sysfs.c | 36 +++++++++++++++++++++++++-----------
include/scsi/scsi_host.h | 2 +-
4 files changed, 64 insertions(+), 28 deletions(-)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index f334859..e84123a 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -316,11 +316,11 @@ static void scsi_host_dev_release(struct device *dev)
kfree(shost);
}
-static unsigned int shost_eh_deadline;
+static unsigned int shost_eh_deadline = -1;
This should probably be "static int shost_eh_deadline = -1;".
And the range tests in scsi_host_alloc() and store_shost_eh_deadline()
below should probably use INT_MAX rather than UINT_MAX.
The maximum value is decreased then.
Hannes, agree?
module_param_named(eh_deadline, shost_eh_deadline, uint, S_IRUGO|S_IWUSR);
MODULE_PARM_DESC(eh_deadline,
- "SCSI EH timeout in seconds (should be between 1 and 2^32-1)");
+ "SCSI EH timeout in seconds (should be between 0 and 2^32-1)");
And the description above should be modified as:
+ "SCSI EH timeout in seconds (should be between 0 and (2^31-1)/HZ)");
static struct device_type scsi_host_type = {
.name = "scsi_host",
@@ -394,7 +394,15 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
shost->unchecked_isa_dma = sht->unchecked_isa_dma;
shost->use_clustering = sht->use_clustering;
shost->ordered_tag = sht->ordered_tag;
- shost->eh_deadline = shost_eh_deadline * HZ;
+ if (shost_eh_deadline == -1)
+ shost->eh_deadline = -1;
+ else if ((ulong) shost_eh_deadline * HZ> UINT_MAX) {
+ printk(KERN_WARNING "scsi%d: eh_deadline %u exceeds the "
+ "maximum, setting to %u\n",
+ shost->host_no, shost_eh_deadline, UINT_MAX / HZ);
+ shost->eh_deadline = UINT_MAX / HZ * HZ;
Just use "shost->eh_deadline = INT_MAX" here, leave off the "/ HZ * HZ".
Nod.
Thanks,
Ren
--
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
