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>

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 08dd07f1000846b044c67c39a11f35fffe736042..5b5ae631ed037ed28f0bc1699285d2a5df3dfd84 100644 (file)
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -67,6 +67,9 @@ enum {
        ATA_ECAT_DUBIOUS_UNK_DEV        = 7,
        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
         * sometimes nice to wait there but it can't be depended upon;
         * otherwise, we wouldn't be resetting.  Just give it enough
@@ -485,6 +488,9 @@ void ata_scsi_error(struct Scsi_Host *host)
                                if (ata_ncq_enabled(dev))
                                        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;
@@ -2088,11 +2094,17 @@ int ata_eh_reset(struct ata_link *link, int classify,
        /*
         * 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);
        ap->pflags |= ATA_PFLAG_RESETTING;
        spin_unlock_irqrestore(ap->lock, flags);
 
        ata_eh_about_to_do(link, NULL, ATA_EH_RESET);
+       ehc->last_reset = jiffies;
 
        ata_link_for_each_dev(dev, link) {
                /* 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,
        /*
         * Perform reset
         */
+       ehc->last_reset = jiffies;
        if (ata_is_host_link(link))
                ata_eh_freeze_port(ap);
 
@@ -2278,6 +2291,7 @@ int ata_eh_reset(struct ata_link *link, int classify,
 
        /* reset successful, schedule revalidation */
        ata_eh_done(link, NULL, ATA_EH_RESET);
+       ehc->last_reset = jiffies;
        ehc->i.action |= ATA_EH_REVALIDATE;
 
        rc = 0;
@@ -2304,9 +2318,9 @@ int ata_eh_reset(struct ata_link *link, int classify,
        if (time_before(now, deadline)) {
                unsigned long delta = deadline - now;
 
-               ata_link_printk(link, KERN_WARNING, "reset failed "
-                               "(errno=%d), retrying in %u secs\n",
-                               rc, (jiffies_to_msecs(delta) + 999) / 1000);
+               ata_link_printk(link, KERN_WARNING,
+                       "reset failed (errno=%d), retrying in %u secs\n",
+                       rc, DIV_ROUND_UP(jiffies_to_msecs(delta), 1000));
 
                while (delta)
                        delta = schedule_timeout_uninterruptible(delta);
@@ -2623,7 +2637,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
 {
        struct ata_link *link;
        struct ata_device *dev;
-       int nr_failed_devs, nr_disabled_devs;
+       int nr_failed_devs;
        int rc;
        unsigned long flags;
 
@@ -2666,7 +2680,6 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
  retry:
        rc = 0;
        nr_failed_devs = 0;
-       nr_disabled_devs = 0;
 
        /* if UNLOADING, finish immediately */
        if (ap->pflags & ATA_PFLAG_UNLOADING)
@@ -2733,8 +2746,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
 
 dev_fail:
                nr_failed_devs++;
-               if (ata_eh_handle_dev_fail(dev, rc))
-                       nr_disabled_devs++;
+               ata_eh_handle_dev_fail(dev, rc);
 
                if (ap->pflags & ATA_PFLAG_FROZEN) {
                        /* PMP reset requires working host port.
@@ -2746,18 +2758,8 @@ dev_fail:
                }
        }
 
-       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);
-               }
-
+       if (nr_failed_devs)
                goto retry;
-       }
 
  out:
        if (rc && r_failed_link)

diff --git a/drivers/ata/libata-pmp.c b/drivers/ata/libata-pmp.c
index 63691d77ac436aa8a52936150165ac2666424dab..b65db309c181f02f76431a4d7b3e24d5e63b3388 100644 (file)
--- 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,
                }
 
                if (tries) {
-                       int sleep = ehc->i.flags & ATA_EHI_DID_RESET;
-
                        /* consecutive revalidation failures? speed down */
                        if (reval_failed)
                                sata_down_spd_limit(link);
                        else
                                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;
                        goto retry;
                } else {
@@ -991,10 +984,7 @@ static int sata_pmp_eh_recover(struct ata_port *ap)
                goto retry;
 
        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;
-               ssleep(5);
                goto retry;
        }
 

diff --git a/include/linux/libata.h b/include/linux/libata.h
index 94110b652b302d4111a216f4e4cb591adddccc8d..9058c2a325a9f19ea1abdc8edb8355451ad6cb8e 100644 (file)
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -602,6 +602,8 @@ struct ata_eh_context {
        unsigned int            did_probe_mask;
        unsigned int            saved_ncq_enabled;
        u8                      saved_xfer_mode[ATA_MAX_DEVICES];
+       /* timestamp for the last reset attempt or success */
+       unsigned long           last_reset;
 };
 
 struct ata_acpi_drive