| Message ID | 20230112140412.667308-3-niklas.cassel@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Command Duration Limits support | expand |
On 1/12/23 15:03, Niklas Cassel wrote: > Current ata_eh_request_sense() calls ata_scsi_set_sense() with > check_condition always set to true, which, in addition to setting the > sense data, unconditionally sets the scsicmd->result to > SAM_STAT_CHECK_CONDITION. > > For Command Duration Limits policy 0xD: > The device shall complete the command without error (SAM_STAT_GOOD) > with the additional sense code set to DATA CURRENTLY UNAVAILABLE. > > It is perfectly fine to have sense data for a command that returned > completion without error. > > In order to support for CDL policy 0xD, we have to remove this > assumption that having sense data means that the command failed > (SAM_STAT_CHECK_CONDITION). > > Add a new parameter to ata_eh_request_sense() to allow us to request > sense data without unconditionally setting SAM_STAT_CHECK_CONDITION. > This new parameter will be used in a follow-up patch. > > Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com> > --- > drivers/ata/libata-eh.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
Similar comment here - just move setting the SAM_STAT_CHECK_CONDITION out of this function and into the caller. It'll need a bool return for that, though.
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 3521f3f67f5a..1c3d55fc1cae 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -1395,6 +1395,7 @@ unsigned int atapi_eh_tur(struct ata_device *dev, u8 *r_sense_key) /** * ata_eh_request_sense - perform REQUEST_SENSE_DATA_EXT * @qc: qc to perform REQUEST_SENSE_SENSE_DATA_EXT to + * @check_condition: if SAM_STAT_CHECK_CONDITION should get set * * Perform REQUEST_SENSE_DATA_EXT after the device reported CHECK * SENSE. This function is an EH helper. @@ -1402,7 +1403,8 @@ unsigned int atapi_eh_tur(struct ata_device *dev, u8 *r_sense_key) * LOCKING: * Kernel thread context (may sleep). */ -static void ata_eh_request_sense(struct ata_queued_cmd *qc) +static void ata_eh_request_sense(struct ata_queued_cmd *qc, + bool check_condition) { struct scsi_cmnd *cmd = qc->scsicmd; struct ata_device *dev = qc->dev; @@ -1432,8 +1434,8 @@ static void ata_eh_request_sense(struct ata_queued_cmd *qc) /* Ignore err_mask; ATA_ERR might be set */ if (tf.status & ATA_SENSE) { if (ata_scsi_sense_is_valid(tf.lbah, tf.lbam, tf.lbal)) { - ata_scsi_set_sense(dev, cmd, true, tf.lbah, tf.lbam, - tf.lbal); + ata_scsi_set_sense(dev, cmd, check_condition, tf.lbah, + tf.lbam, tf.lbal); qc->flags |= ATA_QCFLAG_SENSE_VALID; } } else { @@ -1590,7 +1592,7 @@ static unsigned int ata_eh_analyze_tf(struct ata_queued_cmd *qc) * (i.e. NCQ autosense is not supported by the device). */ if (!(qc->flags & ATA_QCFLAG_SENSE_VALID) && (stat & ATA_SENSE)) - ata_eh_request_sense(qc); + ata_eh_request_sense(qc, true); if (err & ATA_ICRC) qc->err_mask |= AC_ERR_ATA_BUS; if (err & (ATA_UNC | ATA_AMNF))
Current ata_eh_request_sense() calls ata_scsi_set_sense() with check_condition always set to true, which, in addition to setting the sense data, unconditionally sets the scsicmd->result to SAM_STAT_CHECK_CONDITION. For Command Duration Limits policy 0xD: The device shall complete the command without error (SAM_STAT_GOOD) with the additional sense code set to DATA CURRENTLY UNAVAILABLE. It is perfectly fine to have sense data for a command that returned completion without error. In order to support for CDL policy 0xD, we have to remove this assumption that having sense data means that the command failed (SAM_STAT_CHECK_CONDITION). Add a new parameter to ata_eh_request_sense() to allow us to request sense data without unconditionally setting SAM_STAT_CHECK_CONDITION. This new parameter will be used in a follow-up patch. Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com> --- drivers/ata/libata-eh.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)