[PATCH 02/11] libata-eh-fw: clear SError in ata_std_postreset()

[PATCH 02/11] libata-eh-fw: clear SError in ata_std_postreset()

[Tejun Heo]

Clear SError in ata_std_postreset().  This is to clear SError bits
which get set during reset.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

033e89c7ccb4ba7ae815e6b3c2b65e5b0a8e0ce4
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 126adef..554cf8a 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -2527,11 +2527,17 @@ int sata_std_hardreset(struct ata_port *
  */
 void ata_std_postreset(struct ata_port *ap, unsigned int *classes)
 {
+	u32 serror;
+
 	DPRINTK("ENTER\n");
 
 	/* print link status */
 	sata_print_link_status(ap);
 
+	/* clear SError */
+	if (ata_scr_read(ap, SCR_ERROR, &serror) == 0)
+		ata_scr_write(ap, SCR_ERROR, serror);
+
 	/* re-enable interrupts */
 	if (ap->ioaddr.ctl_addr)	/* FIXME: hack. create a hook instead */
 		ata_irq_on(ap);
-- 
1.2.4

[PATCH 04/11] libata-eh-fw: update ata_qc_from_tag() to enforce normal/EH qc ownership

[Tejun Heo]

New EH framework has clear distinction about who owns a qc.  Every qc
starts owned by normal execution path - PIO, interrupt or whatever.
When an exception condition occurs which affects the qc, the qc gets
scheduled for EH.  Note that some events (say, link lost and regained,
command timeout) may schedule qc's which are not directly related but
could have been affected for EH too.  Scheduling for EH is atomic
w.r.t. ap->host_set->lock and once schedule for EH, normal execution
path is not allowed to access the qc in whatever way.  (PIO
synchronization acts a bit different and will be dealt with later)

This patch make ata_qc_from_tag() check whether a qc is active and
owned by normal path before returning it.  If conditions don't match,
NULL is returned and thus access to the qc is denied.
__ata_qc_from_tag() is the original ata_qc_from_tag() and is used by
libata core/EH layers to access inactive/failed qc's.

This change is applied only if the associated LLDD implements new EH
as indicated by non-NULL ->error_handler

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |    4 ++--
 include/linux/libata.h     |   19 +++++++++++++++++--
 2 files changed, 19 insertions(+), 4 deletions(-)

6d50afcc7b9166668faffbdf0b644eeadbb3ed71
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 3de1750..76df36d 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1001,7 +1001,7 @@ unsigned ata_exec_internal(struct ata_de
 
 	if (test_and_set_bit(tag, &ap->qactive))
 		BUG();
-	qc = ata_qc_from_tag(ap, tag);
+	qc = __ata_qc_from_tag(ap, tag);
 
 	qc->tag = tag;
 	qc->scsicmd = NULL;
@@ -4041,7 +4041,7 @@ static struct ata_queued_cmd *ata_qc_new
 	/* the last tag is reserved for internal command. */
 	for (i = 0; i < ATA_MAX_QUEUE - 1; i++)
 		if (!test_and_set_bit(i, &ap->qactive)) {
-			qc = ata_qc_from_tag(ap, i);
+			qc = __ata_qc_from_tag(ap, i);
 			break;
 		}
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 2b384e8..3d6942a 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -832,14 +832,29 @@ static inline void ata_qc_set_polling(st
 	qc->tf.ctl |= ATA_NIEN;
 }
 
-static inline struct ata_queued_cmd *ata_qc_from_tag (struct ata_port *ap,
-						      unsigned int tag)
+static inline struct ata_queued_cmd *__ata_qc_from_tag(struct ata_port *ap,
+						       unsigned int tag)
 {
 	if (likely(ata_tag_valid(tag)))
 		return &ap->qcmd[tag];
 	return NULL;
 }
 
+static inline struct ata_queued_cmd *ata_qc_from_tag(struct ata_port *ap,
+						     unsigned int tag)
+{
+	struct ata_queued_cmd *qc = __ata_qc_from_tag(ap, tag);
+
+	if (unlikely(!qc) || !ap->ops->error_handler)
+		return qc;
+
+	if ((qc->flags & (ATA_QCFLAG_ACTIVE |
+			  ATA_QCFLAG_FAILED)) == ATA_QCFLAG_ACTIVE)
+		return qc;
+
+	return NULL;
+}
+
 static inline void ata_tf_init(struct ata_device *dev, struct ata_taskfile *tf)
 {
 	memset(tf, 0, sizeof(*tf));
-- 
1.2.4

[PATCH 03/11] libata-eh-fw: use special reserved tag and qc for internal commands

[Tejun Heo]

New EH may issue internal commands to recover from error while failed
qc's are still hanging around.  To allow such usage, reserve tag
ATA_MAX_QUEUE-1 for internal command.  This also makes it easy to tell
whether a qc is for internal command or not.  ata_tag_internal() test
implements this test.

To avoid breaking existing drivers, ata_exec_internal() uses
ATA_TAG_INTERNAL only for drivers which implement ->error_handler.
For drivers using old EH, tag 0 is used.  Note that this makes
ata_tag_internal() test valid only when ->error_handler is
implemented.  This is okay as drivers on old EH should not and does
not have any reason to use ata_tag_internal().

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   32 +++++++++++++++++++++++++++++---
 include/linux/libata.h     |    9 ++++++++-
 2 files changed, 37 insertions(+), 4 deletions(-)

9b20eee01cf9046235f34ec372255ea4e22304c8
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 554cf8a..3de1750 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -980,15 +980,39 @@ unsigned ata_exec_internal(struct ata_de
 	struct ata_port *ap = dev->ap;
 	u8 command = tf->command;
 	struct ata_queued_cmd *qc;
+	unsigned int tag, preempted_tag;
 	DECLARE_COMPLETION(wait);
 	unsigned long flags;
 	unsigned int err_mask;
 
 	spin_lock_irqsave(&ap->host_set->lock, flags);
 
-	qc = ata_qc_new_init(dev);
-	BUG_ON(qc == NULL);
+	/* initialize internal qc */
 
+	/* XXX: Tag 0 is used for drivers with legacy EH as some
+	 * drivers choke if any other tag is given.  This breaks
+	 * ata_tag_internal() test for those drivers.  Don't use new
+	 * EH stuff without converting to it.
+	 */
+	if (ap->ops->error_handler)
+		tag = ATA_TAG_INTERNAL;
+	else
+		tag = 0;
+
+	if (test_and_set_bit(tag, &ap->qactive))
+		BUG();
+	qc = ata_qc_from_tag(ap, tag);
+
+	qc->tag = tag;
+	qc->scsicmd = NULL;
+	qc->ap = ap;
+	qc->dev = dev;
+	ata_qc_reinit(qc);
+
+	preempted_tag = ap->active_tag;
+	ap->active_tag = ATA_TAG_POISON;
+
+	/* prepare & issue qc */
 	qc->tf = *tf;
 	if (cdb)
 		memcpy(qc->cdb, cdb, ATAPI_CDB_LEN);
@@ -1035,6 +1059,7 @@ unsigned ata_exec_internal(struct ata_de
 	err_mask = qc->err_mask;
 
 	ata_qc_free(qc);
+	ap->active_tag = preempted_tag;
 
 	/* XXX - Some LLDDs (sata_mv) disable port on command failure.
 	 * Until those drivers are fixed, we detect the condition
@@ -4013,7 +4038,8 @@ static struct ata_queued_cmd *ata_qc_new
 	struct ata_queued_cmd *qc = NULL;
 	unsigned int i;
 
-	for (i = 0; i < ATA_MAX_QUEUE; i++)
+	/* the last tag is reserved for internal command. */
+	for (i = 0; i < ATA_MAX_QUEUE - 1; i++)
 		if (!test_and_set_bit(i, &ap->qactive)) {
 			qc = ata_qc_from_tag(ap, i);
 			break;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 302df2d..2b384e8 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -108,7 +108,9 @@ enum {
 	LIBATA_MAX_PRD		= ATA_MAX_PRD / 2,
 	ATA_MAX_PORTS		= 8,
 	ATA_DEF_QUEUE		= 1,
-	ATA_MAX_QUEUE		= 1,
+	/* tag ATA_MAX_QUEUE - 1 is reserved for internal commands */
+	ATA_MAX_QUEUE		= 2,
+	ATA_TAG_INTERNAL	= ATA_MAX_QUEUE - 1,
 	ATA_MAX_SECTORS		= 200,	/* FIXME */
 	ATA_MAX_BUS		= 2,
 	ATA_DEF_BUSY_WAIT	= 10000,
@@ -717,6 +719,11 @@ static inline unsigned int ata_tag_valid
 	return (tag < ATA_MAX_QUEUE) ? 1 : 0;
 }
 
+static inline unsigned int ata_tag_internal(unsigned int tag)
+{
+	return tag == ATA_MAX_QUEUE - 1;
+}
+
 static inline unsigned int ata_class_enabled(unsigned int class)
 {
 	return class == ATA_DEV_ATA || class == ATA_DEV_ATAPI;
-- 
1.2.4

[PATCHSET 02/11] new EH framework, take 3

[Tejun Heo]

Hello, again.

This is part of patchset series described in [T].

This is the third take of new-EH-framework patchset.  This patchset
contains 11 patches.  Changes from the last take[L] are

* ata_eh_schedule_port() is replaced with ata_port_schedule_eh(),
  ata_port_abort() and ata_port_freeze().  This is cleaner and makes
  adding link level abort easier.

* abort now uses ATA_QCFLAG_FAILED to tag a qc as failed and simply
  call ata_qc_complete().  This simplifies abortion.

* ATA_EH_PENDING is set when EH is requested and cleared right before
  beginning EH.  After completing EH, if the flag is still set, EH is
  repeated.  This way no EH request is lost.  Later EH implementation
  also includes a mechanism to avoid repeating EH actions
  unnecessarily in such cases.

* ATA_DFLAG_FAILED removed.  It's replaced with ehi->dev implemented
  in the EH patchset.

* Port can be in FROZEN state after EH.  Previously, EH framework
  repeated EH automatically in such cases.  Now whether to leave the
  port frozen or not is the EH's call.  (Note that user can request
  another shot at recovery or detection with warm plug request)

* ops->thaw() added.  Reset or postreset methods now don't
  automatically thaw the port.  This change is necessary for later PM
  support where multiple resets are performed while frozen.  It also
  cleans code up a bit.

* Timedout qcs are now handled by ata_scsi_error() after EH kicks in.
  ata_scsi_timed_out() path is used only for old EH.  To transfer a
  timedout qc's ownership to EH, it's necessary to freeze the port.
  Previously, the transfer happened on timeout detection in
  ata_scsi_timed_out() forcing other active commands to get aborted.
  With this change, the transfer is done after all other commands are
  drained.

* ata_exec_internal() EH fixed/improved.  As commands can be aborted
  during PIO, PIO task needs to be flushed whether it timed out or
  not.

* Error messages from SCSI qc completion path is suppressed if new EH
  is in use.  New EH itself reports better.

This patchset is against

  upstream (acc696d93dcf993dec123d69d599979e1456ffec)
  + [1] prep-for-new-EH patchset

--
tejun

[T] http://article.gmane.org/gmane.linux.ide/9957
[L] http://article.gmane.org/gmane.linux.ide/9524
[1] http://article.gmane.org/gmane.linux.ide/9959

[PATCH 01/11] libata-eh-fw: add flags and operations for new EH

[Tejun Heo]

Add ATA_FLAG_EH_{PENDING|FROZEN}, ATA_ATA_QCFLAG_{FAILED|SENSE_VALID}
and ops->freeze, thaw, error_handler, post_internal_cmd() for new EH.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 include/linux/libata.h |   27 ++++++++++++++++++++-------
 1 files changed, 20 insertions(+), 7 deletions(-)

7adb784b36e6b4302540a3edb58cddb3ba0a4b83
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 7b54d92..302df2d 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -146,13 +146,16 @@ enum {
 	ATA_FLAG_PIO_LBA48	= (1 << 8), /* Host DMA engine is LBA28 only */
 	ATA_FLAG_IRQ_MASK	= (1 << 9), /* Mask IRQ in PIO xfers */
 
-	ATA_FLAG_NOINTR		= (1 << 16), /* FIXME: Remove this once
+	ATA_FLAG_NOINTR		= (1 << 13), /* FIXME: Remove this once
 					      * proper HSM is in place. */
-	ATA_FLAG_DEBUGMSG	= (1 << 17),
-	ATA_FLAG_FLUSH_PORT_TASK = (1 << 18), /* flush port task */
+	ATA_FLAG_DEBUGMSG	= (1 << 14),
+	ATA_FLAG_FLUSH_PORT_TASK = (1 << 15), /* flush port task */
 
-	ATA_FLAG_DISABLED	= (1 << 19), /* port is disabled, ignore it */
-	ATA_FLAG_SUSPENDED	= (1 << 20), /* port is suspended */
+	ATA_FLAG_EH_PENDING	= (1 << 16), /* EH pending */
+	ATA_FLAG_FROZEN		= (1 << 17), /* port is frozen */
+
+	ATA_FLAG_DISABLED	= (1 << 22), /* port is disabled, ignore it */
+	ATA_FLAG_SUSPENDED	= (1 << 23), /* port is suspended (power) */
 
 	/* bits 24:31 of ap->flags are reserved for LLDD specific flags */
 
@@ -164,7 +167,9 @@ enum {
 	ATA_QCFLAG_IO		= (1 << 3), /* standard IO command */
 	ATA_QCFLAG_RESULT_TF	= (1 << 4), /* result TF requested */
 
-	ATA_QCFLAG_EH_SCHEDULED = (1 << 16), /* EH scheduled */
+	ATA_QCFLAG_FAILED	= (1 << 16), /* cmd failed and is owned by EH */
+	ATA_QCFLAG_SENSE_VALID	= (1 << 17), /* sense data valid */
+	ATA_QCFLAG_EH_SCHEDULED = (1 << 18), /* EH scheduled (obsolete) */
 
 	/* host set flags */
 	ATA_HOST_SIMPLEX	= (1 << 0),	/* Host is simplex, one DMA channel per host_set only */
@@ -463,7 +468,15 @@ struct ata_port_operations {
 	void (*qc_prep) (struct ata_queued_cmd *qc);
 	unsigned int (*qc_issue) (struct ata_queued_cmd *qc);
 
-	void (*eng_timeout) (struct ata_port *ap);
+	/* Error handlers.  ->error_handler overrides ->eng_timeout and
+	 * indicates that new-style EH is in place.
+	 */
+	void (*eng_timeout) (struct ata_port *ap); /* obsolete */
+
+	void (*freeze) (struct ata_port *ap);
+	void (*thaw) (struct ata_port *ap);
+	void (*error_handler) (struct ata_port *ap);
+	void (*post_internal_cmd) (struct ata_queued_cmd *qc);
 
 	irqreturn_t (*irq_handler)(int, void *, struct pt_regs *);
 	void (*irq_clear) (struct ata_port *);
-- 
1.2.4

[PATCH 08/11] libata-eh-fw: implement new EH scheduling from PIO

[Tejun Heo]

PIO executes without holding host_set lock, so it cannot be
synchronized using the same mechanism as interrupt driven execution.
port_task framework makes sure that EH is not entered until PIO task
is flushed, so PIO task can be sure the qc in progress won't go away
underneath it.  One thing it cannot be sure of is whether the qc has
already been scheduled for EH by another exception condition while
host_set lock was released.

This patch makes ata_poll_qc-complete() handle such conditions
properly and make it freeze the port if HSM violation is detected
during PIO execution.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   23 +++++++++++++++++++----
 1 files changed, 19 insertions(+), 4 deletions(-)

52240d1d208615e461c41a5c297f521a8a892f0d
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 436ff5b..86d6a7d 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -3429,16 +3429,31 @@ skip_map:
  *	LOCKING:
  *	None.  (grabs host lock)
  */
-
 void ata_poll_qc_complete(struct ata_queued_cmd *qc)
 {
 	struct ata_port *ap = qc->ap;
 	unsigned long flags;
 
 	spin_lock_irqsave(&ap->host_set->lock, flags);
-	ap->flags &= ~ATA_FLAG_NOINTR;
-	ata_irq_on(ap);
-	ata_qc_complete(qc);
+
+	if (ap->ops->error_handler) {
+		/* EH might have kicked in while host_set lock is released */
+		qc = ata_qc_from_tag(ap, qc->tag);
+		if (qc) {
+			if (!(qc->err_mask & AC_ERR_HSM)) {
+				ap->flags &= ~ATA_FLAG_NOINTR;
+				ata_irq_on(ap);
+				ata_qc_complete(qc);
+			} else
+				ata_port_freeze(ap);
+		}
+	} else {
+		/* old EH */
+		ap->flags &= ~ATA_FLAG_NOINTR;
+		ata_irq_on(ap);
+		ata_qc_complete(qc);
+	}
+
 	spin_unlock_irqrestore(&ap->host_set->lock, flags);
 }
 
-- 
1.2.4

[PATCH 11/11] libata-eh-fw: update SCSI command completion path for new EH

[Tejun Heo]

SCSI command completion path used to do some part of EH including
printing messages and obtaining sense data.  With new EH, all these
are responsibilities of the EH, update SCSI command completion path to
reflect this.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-scsi.c |   48 ++++++++++++++++++++++++++++++++++----------
 1 files changed, 37 insertions(+), 11 deletions(-)

90c8f6da58a1c4c67e65c73ed303a97d06c077c9
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index fd7064b..e61cc35 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -415,6 +415,7 @@ int ata_scsi_device_suspend(struct scsi_
  *	@sk: the sense key we'll fill out
  *	@asc: the additional sense code we'll fill out
  *	@ascq: the additional sense code qualifier we'll fill out
+ *	@verbose: be verbose
  *
  *	Converts an ATA error into a SCSI error.  Fill out pointers to
  *	SK, ASC, and ASCQ bytes for later use in fixed or descriptor
@@ -424,7 +425,7 @@ int ata_scsi_device_suspend(struct scsi_
  *	spin_lock_irqsave(host_set lock)
  */
 void ata_to_sense_error(unsigned id, u8 drv_stat, u8 drv_err, u8 *sk, u8 *asc,
-			u8 *ascq)
+			u8 *ascq, int verbose)
 {
 	int i;
 
@@ -489,8 +490,9 @@ void ata_to_sense_error(unsigned id, u8 
 			}
 		}
 		/* No immediate match */
-		printk(KERN_WARNING "ata%u: no sense translation for "
-		       "error 0x%02x\n", id, drv_err);
+		if (verbose)
+			printk(KERN_WARNING "ata%u: no sense translation for "
+			       "error 0x%02x\n", id, drv_err);
 	}
 
 	/* Fall back to interpreting status bits */
@@ -503,8 +505,9 @@ void ata_to_sense_error(unsigned id, u8 
 		}
 	}
 	/* No error?  Undecoded? */
-	printk(KERN_WARNING "ata%u: no sense translation for status: 0x%02x\n",
-	       id, drv_stat);
+	if (verbose)
+		printk(KERN_WARNING "ata%u: no sense translation for "
+		       "status: 0x%02x\n", id, drv_stat);
 
 	/* We need a sensible error return here, which is tricky, and one
 	   that won't cause people to do things like return a disk wrongly */
@@ -513,9 +516,10 @@ void ata_to_sense_error(unsigned id, u8 
 	*ascq = 0x00;
 
  translate_done:
-	printk(KERN_ERR "ata%u: translated ATA stat/err 0x%02x/%02x to "
-	       "SCSI SK/ASC/ASCQ 0x%x/%02x/%02x\n", id, drv_stat, drv_err,
-	       *sk, *asc, *ascq);
+	if (verbose)
+		printk(KERN_ERR "ata%u: translated ATA stat/err 0x%02x/%02x "
+		       "to SCSI SK/ASC/ASCQ 0x%x/%02x/%02x\n",
+		       id, drv_stat, drv_err, *sk, *asc, *ascq);
 	return;
 }
 
@@ -538,6 +542,7 @@ void ata_gen_ata_desc_sense(struct ata_q
 	struct ata_taskfile *tf = &qc->result_tf;
 	unsigned char *sb = cmd->sense_buffer;
 	unsigned char *desc = sb + 8;
+	int verbose = qc->ap->ops->error_handler == NULL;
 
 	memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
 
@@ -550,7 +555,7 @@ void ata_gen_ata_desc_sense(struct ata_q
 	if (qc->err_mask ||
 	    tf->command & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ)) {
 		ata_to_sense_error(qc->ap->id, tf->command, tf->feature,
-				   &sb[1], &sb[2], &sb[3]);
+				   &sb[1], &sb[2], &sb[3], verbose);
 		sb[1] &= 0x0f;
 	}
 
@@ -608,6 +613,7 @@ void ata_gen_fixed_sense(struct ata_queu
 	struct scsi_cmnd *cmd = qc->scsicmd;
 	struct ata_taskfile *tf = &qc->result_tf;
 	unsigned char *sb = cmd->sense_buffer;
+	int verbose = qc->ap->ops->error_handler == NULL;
 
 	memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
 
@@ -620,7 +626,7 @@ void ata_gen_fixed_sense(struct ata_queu
 	if (qc->err_mask ||
 	    tf->command & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ)) {
 		ata_to_sense_error(qc->ap->id, tf->command, tf->feature,
-				   &sb[2], &sb[12], &sb[13]);
+				   &sb[2], &sb[12], &sb[13], verbose);
 		sb[2] &= 0x0f;
 	}
 
@@ -1212,7 +1218,7 @@ static void ata_scsi_qc_complete(struct 
 		}
 	}
 
-	if (need_sense)
+	if (need_sense && !qc->ap->ops->error_handler)
 		ata_dump_status(qc->ap->id, &qc->result_tf);
 
 	qc->scsidone(cmd);
@@ -2060,6 +2066,26 @@ static void atapi_qc_complete(struct ata
 
 	VPRINTK("ENTER, err_mask 0x%X\n", err_mask);
 
+	/* handle completion from new EH */
+	if (unlikely(qc->ap->ops->error_handler &&
+		     (err_mask || qc->flags & ATA_QCFLAG_SENSE_VALID))) {
+
+		if (!(qc->flags & ATA_QCFLAG_SENSE_VALID)) {
+			/* FIXME: not quite right; we don't want the
+			 * translation of taskfile registers into a
+			 * sense descriptors, since that's only
+			 * correct for ATA, not ATAPI
+			 */
+			ata_gen_ata_desc_sense(qc);
+		}
+
+		qc->scsicmd->result = SAM_STAT_CHECK_CONDITION;
+		qc->scsidone(cmd);
+		ata_qc_free(qc);
+		return;
+	}
+
+	/* successful completion or old EH failure path */
 	if (unlikely(err_mask & AC_ERR_DEV)) {
 		cmd->result = SAM_STAT_CHECK_CONDITION;
 		atapi_request_sense(qc);
-- 
1.2.4

[PATCH 10/11] libata-eh-fw: update ata_exec_internal() for new EH

[Tejun Heo]

Update ata_exec_internal() such that it uses new EH framework.
->post_internal_cmd() is always invoked regardless of completion
status.  Also, when ata_exec_internal() detects a timeout condition
and new EH is in place, it freezes the port as timeout for normal
commands would do.

Note that ata_port_flush_task() is called regardless of
wait_for_completion status.  This is necessary as exceptions unrelated
to the qc can abort the qc, in which case PIO task could still be
running after the wait for completion returns.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   30 +++++++++++++++++++++++-------
 1 files changed, 23 insertions(+), 7 deletions(-)

a20105e1c3c1639dd1677d2d9f600ccd0669882c
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 86d6a7d..a05be86 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -984,6 +984,7 @@ unsigned ata_exec_internal(struct ata_de
 	DECLARE_COMPLETION(wait);
 	unsigned long flags;
 	unsigned int err_mask;
+	int rc;
 
 	spin_lock_irqsave(&ap->host_set->lock, flags);
 
@@ -1036,20 +1037,25 @@ unsigned ata_exec_internal(struct ata_de
 
 	spin_unlock_irqrestore(&ap->host_set->lock, flags);
 
-	if (!wait_for_completion_timeout(&wait, ATA_TMOUT_INTERNAL)) {
-		ata_port_flush_task(ap);
+	rc = wait_for_completion_timeout(&wait, ATA_TMOUT_INTERNAL);
+
+	ata_port_flush_task(ap);
 
+	if (!rc) {
 		spin_lock_irqsave(&ap->host_set->lock, flags);
 
 		/* We're racing with irq here.  If we lose, the
 		 * following test prevents us from completing the qc
-		 * again.  If completion irq occurs after here but
-		 * before the caller cleans up, it will result in a
-		 * spurious interrupt.  We can live with that.
+		 * twice.  If we win, the port is frozen and will be
+		 * cleaned up by ->post_internal_cmd().
 		 */
 		if (qc->flags & ATA_QCFLAG_ACTIVE) {
-			qc->err_mask = AC_ERR_TIMEOUT;
-			ata_qc_complete(qc);
+			qc->err_mask |= AC_ERR_TIMEOUT;
+
+			if (ap->ops->error_handler)
+				ata_port_freeze(ap);
+			else
+				ata_qc_complete(qc);
 
 			ata_dev_printk(dev, KERN_WARNING,
 				       "qc timeout (cmd 0x%x)\n", command);
@@ -1058,6 +1064,16 @@ unsigned ata_exec_internal(struct ata_de
 		spin_unlock_irqrestore(&ap->host_set->lock, flags);
 	}
 
+	/* do post_internal_cmd */
+	if (ap->ops->post_internal_cmd)
+		ap->ops->post_internal_cmd(qc);
+
+	if (qc->flags & ATA_QCFLAG_FAILED && !qc->err_mask) {
+		ata_dev_printk(dev, KERN_WARNING, "zero err_mask for failed "
+			       "internal command, assuming AC_ERR_OTHER\n");
+		qc->err_mask |= AC_ERR_OTHER;
+	}
+
 	/* finish up */
 	spin_lock_irqsave(&ap->host_set->lock, flags);
 
-- 
1.2.4

[PATCH 06/11] libata-eh-fw: implement ata_port_schedule_eh() and ata_port_abort()

[Tejun Heo]

ata_port_schedule_eh() directly schedules EH for @ap without
associated qc.  Once EH scheduled, no further qc is allowed and EH
kicks in as soon as all currently active qc's are drained.

ata_port_abort() schedules all currently active commands for EH by
qc_completing them with ATA_QCFLAG_FAILED set.  If ata_port_abort()
doesn't find any qc to abort, it directly schedule EH using
ata_port_schedule_eh().

These two functions provide ways to invoke EH for conditions which
aren't directly related to any specfic qc.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |    2 ++
 drivers/scsi/libata-eh.c   |   54 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/libata-scsi.c |   23 +++++++++++++++++++
 drivers/scsi/libata.h      |    1 +
 include/linux/libata.h     |    4 +++
 5 files changed, 84 insertions(+), 0 deletions(-)

8bf0da935c2cc87ba065a10866fdf212cc0abe37
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 2b18033..7483518 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -5382,5 +5382,7 @@ EXPORT_SYMBOL_GPL(ata_scsi_device_suspen
 EXPORT_SYMBOL_GPL(ata_scsi_device_resume);
 
 EXPORT_SYMBOL_GPL(ata_eng_timeout);
+EXPORT_SYMBOL_GPL(ata_port_schedule_eh);
+EXPORT_SYMBOL_GPL(ata_port_abort);
 EXPORT_SYMBOL_GPL(ata_eh_qc_complete);
 EXPORT_SYMBOL_GPL(ata_eh_qc_retry);
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index 471846f..037a561 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -237,6 +237,60 @@ void ata_qc_schedule_eh(struct ata_queue
 	scsi_req_abort_cmd(qc->scsicmd);
 }
 
+/**
+ *	ata_port_schedule_eh - schedule error handling without a qc
+ *	@ap: ATA port to schedule EH for
+ *
+ *	Schedule error handling for @ap.  EH will kick in as soon as
+ *	all commands are drained.
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(host_set lock)
+ */
+void ata_port_schedule_eh(struct ata_port *ap)
+{
+	WARN_ON(!ap->ops->error_handler);
+
+	ap->flags |= ATA_FLAG_EH_PENDING;
+	ata_schedule_scsi_eh(ap->host);
+
+	DPRINTK("port EH scheduled\n");
+}
+
+/**
+ *	ata_port_abort - abort all qc's on the port
+ *	@ap: ATA port to abort qc's for
+ *
+ *	Abort all active qc's of @ap and schedule EH.
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(host_set lock)
+ *
+ *	RETURNS:
+ *	Number of aborted qc's.
+ */
+int ata_port_abort(struct ata_port *ap)
+{
+	int tag, nr_aborted = 0;
+
+	WARN_ON(!ap->ops->error_handler);
+
+	for (tag = 0; tag < ATA_MAX_QUEUE; tag++) {
+		struct ata_queued_cmd *qc = ata_qc_from_tag(ap, tag);
+
+		if (qc) {
+			qc->flags |= ATA_QCFLAG_FAILED;
+			ata_qc_complete(qc);
+			nr_aborted++;
+		}
+	}
+
+	if (!nr_aborted)
+		ata_port_schedule_eh(ap);
+
+	return nr_aborted;
+}
+
 static void ata_eh_scsidone(struct scsi_cmnd *scmd)
 {
 	/* nada */
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index a9b4083..fd7064b 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -2596,3 +2596,26 @@ void ata_scsi_scan_host(struct ata_port 
 	}
 }
 
+/**
+ *	ata_schedule_scsi_eh - schedule EH for SCSI host
+ *	@shost:	SCSI host to invoke error handling on.
+ *
+ *	Schedule SCSI EH without scmd.  This is a hack.
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(host_set lock)
+ **/
+void ata_schedule_scsi_eh(struct Scsi_Host *shost)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(shost->host_lock, flags);
+
+	if (scsi_host_set_state(shost, SHOST_RECOVERY) == 0 ||
+	    scsi_host_set_state(shost, SHOST_CANCEL_RECOVERY) == 0) {
+		shost->host_eh_scheduled++;
+		scsi_eh_wakeup(shost);
+	}
+
+	spin_unlock_irqrestore(shost->host_lock, flags);
+}
diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h
index 1e81c37..e2bbddf 100644
--- a/drivers/scsi/libata.h
+++ b/drivers/scsi/libata.h
@@ -98,6 +98,7 @@ extern void ata_scsi_set_sense(struct sc
 extern void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
                         unsigned int (*actor) (struct ata_scsi_args *args,
                                            u8 *rbuf, unsigned int buflen));
+extern void ata_schedule_scsi_eh(struct Scsi_Host *shost);
 
 /* libata-eh.c */
 extern enum scsi_eh_timer_return ata_scsi_timed_out(struct scsi_cmnd *cmd);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index fbabfb6..4221d7a 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -662,6 +662,10 @@ extern unsigned long ata_pci_default_fil
  * EH
  */
 extern void ata_eng_timeout(struct ata_port *ap);
+
+extern void ata_port_schedule_eh(struct ata_port *ap);
+extern int ata_port_abort(struct ata_port *ap);
+
 extern void ata_eh_qc_complete(struct ata_queued_cmd *qc);
 extern void ata_eh_qc_retry(struct ata_queued_cmd *qc);
 
-- 
1.2.4

[PATCH 05/11] libata-eh-fw: implement new EH scheduling via error completion

[Tejun Heo]

There are several ways a qc can get schedule for EH in new EH.  This
patch implements one of them - completing a qc with ATA_QCFLAG_FAILED
set or with non-zero qc->err_mask.  ALL such qc's are examined by EH.

New EH schedules a qc for EH from completion iff ->error_handler is
implemented, qc is marked as failed or qc->err_mask is non-zero and
the command is not an internal command (internal cmd is handled via
->post_internal_cmd).  The EH scheduling itself is performed by asking
SCSI midlayer to schedule EH for the specified scmd.

For drivers implementing old-EH, nothing changes.  As this change
makes ata_qc_complete() rather large, it's not inlined anymore and
__ata_qc_complete() is exported to other parts of libata for later
use.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   62 +++++++++++++++++++++++++++++++++++++++++++-
 drivers/scsi/libata-eh.c   |   27 +++++++++++++++++++
 drivers/scsi/libata.h      |    2 +
 include/linux/libata.h     |   27 +------------------
 4 files changed, 91 insertions(+), 27 deletions(-)

8291f2fe1c4d682e630a4902424a6195b797ceca
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 76df36d..2b18033 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -4122,6 +4122,66 @@ void __ata_qc_complete(struct ata_queued
 	qc->complete_fn(qc);
 }
 
+/**
+ *	ata_qc_complete - Complete an active ATA command
+ *	@qc: Command to complete
+ *	@err_mask: ATA Status register contents
+ *
+ *	Indicate to the mid and upper layers that an ATA
+ *	command has completed, with either an ok or not-ok status.
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(host_set lock)
+ */
+void ata_qc_complete(struct ata_queued_cmd *qc)
+{
+	struct ata_port *ap = qc->ap;
+
+	/* XXX: New EH and old EH use different mechanisms to
+	 * synchronize EH with regular execution path.
+	 *
+	 * In new EH, a failed qc is marked with ATA_QCFLAG_FAILED.
+	 * Normal execution path is responsible for not accessing a
+	 * failed qc.  libata core enforces the rule by returning NULL
+	 * from ata_qc_from_tag() for failed qcs.
+	 *
+	 * Old EH depends on ata_qc_complete() nullifying completion
+	 * requests if ATA_QCFLAG_EH_SCHEDULED is set.  Old EH does
+	 * not synchronize with interrupt handler.  Only PIO task is
+	 * taken care of.
+	 */
+	if (ap->ops->error_handler) {
+		WARN_ON(ap->flags & ATA_FLAG_FROZEN);
+
+		if (unlikely(qc->err_mask))
+			qc->flags |= ATA_QCFLAG_FAILED;
+
+		if (unlikely(qc->flags & ATA_QCFLAG_FAILED)) {
+			if (!ata_tag_internal(qc->tag)) {
+				/* always fill result TF for failed qc */
+				ap->ops->tf_read(ap, &qc->result_tf);
+				ata_qc_schedule_eh(qc);
+				return;
+			}
+		}
+
+		/* read result TF if requested */
+		if (qc->flags & ATA_QCFLAG_RESULT_TF)
+			ap->ops->tf_read(ap, &qc->result_tf);
+
+		__ata_qc_complete(qc);
+	} else {
+		if (qc->flags & ATA_QCFLAG_EH_SCHEDULED)
+			return;
+
+		/* read result TF if failed or requested */
+		if (qc->err_mask || qc->flags & ATA_QCFLAG_RESULT_TF)
+			ap->ops->tf_read(ap, &qc->result_tf);
+
+		__ata_qc_complete(qc);
+	}
+}
+
 static inline int ata_should_dma_map(struct ata_queued_cmd *qc)
 {
 	struct ata_port *ap = qc->ap;
@@ -5244,7 +5304,7 @@ EXPORT_SYMBOL_GPL(ata_device_add);
 EXPORT_SYMBOL_GPL(ata_host_set_remove);
 EXPORT_SYMBOL_GPL(ata_sg_init);
 EXPORT_SYMBOL_GPL(ata_sg_init_one);
-EXPORT_SYMBOL_GPL(__ata_qc_complete);
+EXPORT_SYMBOL_GPL(ata_qc_complete);
 EXPORT_SYMBOL_GPL(ata_qc_issue_prot);
 EXPORT_SYMBOL_GPL(ata_tf_load);
 EXPORT_SYMBOL_GPL(ata_tf_read);
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index 959a1cd..471846f 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -210,6 +210,33 @@ void ata_eng_timeout(struct ata_port *ap
 	DPRINTK("EXIT\n");
 }
 
+/**
+ *	ata_qc_schedule_eh - schedule qc for error handling
+ *	@qc: command to schedule error handling for
+ *
+ *	Schedule error handling for @qc.  EH will kick in as soon as
+ *	other commands are drained.
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(host_set lock)
+ */
+void ata_qc_schedule_eh(struct ata_queued_cmd *qc)
+{
+	struct ata_port *ap = qc->ap;
+
+	WARN_ON(!ap->ops->error_handler);
+
+	qc->flags |= ATA_QCFLAG_FAILED;
+	qc->ap->flags |= ATA_FLAG_EH_PENDING;
+
+	/* The following will fail if timeout has already expired.
+	 * ata_scsi_error() takes care of such scmds on EH entry.
+	 * Note that ATA_QCFLAG_FAILED is unconditionally set after
+	 * this function completes.
+	 */
+	scsi_req_abort_cmd(qc->scsicmd);
+}
+
 static void ata_eh_scsidone(struct scsi_cmnd *scmd)
 {
 	/* nada */
diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h
index a810c91..1e81c37 100644
--- a/drivers/scsi/libata.h
+++ b/drivers/scsi/libata.h
@@ -57,6 +57,7 @@ extern int ata_do_reset(struct ata_port 
 			unsigned int *classes);
 extern void ata_qc_free(struct ata_queued_cmd *qc);
 extern void ata_qc_issue(struct ata_queued_cmd *qc);
+extern void __ata_qc_complete(struct ata_queued_cmd *qc);
 extern int ata_check_atapi_dma(struct ata_queued_cmd *qc);
 extern void ata_dev_select(struct ata_port *ap, unsigned int device,
                            unsigned int wait, unsigned int can_sleep);
@@ -101,5 +102,6 @@ extern void ata_scsi_rbuf_fill(struct at
 /* libata-eh.c */
 extern enum scsi_eh_timer_return ata_scsi_timed_out(struct scsi_cmnd *cmd);
 extern void ata_scsi_error(struct Scsi_Host *host);
+extern void ata_qc_schedule_eh(struct ata_queued_cmd *qc);
 
 #endif /* __LIBATA_H__ */
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 3d6942a..fbabfb6 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -605,7 +605,7 @@ extern void ata_bmdma_start (struct ata_
 extern void ata_bmdma_stop(struct ata_queued_cmd *qc);
 extern u8   ata_bmdma_status(struct ata_port *ap);
 extern void ata_bmdma_irq_clear(struct ata_port *ap);
-extern void __ata_qc_complete(struct ata_queued_cmd *qc);
+extern void ata_qc_complete(struct ata_queued_cmd *qc);
 extern void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd,
 			      void (*done)(struct scsi_cmnd *));
 extern int ata_std_bios_param(struct scsi_device *sdev,
@@ -883,31 +883,6 @@ static inline void ata_qc_reinit(struct 
 }
 
 /**
- *	ata_qc_complete - Complete an active ATA command
- *	@qc: Command to complete
- *	@err_mask: ATA Status register contents
- *
- *	Indicate to the mid and upper layers that an ATA
- *	command has completed, with either an ok or not-ok status.
- *
- *	LOCKING:
- *	spin_lock_irqsave(host_set lock)
- */
-static inline void ata_qc_complete(struct ata_queued_cmd *qc)
-{
-	struct ata_port *ap = qc->ap;
-
-	if (unlikely(qc->flags & ATA_QCFLAG_EH_SCHEDULED))
-		return;
-
-	/* read result TF if failed or requested */
-	if (qc->err_mask || qc->flags & ATA_QCFLAG_RESULT_TF)
-		ap->ops->tf_read(ap, &qc->result_tf);
-
-	__ata_qc_complete(qc);
-}
-
-/**
  *	ata_irq_on - Enable interrupts on a port.
  *	@ap: Port on which interrupts are enabled.
  *
-- 
1.2.4

[PATCH 07/11] libata-eh-fw: implement freeze/thaw

[Tejun Heo]

Freezing is performed atomic w.r.t. host_set->lock and once frozen
LLDD is not allowed to access the port or any qc on it.  Also, libata
makes sure that no new qc gets issued to a frozen port.

A frozen port is thawed after a reset operation completes
successfully, so reset methods must do its job while the port is
frozen.  During initialization all ports get frozen before requesting
IRQ, so reset methods are always invoked on a frozen port.

Optional ->freeze and ->thaw operations notify LLDD that the port is
being frozen and thawed, respectively.  LLDD can disable/enable
hardware interrupt in these callbacks if the controller's IRQ mask can
be changed dynamically.  If the controller doesn't allow such
operation, LLDD can check for frozen state in the interrupt handler
and ack/clear interrupts unconditionally while frozen.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-core.c |   26 ++++++++++-
 drivers/scsi/libata-eh.c   |  103 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/libata.h     |    4 ++
 3 files changed, 131 insertions(+), 2 deletions(-)

ce9d6dcb786c6bc5e65dc4cd20be3af0bc5e1d69
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 7483518..436ff5b 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -987,6 +987,12 @@ unsigned ata_exec_internal(struct ata_de
 
 	spin_lock_irqsave(&ap->host_set->lock, flags);
 
+	/* no internal command while frozen */
+	if (ap->flags & ATA_FLAG_FROZEN) {
+		spin_unlock_irqrestore(&ap->host_set->lock, flags);
+		return AC_ERR_SYSTEM;
+	}
+
 	/* initialize internal qc */
 
 	/* XXX: Tag 0 is used for drivers with legacy EH as some
@@ -2564,8 +2570,11 @@ void ata_std_postreset(struct ata_port *
 		ata_scr_write(ap, SCR_ERROR, serror);
 
 	/* re-enable interrupts */
-	if (ap->ioaddr.ctl_addr)	/* FIXME: hack. create a hook instead */
-		ata_irq_on(ap);
+	if (!ap->ops->error_handler) {
+		/* FIXME: hack. create a hook instead */
+		if (ap->ioaddr.ctl_addr)
+			ata_irq_on(ap);
+	}
 
 	/* is double-select really necessary? */
 	if (classes[0] != ATA_DEV_NONE)
@@ -2680,6 +2689,8 @@ int ata_drive_probe_reset(struct ata_por
 {
 	int rc = -EINVAL;
 
+	ata_eh_freeze_port(ap);
+
 	if (probeinit)
 		probeinit(ap);
 
@@ -2724,6 +2735,9 @@ int ata_drive_probe_reset(struct ata_por
 	if (rc == 0) {
 		if (postreset)
 			postreset(ap, classes);
+
+		ata_eh_thaw_port(ap);
+
 		if (classes[0] == ATA_DEV_UNKNOWN)
 			rc = -ENODEV;
 	}
@@ -4038,6 +4052,10 @@ static struct ata_queued_cmd *ata_qc_new
 	struct ata_queued_cmd *qc = NULL;
 	unsigned int i;
 
+	/* no command while frozen */
+	if (unlikely(ap->flags & ATA_FLAG_FROZEN))
+		return NULL;
+
 	/* the last tag is reserved for internal command. */
 	for (i = 0; i < ATA_MAX_QUEUE - 1; i++)
 		if (!test_and_set_bit(i, &ap->qactive)) {
@@ -4952,6 +4970,7 @@ int ata_device_add(const struct ata_prob
 
 		ata_chk_status(ap);
 		host_set->ops->irq_clear(ap);
+		ata_eh_freeze_port(ap);	/* freeze port before requesting IRQ */
 		count++;
 	}
 
@@ -5384,5 +5403,8 @@ EXPORT_SYMBOL_GPL(ata_scsi_device_resume
 EXPORT_SYMBOL_GPL(ata_eng_timeout);
 EXPORT_SYMBOL_GPL(ata_port_schedule_eh);
 EXPORT_SYMBOL_GPL(ata_port_abort);
+EXPORT_SYMBOL_GPL(ata_port_freeze);
+EXPORT_SYMBOL_GPL(ata_eh_freeze_port);
+EXPORT_SYMBOL_GPL(ata_eh_thaw_port);
 EXPORT_SYMBOL_GPL(ata_eh_qc_complete);
 EXPORT_SYMBOL_GPL(ata_eh_qc_retry);
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index 037a561..cb4e2b8 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -291,6 +291,109 @@ int ata_port_abort(struct ata_port *ap)
 	return nr_aborted;
 }
 
+/**
+ *	__ata_port_freeze - freeze port
+ *	@ap: ATA port to freeze
+ *
+ *	This function is called when HSM violation or some other
+ *	condition disrupts normal operation of the port.  Frozen port
+ *	is not allowed to perform any operation until the port is
+ *	thawed, which usually follows a successful reset.
+ *
+ *	ap->ops->freeze() callback can be used for freezing the port
+ *	hardware-wise (e.g. mask interrupt and stop DMA engine).  If a
+ *	port cannot be frozen hardware-wise, the interrupt handler
+ *	must ack and clear interrupts unconditionally while the port
+ *	is frozen.
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(host_set lock)
+ */
+static void __ata_port_freeze(struct ata_port *ap)
+{
+	WARN_ON(!ap->ops->error_handler);
+
+	if (ap->ops->freeze)
+		ap->ops->freeze(ap);
+
+	ap->flags |= ATA_FLAG_FROZEN;
+
+	DPRINTK("ata%u port frozen\n", ap->id);
+}
+
+/**
+ *	ata_port_freeze - abort & freeze port
+ *	@ap: ATA port to freeze
+ *
+ *	Abort and freeze @ap.
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(host_set lock)
+ *
+ *	RETURNS:
+ *	Number of aborted commands.
+ */
+int ata_port_freeze(struct ata_port *ap)
+{
+	int nr_aborted;
+
+	WARN_ON(!ap->ops->error_handler);
+
+	nr_aborted = ata_port_abort(ap);
+	__ata_port_freeze(ap);
+
+	return nr_aborted;
+}
+
+/**
+ *	ata_eh_freeze_port - EH helper to freeze port
+ *	@ap: ATA port to freeze
+ *
+ *	Freeze @ap.
+ *
+ *	LOCKING:
+ *	None.
+ */
+void ata_eh_freeze_port(struct ata_port *ap)
+{
+	unsigned long flags;
+
+	if (!ap->ops->error_handler)
+		return;
+
+	spin_lock_irqsave(&ap->host_set->lock, flags);
+	__ata_port_freeze(ap);
+	spin_unlock_irqrestore(&ap->host_set->lock, flags);
+}
+
+/**
+ *	ata_port_thaw_port - EH helper to thaw port
+ *	@ap: ATA port to thaw
+ *
+ *	Thaw frozen port @ap.
+ *
+ *	LOCKING:
+ *	None.
+ */
+void ata_eh_thaw_port(struct ata_port *ap)
+{
+	unsigned long flags;
+
+	if (!ap->ops->error_handler)
+		return;
+
+	spin_lock_irqsave(&ap->host_set->lock, flags);
+
+	ap->flags &= ~ATA_FLAG_FROZEN;
+
+	if (ap->ops->thaw)
+		ap->ops->thaw(ap);
+
+	spin_unlock_irqrestore(&ap->host_set->lock, flags);
+
+	DPRINTK("ata%u port thawed\n", ap->id);
+}
+
 static void ata_eh_scsidone(struct scsi_cmnd *scmd)
 {
 	/* nada */
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 4221d7a..3f22eff 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -665,6 +665,10 @@ extern void ata_eng_timeout(struct ata_p
 
 extern void ata_port_schedule_eh(struct ata_port *ap);
 extern int ata_port_abort(struct ata_port *ap);
+extern int ata_port_freeze(struct ata_port *ap);
+
+extern void ata_eh_freeze_port(struct ata_port *ap);
+extern void ata_eh_thaw_port(struct ata_port *ap);
 
 extern void ata_eh_qc_complete(struct ata_queued_cmd *qc);
 extern void ata_eh_qc_retry(struct ata_queued_cmd *qc);
-- 
1.2.4

[PATCH 09/11] libata-eh-fw: update ata_scsi_error() for new EH

[Tejun Heo]

Update ata_scsi_error() for new EH.  ata_scsi_error() is responsible
for claiming timed out qcs and invoking ->error_handler in safe and
synchronized manner.  As the state of the controller is unknown if a
qc has timed out, the port is frozen in such cases.

Note that ata_scsi_timed_out() isn't used for new EH.  This is because
a timed out qc cannot be claimed by EH without freezing the port and
freezing the port in ata_scsi_timed_out() results in unnecessary
abortion of other active qcs.  ata_scsi_timed_out() can be removed
once all drivers are converted to new EH.

While at it, add 'TODO: kill' comments to old EH functions.

Signed-off-by: Tejun Heo <htejun@gmail.com>

---

 drivers/scsi/libata-eh.c |  136 ++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/libata.h   |    3 +
 2 files changed, 134 insertions(+), 5 deletions(-)

610d1ef7cf0e6a147bc190ebb2f55afabf764df4
diff --git a/drivers/scsi/libata-eh.c b/drivers/scsi/libata-eh.c
index cb4e2b8..0803231 100644
--- a/drivers/scsi/libata-eh.c
+++ b/drivers/scsi/libata-eh.c
@@ -44,6 +44,8 @@
 
 #include "libata.h"
 
+static void __ata_port_freeze(struct ata_port *ap);
+
 /**
  *	ata_scsi_timed_out - SCSI layer time out callback
  *	@cmd: timed out SCSI command
@@ -55,6 +57,8 @@
  *	from finishing it by setting EH_SCHEDULED and return
  *	EH_NOT_HANDLED.
  *
+ *	TODO: kill this function once old EH is gone.
+ *
  *	LOCKING:
  *	Called from timer context
  *
@@ -67,10 +71,16 @@ enum scsi_eh_timer_return ata_scsi_timed
 	struct ata_port *ap = ata_shost_to_port(host);
 	unsigned long flags;
 	struct ata_queued_cmd *qc;
-	enum scsi_eh_timer_return ret = EH_HANDLED;
+	enum scsi_eh_timer_return ret;
 
 	DPRINTK("ENTER\n");
 
+	if (ap->ops->error_handler) {
+		ret = EH_NOT_HANDLED;
+		goto out;
+	}
+
+	ret = EH_HANDLED;
 	spin_lock_irqsave(&ap->host_set->lock, flags);
 	qc = ata_qc_from_tag(ap, ap->active_tag);
 	if (qc) {
@@ -81,6 +91,7 @@ enum scsi_eh_timer_return ata_scsi_timed
 	}
 	spin_unlock_irqrestore(&ap->host_set->lock, flags);
 
+ out:
 	DPRINTK("EXIT, ret=%d\n", ret);
 	return ret;
 }
@@ -100,21 +111,132 @@ enum scsi_eh_timer_return ata_scsi_timed
 void ata_scsi_error(struct Scsi_Host *host)
 {
 	struct ata_port *ap = ata_shost_to_port(host);
+	spinlock_t *hs_lock = &ap->host_set->lock;
+	int i, repeat_cnt = ATA_EH_MAX_REPEAT;
+	unsigned long flags;
 
 	DPRINTK("ENTER\n");
 
-	/* synchronize with IRQ handler and port task */
-	spin_unlock_wait(&ap->host_set->lock);
+	/* synchronize with port task */
 	ata_port_flush_task(ap);
 
-	WARN_ON(ata_qc_from_tag(ap, ap->active_tag) == NULL);
+	/* synchronize with host_set lock and sort out timeouts */
 
-	ap->ops->eng_timeout(ap);
+	/* For new EH, all qcs are finished in one of three ways -
+	 * normal completion, error completion, and SCSI timeout.
+	 * Both cmpletions can race against SCSI timeout.  When normal
+	 * completion wins, the qc never reaches EH.  When error
+	 * completion wins, the qc has ATA_QCFLAG_FAILED set.
+	 *
+	 * When SCSI timeout wins, things are a bit more complex.
+	 * Normal or error completion can occur after the timeout but
+	 * before this point.  In such cases, both types of
+	 * completions are honored.  A scmd is determined to have
+	 * timed out iff its associated qc is active and not failed.
+	 */
+	if (ap->ops->error_handler) {
+		struct scsi_cmnd *scmd, *tmp;
+		int nr_timedout = 0;
+
+		spin_lock_irqsave(hs_lock, flags);
+
+		list_for_each_entry_safe(scmd, tmp, &host->eh_cmd_q, eh_entry) {
+			struct ata_queued_cmd *qc;
+
+			for (i = 0; i < ATA_MAX_QUEUE; i++) {
+				qc = __ata_qc_from_tag(ap, i);
+				if (qc->flags & ATA_QCFLAG_ACTIVE &&
+				    qc->scsicmd == scmd)
+					break;
+			}
+
+			if (i < ATA_MAX_QUEUE) {
+				/* the scmd has an associated qc */
+				if (!(qc->flags & ATA_QCFLAG_FAILED)) {
+					/* which hasn't failed yet, timeout */
+					qc->err_mask |= AC_ERR_TIMEOUT;
+					qc->flags |= ATA_QCFLAG_FAILED;
+					nr_timedout++;
+				}
+			} else {
+				/* Normal completion occurred after
+				 * SCSI timeout but before this point.
+				 * Successfully complete it.
+				 */
+				scmd->retries = scmd->allowed;
+				scsi_eh_finish_cmd(scmd, &ap->eh_done_q);
+			}
+		}
 
+		/* If we have timed out qcs.  They belong to EH from
+		 * this point but the state of the controller is
+		 * unknown.  Freeze the port to make sure the IRQ
+		 * handler doesn't diddle with those qcs.  This must
+		 * be done atomically w.r.t. setting QCFLAG_FAILED.
+		 */
+		if (nr_timedout)
+			__ata_port_freeze(ap);
+
+		spin_unlock_irqrestore(hs_lock, flags);
+	} else
+		spin_unlock_wait(hs_lock);
+
+ repeat:
+	/* invoke error handler */
+	if (ap->ops->error_handler) {
+		/* clear EH pending */
+		spin_lock_irqsave(hs_lock, flags);
+		ap->flags &= ~ATA_FLAG_EH_PENDING;
+		spin_unlock_irqrestore(hs_lock, flags);
+
+		/* invoke EH */
+		ap->ops->error_handler(ap);
+
+		/* Exception might have happend after ->error_handler
+		 * recovered the port but before this point.  Repeat
+		 * EH in such case.
+		 */
+		spin_lock_irqsave(hs_lock, flags);
+
+		if (ap->flags & ATA_FLAG_EH_PENDING) {
+			if (--repeat_cnt) {
+				ata_port_printk(ap, KERN_INFO,
+					"EH pending after completion, "
+					"repeating EH (cnt=%d)\n", repeat_cnt);
+				spin_unlock_irqrestore(hs_lock, flags);
+				goto repeat;
+			}
+			ata_port_printk(ap, KERN_ERR, "EH pending after %d "
+					"tries, giving up\n", ATA_EH_MAX_REPEAT);
+		}
+
+		/* Clear host_eh_scheduled while holding hs_lock such
+		 * that if exception occurs after this point but
+		 * before EH completion, SCSI midlayer will
+		 * re-initiate EH.
+		 */
+		host->host_eh_scheduled = 0;
+
+		spin_unlock_irqrestore(hs_lock, flags);
+	} else {
+		WARN_ON(ata_qc_from_tag(ap, ap->active_tag) == NULL);
+		ap->ops->eng_timeout(ap);
+	}
+
+	/* finish or retry handled scmd's and clean up */
 	WARN_ON(host->host_failed || !list_empty(&host->eh_cmd_q));
 
 	scsi_eh_flush_done_q(&ap->eh_done_q);
 
+	/* clean up */
+	spin_lock_irqsave(hs_lock, flags);
+
+	if (ap->flags & ATA_FLAG_RECOVERED)
+		ata_port_printk(ap, KERN_INFO, "EH complete\n");
+	ap->flags &= ~ATA_FLAG_RECOVERED;
+
+	spin_unlock_irqrestore(hs_lock, flags);
+
 	DPRINTK("EXIT\n");
 }
 
@@ -133,6 +255,8 @@ void ata_scsi_error(struct Scsi_Host *ho
  *	an interrupt was not delivered to the driver, even though the
  *	transaction completed successfully.
  *
+ *	TODO: kill this function once old EH is gone.
+ *
  *	LOCKING:
  *	Inherited from SCSI layer (none, can sleep)
  */
@@ -198,6 +322,8 @@ static void ata_qc_timeout(struct ata_qu
  *	an interrupt was not delivered to the driver, even though the
  *	transaction completed successfully.
  *
+ *	TODO: kill this function once old EH is gone.
+ *
  *	LOCKING:
  *	Inherited from SCSI layer (none, can sleep)
  */
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 3f22eff..e4353cf 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -225,6 +225,9 @@ enum {
 	ATA_PORT_PRIMARY	= (1 << 0),
 	ATA_PORT_SECONDARY	= (1 << 1),
 
+	/* max repeat if error condition is still set after ->error_handler */
+	ATA_EH_MAX_REPEAT	= 5,
+
 	/* how hard are we gonna try to probe/recover devices */
 	ATA_PROBE_MAX_TRIES	= 3,
 };
-- 
1.2.4

Re: [PATCHSET 02/11] new EH framework, take 3

[Jeff Garzik]

Tejun Heo wrote:
> This is part of patchset series described in [T].
> 
> This is the third take of new-EH-framework patchset.  This patchset
> contains 11 patches.  Changes from the last take[L] are
[...]
> This patchset is against
> 
>   upstream (acc696d93dcf993dec123d69d599979e1456ffec)
>   + [1] prep-for-new-EH patchset
> 
> --
> tejun
> 
> [T] http://article.gmane.org/gmane.linux.ide/9957
> [L] http://article.gmane.org/gmane.linux.ide/9524
> [1] http://article.gmane.org/gmane.linux.ide/9959


ACK entire patchset

Re: [PATCH 07/11] libata-eh-fw: implement freeze/thaw

[Albert Lee]

Tejun Heo wrote:
> Freezing is performed atomic w.r.t. host_set->lock and once frozen
> LLDD is not allowed to access the port or any qc on it.  Also, libata
> makes sure that no new qc gets issued to a frozen port.
> 
> A frozen port is thawed after a reset operation completes
> successfully, so reset methods must do its job while the port is
> frozen.  During initialization all ports get frozen before requesting
> IRQ, so reset methods are always invoked on a frozen port.
> 
> Optional ->freeze and ->thaw operations notify LLDD that the port is
> being frozen and thawed, respectively.  LLDD can disable/enable
> hardware interrupt in these callbacks if the controller's IRQ mask can
> be changed dynamically.  If the controller doesn't allow such
> operation, LLDD can check for frozen state in the interrupt handler
> and ack/clear interrupts unconditionally while frozen.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> ---
> 
>  drivers/scsi/libata-core.c |   26 ++++++++++-
>  drivers/scsi/libata-eh.c   |  103 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/libata.h     |    4 ++
>  3 files changed, 131 insertions(+), 2 deletions(-)
> 
> ce9d6dcb786c6bc5e65dc4cd20be3af0bc5e1d69
> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
> index 7483518..436ff5b 100644
> --- a/drivers/scsi/libata-core.c
> +++ b/drivers/scsi/libata-core.c
> @@ -987,6 +987,12 @@ unsigned ata_exec_internal(struct ata_de
>  
>  	spin_lock_irqsave(&ap->host_set->lock, flags);
>  
> +	/* no internal command while frozen */
> +	if (ap->flags & ATA_FLAG_FROZEN) {
> +		spin_unlock_irqrestore(&ap->host_set->lock, flags);
> +		return AC_ERR_SYSTEM;
> +	}
> +
>  	/* initialize internal qc */
>  
>  	/* XXX: Tag 0 is used for drivers with legacy EH as some
> @@ -2564,8 +2570,11 @@ void ata_std_postreset(struct ata_port *
>  		ata_scr_write(ap, SCR_ERROR, serror);
>  
>  	/* re-enable interrupts */
> -	if (ap->ioaddr.ctl_addr)	/* FIXME: hack. create a hook instead */
> -		ata_irq_on(ap);
> +	if (!ap->ops->error_handler) {
> +		/* FIXME: hack. create a hook instead */
> +		if (ap->ioaddr.ctl_addr)
> +			ata_irq_on(ap);
> +	}
>  
>  	/* is double-select really necessary? */
>  	if (classes[0] != ATA_DEV_NONE)
> @@ -2680,6 +2689,8 @@ int ata_drive_probe_reset(struct ata_por
>  {
>  	int rc = -EINVAL;
>  
> +	ata_eh_freeze_port(ap);
> +
>  	if (probeinit)
>  		probeinit(ap);
>  
> @@ -2724,6 +2735,9 @@ int ata_drive_probe_reset(struct ata_por
>  	if (rc == 0) {
>  		if (postreset)
>  			postreset(ap, classes);
> +
> +		ata_eh_thaw_port(ap);
> +
>  		if (classes[0] == ATA_DEV_UNKNOWN)
>  			rc = -ENODEV;
>  	}


With the new ata_drive_probe_reset(), my old Acer 787E drive still got "irq nobody cared"
when probed. (dmesg attached. The problem only seen on this drive; other CD-ROM drives works ok.)
The pata_pdc2027x driver has been converted to the ->probe_reset() interface.
Is there anything else I should do for pata_pdc2027x driver to avoid the "irq nobody cared"?

--
albert


May 16 17:26:45 p4ht-s kernel: ACPI: PCI Interrupt 0000:02:05.0[A] -> Link [LNK1] -> GSI 5 (level, low) -> IRQ 5
May 16 17:26:45 p4ht-s kernel: pata_pdc2027x 0000:02:05.0: PLL input clock 16731 kHz
May 16 17:26:45 p4ht-s kernel: ata5: PATA max UDMA/133 cmd 0xE0A917C0 ctl 0xE0A91FDA bmdma 0xE0A91000 irq 5
May 16 17:26:45 p4ht-s kernel: ata6: PATA max UDMA/133 cmd 0xE0A915C0 ctl 0xE0A91DDA bmdma 0xE0A91008 irq 5
May 16 17:26:46 p4ht-s kernel: ata5.00: ATAPI, max UDMA/33
May 16 17:26:46 p4ht-s kernel: ata5.01: ATAPI, max UDMA/33
May 16 17:26:46 p4ht-s kernel: ata5.00: configured for UDMA/33
May 16 17:26:46 p4ht-s kernel: ata5.01: configured for UDMA/33
May 16 17:26:46 p4ht-s kernel: scsi4 : pata_pdc2027x
May 16 17:26:47 p4ht-s kernel: irq 5: nobody cared (try booting with the "irqpoll" option)
May 16 17:26:47 p4ht-s kernel:  <c013b1c4> __report_bad_irq+0x24/0x7f   <c013b296> note_interrupt+0x77/0x21d

Message from syslogd@p4ht-s at Tue May 16 17:26:47 2006 ...
p4ht-s kernel: Disabling IRQ #5
May 16 17:26:47 p4ht-s kernel:  <c013ac7d> handle_IRQ_event+0x2e/0x5a   <c013ad73> __do_IRQ+0xca/0xd7
May 16 17:26:47 p4ht-s kernel:  <c01047b8> do_IRQ+0x53/0x8b  
May 16 17:26:47 p4ht-s kernel:  =======================
May 16 17:26:47 p4ht-s kernel:  <c0102f7e> common_interrupt+0x1a/0x20   <c011f599> __do_softirq+0x37/0x95
May 16 17:26:47 p4ht-s kernel:  <c010483f> do_softirq+0x4f/0x60  
May 16 17:26:47 p4ht-s kernel:  =======================
May 16 17:26:47 p4ht-s kernel:  <c011f485> irq_exit+0x37/0x39   <c01047bf> do_IRQ+0x5a/0x8b
May 16 17:26:47 p4ht-s kernel:  <e087ff2a> ata_std_softreset+0x0/0xd0 [libata]   <c0102f7e> common_interrupt+0x1a/0x20
May 16 17:26:47 p4ht-s kernel:  <e087ff2a> ata_std_softreset+0x0/0xd0 [libata]   <c019007b> proc_readfd+0x214/0x254
May 16 17:26:47 p4ht-s kernel:  <e0885737> ata_altstatus+0x2a/0x2e [libata]   <e087eaef> ata_std_dev_select+0x23/0x47 [libata]
May 16 17:26:47 p4ht-s kernel:  <e087eb25> ata_devchk+0x12/0xaa [libata]   <e087ff2a> ata_std_softreset+0x0/0xd0 [libata]
May 16 17:26:47 p4ht-s kernel:  <e087ff5d> ata_std_softreset+0x33/0xd0 [libata]   <e087d504> sata_scr_read+0xe/0x2b [libata]
May 16 17:26:47 p4ht-s kernel:  <e087d504> sata_scr_read+0xe/0x2b [libata]   <e087ff2a> ata_std_softreset+0x0/0xd0 [libata]
May 16 17:26:47 p4ht-s kernel:  <e087d6b9> ata_do_reset+0x1f/0x5f [libata]   <e0a8a4f2> pdc2027x_probeinit+0x0/0x5c [pata_pdc2027x]
May 16 17:26:47 p4ht-s kernel:  <e087ee80> ata_drive_probe_reset+0x107/0x139 [libata]   <e087ff2a> ata_std_softreset+0x0/0xd0 [libata]
May 16 17:26:47 p4ht-s kernel:  <e0a8a5ae> pdc2027x_probe_reset+0x60/0x65 [pata_pdc2027x]   <e0881575> ata_std_postreset+0x0/0x136 [libata]
May 16 17:26:47 p4ht-s kernel:  <e0882555> ata_device_add+0x421/0x721 [libata]   <e0a8a449> pdc2027x_init_one+0x29f/0x348 [pata_pdc2027x]
May 16 17:26:47 p4ht-s kernel:  <c01e2ce6> pci_device_probe+0x40/0x5b   <c0241135> driver_probe_device+0x3b/0xb0
May 16 17:26:47 p4ht-s kernel:  <c0241299> __driver_attach+0x82/0x84   <c0240727> bus_for_each_dev+0x39/0x57
May 16 17:26:47 p4ht-s kernel:  <c0241012> driver_attach+0x16/0x1a   <c0241217> __driver_attach+0x0/0x84
May 16 17:26:47 p4ht-s kernel:  <c02409f3> bus_add_driver+0x66/0x116   <c02415fa> driver_register+0x41/0xb4
May 16 17:26:47 p4ht-s kernel:  <c01e2937> __pci_register_driver+0x65/0x86   <c013296f> sys_init_module+0x114/0x175b
May 16 17:26:47 p4ht-s kernel:  <e087d915> ata_port_start+0x0/0x7e [libata]   <c0102db3> syscall_call+0x7/0xb
May 16 17:26:47 p4ht-s kernel: handlers:
May 16 17:26:47 p4ht-s kernel: [<c027279c>] (usb_hcd_irq+0x0/0x51)
May 16 17:26:47 p4ht-s kernel: [<c027279c>] (usb_hcd_irq+0x0/0x51)
May 16 17:26:47 p4ht-s kernel: [<e0882ad1>] (ata_interrupt+0x0/0x1af [libata])
May 16 17:26:47 p4ht-s kernel: Disabling IRQ #5
May 16 17:27:09 p4ht-s samba(pam_unix)[2647]: session opened for user albertcc by (uid=0)
May 16 17:27:17 p4ht-s kernel: ata6.00: qc timeout (cmd 0xa1)
May 16 17:27:17 p4ht-s kernel: ata6.00: failed to IDENTIFY (I/O error, err_mask=0x4)
May 16 17:27:47 p4ht-s kernel: ata6.00: qc timeout (cmd 0xa1)
May 16 17:27:47 p4ht-s kernel: ata6.00: failed to IDENTIFY (I/O error, err_mask=0x4)

May 16 17:28:18 p4ht-s kernel: ata6.00: qc timeout (cmd 0xa1)
May 16 17:28:18 p4ht-s kernel: ata6.00: failed to IDENTIFY (I/O error, err_mask=0x4)
May 16 17:28:18 p4ht-s kernel: ata6.00: limiting speed to PIO0
May 16 17:28:18 p4ht-s kernel: ata6.00: disabled
May 16 17:28:18 p4ht-s kernel: scsi5 : pata_pdc2027x
May 16 17:28:23 p4ht-s kernel: ata5.00: command 0xa0 timeout, stat 0x58 host_stat 0x0
May 16 17:28:23 p4ht-s kernel: ata5: no sense translation for status: 0x40
May 16 17:28:23 p4ht-s kernel: ata5: translated ATA stat/err 0x40/00 to SCSI SK/ASC/ASCQ 0xb/00/00
May 16 17:28:29 p4ht-s kernel: ata5.01: command 0xa0 timeout, stat 0x58 host_stat 0x0
May 16 17:28:29 p4ht-s kernel: ata5: no sense translation for status: 0x40
May 16 17:28:29 p4ht-s kernel: ata5: translated ATA stat/err 0x40/00 to SCSI SK/ASC/ASCQ 0xb/00/00

Re: [PATCH 07/11] libata-eh-fw: implement freeze/thaw

[Tejun Heo]

Albert Lee wrote:
> With the new ata_drive_probe_reset(), my old Acer 787E drive still got "irq nobody cared"
> when probed. (dmesg attached. The problem only seen on this drive; other CD-ROM drives works ok.)
> The pata_pdc2027x driver has been converted to the ->probe_reset() interface.
> Is there anything else I should do for pata_pdc2027x driver to avoid the "irq nobody cared"?

You need to implement both ->probe_reset() and ->error_handler() & 
friends to avoid "irq nobody cared".  Note that hotplug merges probing 
into ->error_handler() and removes ->probe_reset().

The actual part which helps avoiding "irq nobody cared" during 
initialization is proper implementation of ->freeze() and ->thaw().  If 
the controller can detect spurious interrupts, freezing on spurious 
interrupts can help avoiding screaming interrupts, too.

-- 
tejun

Re: [PATCH 07/11] libata-eh-fw: implement freeze/thaw

[Albert Lee]

Tejun Heo wrote:
> Albert Lee wrote:
> 
>> With the new ata_drive_probe_reset(), my old Acer 787E drive still got
>> "irq nobody cared"
>> when probed. (dmesg attached. The problem only seen on this drive;
>> other CD-ROM drives works ok.)
>> The pata_pdc2027x driver has been converted to the ->probe_reset()
>> interface.
>> Is there anything else I should do for pata_pdc2027x driver to avoid
>> the "irq nobody cared"?
> 
> 
> You need to implement both ->probe_reset() and ->error_handler() &
> friends to avoid "irq nobody cared".  Note that hotplug merges probing
> into ->error_handler() and removes ->probe_reset().
> 
> The actual part which helps avoiding "irq nobody cared" during
> initialization is proper implementation of ->freeze() and ->thaw().  If
> the controller can detect spurious interrupts, freezing on spurious
> interrupts can help avoiding screaming interrupts, too.
> 

I will try adding ->freeze() etc.
Thanks for the advice,

Albert

Re: [PATCH 07/11] libata-eh-fw: implement freeze/thaw

[Albert Lee]

Tejun Heo wrote:
> Albert Lee wrote:
> 
>> With the new ata_drive_probe_reset(), my old Acer 787E drive still got
>> "irq nobody cared"
>> when probed. (dmesg attached. The problem only seen on this drive;
>> other CD-ROM drives works ok.)
>> The pata_pdc2027x driver has been converted to the ->probe_reset()
>> interface.
>> Is there anything else I should do for pata_pdc2027x driver to avoid
>> the "irq nobody cared"?
> 
> 
> You need to implement both ->probe_reset() and ->error_handler() &
> friends to avoid "irq nobody cared".  Note that hotplug merges probing
> into ->error_handler() and removes ->probe_reset().
> 
> The actual part which helps avoiding "irq nobody cared" during
> initialization is proper implementation of ->freeze() and ->thaw().  If
> the controller can detect spurious interrupts, freezing on spurious
> interrupts can help avoiding screaming interrupts, too.
> 

Adding the required ->freeze() etc fixes the "irq nobody cares" problem.

Although the Acer 787E drive still failed to IDENTIFY and doesn't work,
the other two good drives are probed and work fine.
With the new EH, libata looks more robust with faulty hardware. :)

Thanks,

Albert


===

pata_pdc2027x 0000:02:05.0: version 0.74
ACPI: PCI Interrupt 0000:02:05.0[A] -> Link [LNK1] -> GSI 5 (level, low) -> IRQ 5
pata_pdc2027x 0000:02:05.0: PLL input clock 16728 kHz
ata5: PATA max UDMA/133 cmd 0xE0A917C0 ctl 0xE0A91FDA bmdma 0xE0A91000 irq 5
ata6: PATA max UDMA/133 cmd 0xE0A915C0 ctl 0xE0A91DDA bmdma 0xE0A91008 irq 5
ata5.00: cfg 49:0b00 82:0210 83:1000 84:0000 85:0000 86:0000 87:0000 88:0407
ata5.00: ATAPI, max UDMA/33
ata5.01: cfg 49:0f00 82:421c 83:0000 84:0000 85:0000 86:0000 87:0000 88:0407
ata5.01: ATAPI, max UDMA/33
ata5.00: configured for UDMA/33
ata5.01: configured for UDMA/33
scsi4 : pata_pdc2027x
ata6.00: cfg 49:0b00 82:0000 83:0000 84:0000 85:0000 86:0000 87:0000 88:0000
ata6.00: ATAPI, max MWDMA1
ata6.00: qc timeout (cmd 0xa1)
ata6.00: failed to IDENTIFY (I/O error, err_mask=0x4)
ata6.00: revalidation failed (errno=-5)
ata6.00: limiting speed to MWDMA0
ata6.00: cfg 49:2020 82:0000 83:0000 84:0000 85:0000 86:0000 87:0000 88:0000
ata6.00: ATAPI, max PIO0
ata6.00: failed to set xfermode (err_mask=0x1)
ata6.00: disabled
scsi5 : pata_pdc2027x
  Vendor: LITE-ON   Model: CD-RW SOHR-5238S  Rev: 4S07
  Type:   CD-ROM                             ANSI SCSI revision: 05
  Vendor: HL-DT-ST  Model: DVDRAM GSA-4163B  Rev: A101
  Type:   CD-ROM                             ANSI SCSI revision: 05
sr0: scsi3-mmc drive: 40x/40x writer cd/rw xa/form2 cdda tray
Uniform CD-ROM driver Revision: 3.20
sr 4:0:0:0: Attached scsi CD-ROM sr0
sr1: scsi3-mmc drive: 40x/40x writer dvd-ram cd/rw xa/form2 cdda tray
sr 4:0:1:0: Attached scsi CD-ROM sr1

Re: [PATCH 08/11] libata-eh-fw: implement new EH scheduling from PIO

[Albert Lee]

Tejun Heo wrote:
> PIO executes without holding host_set lock, so it cannot be
> synchronized using the same mechanism as interrupt driven execution.
> port_task framework makes sure that EH is not entered until PIO task
> is flushed, so PIO task can be sure the qc in progress won't go away
> underneath it.  One thing it cannot be sure of is whether the qc has
> already been scheduled for EH by another exception condition while
> host_set lock was released.
> 
> This patch makes ata_poll_qc-complete() handle such conditions
> properly and make it freeze the port if HSM violation is detected
> during PIO execution.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> ---
> 
>  drivers/scsi/libata-core.c |   23 +++++++++++++++++++----
>  1 files changed, 19 insertions(+), 4 deletions(-)
> 
> 52240d1d208615e461c41a5c297f521a8a892f0d
> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
> index 436ff5b..86d6a7d 100644
> --- a/drivers/scsi/libata-core.c
> +++ b/drivers/scsi/libata-core.c
> @@ -3429,16 +3429,31 @@ skip_map:
>   *	LOCKING:
>   *	None.  (grabs host lock)
>   */
> -
>  void ata_poll_qc_complete(struct ata_queued_cmd *qc)
>  {
>  	struct ata_port *ap = qc->ap;
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&ap->host_set->lock, flags);
> -	ap->flags &= ~ATA_FLAG_NOINTR;
> -	ata_irq_on(ap);
> -	ata_qc_complete(qc);
> +
> +	if (ap->ops->error_handler) {
> +		/* EH might have kicked in while host_set lock is released */
> +		qc = ata_qc_from_tag(ap, qc->tag);
> +		if (qc) {
> +			if (!(qc->err_mask & AC_ERR_HSM)) {
> +				ap->flags &= ~ATA_FLAG_NOINTR;
> +				ata_irq_on(ap);
> +				ata_qc_complete(qc);
> +			} else
> +				ata_port_freeze(ap);

I don't quite understand here. For the AC_ERR_HSM error, how does the
EH get triggered after ata_port_freeze()? Do we wait until time out?



> +		}
> +	} else {
> +		/* old EH */
> +		ap->flags &= ~ATA_FLAG_NOINTR;
> +		ata_irq_on(ap);
> +		ata_qc_complete(qc);
> +	}
> +
>  	spin_unlock_irqrestore(&ap->host_set->lock, flags);
>  }
>  

Re: [PATCH 08/11] libata-eh-fw: implement new EH scheduling from PIO

[Tejun Heo]

Albert Lee wrote:
> Tejun Heo wrote:
>> PIO executes without holding host_set lock, so it cannot be
>> synchronized using the same mechanism as interrupt driven execution.
>> port_task framework makes sure that EH is not entered until PIO task
>> is flushed, so PIO task can be sure the qc in progress won't go away
>> underneath it.  One thing it cannot be sure of is whether the qc has
>> already been scheduled for EH by another exception condition while
>> host_set lock was released.
>>
>> This patch makes ata_poll_qc-complete() handle such conditions
>> properly and make it freeze the port if HSM violation is detected
>> during PIO execution.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
>>
>> ---
>>
>>  drivers/scsi/libata-core.c |   23 +++++++++++++++++++----
>>  1 files changed, 19 insertions(+), 4 deletions(-)
>>
>> 52240d1d208615e461c41a5c297f521a8a892f0d
>> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
>> index 436ff5b..86d6a7d 100644
>> --- a/drivers/scsi/libata-core.c
>> +++ b/drivers/scsi/libata-core.c
>> @@ -3429,16 +3429,31 @@ skip_map:
>>   *	LOCKING:
>>   *	None.  (grabs host lock)
>>   */
>> -
>>  void ata_poll_qc_complete(struct ata_queued_cmd *qc)
>>  {
>>  	struct ata_port *ap = qc->ap;
>>  	unsigned long flags;
>>  
>>  	spin_lock_irqsave(&ap->host_set->lock, flags);
>> -	ap->flags &= ~ATA_FLAG_NOINTR;
>> -	ata_irq_on(ap);
>> -	ata_qc_complete(qc);
>> +
>> +	if (ap->ops->error_handler) {
>> +		/* EH might have kicked in while host_set lock is released */
>> +		qc = ata_qc_from_tag(ap, qc->tag);
>> +		if (qc) {
>> +			if (!(qc->err_mask & AC_ERR_HSM)) {
>> +				ap->flags &= ~ATA_FLAG_NOINTR;
>> +				ata_irq_on(ap);
>> +				ata_qc_complete(qc);
>> +			} else
>> +				ata_port_freeze(ap);
> 
> I don't quite understand here. For the AC_ERR_HSM error, how does the
> EH get triggered after ata_port_freeze()? Do we wait until time out?
> 

/**
  *	ata_port_freeze - abort & freeze port
  *	@ap: ATA port to freeze
  *
  *	Abort and freeze @ap.
         ^^^^^^
  *
  *	LOCKING:
  *	spin_lock_irqsave(host_set lock)
  *
  *	RETURNS:
  *	Number of aborted commands.
  */
int ata_port_freeze(struct ata_port *ap)
{
	int nr_aborted;

	WARN_ON(!ap->ops->error_handler);

	nr_aborted = ata_port_abort(ap);
                      ^^^^^^^^^^^^^^^^^^^
	__ata_port_freeze(ap);

	return nr_aborted;
}

-- 
tejun

Re: [PATCH 08/11] libata-eh-fw: implement new EH scheduling from PIO

[Albert Lee]

Tejun Heo wrote:
> Albert Lee wrote:
> 
>> Tejun Heo wrote:
>>
>>> PIO executes without holding host_set lock, so it cannot be
>>> synchronized using the same mechanism as interrupt driven execution.
>>> port_task framework makes sure that EH is not entered until PIO task
>>> is flushed, so PIO task can be sure the qc in progress won't go away
>>> underneath it.  One thing it cannot be sure of is whether the qc has
>>> already been scheduled for EH by another exception condition while
>>> host_set lock was released.
>>>
>>> This patch makes ata_poll_qc-complete() handle such conditions
>>> properly and make it freeze the port if HSM violation is detected
>>> during PIO execution.
>>>
>>> Signed-off-by: Tejun Heo <htejun@gmail.com>
>>>
>>> ---
>>>
>>>  drivers/scsi/libata-core.c |   23 +++++++++++++++++++----
>>>  1 files changed, 19 insertions(+), 4 deletions(-)
>>>
>>> 52240d1d208615e461c41a5c297f521a8a892f0d
>>> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
>>> index 436ff5b..86d6a7d 100644
>>> --- a/drivers/scsi/libata-core.c
>>> +++ b/drivers/scsi/libata-core.c
>>> @@ -3429,16 +3429,31 @@ skip_map:
>>>   *    LOCKING:
>>>   *    None.  (grabs host lock)
>>>   */
>>> -
>>>  void ata_poll_qc_complete(struct ata_queued_cmd *qc)
>>>  {
>>>      struct ata_port *ap = qc->ap;
>>>      unsigned long flags;
>>>  
>>>      spin_lock_irqsave(&ap->host_set->lock, flags);
>>> -    ap->flags &= ~ATA_FLAG_NOINTR;
>>> -    ata_irq_on(ap);
>>> -    ata_qc_complete(qc);
>>> +
>>> +    if (ap->ops->error_handler) {
>>> +        /* EH might have kicked in while host_set lock is released */
>>> +        qc = ata_qc_from_tag(ap, qc->tag);
>>> +        if (qc) {
>>> +            if (!(qc->err_mask & AC_ERR_HSM)) {
>>> +                ap->flags &= ~ATA_FLAG_NOINTR;
>>> +                ata_irq_on(ap);
>>> +                ata_qc_complete(qc);
>>> +            } else
>>> +                ata_port_freeze(ap);
>>
>>
>> I don't quite understand here. For the AC_ERR_HSM error, how does the
>> EH get triggered after ata_port_freeze()? Do we wait until time out?
>>
> 
> /**
>  *    ata_port_freeze - abort & freeze port
>  *    @ap: ATA port to freeze
>  *
>  *    Abort and freeze @ap.
>         ^^^^^^
>  *
>  *    LOCKING:
>  *    spin_lock_irqsave(host_set lock)
>  *
>  *    RETURNS:
>  *    Number of aborted commands.
>  */
> int ata_port_freeze(struct ata_port *ap)
> {
>     int nr_aborted;
> 
>     WARN_ON(!ap->ops->error_handler);
> 
>     nr_aborted = ata_port_abort(ap);
>                      ^^^^^^^^^^^^^^^^^^^
>     __ata_port_freeze(ap);
> 
>     return nr_aborted;
> }
> 

Ah, I see. ata_port_freeze() is more than freeze. It does recovery, too.
Thanks for the pointer.

-- albert