Patchwork [#upstream-fixes] libata: fix internal command failure handling

login
register
mail settings
Submitter Tejun Heo
Date Oct. 16, 2009, 4 a.m.
Message ID <4AD7EFF3.1000902@kernel.org>
Download mbox | patch
Permalink /patch/36157/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Tejun Heo - Oct. 16, 2009, 4 a.m.
When an internal command fails, it should be failed directly without
invoking EH.  In the original implemetation, this was accomplished by
letting internal command bypass failure handling in ata_qc_complete().
However, later changes added post-successful-completion handling to
that code path and the success path is no longer adequate as internal
command failure path.  One of the visible problems is that internal
command failure due to timeout or other freeze conditions would
spuriously trigger WARN_ON_ONCE() in the success path.

This patch updates failure path such that internal command failure
handling is contained there.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: stable@kernel.org
---
 drivers/ata/libata-core.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

--
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 - Oct. 16, 2009, 10:27 a.m.
On 10/16/2009 12:00 AM, Tejun Heo wrote:
> When an internal command fails, it should be failed directly without
> invoking EH.  In the original implemetation, this was accomplished by
> letting internal command bypass failure handling in ata_qc_complete().
> However, later changes added post-successful-completion handling to
> that code path and the success path is no longer adequate as internal
> command failure path.  One of the visible problems is that internal
> command failure due to timeout or other freeze conditions would
> spuriously trigger WARN_ON_ONCE() in the success path.
>
> This patch updates failure path such that internal command failure
> handling is contained there.
>
> Signed-off-by: Tejun Heo<tj@kernel.org>
> Cc: stable@kernel.org
> ---
>   drivers/ata/libata-core.c |   12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)

applied -- thanks for fixing!!


--
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/libata-core.c b/drivers/ata/libata-core.c
index b525a09..d7f0f1b 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5028,12 +5028,14 @@  void ata_qc_complete(struct ata_queued_cmd *qc)
 			qc->flags |= ATA_QCFLAG_FAILED;

 		if (unlikely(qc->flags & ATA_QCFLAG_FAILED)) {
-			if (!ata_tag_internal(qc->tag)) {
-				/* always fill result TF for failed qc */
-				fill_result_tf(qc);
+			/* always fill result TF for failed qc */
+			fill_result_tf(qc);
+
+			if (!ata_tag_internal(qc->tag))
 				ata_qc_schedule_eh(qc);
-				return;
-			}
+			else
+				__ata_qc_complete(qc);
+			return;
 		}

 		WARN_ON_ONCE(ap->pflags & ATA_PFLAG_FROZEN);