On Wed, Sep 20, 2023 at 01:30:02PM +0200, Greg Kroah-Hartman wrote:
> 6.5-stable review patch. If anyone has any objections, please let me know.
Hello Greg,
I don't think that we should backport this commit.
While the patch did apply without conflicts, it was part of a series that
did a bunch of other cleanups as well.
I think that it is best to either have that whole series (and we don't want
to backport the whole series), or none of the patches in that series.
(So that at least we know that we have one or the other, not some half-way
cleanup that will only live in v6.5 stable.)
>
> ------------------
>
> From: Hannes Reinecke <hare@xxxxxxx>
>
> [ Upstream commit ff8072d589dcff7c1f0345a6ec98b5fc1e9ee2a1 ]
>
> With commit 65a15d6560df ("scsi: ipr: Remove SATA support") all
> libata drivers now have the error_handler() callback provided,
> so we can stop checking for non-existing error_handler callback.
>
> Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
> [niklas: fixed review comments, rebased, solved conflicts during rebase,
> fixed bug that unconditionally dumped all QCs, removed the now unused
> function ata_dump_status(), removed the now unreachable failure paths in
> atapi_qc_complete(), removed the non-EH function to request ATAPI sense]
> Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
> Reviewed-by: John Garry <john.g.garry@xxxxxxxxxx>
> Reviewed-by: Jason Yan <yanaijie@xxxxxxxxxx>
> Reviewed-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
> Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
> Stable-dep-of: 5e35a9ac3fe3 ("ata: libata-core: fetch sense data for successful commands iff CDL enabled")
Yes, it is true that
5e35a9ac3fe3 ("ata: libata-core: fetch sense data for successful commands iff CDL enabled")
does not apply cleanly to v6.5 stable without this big commit.
I'm attaching a backported version of that patch (which is only 2 lines or so)
that can be applied to v6.5 stable instead.
Kind regards,
Niklas
From e8d3e482fd2870fc56e2af7eb3812263e446564f Mon Sep 17 00:00:00 2001
From: Niklas Cassel <niklas.cassel@xxxxxxx>
Date: Wed, 13 Sep 2023 17:04:43 +0200
Subject: [PATCH] ata: libata-core: fetch sense data for successful commands
iff CDL enabled
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@xxxxxxx>
Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
(cherry picked from commit 5e35a9ac3fe3a0d571b899a16ca84253e53dc70c)
Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
---
drivers/ata/libata-core.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 04db0f2c683a..79d02eb4e479 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4935,11 +4935,8 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
* 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.
--
2.41.0
