Patchwork [#upstream-fixes] ahci: fix hang on failed softreset

login
register
mail settings
Submitter Tejun Heo
Date Aug. 27, 2010, 9:09 a.m.
Message ID <4C7780BB.6030604@kernel.org>
Download mbox | patch
Permalink /patch/62834/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Tejun Heo - Aug. 27, 2010, 9:09 a.m.
ahci_do_softreset() compared the current time and deadline in reverse
when calculating timeout for SRST issue.  The result is that if
@deadline is in future, SRST is issued with 0 timeout, which hasn't
caused any problem because it later waits for DRDY with the correct
timeout.  If deadline is already exceeded by the time SRST is about to
be issued, the timeout calculation underflows and if the device
doesn't respond, timeout doesn't trigger for a _very_ long time.

Reverse the incorrect comparison order.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Anssi Hannula <anssi.hannula@iki.fi>
Tested-by: Gwendal Grignou <gwendal@google.com>
Cc: stable@kernel.org
---
 drivers/ata/libahci.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeff Garzik - Sept. 5, 2010, 8:39 p.m.
On 08/27/2010 05:09 AM, Tejun Heo wrote:
> ahci_do_softreset() compared the current time and deadline in reverse
> when calculating timeout for SRST issue.  The result is that if
> @deadline is in future, SRST is issued with 0 timeout, which hasn't
> caused any problem because it later waits for DRDY with the correct
> timeout.  If deadline is already exceeded by the time SRST is about to
> be issued, the timeout calculation underflows and if the device
> doesn't respond, timeout doesn't trigger for a _very_ long time.
>
> Reverse the incorrect comparison order.
>
> Signed-off-by: Tejun Heo<tj@kernel.org>
> Reported-by: Anssi Hannula<anssi.hannula@iki.fi>
> Tested-by: Gwendal Grignou<gwendal@google.com>
> Cc: stable@kernel.org
> ---
>   drivers/ata/libahci.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

applied, good catch, sorry for the delay.


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 81e772a..98c80e1 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1320,7 +1320,7 @@  int ahci_do_softreset(struct ata_link *link, unsigned int *class,
 	/* issue the first D2H Register FIS */
 	msecs = 0;
 	now = jiffies;
-	if (time_after(now, deadline))
+	if (time_after(deadline, now))
 		msecs = jiffies_to_msecs(deadline - now);

 	tf.ctl |= ATA_SRST;