diff mbox

[libata/for-4.2-fixes] libata: disable NCQ autosense

Message ID 20150803151621.GF32599@mtj.duckdns.org (mailing list archive)
State New, archived
Headers show

Commit Message

Tejun Heo Aug. 3, 2015, 3:16 p.m. UTC
From 8c0fa3e7ca99b0d6d96cb2cf194a912f643da7c5 Mon Sep 17 00:00:00 2001
From: Tejun Heo <tj@kernel.org>
Date: Mon, 3 Aug 2015 11:10:45 -0400

fe7173c206de ("libata: Implement support for sense data reporting")
and subsequent patches enabled NCQ autosense reporting; unfortunately,
this breaks libata EH behavior for devices which support the feature
because it assumes that only ATAPI devices report sense data and that
sense data trumps explicit device error indication.

For now, disable NCQ autosense.

Cc: stable@vger.kernel.org # v4.1+
Fixes: fe7173c206de ("libata: Implement support for sense data reporting")
---
 drivers/ata/libata-core.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Tejun Heo Aug. 3, 2015, 3:39 p.m. UTC | #1
On Mon, Aug 03, 2015 at 11:16:21AM -0400, Tejun Heo wrote:
> From 8c0fa3e7ca99b0d6d96cb2cf194a912f643da7c5 Mon Sep 17 00:00:00 2001
> From: Tejun Heo <tj@kernel.org>
> Date: Mon, 3 Aug 2015 11:10:45 -0400
> 
> fe7173c206de ("libata: Implement support for sense data reporting")
> and subsequent patches enabled NCQ autosense reporting; unfortunately,
> this breaks libata EH behavior for devices which support the feature
> because it assumes that only ATAPI devices report sense data and that
> sense data trumps explicit device error indication.

So, this isn't enough and apparently you're bypassing the entire
device side error analyzing when the device reports sense data.  Sorry
that I missed this on the first submission but I can't see how this
would work.  Even if you fix the immediate issue in this thread, how
is it gonna handle link error reported by the device?  Who's gonna
tell libata EH that the link better be reset and may be sped down if
errors keep occurring?  This is a significant regression in EH
behavior.

I'm gonna revert the whole ACS-4 sense thing.  ATM, I'm pretty
doubtful that this buys antying substantial for us and even if so we
need to figure out a better way to integrate this.

Thanks.
diff mbox

Patch

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index db5d9f7..32451ac 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2151,6 +2151,13 @@  static void ata_dev_config_sense_reporting(struct ata_device *dev)
 {
 	unsigned int err_mask;
 
+	/*
+	 * FIXME: This makes ATA devices report sense data on failure which
+	 * in turn makes libata EH ignore AC_ERR_DEV leading to incorrect
+	 * error handling behaviors.
+	 */
+	return;
+
 	if (!ata_id_has_sense_reporting(dev->id))
 		return;