You have a point, but setting the flag LOCKED when an error happen
will mean that a braindead device [which always trigger
ata_id_locked()] will work until an internal disk error happens; that
will make debugging difficult.
Instead, I am working on adding the provision for a blacklist to allow
bad devices to bypass the test.
Gwendal.
On Tue, Sep 1, 2009 at 6:39 PM, Tejun Heo<tj@xxxxxxxxxx> wrote:
> Gwendal Grignou wrote:
>> Send a Read/Write command to a locked disk triggers the error handler.
>> Given the disk returns a generic device error code, the error handler can come
>> to the conclusion of reducing the link speed, which is bad.
>> Also, if drives are still locked at boot, this fix speeds up the boot process
>> by returning errors without invoquing the error handler thread.
>>
>> Signed-off-by: Gwendal Grignou <gwendal@xxxxxxxxxx>
>> ---
>> drivers/ata/libata-core.c | 2 ++
>> drivers/ata/libata-scsi.c | 6 ++++++
>> include/linux/ata.h | 3 +++
>> 3 files changed, 11 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>> index 072ba5e..3f80570 100644
>> --- a/drivers/ata/libata-core.c
>> +++ b/drivers/ata/libata-core.c
>> @@ -5023,6 +5023,8 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
>> qc->tf.feature != SETFEATURES_WC_OFF)
>> break;
>> /* fall through */
>> + case ATA_CMD_SEC_UNLOCK: /* Read/Write access now work */
>> + case ATA_CMD_SEC_ERASE_UNIT: /* Read/Write access now work */
>> case ATA_CMD_INIT_DEV_PARAMS: /* CHS translation changed */
>> case ATA_CMD_SET_MULTI: /* multi_count changed */
>> /* revalidate device */
>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>> index d0dfeef..8a41767 100644
>> --- a/drivers/ata/libata-scsi.c
>> +++ b/drivers/ata/libata-scsi.c
>> @@ -1658,6 +1658,12 @@ static unsigned int ata_scsi_rw_xlat(struct ata_queued_cmd *qc)
>> u32 n_block;
>> int rc;
>>
>> + if (unlikely(ata_id_locked(qc->dev->id))) {
>> + /* Terminate RW commands early when the disk is locked */
>> + ata_scsi_set_sense(scmd, ABORTED_COMMAND, 0, 0);
>> + return 1;
>> + }
>> +
>> if (cdb[0] == WRITE_10 || cdb[0] == WRITE_6 || cdb[0] == WRITE_16)
>> tf_flags |= ATA_TFLAG_WRITE;
>
> Hmmm... how sure are we that there aren't some braindead devices which
> incorrectly would trigger the ata_id_locked() condition? I think it
> would be safer to have an extra DFLAG, say ATA_DFLAG_LOCKED which gets
> set when EH detects unknown device error && ata_id_locked(); then, in
> the issue path, we can do
>
> if (ATA_DFLAG_LOCKED is set) {
> if (ata_id_locked())
> Terminate early;
> else
> Clear ATA_DFLAG_LOCKED;
> }
>
> Thanks.
>
> --
> 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
