[PATCHSET] prep for PMP support, take 3

[PATCHSET] prep for PMP support, take 3

[Tejun Heo]

Hello, all.

This is the third take of libata-pmp-prep patchset.  This patchset
contains 5 patches and implements various stuff needed for PMP.

Changes from the last take[L] are.

* rebased
* sata_link_hardreset() separation is done earlier by vt8251-ahci
  patchset and thus dropped from this patchset.

This patchset is against

  upstream (da54f5fe54c7d75e2db7d17961fb36a8c28a8501)
  + [1] hp-poll patchset, take #5
  + [2] vt8251-ahci patchset, take #2
  + [3] libata-link patchset, take #3

Git tree can be accessed at

  http://htj.dyndns.org/git/?p=libata-tj.git;a=shortlog;h=pmp-prep
  git://htj.dyndns.org/libata-tj pmp-prep

Jeff, I couldn't merge ->qc_defer() into ->qc_prep() or ->qc_issue().
The problem is that ->qc_prep() is done after sg mapping is done which
is a bit costly to throw away if the qc has to be deferred.
->qc_defer() is called right after qc is allocated and translated but
before any resource for qc execution is acuquired.

Another way would be to let ->qc_issue() determine whether deferring
is needed and cache prepared qc while deferred.  This has the
following problems.

* qc and related resources are held by deferred command.  Currently,
  qc directly maps to hardware command slots and w/ PMP this can get
  messy.

* Separate requeueing/restarting mechanism must be implemented.  ATM,
  libata depends on SCSI for retrying deferred commands with
  reasonable fairness.  To hold prepared qc, libata needs to implement
  its own mechanism and I don't think the added benefits justify the
  complexity.

So, I think it's better to keep ->qc_defer().

Thanks.

--
tejun

[L] http://article.gmane.org/gmane.linux.ide/11928
[1] http://article.gmane.org/gmane.linux.ide/13467
[2] http://article.gmane.org/gmane.linux.ide/13471
[3] http://article.gmane.org/gmane.linux.ide/13478

[PATCH 5/5] [PATCH] libata-pmp-prep: implement qc_defer helpers

[Tejun Heo]

Implement ap->nr_active_links (the number of links with active qcs),
ap->excl_link (pointer to link which can be used by ->qc_defer and is
cleared when a qc with ATA_QCFLAG_CLEAR_EXCL completes), and
ata_link_active().

These can be used by ->qc_defer() to implement proper command
exclusion.  This set of helpers seem enough for both sil24 (ATAPI
exclusion needed) and cmd-switching PMP.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-core.c |   21 +++++++++++++++++++--
 drivers/ata/libata-eh.c   |    5 +++++
 include/linux/libata.h    |    8 ++++++++
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 5f0835d..58836e3 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1031,6 +1031,7 @@ unsigned ata_exec_internal(struct ata_de
 	struct ata_queued_cmd *qc;
 	unsigned int tag, preempted_tag;
 	u32 preempted_sactive, preempted_qc_active;
+	int preempted_nr_active_links;
 	DECLARE_COMPLETION_ONSTACK(wait);
 	unsigned long flags;
 	unsigned int err_mask;
@@ -1069,9 +1070,11 @@ unsigned ata_exec_internal(struct ata_de
 	preempted_tag = link->active_tag;
 	preempted_sactive = link->sactive;
 	preempted_qc_active = ap->qc_active;
+	preempted_nr_active_links = ap->nr_active_links;
 	link->active_tag = ATA_TAG_POISON;
 	link->sactive = 0;
 	ap->qc_active = 0;
+	ap->nr_active_links = 0;
 
 	/* prepare & issue qc */
 	qc->tf = *tf;
@@ -1141,6 +1144,7 @@ unsigned ata_exec_internal(struct ata_de
 	link->active_tag = preempted_tag;
 	link->sactive = preempted_sactive;
 	ap->qc_active = preempted_qc_active;
+	ap->nr_active_links = preempted_nr_active_links;
 
 	/* XXX - Some LLDDs (sata_mv) disable port on command failure.
 	 * Until those drivers are fixed, we detect the condition
@@ -4580,10 +4584,18 @@ void __ata_qc_complete(struct ata_queued
 		ata_sg_clean(qc);
 
 	/* command should be marked inactive atomically with qc completion */
-	if (qc->tf.protocol == ATA_PROT_NCQ)
+	if (qc->tf.protocol == ATA_PROT_NCQ) {
 		link->sactive &= ~(1 << qc->tag);
-	else
+		if (!link->sactive)
+			ap->nr_active_links--;
+	} else {
 		link->active_tag = ATA_TAG_POISON;
+		ap->nr_active_links--;
+	}
+
+	/* clear exclusive status */
+	if (unlikely(qc->flags & ATA_QCFLAG_CLEAR_EXCL))
+		ap->excl_link = NULL;
 
 	/* atapi: mark qc as inactive to prevent the interrupt handler
 	 * from completing the command twice later, before the error handler
@@ -4754,9 +4766,14 @@ void ata_qc_issue(struct ata_queued_cmd 
 
 	if (qc->tf.protocol == ATA_PROT_NCQ) {
 		WARN_ON(link->sactive & (1 << qc->tag));
+
+		if (!link->sactive)
+			ap->nr_active_links++;
 		link->sactive |= 1 << qc->tag;
 	} else {
 		WARN_ON(link->sactive);
+
+		ap->nr_active_links++;
 		link->active_tag = qc->tag;
 	}
 
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index ba8b173..103fc5e 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -286,6 +286,7 @@ void ata_scsi_error(struct Scsi_Host *ho
 
 		ap->pflags |= ATA_PFLAG_EH_IN_PROGRESS;
 		ap->pflags &= ~ATA_PFLAG_EH_PENDING;
+		ap->excl_link = NULL;	/* don't maintain exclusion over EH */
 
 		spin_unlock_irqrestore(ap->lock, flags);
 
@@ -2246,6 +2247,10 @@ void ata_eh_finish(struct ata_port *ap)
 			}
 		}
 	}
