Patchwork [#upstream] libata: clear eh_info on reset completion

login
register
mail settings
Submitter Tejun Heo
Date July 17, 2009, 2:58 a.m.
Message ID <4A5FE8D9.2040702@kernel.org>
Download mbox | patch
Permalink /patch/29898/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Tejun Heo - July 17, 2009, 2:58 a.m.
Resets are done with port frozen but some controllers still issue
interrupts during reset and they may end up recording error conditions
in ehi leading to unnecessary EH retrials.

This patch makes ata_eh_reset() clear ehi on reset completion.  As
reset is the most severe recovery action, there's nothing to lose by
clearing ehi on its completion.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Zdenek Kaspar <zkaspar82@gmail.com>
---
Zdenek, I moved the clearing above a bit.  It should work the same but
can you please test this one too?

Jeff, after Zdenek's verification, please commit this to #upstream.

Thanks.

 drivers/ata/libata-eh.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 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
Zdenek Kaspar - July 17, 2009, 2:32 p.m.
Tejun Heo napsal(a):
> Resets are done with port frozen but some controllers still issue
> interrupts during reset and they may end up recording error conditions
> in ehi leading to unnecessary EH retrials.
> 
> This patch makes ata_eh_reset() clear ehi on reset completion.  As
> reset is the most severe recovery action, there's nothing to lose by
> clearing ehi on its completion.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Zdenek Kaspar <zkaspar82@gmail.com>
> ---
> Zdenek, I moved the clearing above a bit.  It should work the same but
> can you please test this one too?
> 
> Jeff, after Zdenek's verification, please commit this to #upstream.
> 
> Thanks.
> 
>  drivers/ata/libata-eh.c |   12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
> index a3df9fe..924a8de 100644
> --- a/drivers/ata/libata-eh.c
> +++ b/drivers/ata/libata-eh.c
> @@ -2572,11 +2572,17 @@ int ata_eh_reset(struct ata_link *link, int classify,
>  			postreset(slave, classes);
>  	}
> 
> -	/* clear cached SError */
> +	/*
> +	 * Some controllers can't be frozen very well and may set
> +	 * spuruious error conditions during reset.  Clear accumulated
> +	 * error information.  As reset is the final recovery action,
> +	 * nothing is lost by doing this.
> +	 */
>  	spin_lock_irqsave(link->ap->lock, flags);
> -	link->eh_info.serror = 0;
> +	memset(&link->eh_info, 0, sizeof(link->eh_info));
>  	if (slave)
> -		slave->eh_info.serror = 0;
> +		memset(&slave->eh_info, 0, sizeof(link->eh_info));
> +	ap->pflags &= ~ATA_PFLAG_EH_PENDING;
>  	spin_unlock_irqrestore(link->ap->lock, flags);
> 
>  	/* Make sure onlineness and classification result correspond.

Successfully tested with ICH8 and ICH7R controllers on two different
machines with 2.6.31-rc3.

Z.
--
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 - July 29, 2009, 1:20 a.m.
Tejun Heo wrote:
> Resets are done with port frozen but some controllers still issue
> interrupts during reset and they may end up recording error conditions
> in ehi leading to unnecessary EH retrials.
> 
> This patch makes ata_eh_reset() clear ehi on reset completion.  As
> reset is the most severe recovery action, there's nothing to lose by
> clearing ehi on its completion.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reported-by: Zdenek Kaspar <zkaspar82@gmail.com>
> ---
> Zdenek, I moved the clearing above a bit.  It should work the same but
> can you please test this one too?
> 
> Jeff, after Zdenek's verification, please commit this to #upstream.
> 
> Thanks.
> 
>  drivers/ata/libata-eh.c |   12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

applied


--
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-eh.c b/drivers/ata/libata-eh.c
index a3df9fe..924a8de 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2572,11 +2572,17 @@  int ata_eh_reset(struct ata_link *link, int classify,
 			postreset(slave, classes);
 	}

-	/* clear cached SError */
+	/*
+	 * Some controllers can't be frozen very well and may set
+	 * spuruious error conditions during reset.  Clear accumulated
+	 * error information.  As reset is the final recovery action,
+	 * nothing is lost by doing this.
+	 */
 	spin_lock_irqsave(link->ap->lock, flags);
-	link->eh_info.serror = 0;
+	memset(&link->eh_info, 0, sizeof(link->eh_info));
 	if (slave)
-		slave->eh_info.serror = 0;
+		memset(&slave->eh_info, 0, sizeof(link->eh_info));
+	ap->pflags &= ~ATA_PFLAG_EH_PENDING;
 	spin_unlock_irqrestore(link->ap->lock, flags);

 	/* Make sure onlineness and classification result correspond.