Re: [PATCH] libata-eh don't waste time retrying media errors (v3)

[Tejun Heo <htejun@gmail.com>]

On Wed, May 02, 2012 at 03:22:52PM -0400, Mark Lord wrote:
> ATA and SATA drives have had built-in retries for media errors
> for as long as they've been commonplace in computers (early 1990s).
> 
> When libata stumbles across a bad sector, it can waste minutes
> sitting there doing retry after retry before finally giving up
> and letting the higher layers deal with it.
> 
> This patch removes retries for media errors only.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>
> ---
> version 3: try to improve readability.
> 
> --- old/drivers/ata/libata-eh.c	2012-04-27 13:17:35.000000000 -0400
> +++ linux/drivers/ata/libata-eh.c	2012-05-02 15:20:19.946827031 -0400
> @@ -2046,6 +2046,26 @@
>  }
> 
>  /**
> + *	ata_eh_worth_retry - analyze error and decide whether to retry
> + *	@qc: qc to possibly retry
> + *
> + *	Look at the cause of the error and decide if a retry
> + * 	might be useful or not.  We don't want to retry media errors
> + *	because the drive itself has probably already taken 10-30 seconds
> + *	doing its own internal retries before reporting the failure.
> + */
> +static inline int ata_eh_worth_retry(struct ata_queued_cmd *qc)

Return bool? && maybe split the patch into two - the first separating
out the logic into a function, the latter changing emedia handling?

> +{
> +	if (qc->flags & AC_ERR_MEDIA)
> +		return 0;	/* don't retry media errors */
> +	if (qc->flags & ATA_QCFLAG_IO)
> +		return 1;	/* otherwise retry anything from fs stack */
> +	if (qc->err_mask & AC_ERR_INVALID)
> +		return 0;	/* don't retry these */
> +	return qc->err_mask != AC_ERR_DEV;  /* retry if not dev error */
> +}

Other than that,

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

Thanks.

-- 
tejun