[PATCH] When a disk needs to be waken up from sleep, ensure that the command is retried by SCSI EH.

[PATCH] When a disk needs to be waken up from sleep, ensure that the command is retried by SCSI EH.

[Gwendal Grignou]

Signed-off-by: Gwendal Grignou <gwendal@google.com>
---
 drivers/ata/libata-core.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 8ac98ff..5b7616d 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5145,6 +5145,8 @@ void ata_qc_issue(struct ata_queued_cmd *qc)
 	/* if device is sleeping, schedule reset and abort the link */
 	if (unlikely(qc->dev->flags & ATA_DFLAG_SLEEPING)) {
 		link->eh_info.action |= ATA_EH_RESET;
+		if (qc->scsicmd) 
+			qc->scsicmd->allowed++;
 		ata_ehi_push_desc(&link->eh_info, "waking up from sleep");
 		ata_link_abort(link);
 		return;
-- 
1.5.4.3

Re: [PATCH] When a disk needs to be waken up from sleep, ensure that the command is retried by SCSI EH.

[Gwendal Grignou]

The explanation did not go through git-send-email:

 Without this fix, ATA passthrough commands are not resend to the
drive, and no error is
 signalled to the caller because:
 - allowed retry count is 1
 - ata_eh_qc_complete fill the sense data, so result is valid
 - sense data is filled with untouched ATA registers.

Gwendal.

On Thu, Aug 6, 2009 at 10:25 AM, Gwendal Grignou<gwendal@google.com> wrote:
>
> Signed-off-by: Gwendal Grignou <gwendal@google.com>
> ---
>  drivers/ata/libata-core.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 8ac98ff..5b7616d 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -5145,6 +5145,8 @@ void ata_qc_issue(struct ata_queued_cmd *qc)
>        /* if device is sleeping, schedule reset and abort the link */
>        if (unlikely(qc->dev->flags & ATA_DFLAG_SLEEPING)) {
>                link->eh_info.action |= ATA_EH_RESET;
> +               if (qc->scsicmd)
> +                       qc->scsicmd->allowed++;
>                ata_ehi_push_desc(&link->eh_info, "waking up from sleep");
>                ata_link_abort(link);
>                return;
> --
> 1.5.4.3
>
>

Re: [PATCH] When a disk needs to be waken up from sleep, ensure that the command is retried by SCSI EH.

[Tejun Heo]

Gwendal Grignou wrote:
> The explanation did not go through git-send-email:
> 
>  Without this fix, ATA passthrough commands are not resend to the
> drive, and no error is
>  signalled to the caller because:
>  - allowed retry count is 1
>  - ata_eh_qc_complete fill the sense data, so result is valid
>  - sense data is filled with untouched ATA registers.

Would setting ATA_QCFLAG_RETRY achieve the same result?

Thanks.

-- 
tejun

Re: [PATCH] When a disk needs to be waken up from sleep, ensure that the command is retried by SCSI EH.

[Gwendal Grignou]

ATA_QCFLAG_RETRY is already set [by _autopsy()], ata_eh_qc_retry() was
already called, but given retries is 0, we can not reduce it.
However, I will submit a better patch, in ata_eh_qc_retry().

Gwendal.

On Fri, Aug 7, 2009 at 1:34 AM, Tejun Heo<tj@kernel.org> wrote:
> Gwendal Grignou wrote:
>> The explanation did not go through git-send-email:
>>
>>  Without this fix, ATA passthrough commands are not resend to the
>> drive, and no error is
>>  signalled to the caller because:
>>  - allowed retry count is 1
>>  - ata_eh_qc_complete fill the sense data, so result is valid
My mistake: __ata_eh_qc_complete() instead of ata_eh_qc_complete().
>>  - sense data is filled with untouched ATA registers.
>
> Would setting ATA_QCFLAG_RETRY achieve the same result?
>
> Thanks.
>
> --
> tejun
>

[PATCH] When a disk needs to be waken up from sleep, ensure that the command is retried by SCSI EH.

[Gwendal Grignou]

---
 drivers/ata/libata-eh.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 79711b6..202da9c 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1183,14 +1183,14 @@ void ata_eh_qc_complete(struct ata_queued_cmd *qc)
  *	should be retried.  To be used from EH.
  *
  *	SCSI midlayer limits the number of retries to scmd->allowed.
- *	scmd->retries is decremented for commands which get retried
+ *	scmd->allowed is incremented for commands which get retried
  *	due to unrelated failures (qc->err_mask is zero).
  */
 void ata_eh_qc_retry(struct ata_queued_cmd *qc)
 {
 	struct scsi_cmnd *scmd = qc->scsicmd;
-	if (!qc->err_mask && scmd->retries)
-		scmd->retries--;
+	if (!qc->err_mask)
+		scmd->allowed++;
 	__ata_eh_qc_complete(qc);
 }
 
-- 
1.5.4.3

Re: [PATCH] When a disk needs to be waken up from sleep, ensure that the command is retried by SCSI EH.

[Tejun Heo]

Gwendal Grignou wrote:
> ---
>  drivers/ata/libata-eh.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)

The patch description is missing again but other than that

Acked-by: Tejun Heo <tj@kernel.org>

-- 
tejun

[PATCH] When a disk needs to be waken up from sleep, ensure that the command is retried by SCSI EH.

[Gwendal Grignou]

 Without this fix, ATA passthrough commands are not resend to the drive, and no error is
 signalled to the caller because:
 - allowed retry count is 1
 - ata_eh_qc_complete fill the sense data, so result is valid
 - sense data is filled with untouched ATA registers.

---
 drivers/ata/libata-eh.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 79711b6..202da9c 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1183,14 +1183,14 @@ void ata_eh_qc_complete(struct ata_queued_cmd *qc)
  *	should be retried.  To be used from EH.
  *
  *	SCSI midlayer limits the number of retries to scmd->allowed.
- *	scmd->retries is decremented for commands which get retried
+ *	scmd->allowed is incremented for commands which get retried
  *	due to unrelated failures (qc->err_mask is zero).
  */
 void ata_eh_qc_retry(struct ata_queued_cmd *qc)
 {
 	struct scsi_cmnd *scmd = qc->scsicmd;
-	if (!qc->err_mask && scmd->retries)
-		scmd->retries--;
+	if (!qc->err_mask)
+		scmd->allowed++;
 	__ata_eh_qc_complete(qc);
 }
 
-- 
1.5.4.3

Re: [PATCH] When a disk needs to be waken up from sleep, ensure that the command is retried by SCSI EH.

[Tejun Heo]

Gwendal Grignou wrote:
>  Without this fix, ATA passthrough commands are not resend to the drive, and no error is

80 column.

>  signalled to the caller because:
>  - allowed retry count is 1
>  - ata_eh_qc_complete fill the sense data, so result is valid
>  - sense data is filled with untouched ATA registers.

and Signed-off-by:

Thanks.

-- 
tejun

[PATCH] When a disk wakes up from sleep, ensure that the command is retried by SCSI EH.

[Gwendal Grignou]

Without this fix, ATA passthrough commands are not resend to the drive, and no
error is signalled to the caller because:
- allowed retry count is 1
- ata_eh_qc_complete fill the sense data, so result is valid
- sense data is filled with untouched ATA registers.

Signed-off-by: Gwendal Grignou <gwendal@google.com>
---
 drivers/ata/libata-eh.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 79711b6..202da9c 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1183,14 +1183,14 @@ void ata_eh_qc_complete(struct ata_queued_cmd *qc)
  *	should be retried.  To be used from EH.
  *
  *	SCSI midlayer limits the number of retries to scmd->allowed.
- *	scmd->retries is decremented for commands which get retried
+ *	scmd->allowed is incremented for commands which get retried
  *	due to unrelated failures (qc->err_mask is zero).
  */
 void ata_eh_qc_retry(struct ata_queued_cmd *qc)
 {
 	struct scsi_cmnd *scmd = qc->scsicmd;
-	if (!qc->err_mask && scmd->retries)
-		scmd->retries--;
+	if (!qc->err_mask)
+		scmd->allowed++;
 	__ata_eh_qc_complete(qc);
 }
 
-- 
1.5.4.3

Re: [PATCH] When a disk wakes up from sleep, ensure that the command is retried by SCSI EH.

[Tejun Heo]

Gwendal Grignou wrote:
> Without this fix, ATA passthrough commands are not resend to the drive, and no
> error is signalled to the caller because:
> - allowed retry count is 1
> - ata_eh_qc_complete fill the sense data, so result is valid
> - sense data is filled with untouched ATA registers.
> 
> Signed-off-by: Gwendal Grignou <gwendal@google.com>

Acked-by: Tejun Heo <tj@kernel.org>

-- 
tejun

Re: [PATCH] When a disk wakes up from sleep, ensure that the command is retried by SCSI EH.

[Gwendal Grignou]

Just looking into 2.6.33 code, and I did not find this patch. Is there
anything I should do to have it merge?

Thanks,
Gwendal.

On Fri, Aug 7, 2009 at 4:23 PM, Tejun Heo <tj@kernel.org> wrote:
> Gwendal Grignou wrote:
>> Without this fix, ATA passthrough commands are not resend to the drive, and no
>> error is signalled to the caller because:
>> - allowed retry count is 1
>> - ata_eh_qc_complete fill the sense data, so result is valid
>> - sense data is filled with untouched ATA registers.
>>
>> Signed-off-by: Gwendal Grignou <gwendal@google.com>
>
> Acked-by: Tejun Heo <tj@kernel.org>
>
> --
> tejun
>

Re: [PATCH] When a disk wakes up from sleep, ensure that the command is retried by SCSI EH.

[Tejun Heo]

Hello,

On 04/22/2010 07:43 PM, Gwendal Grignou wrote:
> Just looking into 2.6.33 code, and I did not find this patch. Is there
> anything I should do to have it merge?

I think Jeff just hasn't come around yet.  Please wait a bit more for
Jeff's "applied" mail.  If it doesn't happen in a week or so, pinging
him again would be a good idea.

-- 
tejun

Re: [PATCH] When a disk wakes up from sleep, ensure that the command is retried by SCSI EH.

[Gwendal Grignou]

Tejun Heo <tj <at> kernel.org> writes:

> 
> Hello,
> 
> On 04/22/2010 07:43 PM, Gwendal Grignou wrote:
> > Just looking into 2.6.33 code, and I did not find this patch. Is there
> > anything I should do to have it merge?
> 
> I think Jeff just hasn't come around yet.  Please wait a bit more for
> Jeff's "applied" mail.  If it doesn't happen in a week or so, pinging
> him again would be a good idea.
> 


Ping, years later! The patch still applies to 3.12.0-rc3.

Gwendal.

Re: [PATCH] When a disk wakes up from sleep, ensure that the command ?is retried by SCSI EH.

[Tejun Heo]

Hello, Gwendal.

Applied to libata/for-3.12-fixes w/ commit message updated and stable
cc'd.

Thanks.

>From f13e220161e738c2710b9904dcb3cf8bb0bcce61 Mon Sep 17 00:00:00 2001
From: Gwendal Grignou <gwendal@google.com>
Date: Fri, 7 Aug 2009 16:17:49 -0700
Subject: [PATCH] libata: make ata_eh_qc_retry() bump scmd->allowed on bogus
 failures

libata EH decrements scmd->retries when the command failed for reasons
unrelated to the command itself so that, for example, commands aborted
due to suspend / resume cycle don't get penalized; however,
decrementing scmd->retries isn't enough for ATA passthrough commands.

Without this fix, ATA passthrough commands are not resend to the
drive, and no error is signalled to the caller because:

- allowed retry count is 1
- ata_eh_qc_complete fill the sense data, so result is valid
- sense data is filled with untouched ATA registers.

Signed-off-by: Gwendal Grignou <gwendal@google.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: stable@vger.kernel.org
---
 drivers/ata/libata-eh.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index c69fcce..370462f 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1322,14 +1322,14 @@ void ata_eh_qc_complete(struct ata_queued_cmd *qc)
  *	should be retried.  To be used from EH.
  *
  *	SCSI midlayer limits the number of retries to scmd->allowed.
- *	scmd->retries is decremented for commands which get retried
+ *	scmd->allowed is incremented for commands which get retried
  *	due to unrelated failures (qc->err_mask is zero).
  */
 void ata_eh_qc_retry(struct ata_queued_cmd *qc)
 {
 	struct scsi_cmnd *scmd = qc->scsicmd;
-	if (!qc->err_mask && scmd->retries)
-		scmd->retries--;
+	if (!qc->err_mask)
+		scmd->allowed++;
 	__ata_eh_qc_complete(qc);
 }
 
-- 
1.8.3.1