Mark Lord wrote:
Gents,
I am debugging a drive hang issue for a client,
which involves "multiple sector" read/write commands (PIO).
Libata does not appear (as of current #upstream at least)
to correctly implement this functionality.
Specifically, libata doesn't reset dev->multi_count to zero
on drive resets (whereas the drive firmware often *does*),
and libata does not deal well with dubious IDENTIFY data.
So, on resets, we should either clear dev->multi_count,
or re-probe it from the drive, or issue a SET_MULTIPLE command
to restore the old value.
When probing:
If (dev->id[59] == 0xffff), ignore it.
If (dev->id[59] & 0x00ff) is not an even power of two, ignore it.
it should also be ignored (this catches the 0xffff case too).
The powers of two is from the ATA8 standard, which strongly suggests
that only powers of two are valid for use with SET_MULTIPLE.
The probe stuff is trivial to fix (below), but I'm not really
up to speed on figuring out how to get this code re-invoked
after drive resets, and on how to ensure that a SET_MULTIPLE
gets issued to restore the previous working value (with eventual
fallback to not using it after repeated errors). Tejun?
..
Mmm.. we also have to re-check and/or re-set dev->multi_count
after any ata_acpi_on_devcfg() call, because some machines use
ACPI taskfiles to issue a "set multi_count" command to the drive.
This appears to be taken care of already though,
as as ata_dev_configure() is where we now invoke ata_acpi_on_devcfg(),
and this all gets called from ata_dev_revalidate().
So with the patch I provided, newer kernels should then be fine.
But I suppose the patch itself could also be even more finicky
about things, and not bother with R/W multi commands if multi_count==1,
and also validate multi_count against the drive's reported maximum
from ID word 47.
Alan: do you reckon we'll be fine enough by just checking
that word on its own, or would its interpretation depend upon
the supported ATA revisions of the device? If so, then so would
the rest of the mult_count support in libata, right?
I'll re-spin a v3 of the patch to check against word 47.
Cheers
--
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