+
+	/* make sure nr_active_links is zero after EH */
+	WARN_ON(ap->nr_active_links);
+	ap->nr_active_links = 0;
 }
 
 /**
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 12e866d..0191ac8 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -206,6 +206,7 @@ enum {
 	ATA_QCFLAG_DMAMAP	= ATA_QCFLAG_SG | ATA_QCFLAG_SINGLE,
 	ATA_QCFLAG_IO		= (1 << 3), /* standard IO command */
 	ATA_QCFLAG_RESULT_TF	= (1 << 4), /* result TF requested */
+	ATA_QCFLAG_CLEAR_EXCL	= (1 << 5), /* clear excl_link on completion */
 
 	ATA_QCFLAG_FAILED	= (1 << 16), /* cmd failed and is owned by EH */
 	ATA_QCFLAG_SENSE_VALID	= (1 << 17), /* sense data valid */
@@ -579,11 +580,13 @@ struct ata_port {
 	struct ata_queued_cmd	qcmd[ATA_MAX_QUEUE];
 	unsigned long		qc_allocated;
 	unsigned int		qc_active;
+	int			nr_active_links; /* #links with active qcs */
 
 	struct ata_link		link;	/* host default link */
 
 	int			nr_pmp_links;	/* nr of available PMP links */
 	struct ata_link		*pmp_link;	/* array of PMP links */
+	struct ata_link		*excl_link;	/* for PMP qc exclusion */
 
 	struct ata_port_stats	stats;
 	struct ata_host		*host;
@@ -1068,6 +1071,11 @@ static inline int ata_link_max_devices(c
 	return 1;
 }
 
+static inline int ata_link_active(struct ata_link *link)
+{
+	return ata_tag_valid(link->active_tag) || link->sactive;
+}
+
 static inline struct ata_link *ata_port_first_link(struct ata_port *ap)
 {
 	if (ap->nr_pmp_links)
-- 
1.4.2.3

[PATCH 4/5] [PATCH] libata-pmp-prep: implement ops->qc_defer()

[Tejun Heo]

Controllers which support PMP have various restrictions on which
combinations of commands are allowed to what number of devices
concurrently.  This patch implements ops->qc_defer() which determines
whether a qc can be issued at the moment or should be deferred.

If the function returns ATA_DEFER_LINK, the qc will be deferred until
a qc completes on the link.  If ATA_DEFER_PORT, until a qc completes
on any link.  The defer conditions are advisory and in general
ATA_DEFER_LINK can be considered as lower priority deferring than
ATA_DEFER_PORT.

ops->qc_defer() replaces fixed ata_scmd_need_defer().  For standard
NCQ/non-NCQ exclusion, ata_std_qc_defer() is implemented.  ahci and
sata_sil24 are converted to use ata_std_qc_defer().

ops->qc_defer() is heavier than the original mechanism because full qc
is prepped before determining to defer it, but various information is
needed to determine defer conditinos and fully translating a qc is the
only way to supply such information in generic manner.

IMHO, this shouldn't cause any noticeable performance issues as

* for most cases deferring occurs rarely (except for NCQ-aware
  cmd-switching PMP)
* translation itself isn't that expensive
* once deferred the command won't be repeated until another command
  completes which usually is a very long time cpu-wise.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/ahci.c        |    2 ++
 drivers/ata/libata-core.c |   39 +++++++++++++++++++++++++++++++
 drivers/ata/libata-scsi.c |   57 +++++++++++++++------------------------------
 drivers/ata/sata_sil24.c  |    1 +
 include/linux/libata.h    |    6 +++++
 5 files changed, 67 insertions(+), 38 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 3878dea..d2d43cf 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -251,6 +251,7 @@ static const struct ata_port_operations 
 
 	.tf_read		= ahci_tf_read,
 
+	.qc_defer		= ata_std_qc_defer,
 	.qc_prep		= ahci_qc_prep,
 	.qc_issue		= ahci_qc_issue,
 
@@ -284,6 +285,7 @@ static const struct ata_port_operations 
 
 	.tf_read		= ahci_tf_read,
 
+	.qc_defer		= ata_std_qc_defer,
 	.qc_prep		= ahci_qc_prep,
 	.qc_issue		= ahci_qc_issue,
 
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 6b3a013..5f0835d 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3485,6 +3485,37 @@ int ata_check_atapi_dma(struct ata_queue
 
 	return rc;
 }
+
+/**
+ *	ata_std_qc_defer - Check whether a qc needs to be deferred
+ *	@qc: ATA command in question
+ *
+ *	Non-NCQ commands cannot run with any other command, NCQ or
+ *	not.  As upper layer only knows the queue depth, we are
+ *	responsible for maintaining exclusion.  This function checks
+ *	whether a new command @qc can be issued.
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(host lock)
+ *
+ *	RETURNS:
+ *	ATA_DEFER_* if deferring is needed, 0 otherwise.
+ */
+int ata_std_qc_defer(struct ata_queued_cmd *qc)
+{
+	struct ata_link *link = qc->dev->link;
+
+	if (qc->tf.protocol == ATA_PROT_NCQ) {
+		if (!ata_tag_valid(link->active_tag))
+			return 0;
+	} else {
+		if (!ata_tag_valid(link->active_tag) && !link->sactive)
+			return 0;
+	}
+
+	return ATA_DEFER_LINK;
+}
+
 /**
  *	ata_qc_prep - Prepare taskfile for submission
  *	@qc: Metadata associated with taskfile to be prepared
@@ -5599,6 +5630,13 @@ static struct ata_port * ata_port_add(co
 
 	shost->transportt = &ata_scsi_transport_template;
 
+	/* Schedule policy is determined by ->qc_defer() callback and
+	 * it needs to see every deferred qc.  Set host_blocked to 1
+	 * to prevent SCSI midlayer from automatically deferring
+	 * requests.
+	 */
+	shost->max_host_blocked = 1;
+
 	ap = ata_shost_to_port(shost);
 
 	ata_port_init(ap, host, ent, port_no);
@@ -6299,6 +6337,7 @@ EXPORT_SYMBOL_GPL(ata_interrupt);
 EXPORT_SYMBOL_GPL(ata_mmio_data_xfer);
 EXPORT_SYMBOL_GPL(ata_pio_data_xfer);
 EXPORT_SYMBOL_GPL(ata_pio_data_xfer_noirq);
+EXPORT_SYMBOL_GPL(ata_std_qc_defer);
 EXPORT_SYMBOL_GPL(ata_qc_prep);
 EXPORT_SYMBOL_GPL(ata_noop_qc_prep);
 EXPORT_SYMBOL_GPL(ata_bmdma_setup);
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index feae473..c62992f 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -810,6 +810,13 @@ static void ata_scsi_dev_config(struct s
 {
 	unsigned int max_sectors;
 
+	/* Schedule policy is determined by ->qc_defer() callback and
+	 * it needs to see every deferred qc.  Set dev_blocked to 1 to
+	 * prevent SCSI midlayer from automatically deferring
+	 * requests.
+	 */
+	sdev->max_device_blocked = 1;
+
 	/* TODO: 2048 is an arbitrary number, not the
 	 * hardware maximum.  This should be increased to
 	 * 65534 when Jens Axboe's patch for dynamically
@@ -1500,39 +1507,6 @@ static void ata_scsi_qc_complete(struct 
 }
 
 /**
- *	ata_scmd_need_defer - Check whether we need to defer scmd
- *	@dev: ATA device to which the command is addressed
- *	@is_io: Is the command IO (and thus possibly NCQ)?
- *
- *	NCQ and non-NCQ commands cannot run together.  As upper layer
- *	only knows the queue depth, we are responsible for maintaining
- *	exclusion.  This function checks whether a new command can be
- *	issued to @dev.
- *
- *	LOCKING:
- *	spin_lock_irqsave(host lock)
- *
- *	RETURNS:
- *	1 if deferring is needed, 0 otherwise.
- */
-static int ata_scmd_need_defer(struct ata_device *dev, int is_io)
-{
-	struct ata_link *link = dev->link;
-
-	if (!(dev->flags & ATA_DFLAG_NCQ))
-		return 0;
-
-	if (is_io) {
-		if (!ata_tag_valid(link->active_tag))
-			return 0;
-	} else {
-		if (!ata_tag_valid(link->active_tag) && !link->sactive)
-			return 0;
-	}
-	return 1;
-}
-
-/**
  *	ata_scsi_translate - Translate then issue SCSI command to ATA device
  *	@dev: ATA device to which the command is addressed
  *	@cmd: SCSI command to execute
@@ -1563,15 +1537,13 @@ static int ata_scsi_translate(struct ata
 			      void (*done)(struct scsi_cmnd *),
 			      ata_xlat_func_t xlat_func)
 {
+	struct ata_port *ap = dev->link->ap;
 	struct ata_queued_cmd *qc;
 	u8 *scsicmd = cmd->cmnd;
-	int is_io = xlat_func == ata_scsi_rw_xlat;
+	int rc;
 
 	VPRINTK("ENTER\n");
 
-	if (unlikely(ata_scmd_need_defer(dev, is_io)))
-		goto defer;
-
 	qc = ata_scsi_qc_new(dev, cmd, done);
 	if (!qc)
 		goto err_mem;
@@ -1599,6 +1571,11 @@ static int ata_scsi_translate(struct ata
 	if (xlat_func(qc, scsicmd))
 		goto early_finish;
 
+	if (ap->ops->qc_defer) {
+		if ((rc = ap->ops->qc_defer(qc)))
+			goto defer;
+	}
+
 	/* select device, send command to hardware */
 	ata_qc_issue(qc);
 
@@ -1620,8 +1597,12 @@ err_mem:
 	return 0;
 
 defer:
+	ata_qc_free(qc);
 	DPRINTK("EXIT - defer\n");
-	return SCSI_MLQUEUE_DEVICE_BUSY;
+	if (rc == ATA_DEFER_LINK)
+		return SCSI_MLQUEUE_DEVICE_BUSY;
+	else
+		return SCSI_MLQUEUE_HOST_BUSY;
 }
 
 /**
diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
index 1030496..9564e49 100644
--- a/drivers/ata/sata_sil24.c
+++ b/drivers/ata/sata_sil24.c
@@ -396,6 +396,7 @@ static const struct ata_port_operations 
 
 	.tf_read		= sil24_tf_read,
 
+	.qc_defer		= ata_std_qc_defer,
 	.qc_prep		= sil24_qc_prep,
 	.qc_issue		= sil24_qc_issue,
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 1b91a7c..12e866d 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -263,6 +263,10 @@ enum {
 	/* ering size */
 	ATA_ERING_SIZE		= 32,
 
+	/* return values for ->qc_defer */
+	ATA_DEFER_LINK		= 1,
+	ATA_DEFER_PORT		= 2,
+
 	/* desc_len for ata_eh_info and context */
 	ATA_EH_DESC_LEN		= 80,
 
@@ -633,6 +637,7 @@ struct ata_port_operations {
 
 	void (*data_xfer) (struct ata_device *, unsigned char *, unsigned int, int);
 
+	int (*qc_defer) (struct ata_queued_cmd *qc);
 	void (*qc_prep) (struct ata_queued_cmd *qc);
 	unsigned int (*qc_issue) (struct ata_queued_cmd *qc);
 
@@ -805,6 +810,7 @@ extern void ata_pio_data_xfer(struct ata
 			      unsigned int buflen, int write_data);
 extern void ata_pio_data_xfer_noirq(struct ata_device *adev, unsigned char *buf,
 			      unsigned int buflen, int write_data);
+extern int ata_std_qc_defer(struct ata_queued_cmd *qc);
 extern void ata_qc_prep(struct ata_queued_cmd *qc);
 extern void ata_noop_qc_prep(struct ata_queued_cmd *qc);
 extern unsigned int ata_qc_issue_prot(struct ata_queued_cmd *qc);
-- 
1.4.2.3

[PATCH 3/5] [PATCH] libata-pmp-prep: add @is_cmd to ata_tf_to_fis()

[Tejun Heo]

Add @is_cmd to ata_tf_to_fis().  This controls bit 7 of the second
byte which tells the device whether this H2D FIS is for a command or
not.  This cleans up ahci a bit and will be used by PMP.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/ahci.c        |   10 ++++------
 drivers/ata/libata-core.c |   14 ++++++++------
 drivers/ata/sata_qstor.c  |    2 +-
 drivers/ata/sata_sil24.c  |    2 +-
 include/linux/libata.h    |    3 ++-
 5 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 08611e2..3878dea 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -785,8 +785,7 @@ static int ahci_softreset(struct ata_lin
 			   cmd_fis_len | AHCI_CMD_RESET | AHCI_CMD_CLR_BUSY);
 
 	tf.ctl |= ATA_SRST;
-	ata_tf_to_fis(&tf, fis, 0);
-	fis[1] &= ~(1 << 7);	/* turn off Command FIS bit */
+	ata_tf_to_fis(&tf, 0, 0, fis);
 
 	writel(1, port_mmio + PORT_CMD_ISSUE);
 
@@ -804,8 +803,7 @@ static int ahci_softreset(struct ata_lin
 	ahci_fill_cmd_slot(pp, 0, cmd_fis_len);
 
 	tf.ctl &= ~ATA_SRST;
-	ata_tf_to_fis(&tf, fis, 0);
-	fis[1] &= ~(1 << 7);	/* turn off Command FIS bit */
+	ata_tf_to_fis(&tf, 0, 0, fis);
 
 	writel(1, port_mmio + PORT_CMD_ISSUE);
 	readl(port_mmio + PORT_CMD_ISSUE);	/* flush */
@@ -857,7 +855,7 @@ static int ahci_hardreset(struct ata_lin
 	/* clear D2H reception area to properly wait for D2H FIS */
 	ata_tf_init(link->device, &tf);
 	tf.command = 0xff;
-	ata_tf_to_fis(&tf, d2h_fis, 0);
+	ata_tf_to_fis(&tf, 0, 0, d2h_fis);
 
 	rc = sata_std_hardreset(link, class);
 
@@ -976,7 +974,7 @@ static void ahci_qc_prep(struct ata_queu
 	 */
 	cmd_tbl = pp->cmd_tbl + qc->tag * AHCI_CMD_TBL_SZ;
 
-	ata_tf_to_fis(&qc->tf, cmd_tbl, 0);
+	ata_tf_to_fis(&qc->tf, 0, 1, cmd_tbl);
 	if (is_atapi) {
 		memset(cmd_tbl + AHCI_CMD_TBL_CDB, 0, 32);
 		memcpy(cmd_tbl + AHCI_CMD_TBL_CDB, qc->cdb, qc->dev->cdb_len);
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 0dd5c4c..6b3a013 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -99,8 +99,9 @@ MODULE_VERSION(DRV_VERSION);
 /**
  *	ata_tf_to_fis - Convert ATA taskfile to SATA FIS structure
  *	@tf: Taskfile to convert
- *	@fis: Buffer into which data will output
  *	@pmp: Port multiplier port
+ *	@is_cmd: This FIS is for command
+ *	@fis: Buffer into which data will output
  *
  *	Converts a standard ATA taskfile to a Serial ATA
  *	FIS structure (Register - Host to Device).
@@ -108,12 +109,13 @@ MODULE_VERSION(DRV_VERSION);
  *	LOCKING:
  *	Inherited from caller.
  */
-
-void ata_tf_to_fis(const struct ata_taskfile *tf, u8 *fis, u8 pmp)
+void ata_tf_to_fis(const struct ata_taskfile *tf, u8 pmp, int is_cmd, u8 *fis)
 {
-	fis[0] = 0x27;	/* Register - Host to Device FIS */
-	fis[1] = (pmp & 0xf) | (1 << 7); /* Port multiplier number,
-					    bit 7 indicates Command FIS */
+	fis[0] = 0x27;			/* Register - Host to Device FIS */
+	fis[1] = pmp & 0xf;		/* Port multiplier number*/
+	if (is_cmd)
+		fis[1] |= (1 << 7);	/* bit 7 indicates Command FIS */
+
 	fis[2] = tf->command;
 	fis[3] = tf->feature;
 
diff --git a/drivers/ata/sata_qstor.c b/drivers/ata/sata_qstor.c
index bf1f3e2..da69273 100644
--- a/drivers/ata/sata_qstor.c
+++ b/drivers/ata/sata_qstor.c
@@ -335,7 +335,7 @@ static void qs_qc_prep(struct ata_queued
 	buf[28] = dflags;
 
 	/* frame information structure (FIS) */
-	ata_tf_to_fis(&qc->tf, &buf[32], 0);
+	ata_tf_to_fis(&qc->tf, 0, 1, &buf[32]);
 }
 
 static inline void qs_packet_start(struct ata_queued_cmd *qc)
diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
index 8d6651f..1030496 100644
--- a/drivers/ata/sata_sil24.c
+++ b/drivers/ata/sata_sil24.c
@@ -711,7 +711,7 @@ static void sil24_qc_prep(struct ata_que
 	}
 
 	prb->ctrl = cpu_to_le16(ctrl);
-	ata_tf_to_fis(&qc->tf, prb->fis, 0);
+	ata_tf_to_fis(&qc->tf, qc->dev->link->pmp, 1, prb->fis);
 
 	if (qc->flags & ATA_QCFLAG_DMAMAP)
 		sil24_fill_sg(qc, sge);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 676e1d9..1b91a7c 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -787,7 +787,8 @@ extern u32 ata_wait_register(void __iome
  */
 extern void ata_tf_load(struct ata_port *ap, const struct ata_taskfile *tf);
 extern void ata_tf_read(struct ata_port *ap, struct ata_taskfile *tf);
-extern void ata_tf_to_fis(const struct ata_taskfile *tf, u8 *fis, u8 pmp);
+extern void ata_tf_to_fis(const struct ata_taskfile *tf,
+			  u8 pmp, int is_cmd, u8 *fis);
 extern void ata_tf_from_fis(const u8 *fis, struct ata_taskfile *tf);
 extern void ata_noop_dev_select (struct ata_port *ap, unsigned int device);
 extern void ata_std_dev_select (struct ata_port *ap, unsigned int device);
-- 
1.4.2.3

[PATCH 1/5] [PATCH] libata-pmp-prep: add @new_class to ata_dev_revalidate()

[Tejun Heo]

Consider newly found class code while revalidating.  PMP resetting
always results in valid class code and issuing PMP commands to
ATA/ATAPI device isn't very attractive.  Add @new_class to
ata_dev_revalidate() and check class code for revalidation.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-core.c |   20 +++++++++++++++-----
 drivers/ata/libata-eh.c   |    2 +-
 include/linux/libata.h    |    3 ++-
 3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 0c94b65..d7b28f6 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2169,7 +2169,7 @@ static int ata_dev_set_mode(struct ata_d
 		return -EIO;
 	}
 
-	rc = ata_dev_revalidate(dev, 0);
+	rc = ata_dev_revalidate(dev, ATA_DEV_UNKNOWN, 0);
 	if (rc)
 		return rc;
 
@@ -3073,6 +3073,7 @@ static int ata_dev_same_device(struct at
 /**
  *	ata_dev_revalidate - Revalidate ATA device
  *	@dev: device to revalidate
+ *	@new_class: new class code
  *	@post_reset: is this revalidation after reset?
  *
  *	Re-read IDENTIFY page and make sure @dev is still attached to
@@ -3084,9 +3085,9 @@ static int ata_dev_same_device(struct at
  *	RETURNS:
  *	0 on success, negative errno otherwise
  */
-int ata_dev_revalidate(struct ata_device *dev, int post_reset)
+int ata_dev_revalidate(struct ata_device *dev, unsigned int new_class,
+		       int post_reset)
 {
-	unsigned int class = dev->class;
 	u16 *id = (void *)dev->link->ap->sector_buf;
 	int rc;
 
@@ -3095,13 +3096,22 @@ int ata_dev_revalidate(struct ata_device
 		goto fail;
 	}
 
+	if (ata_class_enabled(new_class)) {
+		/* sometimes wrong class is reported, let it retry */
+		if (new_class != ATA_DEV_ATA && new_class != ATA_DEV_ATAPI) {
+			rc = -EIO;
+			goto fail;
+		}
+	} else
+		new_class = dev->class;
+
 	/* read ID data */
-	rc = ata_dev_read_id(dev, &class, post_reset, id);
+	rc = ata_dev_read_id(dev, &new_class, post_reset, id);
 	if (rc)
 		goto fail;
 
 	/* is the device still there? */
-	if (!ata_dev_same_device(dev, class, id)) {
+	if (!ata_dev_same_device(dev, new_class, id)) {
 		rc = -ENODEV;
 		goto fail;
 	}
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index ead460b..8ee4674 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1738,7 +1738,7 @@ static int ata_eh_revalidate_and_attach(
 			}
 
 			ata_eh_about_to_do(link, dev, ATA_EH_REVALIDATE);
-			rc = ata_dev_revalidate(dev,
+			rc = ata_dev_revalidate(dev, ehc->classes[dev->devno],
 					ehc->i.flags & ATA_EHI_DID_RESET);
 			if (rc)
 				break;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index e09dfe6..676e1d9 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -729,7 +729,8 @@ extern int sata_std_hardreset(struct ata
 extern void ata_std_postreset(struct ata_link *link, unsigned int *classes);
 extern void sata_std_hp_poll_activate(struct ata_port *ap);
 extern int sata_std_hp_poll(struct ata_port *ap);
-extern int ata_dev_revalidate(struct ata_device *dev, int post_reset);
+extern int ata_dev_revalidate(struct ata_device *dev, unsigned int new_class,
+			      int post_reset);
 extern void ata_port_disable(struct ata_port *);
 extern void ata_std_ports(struct ata_ioports *ioaddr);
 #ifdef CONFIG_PCI
-- 
1.4.2.3

[PATCH 2/5] [PATCH] libata-pmp-prep: make a number of functions global to libata

[Tejun Heo]

Make a number of functions from libata-core.c and libata-eh.c global
to libata (drivers/ata/libata.h).  These will be used by PMP.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-core.c |    8 ++++----
 drivers/ata/libata-eh.c   |   34 ++++++++++++++++------------------
 drivers/ata/libata.h      |   20 ++++++++++++++++++++
 3 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index d7b28f6..0dd5c4c 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1706,7 +1706,7 @@ void ata_port_probe(struct ata_port *ap)
  *	LOCKING:
  *	None.
  */
-static void sata_print_link_status(struct ata_link *link)
+void sata_print_link_status(struct ata_link *link)
 {
 	u32 sstatus, scontrol, tmp;
 
@@ -2639,7 +2639,7 @@ int sata_link_resume(struct ata_link *li
 	return sata_link_debounce(link, params);
 }
 
-static void ata_wait_spinup(struct ata_link *link)
+void ata_wait_spinup(struct ata_link *link)
 {
 	struct ata_eh_context *ehc = &link->eh_context;
 	unsigned long end, secs;
@@ -5421,7 +5421,7 @@ void ata_dev_init(struct ata_device *dev
  *	LOCKING:
  *	Kernel thread context (may sleep)
  */
-static void ata_link_init(struct ata_port *ap, struct ata_link *link, int pmp)
+void ata_link_init(struct ata_port *ap, struct ata_link *link, int pmp)
 {
 	struct ata_device *dev;
 
@@ -5454,7 +5454,7 @@ static void ata_link_init(struct ata_por
  *	RETURNS:
  *	0 on success, -errno on failure.
  */
-static int sata_link_init_spd_limit(struct ata_link *link)
+int sata_link_init_spd_limit(struct ata_link *link)
 {
 
 	u32 scontrol, spd;
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 8ee4674..ba8b173 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -51,10 +51,8 @@ MODULE_PARM_DESC(hotplug_polling_interva
 		 "Hotplug polling interval in milliseconds (default 2000)");
 
 static void __ata_port_freeze(struct ata_port *ap);
-static void ata_eh_finish(struct ata_port *ap);
 static void ata_eh_handle_port_suspend(struct ata_port *ap);
 static void ata_eh_handle_port_resume(struct ata_port *ap);
-static void ata_hp_poll_activate(struct ata_port *ap);
 
 
 static void ata_ering_record(struct ata_ering *ering, int is_io,
@@ -767,7 +765,7 @@ void ata_eh_qc_retry(struct ata_queued_c
  *	LOCKING:
  *	None.
  */
-static void ata_eh_detach_dev(struct ata_device *dev)
+void ata_eh_detach_dev(struct ata_device *dev)
 {
 	struct ata_link *link = dev->link;
 	struct ata_port *ap = link->ap;
@@ -804,8 +802,8 @@ static void ata_eh_detach_dev(struct ata
  *	LOCKING:
  *	None.
  */
-static void ata_eh_about_to_do(struct ata_link *link, struct ata_device *dev,
-			       unsigned int action)
+void ata_eh_about_to_do(struct ata_link *link, struct ata_device *dev,
+			unsigned int action)
 {
 	struct ata_port *ap = link->ap;
 	struct ata_eh_info *ehi = &link->eh_info;
@@ -847,8 +845,8 @@ static void ata_eh_about_to_do(struct at
  *	LOCKING:
  *	None.
  */
-static void ata_eh_done(struct ata_link *link, struct ata_device *dev,
-			unsigned int action)
+void ata_eh_done(struct ata_link *link, struct ata_device *dev,
+		 unsigned int action)
 {
 	struct ata_eh_context *ehc = &link->eh_context;
 
@@ -1441,7 +1439,7 @@ static void ata_eh_link_autopsy(struct a
  *	LOCKING:
  *	Kernel thread context (may sleep).
  */
-static void ata_eh_autopsy(struct ata_port *ap)
+void ata_eh_autopsy(struct ata_port *ap)
 {
 	struct ata_link *link;
 
@@ -1527,7 +1525,7 @@ static void ata_eh_link_report(struct at
  *	LOCKING:
  *	None.
  */
-static void ata_eh_report(struct ata_port *ap)
+void ata_eh_report(struct ata_port *ap)
 {
 	struct ata_link *link;
 
@@ -1578,9 +1576,9 @@ static int ata_eh_followup_srst_needed(i
 	return 0;
 }
 
-static int ata_eh_reset(struct ata_link *link, int classify,
-			ata_prereset_fn_t prereset, ata_reset_fn_t softreset,
-			ata_reset_fn_t hardreset, ata_postreset_fn_t postreset)
+int ata_eh_reset(struct ata_link *link, int classify,
+		 ata_prereset_fn_t prereset, ata_reset_fn_t softreset,
+		 ata_reset_fn_t hardreset, ata_postreset_fn_t postreset)
 {
 	struct ata_eh_context *ehc = &link->eh_context;
 	unsigned int *classes = ehc->classes;
@@ -2061,10 +2059,10 @@ static void ata_eh_handle_dev_fail(struc
  *	RETURNS:
  *	0 on success, -errno on failure.
  */
-static int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
-			  ata_reset_fn_t softreset, ata_reset_fn_t hardreset,
-			  ata_postreset_fn_t postreset,
-			  struct ata_link **r_failed_link)
+int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
+		   ata_reset_fn_t softreset, ata_reset_fn_t hardreset,
+		   ata_postreset_fn_t postreset,
+		   struct ata_link **r_failed_link)
 {
 	struct ata_link *link;
 	struct ata_device *dev;
@@ -2218,7 +2216,7 @@ static int ata_eh_recover(struct ata_por
  *	LOCKING:
  *	None.
  */
-static void ata_eh_finish(struct ata_port *ap)
+void ata_eh_finish(struct ata_port *ap)
 {
 	int tag;
 
@@ -2428,7 +2426,7 @@ static unsigned long ata_hp_poll_delay(v
  *	LOCKING:
  *	Kernel thread context (may sleep).
  */
-static void ata_hp_poll_activate(struct ata_port *ap)
+void ata_hp_poll_activate(struct ata_port *ap)
 {
 	unsigned long flags;
 
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 92ae977..342888a 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -54,11 +54,13 @@ extern unsigned int ata_do_simple_cmd(st
 extern int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
 			   int post_reset, u16 *id);
 extern int ata_dev_configure(struct ata_device *dev, int print_info);
+extern void sata_print_link_status(struct ata_link *link);
 extern int sata_down_spd_limit(struct ata_link *link);
 extern int sata_set_spd_needed(struct ata_link *link);
 extern int ata_down_xfermask_limit(struct ata_device *dev, int force_pio0);
 extern int ata_set_mode(struct ata_link *link,
 			struct ata_device **r_failed_dev);
+extern void ata_wait_spinup(struct ata_link *link);
 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);
@@ -68,6 +70,8 @@ extern void ata_dev_select(struct ata_po
 extern void swap_buf_le16(u16 *buf, unsigned int buf_words);
 extern int ata_flush_cache(struct ata_device *dev);
 extern void ata_dev_init(struct ata_device *dev);
+extern void ata_link_init(struct ata_port *ap, struct ata_link *link, int pmp);
+extern int sata_link_init_spd_limit(struct ata_link *link);
 extern int ata_task_ioctl(struct scsi_device *scsidev, void __user *arg);
 extern int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg);
 extern void ata_port_init(struct ata_port *ap, struct ata_host *host,
@@ -119,7 +123,23 @@ extern enum scsi_eh_timer_return ata_scs
 extern void ata_scsi_error(struct Scsi_Host *host);
 extern void ata_port_wait_eh(struct ata_port *ap);
 extern void ata_qc_schedule_eh(struct ata_queued_cmd *qc);
+extern void ata_eh_detach_dev(struct ata_device *dev);
+extern void ata_eh_about_to_do(struct ata_link *link, struct ata_device *dev,
+			       unsigned int action);
+extern void ata_eh_done(struct ata_link *link, struct ata_device *dev,
+			unsigned int action);
+extern void ata_eh_autopsy(struct ata_port *ap);
+extern void ata_eh_report(struct ata_port *ap);
+extern int ata_eh_reset(struct ata_link *link, int classify,
+			ata_prereset_fn_t prereset, ata_reset_fn_t softreset,
+			ata_reset_fn_t hardreset, ata_postreset_fn_t postreset);
+extern int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
+			  ata_reset_fn_t softreset, ata_reset_fn_t hardreset,
+			  ata_postreset_fn_t postreset,
+			  struct ata_link **r_failed_disk);
+extern void ata_eh_finish(struct ata_port *ap);
 extern void ata_hp_poll_worker(void *data);
+extern void ata_hp_poll_activate(struct ata_port *ap);
 extern void ata_hp_poll_deactivate(struct ata_port *ap);
 
 #endif /* __LIBATA_H__ */
-- 
1.4.2.3

Re: [PATCHSET] prep for PMP support, take 3

[Tejun Heo]

Tejun Heo wrote:
> Hello, all.
> 
> This is the third take of libata-pmp-prep patchset.  This patchset
> contains 5 patches and implements various stuff needed for PMP.
> 
> Changes from the last take[L] are.
> 
> * rebased
> * sata_link_hardreset() separation is done earlier by vt8251-ahci
>   patchset and thus dropped from this patchset.

* sdev->max_device_blocked and shost->max_host_block are adjusted to 1 
to make retrials of deferred commands fair.  This fixes starvation which 
occurs to all other devices when ATAPI device is active on a PMP 
connected to sil24.

-- 
tejun

Re: [PATCHSET] prep for PMP support, take 3

[Jeff Garzik]

Tejun Heo wrote:
> Hello, all.
> 
> This is the third take of libata-pmp-prep patchset.  This patchset
> contains 5 patches and implements various stuff needed for PMP.
> 
> Changes from the last take[L] are.
> 
> * rebased
> * sata_link_hardreset() separation is done earlier by vt8251-ahci
>   patchset and thus dropped from this patchset.
> 
> This patchset is against
> 
>   upstream (da54f5fe54c7d75e2db7d17961fb36a8c28a8501)
>   + [1] hp-poll patchset, take #5
>   + [2] vt8251-ahci patchset, take #2
>   + [3] libata-link patchset, take #3
> 
> Git tree can be accessed at
> 
>   http://htj.dyndns.org/git/?p=libata-tj.git;a=shortlog;h=pmp-prep
>   git://htj.dyndns.org/libata-tj pmp-prep
> 
> Jeff, I couldn't merge ->qc_defer() into ->qc_prep() or ->qc_issue().
> The problem is that ->qc_prep() is done after sg mapping is done which
> is a bit costly to throw away if the qc has to be deferred.
> ->qc_defer() is called right after qc is allocated and translated but
> before any resource for qc execution is acuquired.
> 
> Another way would be to let ->qc_issue() determine whether deferring
> is needed and cache prepared qc while deferred.  This has the
> following problems.
> 
> * qc and related resources are held by deferred command.  Currently,
>   qc directly maps to hardware command slots and w/ PMP this can get
>   messy.
> 
> * Separate requeueing/restarting mechanism must be implemented.  ATM,
>   libata depends on SCSI for retrying deferred commands with
>   reasonable fairness.  To hold prepared qc, libata needs to implement
>   its own mechanism and I don't think the added benefits justify the
>   complexity.
> 
> So, I think it's better to keep ->qc_defer().

Weak ACK for the patchset:

SET FEATURES - XFER MODE also needs to use the defer capability.  And we 
also have such a need for simplex devices.  It would be useful to have 
all of this stuff collected into an "ATA command scheduling / 
sequencing" area, rather than just a new hook confined to a single area. 
  I think the problem space is more complex than that.

	Jeff

Re: [PATCHSET] prep for PMP support, take 3

[Tejun Heo]

Jeff Garzik wrote:
> Weak ACK for the patchset:
> 
> SET FEATURES - XFER MODE also needs to use the defer capability.

We need to reconfigure the controller and revalidate devices after XFER 
MODE, so simple command exclusion isn't enough for XFER MODE.  I think 
we need EH level synchronization.  e.g. Something like the following.

* Controller has two ports portA and portB.  It needs host-wide 
exclusion for XFER_MODE and the associated controller reprogramming.

1. User (or whatever) issues XFER MODE for portA.

2. portA EH is invoked for XFER MODE. (local exclusion automatically 
achieved)

3. portA EH asks portB EH to participate in host-wide exclusion.

4. portB EH is invoked and waits for port A EH to complete.

5. portA EH now has host-wide exclusion.  Do XFER MODE, reprogram 
controller and revalidate.

6. portA EH tells portB EH that it's free to go and portA resumes.

7. portB resumes.

> And we also have such a need for simplex devices.  It would be useful
> to have all of this stuff collected into an "ATA command scheduling /
>  sequencing" area, rather than just a new hook confined to a single
> area. I think the problem space is more complex than that.

For simplex controllers, I dunno.  The current code depends on SCSI 
midlayer for command queue management and, as each ATA port is 
represented as separate SCSI host, it's difficult to implement 
cross-host exclusion properly.  Also, are simplex controllers big deal? 
    Aren't they mostly historical?

So, although per-host scheduling by asking SCSI midlayer to retry on 
certain conditions is a bit crude, I think it covers most common use 
cases (NCQ, PMP and whatever port level exclusion).

As you pointed out previously, it would be better if we can do it via 
return value of ->qc_prep or ->qc_issue, but we can't because a prepped 
qc cannot be given back to SCSI queue for retrial, which makes the 
separation between ->qc_prep and ->qc_issue meaningless at the moment. 
So, we end up with, ->qc_defer (before qc prep) and ->qc_prep/issue 
(after qc prep).

Thanks.

-- 
tejun