ata: libata-scsi: Do not set the INFORMATION field twice for ATA PT

For ATA PASS-THROUGH + fixed format sense data + NCQ autosense
the INFORMATION sense data field is being written twice:

- 1st write: (redundant)
scsi_set_sense_information() sets the INFORMATION field to ATA LBA.
This is incorrect for ATA PASS-THROUGH.

- 2nd write: (correct)
ata_scsi_set_passthru_sense_fields() sets the INFORMATION field
to ATA ERROR/STATUS/DEVICE/COUNT(7:0) as per SAT spec.

There is no user-visible issue because second write overwrites
the incorrect data from the first write.

This patch eliminates the reduntant write by moving the INFORMATION
sense data field population logic to ata_scsi_qc_complete().

Signed-off-by: Igor Pylypiv <ipylypiv@google.com>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>

diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index 4e3034af285e4943caf82517d65c8c542178cde2..cb46ce276bb119859aea71683e3b13e7cc7f5b06 100644 (file)
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -1659,8 +1659,6 @@ void ata_eh_analyze_ncq_error(struct ata_link *link)
                if (ata_scsi_sense_is_valid(sense_key, asc, ascq)) {
                        ata_scsi_set_sense(dev, qc->scsicmd, sense_key, asc,
                                           ascq);
-                       ata_scsi_set_sense_information(dev, qc->scsicmd,
-                                                      &qc->result_tf);
                        qc->flags |= ATA_QCFLAG_SENSE_VALID;
                }
        }

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 2796c0da82578ac1f020f11395ba03e56a54c1fc..ef117a0bc248d5b6fc7dad394a8b6d6786e7f7c9 100644 (file)
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -216,17 +216,21 @@ void ata_scsi_set_sense(struct ata_device *dev, struct scsi_cmnd *cmd,
        scsi_build_sense(cmd, d_sense, sk, asc, ascq);
 }
 
-void ata_scsi_set_sense_information(struct ata_device *dev,
-                                   struct scsi_cmnd *cmd,
-                                   const struct ata_taskfile *tf)
+static void ata_scsi_set_sense_information(struct ata_queued_cmd *qc)
 {
        u64 information;
 
-       information = ata_tf_read_block(tf, dev);
+       if (!(qc->flags & ATA_QCFLAG_RTF_FILLED)) {
+               ata_dev_dbg(qc->dev,
+                           "missing result TF: can't set INFORMATION sense field\n");
+               return;
+       }
+
+       information = ata_tf_read_block(&qc->result_tf, qc->dev);
        if (information == U64_MAX)
                return;
 
-       scsi_set_sense_information(cmd->sense_buffer,
+       scsi_set_sense_information(qc->scsicmd->sense_buffer,
                                   SCSI_SENSE_BUFFERSIZE, information);
 }
 
@@ -971,8 +975,7 @@ static void ata_gen_passthru_sense(struct ata_queued_cmd *qc)
  *     ata_gen_ata_sense - generate a SCSI fixed sense block
  *     @qc: Command that we are erroring out
  *
- *     Generate sense block for a failed ATA command @qc.  Descriptor
- *     format is used to accommodate LBA48 block address.
+ *     Generate sense block for a failed ATA command @qc.
  *
  *     LOCKING:
  *     None.
@@ -982,8 +985,6 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
        struct ata_device *dev = qc->dev;
        struct scsi_cmnd *cmd = qc->scsicmd;
        struct ata_taskfile *tf = &qc->result_tf;
-       unsigned char *sb = cmd->sense_buffer;
-       u64 block;
        u8 sense_key, asc, ascq;
 
        if (ata_dev_disabled(dev)) {
@@ -1014,12 +1015,6 @@ static void ata_gen_ata_sense(struct ata_queued_cmd *qc)
                ata_scsi_set_sense(dev, cmd, ABORTED_COMMAND, 0, 0);
                return;
        }
-
-       block = ata_tf_read_block(&qc->result_tf, dev);
-       if (block == U64_MAX)
-               return;
-
-       scsi_set_sense_information(sb, SCSI_SENSE_BUFFERSIZE, block);
 }
 
 void ata_scsi_sdev_config(struct scsi_device *sdev)
@@ -1679,8 +1674,10 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
                ata_scsi_set_passthru_sense_fields(qc);
                if (is_ck_cond_request)
                        set_status_byte(qc->scsicmd, SAM_STAT_CHECK_CONDITION);
-       } else if (is_error && !have_sense) {
-               ata_gen_ata_sense(qc);
+       } else if (is_error) {
+               if (!have_sense)
+                       ata_gen_ata_sense(qc);
+               ata_scsi_set_sense_information(qc);
        }
 
        ata_qc_done(qc);

diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 0337be4faec7c4c0e480fff45aaee62d1a503e4d..ce5c628fa6fdcbf02cc070e64914d4142c7bbcef 100644 (file)
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -141,9 +141,6 @@ extern int ata_scsi_offline_dev(struct ata_device *dev);
 extern bool ata_scsi_sense_is_valid(u8 sk, u8 asc, u8 ascq);
 extern void ata_scsi_set_sense(struct ata_device *dev,
                               struct scsi_cmnd *cmd, u8 sk, u8 asc, u8 ascq);
-extern void ata_scsi_set_sense_information(struct ata_device *dev,
-                                          struct scsi_cmnd *cmd,
-                                          const struct ata_taskfile *tf);
 extern void ata_scsi_media_change_notify(struct ata_device *dev);
 extern void ata_scsi_hotplug(struct work_struct *work);
 extern void ata_scsi_dev_rescan(struct work_struct *work);