Commit 0a2c0f56 authored by Tejun Heo's avatar Tejun Heo Committed by Jeff Garzik

libata: improve EH retry delay handling

EH retries were delayed by 5 seconds to ensure that resets don't occur
back-to-back.  However, this 5 second delay is superflous or excessive
in many cases.  For example, after IDENTIFY times out, there's no
reason to wait five more seconds before retrying.

This patch adds ehc->last_reset timestamp and record the timestamp for
the last reset trial or success and uses it to space resets by
ATA_EH_RESET_COOL_DOWN which is 5 secs and removes unconditional 5 sec
sleeps.

As this change makes inter-try waits often shorter and they're
redundant in nature, this patch also removes the "retrying..."
messages.

While at it, convert explicit rounding up division to DIV_ROUND_UP().

This change speeds up EH in many cases w/o sacrificing robustness.
Signed-off-by: default avatarTejun Heo <htejun@gmail.com>
Signed-off-by: default avatarJeff Garzik <jgarzik@redhat.com>
parent 341c2c95
...@@ -67,6 +67,9 @@ enum { ...@@ -67,6 +67,9 @@ enum {
ATA_ECAT_DUBIOUS_UNK_DEV = 7, ATA_ECAT_DUBIOUS_UNK_DEV = 7,
ATA_ECAT_NR = 8, ATA_ECAT_NR = 8,
/* always put at least this amount of time between resets */
ATA_EH_RESET_COOL_DOWN = 5000,
/* Waiting in ->prereset can never be reliable. It's /* Waiting in ->prereset can never be reliable. It's
* sometimes nice to wait there but it can't be depended upon; * sometimes nice to wait there but it can't be depended upon;
* otherwise, we wouldn't be resetting. Just give it enough * otherwise, we wouldn't be resetting. Just give it enough
...@@ -485,6 +488,9 @@ void ata_scsi_error(struct Scsi_Host *host) ...@@ -485,6 +488,9 @@ void ata_scsi_error(struct Scsi_Host *host)
if (ata_ncq_enabled(dev)) if (ata_ncq_enabled(dev))
ehc->saved_ncq_enabled |= 1 << devno; ehc->saved_ncq_enabled |= 1 << devno;
} }
/* set last reset timestamp to some time in the past */
ehc->last_reset = jiffies - 60 * HZ;
} }
ap->pflags |= ATA_PFLAG_EH_IN_PROGRESS; ap->pflags |= ATA_PFLAG_EH_IN_PROGRESS;
...@@ -2088,11 +2094,17 @@ int ata_eh_reset(struct ata_link *link, int classify, ...@@ -2088,11 +2094,17 @@ int ata_eh_reset(struct ata_link *link, int classify,
/* /*
* Prepare to reset * Prepare to reset
*/ */
now = jiffies;
deadline = ata_deadline(ehc->last_reset, ATA_EH_RESET_COOL_DOWN);
if (time_before(now, deadline))
schedule_timeout_uninterruptible(deadline - now);
spin_lock_irqsave(ap->lock, flags); spin_lock_irqsave(ap->lock, flags);
ap->pflags |= ATA_PFLAG_RESETTING; ap->pflags |= ATA_PFLAG_RESETTING;
spin_unlock_irqrestore(ap->lock, flags); spin_unlock_irqrestore(ap->lock, flags);
ata_eh_about_to_do(link, NULL, ATA_EH_RESET); ata_eh_about_to_do(link, NULL, ATA_EH_RESET);
ehc->last_reset = jiffies;
ata_link_for_each_dev(dev, link) { ata_link_for_each_dev(dev, link) {
/* If we issue an SRST then an ATA drive (not ATAPI) /* If we issue an SRST then an ATA drive (not ATAPI)
...@@ -2158,6 +2170,7 @@ int ata_eh_reset(struct ata_link *link, int classify, ...@@ -2158,6 +2170,7 @@ int ata_eh_reset(struct ata_link *link, int classify,
/* /*
* Perform reset * Perform reset
*/ */
ehc->last_reset = jiffies;
if (ata_is_host_link(link)) if (ata_is_host_link(link))
ata_eh_freeze_port(ap); ata_eh_freeze_port(ap);
...@@ -2278,6 +2291,7 @@ int ata_eh_reset(struct ata_link *link, int classify, ...@@ -2278,6 +2291,7 @@ int ata_eh_reset(struct ata_link *link, int classify,
/* reset successful, schedule revalidation */ /* reset successful, schedule revalidation */
ata_eh_done(link, NULL, ATA_EH_RESET); ata_eh_done(link, NULL, ATA_EH_RESET);
ehc->last_reset = jiffies;
ehc->i.action |= ATA_EH_REVALIDATE; ehc->i.action |= ATA_EH_REVALIDATE;
rc = 0; rc = 0;
...@@ -2304,9 +2318,9 @@ int ata_eh_reset(struct ata_link *link, int classify, ...@@ -2304,9 +2318,9 @@ int ata_eh_reset(struct ata_link *link, int classify,
if (time_before(now, deadline)) { if (time_before(now, deadline)) {
unsigned long delta = deadline - now; unsigned long delta = deadline - now;
ata_link_printk(link, KERN_WARNING, "reset failed " ata_link_printk(link, KERN_WARNING,
"(errno=%d), retrying in %u secs\n", "reset failed (errno=%d), retrying in %u secs\n",
rc, (jiffies_to_msecs(delta) + 999) / 1000); rc, DIV_ROUND_UP(jiffies_to_msecs(delta), 1000));
while (delta) while (delta)
delta = schedule_timeout_uninterruptible(delta); delta = schedule_timeout_uninterruptible(delta);
...@@ -2623,7 +2637,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, ...@@ -2623,7 +2637,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
{ {
struct ata_link *link; struct ata_link *link;
struct ata_device *dev; struct ata_device *dev;
int nr_failed_devs, nr_disabled_devs; int nr_failed_devs;
int rc; int rc;
unsigned long flags; unsigned long flags;
...@@ -2666,7 +2680,6 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, ...@@ -2666,7 +2680,6 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
retry: retry:
rc = 0; rc = 0;
nr_failed_devs = 0; nr_failed_devs = 0;
nr_disabled_devs = 0;
/* if UNLOADING, finish immediately */ /* if UNLOADING, finish immediately */
if (ap->pflags & ATA_PFLAG_UNLOADING) if (ap->pflags & ATA_PFLAG_UNLOADING)
...@@ -2733,8 +2746,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, ...@@ -2733,8 +2746,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
dev_fail: dev_fail:
nr_failed_devs++; nr_failed_devs++;
if (ata_eh_handle_dev_fail(dev, rc)) ata_eh_handle_dev_fail(dev, rc);
nr_disabled_devs++;
if (ap->pflags & ATA_PFLAG_FROZEN) { if (ap->pflags & ATA_PFLAG_FROZEN) {
/* PMP reset requires working host port. /* PMP reset requires working host port.
...@@ -2746,18 +2758,8 @@ dev_fail: ...@@ -2746,18 +2758,8 @@ dev_fail:
} }
} }
if (nr_failed_devs) { if (nr_failed_devs)
if (nr_failed_devs != nr_disabled_devs) {
ata_port_printk(ap, KERN_WARNING, "failed to recover "
"some devices, retrying in 5 secs\n");
ssleep(5);
} else {
/* no device left to recover, repeat fast */
msleep(500);
}
goto retry; goto retry;
}
out: out:
if (rc && r_failed_link) if (rc && r_failed_link)
......
...@@ -727,19 +727,12 @@ static int sata_pmp_eh_recover_pmp(struct ata_port *ap, ...@@ -727,19 +727,12 @@ static int sata_pmp_eh_recover_pmp(struct ata_port *ap,
} }
if (tries) { if (tries) {
int sleep = ehc->i.flags & ATA_EHI_DID_RESET;
/* consecutive revalidation failures? speed down */ /* consecutive revalidation failures? speed down */
if (reval_failed) if (reval_failed)
sata_down_spd_limit(link); sata_down_spd_limit(link);
else else
reval_failed = 1; reval_failed = 1;
ata_dev_printk(dev, KERN_WARNING,
"retrying reset%s\n",
sleep ? " in 5 secs" : "");
if (sleep)
ssleep(5);
ehc->i.action |= ATA_EH_RESET; ehc->i.action |= ATA_EH_RESET;
goto retry; goto retry;
} else { } else {
...@@ -991,10 +984,7 @@ static int sata_pmp_eh_recover(struct ata_port *ap) ...@@ -991,10 +984,7 @@ static int sata_pmp_eh_recover(struct ata_port *ap)
goto retry; goto retry;
if (--pmp_tries) { if (--pmp_tries) {
ata_port_printk(ap, KERN_WARNING,
"failed to recover PMP, retrying in 5 secs\n");
pmp_ehc->i.action |= ATA_EH_RESET; pmp_ehc->i.action |= ATA_EH_RESET;
ssleep(5);
goto retry; goto retry;
} }
......
...@@ -602,6 +602,8 @@ struct ata_eh_context { ...@@ -602,6 +602,8 @@ struct ata_eh_context {
unsigned int did_probe_mask; unsigned int did_probe_mask;
unsigned int saved_ncq_enabled; unsigned int saved_ncq_enabled;
u8 saved_xfer_mode[ATA_MAX_DEVICES]; u8 saved_xfer_mode[ATA_MAX_DEVICES];
/* timestamp for the last reset attempt or success */
unsigned long last_reset;
}; };
struct ata_acpi_drive struct ata_acpi_drive
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment