diff options
author | Tejun Heo <htejun@gmail.com> | 2008-05-19 13:17:52 -0400 |
---|---|---|
committer | Jeff Garzik <jgarzik@redhat.com> | 2008-07-14 15:59:32 -0400 |
commit | 0a2c0f56159999e20015241d3b8fa89b1ab14309 (patch) | |
tree | 2c516452d3b3f85e9a4092b6092160e123d5f5d4 /drivers | |
parent | 341c2c958ec7bdd9f54733a8b0b432fe76842a82 (diff) |
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: Tejun Heo <htejun@gmail.com>
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
Diffstat (limited to 'drivers')
-rw-r--r-- | drivers/ata/libata-eh.c | 38 | ||||
-rw-r--r-- | drivers/ata/libata-pmp.c | 10 |
2 files changed, 20 insertions, 28 deletions
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 08dd07f10008..5b5ae631ed03 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c | |||
@@ -67,6 +67,9 @@ enum { | |||
67 | ATA_ECAT_DUBIOUS_UNK_DEV = 7, | 67 | ATA_ECAT_DUBIOUS_UNK_DEV = 7, |
68 | ATA_ECAT_NR = 8, | 68 | ATA_ECAT_NR = 8, |
69 | 69 | ||
70 | /* always put at least this amount of time between resets */ | ||
71 | ATA_EH_RESET_COOL_DOWN = 5000, | ||
72 | |||
70 | /* Waiting in ->prereset can never be reliable. It's | 73 | /* Waiting in ->prereset can never be reliable. It's |
71 | * sometimes nice to wait there but it can't be depended upon; | 74 | * sometimes nice to wait there but it can't be depended upon; |
72 | * otherwise, we wouldn't be resetting. Just give it enough | 75 | * otherwise, we wouldn't be resetting. Just give it enough |
@@ -485,6 +488,9 @@ void ata_scsi_error(struct Scsi_Host *host) | |||
485 | if (ata_ncq_enabled(dev)) | 488 | if (ata_ncq_enabled(dev)) |
486 | ehc->saved_ncq_enabled |= 1 << devno; | 489 | ehc->saved_ncq_enabled |= 1 << devno; |
487 | } | 490 | } |
491 | |||
492 | /* set last reset timestamp to some time in the past */ | ||
493 | ehc->last_reset = jiffies - 60 * HZ; | ||
488 | } | 494 | } |
489 | 495 | ||
490 | ap->pflags |= ATA_PFLAG_EH_IN_PROGRESS; | 496 | ap->pflags |= ATA_PFLAG_EH_IN_PROGRESS; |
@@ -2088,11 +2094,17 @@ int ata_eh_reset(struct ata_link *link, int classify, | |||
2088 | /* | 2094 | /* |
2089 | * Prepare to reset | 2095 | * Prepare to reset |
2090 | */ | 2096 | */ |
2097 | now = jiffies; | ||
2098 | deadline = ata_deadline(ehc->last_reset, ATA_EH_RESET_COOL_DOWN); | ||
2099 | if (time_before(now, deadline)) | ||
2100 | schedule_timeout_uninterruptible(deadline - now); | ||
2101 | |||
2091 | spin_lock_irqsave(ap->lock, flags); | 2102 | spin_lock_irqsave(ap->lock, flags); |
2092 | ap->pflags |= ATA_PFLAG_RESETTING; | 2103 | ap->pflags |= ATA_PFLAG_RESETTING; |
2093 | spin_unlock_irqrestore(ap->lock, flags); | 2104 | spin_unlock_irqrestore(ap->lock, flags); |
2094 | 2105 | ||
2095 | ata_eh_about_to_do(link, NULL, ATA_EH_RESET); | 2106 | ata_eh_about_to_do(link, NULL, ATA_EH_RESET); |
2107 | ehc->last_reset = jiffies; | ||
2096 | 2108 | ||
2097 | ata_link_for_each_dev(dev, link) { | 2109 | ata_link_for_each_dev(dev, link) { |
2098 | /* If we issue an SRST then an ATA drive (not ATAPI) | 2110 | /* 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 | /* | 2170 | /* |
2159 | * Perform reset | 2171 | * Perform reset |
2160 | */ | 2172 | */ |
2173 | ehc->last_reset = jiffies; | ||
2161 | if (ata_is_host_link(link)) | 2174 | if (ata_is_host_link(link)) |
2162 | ata_eh_freeze_port(ap); | 2175 | ata_eh_freeze_port(ap); |
2163 | 2176 | ||
@@ -2278,6 +2291,7 @@ int ata_eh_reset(struct ata_link *link, int classify, | |||
2278 | 2291 | ||
2279 | /* reset successful, schedule revalidation */ | 2292 | /* reset successful, schedule revalidation */ |
2280 | ata_eh_done(link, NULL, ATA_EH_RESET); | 2293 | ata_eh_done(link, NULL, ATA_EH_RESET); |
2294 | ehc->last_reset = jiffies; | ||
2281 | ehc->i.action |= ATA_EH_REVALIDATE; | 2295 | ehc->i.action |= ATA_EH_REVALIDATE; |
2282 | 2296 | ||
2283 | rc = 0; | 2297 | rc = 0; |
@@ -2304,9 +2318,9 @@ int ata_eh_reset(struct ata_link *link, int classify, | |||
2304 | if (time_before(now, deadline)) { | 2318 | if (time_before(now, deadline)) { |
2305 | unsigned long delta = deadline - now; | 2319 | unsigned long delta = deadline - now; |
2306 | 2320 | ||
2307 | ata_link_printk(link, KERN_WARNING, "reset failed " | 2321 | ata_link_printk(link, KERN_WARNING, |
2308 | "(errno=%d), retrying in %u secs\n", | 2322 | "reset failed (errno=%d), retrying in %u secs\n", |
2309 | rc, (jiffies_to_msecs(delta) + 999) / 1000); | 2323 | rc, DIV_ROUND_UP(jiffies_to_msecs(delta), 1000)); |
2310 | 2324 | ||
2311 | while (delta) | 2325 | while (delta) |
2312 | delta = schedule_timeout_uninterruptible(delta); | 2326 | delta = schedule_timeout_uninterruptible(delta); |
@@ -2623,7 +2637,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, | |||
2623 | { | 2637 | { |
2624 | struct ata_link *link; | 2638 | struct ata_link *link; |
2625 | struct ata_device *dev; | 2639 | struct ata_device *dev; |
2626 | int nr_failed_devs, nr_disabled_devs; | 2640 | int nr_failed_devs; |
2627 | int rc; | 2641 | int rc; |
2628 | unsigned long flags; | 2642 | unsigned long flags; |
2629 | 2643 | ||
@@ -2666,7 +2680,6 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, | |||
2666 | retry: | 2680 | retry: |
2667 | rc = 0; | 2681 | rc = 0; |
2668 | nr_failed_devs = 0; | 2682 | nr_failed_devs = 0; |
2669 | nr_disabled_devs = 0; | ||
2670 | 2683 | ||
2671 | /* if UNLOADING, finish immediately */ | 2684 | /* if UNLOADING, finish immediately */ |
2672 | if (ap->pflags & ATA_PFLAG_UNLOADING) | 2685 | if (ap->pflags & ATA_PFLAG_UNLOADING) |
@@ -2733,8 +2746,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset, | |||
2733 | 2746 | ||
2734 | dev_fail: | 2747 | dev_fail: |
2735 | nr_failed_devs++; | 2748 | nr_failed_devs++; |
2736 | if (ata_eh_handle_dev_fail(dev, rc)) | 2749 | ata_eh_handle_dev_fail(dev, rc); |
2737 | nr_disabled_devs++; | ||
2738 | 2750 | ||
2739 | if (ap->pflags & ATA_PFLAG_FROZEN) { | 2751 | if (ap->pflags & ATA_PFLAG_FROZEN) { |
2740 | /* PMP reset requires working host port. | 2752 | /* PMP reset requires working host port. |
@@ -2746,18 +2758,8 @@ dev_fail: | |||
2746 | } | 2758 | } |
2747 | } | 2759 | } |
2748 | 2760 | ||
2749 | if (nr_failed_devs) { | 2761 | if (nr_failed_devs) |
2750 | if (nr_failed_devs != nr_disabled_devs) { | ||
2751 | ata_port_printk(ap, KERN_WARNING, "failed to recover " | ||
2752 | "some devices, retrying in 5 secs\n"); | ||
2753 | ssleep(5); | ||
2754 | } else { | ||
2755 | /* no device left to recover, repeat fast */ | ||
2756 | msleep(500); | ||
2757 | } | ||
2758 | |||
2759 | goto retry; | 2762 | goto retry; |
2760 | } | ||
2761 | 2763 | ||
2762 | out: | 2764 | out: |
2763 | if (rc && r_failed_link) | 2765 | if (rc && r_failed_link) |
diff --git a/drivers/ata/libata-pmp.c b/drivers/ata/libata-pmp.c index 63691d77ac43..b65db309c181 100644 --- a/drivers/ata/libata-pmp.c +++ b/drivers/ata/libata-pmp.c | |||
@@ -727,19 +727,12 @@ static int sata_pmp_eh_recover_pmp(struct ata_port *ap, | |||
727 | } | 727 | } |
728 | 728 | ||
729 | if (tries) { | 729 | if (tries) { |
730 | int sleep = ehc->i.flags & ATA_EHI_DID_RESET; | ||
731 | |||
732 | /* consecutive revalidation failures? speed down */ | 730 | /* consecutive revalidation failures? speed down */ |
733 | if (reval_failed) | 731 | if (reval_failed) |
734 | sata_down_spd_limit(link); | 732 | sata_down_spd_limit(link); |
735 | else | 733 | else |
736 | reval_failed = 1; | 734 | reval_failed = 1; |
737 | 735 | ||
738 | ata_dev_printk(dev, KERN_WARNING, | ||
739 | "retrying reset%s\n", | ||
740 | sleep ? " in 5 secs" : ""); | ||
741 | if (sleep) | ||
742 | ssleep(5); | ||
743 | ehc->i.action |= ATA_EH_RESET; | 736 | ehc->i.action |= ATA_EH_RESET; |
744 | goto retry; | 737 | goto retry; |
745 | } else { | 738 | } else { |
@@ -991,10 +984,7 @@ static int sata_pmp_eh_recover(struct ata_port *ap) | |||
991 | goto retry; | 984 | goto retry; |
992 | 985 | ||
993 | if (--pmp_tries) { | 986 | if (--pmp_tries) { |
994 | ata_port_printk(ap, KERN_WARNING, | ||
995 | "failed to recover PMP, retrying in 5 secs\n"); | ||
996 | pmp_ehc->i.action |= ATA_EH_RESET; | 987 | pmp_ehc->i.action |= ATA_EH_RESET; |
997 | ssleep(5); | ||
998 | goto retry; | 988 | goto retry; |
999 | } | 989 | } |
1000 | 990 | ||