ata: libata-core: fetch sense data for successful commands iff CDL enabled
authorNiklas Cassel <niklas.cassel@wdc.com>
Wed, 13 Sep 2023 15:04:43 +0000 (17:04 +0200)
committerDamien Le Moal <dlemoal@kernel.org>
Sat, 16 Sep 2023 12:12:19 +0000 (21:12 +0900)
Currently, we fetch sense data for a _successful_ command if either:
1) Command was NCQ and ATA_DFLAG_CDL_ENABLED flag set (flag
   ATA_DFLAG_CDL_ENABLED will only be set if the Successful NCQ command
   sense data supported bit is set); or
2) Command was non-NCQ and regular sense data reporting is enabled.

This means that case 2) will trigger for a non-NCQ command which has
ATA_SENSE bit set, regardless if CDL is enabled or not.

This decision was by design. If the device reports that it has sense data
available, it makes sense to fetch that sense data, since the sk/asc/ascq
could be important information regardless if CDL is enabled or not.

However, the fetching of sense data for a successful command is done via
ATA EH. Considering how intricate the ATA EH is, we really do not want to
invoke ATA EH unless absolutely needed.

Before commit 18bd7718b5c4 ("scsi: ata: libata: Handle completion of CDL
commands using policy 0xD") we never fetched sense data for successful
commands.

In order to not invoke the ATA EH unless absolutely necessary, even if the
device claims support for sense data reporting, only fetch sense data for
successful (NCQ and non-NCQ commands) commands that are using CDL.

[Damien] Modified the check to test the qc flag ATA_QCFLAG_HAS_CDL
instead of the device support for CDL, which is implied for commands
using CDL.

Fixes: 3ac873c76d79 ("ata: libata-core: fix when to fetch sense data for successful commands")
Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
drivers/ata/libata-core.c

index 74314311295fe43d56241fae063bee6cd530267a..0072e0f9ad391195368ae2e9c77e5e7fa4cc0195 100644 (file)
@@ -4783,11 +4783,8 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
         * been aborted by the device due to a limit timeout using the policy
         * 0xD. For these commands, invoke EH to get the command sense data.
         */
-       if (qc->result_tf.status & ATA_SENSE &&
-           ((ata_is_ncq(qc->tf.protocol) &&
-             dev->flags & ATA_DFLAG_CDL_ENABLED) ||
-            (!ata_is_ncq(qc->tf.protocol) &&
-             ata_id_sense_reporting_enabled(dev->id)))) {
+       if (qc->flags & ATA_QCFLAG_HAS_CDL &&
+           qc->result_tf.status & ATA_SENSE) {
                /*
                 * Tell SCSI EH to not overwrite scmd->result even if this
                 * command is finished with result SAM_STAT_GOOD.