On 12-05-01 04:12 PM, Mark Lord wrote:
> In libata-eh.c, the function ata_eh_link_autopsy() tries to decide
> if an operation is worth retrying.
>
> I notice that for MEDIA ERRORs, it always sits there for minutes
> doing futile retries, when it's pretty obvious that the drive
> is reporting an uncorrectable error.
>
> Is this code correct?
>
>
> /* determine whether the command is worth retrying */
> if (qc->flags & ATA_QCFLAG_IO ||
> (!(qc->err_mask & AC_ERR_INVALID) &&
> qc->err_mask != AC_ERR_DEV))
> qc->flags |= ATA_QCFLAG_RETRY;
>
>
> I think this part may be incorrect: qc->err_mask != AC_ERR_DEV
> Shouldn't that test be this instead: !(qc->err_mask & AC_ERR_DEV) ??
MMmm.. even that isn't good enough, because the first ATA_QCFLAG_IO test
bypasses the rest of that logic and triggers unconditional retries. Ugh.
So something like this perhaps:
/* determine whether the command is worth retrying */
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;
> The mask is just about never exactly equal to AC_ERR_DEV.
> In the case of a media error, it is (AC_ERR_DEV | AC_ERR_MEDIA) at this point.
>
> Tejun?
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
