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

[Tejun Heo <htejun@gmail.com>]

On Tue, May 01, 2012 at 11:03:19PM -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 2; the original patch changed more than intended.
> 
> --- linux/drivers/ata/libata-eh.c.orig	2012-04-27 13:17:35.000000000 -0400
> +++ linux/drivers/ata/libata-eh.c	2012-05-01 22:40:00.182425015 -0400
> @@ -2122,7 +2122,8 @@
>  		if (qc->flags & ATA_QCFLAG_IO ||
>  		    (!(qc->err_mask & AC_ERR_INVALID) &&
>  		     qc->err_mask != AC_ERR_DEV))
> -			qc->flags |= ATA_QCFLAG_RETRY;
> +			if (!(qc->err_mask & AC_ERR_MEDIA))
> +				qc->flags |= ATA_QCFLAG_RETRY;

It can be combined like the following,

	if (!(qc->err_mask & AC_ERR_MEDIA) &&
	    (qc->flags & ATA_QCFLAG_IO ||
	     (!(qc->err_mask & AC_ERR_INVALID) &&
	      qc->err_mask != AC_ERR_DEV)))

which doesn't look any prettier.  Hmm... maybe using local vars would
make it better?

	bool emedia = qc->err_mask & AC_ERR_MEDIA;
	bool einval = qc->err_mask & AC_ERR_INVALID;
	bool edev = qc->err_mask == AC_ERR_DEV;
	bool is_io = qc->flags & ATA_QCFLAG_IO;

	if (!emedia && (is_io || (!einval && !edev)))

-- 
tejun