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

[Mark Lord <kernel@teksavvy.com>]

On 12-05-02 11:54 AM, Tejun Heo wrote:
> 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.

Yeah, I considered that.  But really a bit main reason it looks fugly
is the historical insistence on wrapping lines longer than 80 chars.

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

Anything else I try there ends up just as ugly.  :)