Linux ATA/IDE development
 help / color / mirror / Atom feed
* [PATCH] ahci: qoriq: added ls1046a platform support
From: yuantian.tang @ 2016-10-09  8:51 UTC (permalink / raw)
  To: tj; +Cc: linux-ide, linux-kernel, Tang Yuantian, Tang Yuantian

From: Tang Yuantian <Yuantian.Tang@nxp.com>

Ls1046a is a new introduced soc which supports ATA3.0.

Signed-off-by: Tang Yuantian <yuantian.tang@nxp.com>
---
 drivers/ata/ahci_qoriq.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/ahci_qoriq.c b/drivers/ata/ahci_qoriq.c
index 1eba8df..9884c8c 100644
--- a/drivers/ata/ahci_qoriq.c
+++ b/drivers/ata/ahci_qoriq.c
@@ -46,11 +46,13 @@
 #define LS1021A_AXICC_ADDR	0xC0
 
 #define SATA_ECC_DISABLE	0x00020000
+#define LS1046A_SATA_ECC_DIS	0x80000000
 
 enum ahci_qoriq_type {
 	AHCI_LS1021A,
 	AHCI_LS1043A,
 	AHCI_LS2080A,
+	AHCI_LS1046A,
 };
 
 struct ahci_qoriq_priv {
@@ -63,6 +65,7 @@ static const struct of_device_id ahci_qoriq_of_match[] = {
 	{ .compatible = "fsl,ls1021a-ahci", .data = (void *)AHCI_LS1021A},
 	{ .compatible = "fsl,ls1043a-ahci", .data = (void *)AHCI_LS1043A},
 	{ .compatible = "fsl,ls2080a-ahci", .data = (void *)AHCI_LS2080A},
+	{ .compatible = "fsl,ls1046a-ahci", .data = (void *)AHCI_LS1046A},
 	{},
 };
 MODULE_DEVICE_TABLE(of, ahci_qoriq_of_match);
@@ -175,6 +178,13 @@ static int ahci_qoriq_phy_init(struct ahci_host_priv *hpriv)
 		writel(AHCI_PORT_TRANS_CFG, reg_base + PORT_TRANS);
 		writel(AHCI_PORT_AXICC_CFG, reg_base + PORT_AXICC);
 		break;
+
+	case AHCI_LS1046A:
+		writel(LS1046A_SATA_ECC_DIS, qpriv->ecc_addr);
+		writel(AHCI_PORT_PHY_1_CFG, reg_base + PORT_PHY1);
+		writel(AHCI_PORT_TRANS_CFG, reg_base + PORT_TRANS);
+		writel(AHCI_PORT_AXICC_CFG, reg_base + PORT_AXICC);
+		break;
 	}
 
 	return 0;
@@ -204,9 +214,9 @@ static int ahci_qoriq_probe(struct platform_device *pdev)
 
 	qoriq_priv->type = (enum ahci_qoriq_type)of_id->data;
 
-	if (qoriq_priv->type == AHCI_LS1021A) {
-		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-				"sata-ecc");
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+			"sata-ecc");
+	if (res) {
 		qoriq_priv->ecc_addr = devm_ioremap_resource(dev, res);
 		if (IS_ERR(qoriq_priv->ecc_addr))
 			return PTR_ERR(qoriq_priv->ecc_addr);
-- 
2.1.0.27.g96db324


^ permalink raw reply related

* [Bug 176931] ata failed to IDENTIFY, suspected regression
From: bugzilla-daemon @ 2016-10-07  4:25 UTC (permalink / raw)
  To: linux-ide
In-Reply-To: <bug-176931-11633@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=176931

be11f157cd19c4a2ba1e9c70a38b1a74@protonmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Regression|No                          |Yes

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* [Bug 176931] New: ata failed to IDENTIFY, suspected regression
From: bugzilla-daemon @ 2016-10-07  4:13 UTC (permalink / raw)
  To: linux-ide

https://bugzilla.kernel.org/show_bug.cgi?id=176931

            Bug ID: 176931
           Summary: ata failed to IDENTIFY, suspected regression
           Product: IO/Storage
           Version: 2.5
    Kernel Version: 4.8
          Hardware: x86-64
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: low
          Priority: P1
         Component: IDE
          Assignee: io_ide@kernel-bugs.osdl.org
          Reporter: be11f157cd19c4a2ba1e9c70a38b1a74@protonmail.com
        Regression: No

I have a old hard drive that shows up as this in lspci:

IDE Interface: NVIDIA Corporation MCP78S [GeForce 8200] SATA Controller
(non-AHCI mode) (rev a1)

When I try to boot the machine with kernel 4.8, it will tell me that it cannot
mount /dev/sda3 on root, and that the available choices only include sr0. The
output before that says something along the lines of

ata1: failed to IDENTIFY

However, the exact same kernel .config(using default options when `make`
prompts) with kernel 4.7.6 works fine.

I can offer SSH(root) access and type commands manually for you if that
breaks(but I can only type manually during EST non-business hours). SSH is set
to autostart if you need to reboot the machine. I have a live CD on hand. The
machine has no important data on it so you don't have to worry about breaking
something. Send me a public key if you're interested.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* Re: [PATCH v2 2/2] block: Add iocontext priority to request
From: Jeff Moyer @ 2016-10-06 19:46 UTC (permalink / raw)
  To: Adam Manzanares; +Cc: axboe, tj, linux-block, linux-ide
In-Reply-To: <1475694007-11999-3-git-send-email-adam.manzanares@hgst.com>

Hi, Adam,

Adam Manzanares <adam.manzanares@hgst.com> writes:

> Patch adds an association between iocontext ioprio and the ioprio of
> a request. This feature is only enabled if a queue flag is set to
> indicate that requests should have ioprio associated with them. The
> queue flag is exposed as the req_prio queue sysfs entry.
>
> Signed-off-by: Adam Mananzanares <adam.manzanares@hgst.com>

I like the idea of the patch, but I have a few comments.

First, don't add a tunable, there's no need for it.  (And in the future,
if you do add tunables, document them.)  That should make your patch
much smaller.

> @@ -1648,6 +1649,7 @@ out:
>  
>  void init_request_from_bio(struct request *req, struct bio *bio)
>  {
> +	struct io_context *ioc = rq_ioc(bio);

That can return NULL, and you blindly dereference it later.

> @@ -1656,7 +1658,11 @@ void init_request_from_bio(struct request *req, struct bio *bio)
>  
>  	req->errors = 0;
>  	req->__sector = bio->bi_iter.bi_sector;
> -	req->ioprio = bio_prio(bio);
> +	if (blk_queue_req_prio(req->q))
> +		req->ioprio = ioprio_best(bio_prio(bio), ioc->ioprio);
> +	else
> +		req->ioprio = bio_prio(bio);
> +

If the bio actually has an ioprio (only happens for bcache at this
point), you should use it.  Something like this:

        req->ioprio = bio_prio(bio);
        if (!req->ioprio && ioc)
		req->ioprio = ioc->ioprio;

Finally, please re-order your series as Hannes suggested.

Thanks!
Jeff

^ permalink raw reply

* Re: [PATCH v2 2/2] block: Add iocontext priority to request
From: Hannes Reinecke @ 2016-10-06  6:24 UTC (permalink / raw)
  To: Adam Manzanares, axboe, tj; +Cc: linux-block, linux-ide
In-Reply-To: <1475694007-11999-3-git-send-email-adam.manzanares@hgst.com>

On 10/05/2016 09:00 PM, Adam Manzanares wrote:
> Patch adds an association between iocontext ioprio and the ioprio of
> a request. This feature is only enabled if a queue flag is set to
> indicate that requests should have ioprio associated with them. The
> queue flag is exposed as the req_prio queue sysfs entry.
>
> Signed-off-by: Adam Mananzanares <adam.manzanares@hgst.com>
> ---
>  block/blk-core.c       |  8 +++++++-
>  block/blk-sysfs.c      | 32 ++++++++++++++++++++++++++++++++
>  include/linux/blkdev.h |  2 ++
>  3 files changed, 41 insertions(+), 1 deletion(-)
>
As the previous patch depends on this one it should actually be the 
first in the series.

But other than that:

Reviewed-by: Hannes Reinecke <hare@xuse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

^ permalink raw reply

* Re: [PATCH v2 1/2] ata: Enabling ATA Command Priorities
From: Hannes Reinecke @ 2016-10-06  6:23 UTC (permalink / raw)
  To: Adam Manzanares, axboe, tj; +Cc: linux-block, linux-ide
In-Reply-To: <1475694007-11999-2-git-send-email-adam.manzanares@hgst.com>

On 10/05/2016 09:00 PM, Adam Manzanares wrote:
> This patch checks to see if an ATA device supports NCQ command priorities.
> If so and the user has specified an iocontext that indicates IO_PRIO_CLASS_RT
> and also enables request priorities in the block queue then we build a tf
> with a high priority command.
>
> This patch depends on patch block-Add-iocontext-priority-to-request
>
> Signed-off-by: Adam Manzanares <adam.manzanares@hgst.com>
> ---
>  drivers/ata/libata-core.c | 35 ++++++++++++++++++++++++++++++++++-
>  drivers/ata/libata-scsi.c | 10 +++++++++-
>  drivers/ata/libata.h      |  2 +-
>  include/linux/ata.h       |  6 ++++++
>  include/linux/libata.h    | 18 ++++++++++++++++++
>  5 files changed, 68 insertions(+), 3 deletions(-)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

^ permalink raw reply

* [PATCH v2 2/2] block: Add iocontext priority to request
From: Adam Manzanares @ 2016-10-05 19:00 UTC (permalink / raw)
  To: axboe, tj; +Cc: linux-block, linux-ide, Adam Manzanares
In-Reply-To: <1475694007-11999-1-git-send-email-adam.manzanares@hgst.com>

Patch adds an association between iocontext ioprio and the ioprio of
a request. This feature is only enabled if a queue flag is set to
indicate that requests should have ioprio associated with them. The
queue flag is exposed as the req_prio queue sysfs entry.

Signed-off-by: Adam Mananzanares <adam.manzanares@hgst.com>
---
 block/blk-core.c       |  8 +++++++-
 block/blk-sysfs.c      | 32 ++++++++++++++++++++++++++++++++
 include/linux/blkdev.h |  2 ++
 3 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 36c7ac3..17c3ce5 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -33,6 +33,7 @@
 #include <linux/ratelimit.h>
 #include <linux/pm_runtime.h>
 #include <linux/blk-cgroup.h>
+#include <linux/ioprio.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/block.h>
@@ -1648,6 +1649,7 @@ out:
 
 void init_request_from_bio(struct request *req, struct bio *bio)
 {
+	struct io_context *ioc = rq_ioc(bio);
 	req->cmd_type = REQ_TYPE_FS;
 
 	req->cmd_flags |= bio->bi_opf & REQ_COMMON_MASK;
@@ -1656,7 +1658,11 @@ void init_request_from_bio(struct request *req, struct bio *bio)
 
 	req->errors = 0;
 	req->__sector = bio->bi_iter.bi_sector;
-	req->ioprio = bio_prio(bio);
+	if (blk_queue_req_prio(req->q))
+		req->ioprio = ioprio_best(bio_prio(bio), ioc->ioprio);
+	else
+		req->ioprio = bio_prio(bio);
+
 	blk_rq_bio_prep(req->q, req, bio);
 }
 
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index f87a7e7..268a71a 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -384,6 +384,31 @@ static ssize_t queue_dax_show(struct request_queue *q, char *page)
 	return queue_var_show(blk_queue_dax(q), page);
 }
 
+static ssize_t queue_req_prio_show(struct request_queue *q, char *page)
+{
+	return queue_var_show(blk_queue_req_prio(q), page);
+}
+
+static ssize_t queue_req_prio_store(struct request_queue *q, const char *page,
+				    size_t count)
+{
+	unsigned long req_prio_on;
+	ssize_t ret;
+
+	ret = queue_var_store(&req_prio_on, page, count);
+	if (ret < 0)
+		return ret;
+
+	spin_lock_irq(q->queue_lock);
+	if (req_prio_on)
+		queue_flag_set(QUEUE_FLAG_REQ_PRIO, q);
+	else
+		queue_flag_clear(QUEUE_FLAG_REQ_PRIO, q);
+	spin_unlock_irq(q->queue_lock);
+
+	return ret;
+}
+
 static struct queue_sysfs_entry queue_requests_entry = {
 	.attr = {.name = "nr_requests", .mode = S_IRUGO | S_IWUSR },
 	.show = queue_requests_show,
@@ -526,6 +551,12 @@ static struct queue_sysfs_entry queue_dax_entry = {
 	.show = queue_dax_show,
 };
 
+static struct queue_sysfs_entry queue_req_prio_entry = {
+	.attr = {.name = "req_prio", .mode = S_IRUGO | S_IWUSR },
+	.show = queue_req_prio_show,
+	.store = queue_req_prio_store,
+};
+
 static struct attribute *default_attrs[] = {
 	&queue_requests_entry.attr,
 	&queue_ra_entry.attr,
@@ -553,6 +584,7 @@ static struct attribute *default_attrs[] = {
 	&queue_poll_entry.attr,
 	&queue_wc_entry.attr,
 	&queue_dax_entry.attr,
+	&queue_req_prio_entry.attr,
 	NULL,
 };
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index e79055c..23e1e2d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -505,6 +505,7 @@ struct request_queue {
 #define QUEUE_FLAG_FUA	       24	/* device supports FUA writes */
 #define QUEUE_FLAG_FLUSH_NQ    25	/* flush not queueuable */
 #define QUEUE_FLAG_DAX         26	/* device supports DAX */
+#define QUEUE_FLAG_REQ_PRIO    27	/* Use iocontext ioprio */
 
 #define QUEUE_FLAG_DEFAULT	((1 << QUEUE_FLAG_IO_STAT) |		\
 				 (1 << QUEUE_FLAG_STACKABLE)	|	\
@@ -595,6 +596,7 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
 #define blk_queue_secure_erase(q) \
 	(test_bit(QUEUE_FLAG_SECERASE, &(q)->queue_flags))
 #define blk_queue_dax(q)	test_bit(QUEUE_FLAG_DAX, &(q)->queue_flags)
+#define blk_queue_req_prio(q)	test_bit(QUEUE_FLAG_REQ_PRIO, &(q)->queue_flags)
 
 #define blk_noretry_request(rq) \
 	((rq)->cmd_flags & (REQ_FAILFAST_DEV|REQ_FAILFAST_TRANSPORT| \
-- 
2.1.4


^ permalink raw reply related

* [PATCH v2 1/2] ata: Enabling ATA Command Priorities
From: Adam Manzanares @ 2016-10-05 19:00 UTC (permalink / raw)
  To: axboe, tj; +Cc: linux-block, linux-ide, Adam Manzanares
In-Reply-To: <1475694007-11999-1-git-send-email-adam.manzanares@hgst.com>

This patch checks to see if an ATA device supports NCQ command priorities.
If so and the user has specified an iocontext that indicates IO_PRIO_CLASS_RT
and also enables request priorities in the block queue then we build a tf
with a high priority command.

This patch depends on patch block-Add-iocontext-priority-to-request

Signed-off-by: Adam Manzanares <adam.manzanares@hgst.com>
---
 drivers/ata/libata-core.c | 35 ++++++++++++++++++++++++++++++++++-
 drivers/ata/libata-scsi.c | 10 +++++++++-
 drivers/ata/libata.h      |  2 +-
 include/linux/ata.h       |  6 ++++++
 include/linux/libata.h    | 18 ++++++++++++++++++
 5 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 223a770..181b530 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -739,6 +739,7 @@ u64 ata_tf_read_block(const struct ata_taskfile *tf, struct ata_device *dev)
  *	@n_block: Number of blocks
  *	@tf_flags: RW/FUA etc...
  *	@tag: tag
+ *	@class: IO priority class
  *
  *	LOCKING:
  *	None.
@@ -753,7 +754,7 @@ u64 ata_tf_read_block(const struct ata_taskfile *tf, struct ata_device *dev)
  */
 int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,
 		    u64 block, u32 n_block, unsigned int tf_flags,
-		    unsigned int tag)
+		    unsigned int tag, int class)
 {
 	tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
 	tf->flags |= tf_flags;
@@ -785,6 +786,12 @@ int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,
 		tf->device = ATA_LBA;
 		if (tf->flags & ATA_TFLAG_FUA)
 			tf->device |= 1 << 7;
+
+		if (ata_ncq_prio_enabled(dev)) {
+			if (class == IOPRIO_CLASS_RT)
+				tf->hob_nsect |= ATA_PRIO_HIGH <<
+						 ATA_SHIFT_PRIO;
+		}
 	} else if (dev->flags & ATA_DFLAG_LBA) {
 		tf->flags |= ATA_TFLAG_LBA;
 
@@ -2156,6 +2163,30 @@ static void ata_dev_config_ncq_non_data(struct ata_device *dev)
 	}
 }
 
+static void ata_dev_config_ncq_prio(struct ata_device *dev)
+{
+	struct ata_port *ap = dev->link->ap;
+	unsigned int err_mask;
+
+	err_mask = ata_read_log_page(dev,
+				     ATA_LOG_SATA_ID_DEV_DATA,
+				     ATA_LOG_SATA_SETTINGS,
+				     ap->sector_buf,
+				     1);
+	if (err_mask) {
+		ata_dev_dbg(dev,
+			    "failed to get Identify Device data, Emask 0x%x\n",
+			    err_mask);
+		return;
+	}
+
+	if (ap->sector_buf[ATA_LOG_NCQ_PRIO_OFFSET] & BIT(3))
+		dev->flags |= ATA_DFLAG_NCQ_PRIO;
+	else
+		ata_dev_dbg(dev, "SATA page does not support priority\n");
+
+}
+
 static int ata_dev_config_ncq(struct ata_device *dev,
 			       char *desc, size_t desc_sz)
 {
@@ -2205,6 +2236,8 @@ static int ata_dev_config_ncq(struct ata_device *dev,
 			ata_dev_config_ncq_send_recv(dev);
 		if (ata_id_has_ncq_non_data(dev->id))
 			ata_dev_config_ncq_non_data(dev);
+		if (ata_id_has_ncq_prio(dev->id))
+			ata_dev_config_ncq_prio(dev);
 	}
 
 	return 0;
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index e207b33..65ea15d 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -50,6 +50,7 @@
 #include <linux/uaccess.h>
 #include <linux/suspend.h>
 #include <asm/unaligned.h>
+#include <linux/ioprio.h>
 
 #include "libata.h"
 #include "libata-transport.h"
@@ -1757,6 +1758,8 @@ static unsigned int ata_scsi_rw_xlat(struct ata_queued_cmd *qc)
 {
 	struct scsi_cmnd *scmd = qc->scsicmd;
 	const u8 *cdb = scmd->cmnd;
+	struct request *rq = scmd->request;
+	int class = 0;
 	unsigned int tf_flags = 0;
 	u64 block;
 	u32 n_block;
@@ -1822,8 +1825,13 @@ static unsigned int ata_scsi_rw_xlat(struct ata_queued_cmd *qc)
 	qc->flags |= ATA_QCFLAG_IO;
 	qc->nbytes = n_block * scmd->device->sector_size;
 
+	/* If queue supports req prio pass it onto the task file */
+	if (blk_queue_req_prio(rq->q))
+		class = IOPRIO_PRIO_CLASS(req_get_ioprio(rq));
+
 	rc = ata_build_rw_tf(&qc->tf, qc->dev, block, n_block, tf_flags,
-			     qc->tag);
+			     qc->tag, class);
+
 	if (likely(rc == 0))
 		return 0;
 
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 3b301a4..8f3a559 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -66,7 +66,7 @@ extern u64 ata_tf_to_lba48(const struct ata_taskfile *tf);
 extern struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev, int tag);
 extern int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,
 			   u64 block, u32 n_block, unsigned int tf_flags,
-			   unsigned int tag);
+			   unsigned int tag, int class);
 extern u64 ata_tf_read_block(const struct ata_taskfile *tf,
 			     struct ata_device *dev);
 extern unsigned ata_exec_internal(struct ata_device *dev,
diff --git a/include/linux/ata.h b/include/linux/ata.h
index adbc812..ebe4c3b 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -347,6 +347,7 @@ enum {
 	ATA_LOG_DEVSLP_DETO	  = 0x01,
 	ATA_LOG_DEVSLP_VALID	  = 0x07,
 	ATA_LOG_DEVSLP_VALID_MASK = 0x80,
+	ATA_LOG_NCQ_PRIO_OFFSET   = 0x09,
 
 	/* NCQ send and receive log */
 	ATA_LOG_NCQ_SEND_RECV_SUBCMDS_OFFSET	= 0x00,
@@ -897,6 +898,11 @@ static inline bool ata_id_has_ncq_non_data(const u16 *id)
 	return id[ATA_ID_SATA_CAPABILITY_2] & BIT(5);
 }
 
+static inline bool ata_id_has_ncq_prio(const u16 *id)
+{
+	return id[ATA_ID_SATA_CAPABILITY] & BIT(12);
+}
+
 static inline bool ata_id_has_trim(const u16 *id)
 {
 	if (ata_id_major_version(id) >= 7 &&
diff --git a/include/linux/libata.h b/include/linux/libata.h
index e37d4f9..244f261 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -165,6 +165,7 @@ enum {
 	ATA_DFLAG_NO_UNLOAD	= (1 << 17), /* device doesn't support unload */
 	ATA_DFLAG_UNLOCK_HPA	= (1 << 18), /* unlock HPA */
 	ATA_DFLAG_NCQ_SEND_RECV = (1 << 19), /* device supports NCQ SEND and RECV */
+	ATA_DFLAG_NCQ_PRIO	= (1 << 20), /* device supports NCQ priority */
 	ATA_DFLAG_INIT_MASK	= (1 << 24) - 1,
 
 	ATA_DFLAG_DETACH	= (1 << 24),
@@ -341,7 +342,9 @@ enum {
 	ATA_SHIFT_PIO		= 0,
 	ATA_SHIFT_MWDMA		= ATA_SHIFT_PIO + ATA_NR_PIO_MODES,
 	ATA_SHIFT_UDMA		= ATA_SHIFT_MWDMA + ATA_NR_MWDMA_MODES,
+	ATA_SHIFT_PRIO		= 6,
 
+	ATA_PRIO_HIGH		= 2,
 	/* size of buffer to pad xfers ending on unaligned boundaries */
 	ATA_DMA_PAD_SZ		= 4,
 
@@ -1609,6 +1612,21 @@ static inline int ata_ncq_enabled(struct ata_device *dev)
 			      ATA_DFLAG_NCQ)) == ATA_DFLAG_NCQ;
 }
 
+/**
+ *	ata_ncq_prio_enabled - Test whether NCQ prio is enabled
+ *	@dev: ATA device to test for
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(host lock)
+ *
+ *	RETURNS:
+ *	1 if NCQ prio is enabled for @dev, 0 otherwise.
+ */
+static inline int ata_ncq_prio_enabled(struct ata_device *dev)
+{
+	return (dev->flags & ATA_DFLAG_NCQ_PRIO) == ATA_DFLAG_NCQ_PRIO;
+}
+
 static inline bool ata_fpdma_dsm_supported(struct ata_device *dev)
 {
 	return (dev->flags & ATA_DFLAG_NCQ_SEND_RECV) &&
-- 
2.1.4


^ permalink raw reply related

* [PATCH v2 0/2] Enabling ATA Command Priorities
From: Adam Manzanares @ 2016-10-05 19:00 UTC (permalink / raw)
  To: axboe, tj; +Cc: linux-block, linux-ide, Adam Manzanares

This patch builds ATA commands with high priority if the iocontext
of a process is set to real time. The goal of the patch is to
improve tail latencies of workloads that use higher queue depths.

This patch has been tested with an Ultrastar HE8 HDD and cuts the 
the p99.99 tail latency of foreground IO from 2s down to 72ms when
using the deadline scheduler. This patch works independently of the
scheduler so it can be used with all of the currently available 
request based schedulers. 

Foreground IO, for the previously described results, is an async fio job 
submitting 4K read requests at a QD of 1 to the HDD. The foreground IO is set 
with the iopriority class of real time. The background workload is another fio
job submitting read requests at a QD of 32 to the same HDD with default 
iopriority.

This feature is enabled by setting a queue flag that is exposed as a sysfs
entry named req_prio. If this feature is enabled the request ioprio comes
from the highest priority between the iocontext and the bio. 

v2:
 - Add queue flag to set iopriority going to the request
 - If queue flag set, send iopriority class to ata_build_rw_tf
 - Remove redundant code in ata_ncq_prio_enabled function.


Adam Manzanares (2):
  ata: Enabling ATA Command Priorities
  block: Add iocontext priority to request

 block/blk-core.c       |  8 +++++++-
 block/blk-sysfs.c      | 32 ++++++++++++++++++++++++++++++++
 include/linux/blkdev.h |  2 ++
 3 files changed, 41 insertions(+), 1 deletion(-)

-- 
2.1.4


^ permalink raw reply

* Re: [PATCH 1/3] block: Add iocontext priority to request
From: Tejun Heo @ 2016-10-04 20:52 UTC (permalink / raw)
  To: Adam Manzanares; +Cc: axboe, linux-block, linux-ide
In-Reply-To: <20161004154917.GA4764@hgst.com>

Hello, Adam.

On Tue, Oct 04, 2016 at 08:49:18AM -0700, Adam Manzanares wrote:
> > I wonder whether the right thing to do is adding bio->bi_ioprio which
> > is initialized on bio submission and carried through req->ioprio.
> 
> I looked around and thought about this and I'm not sure if this will help. 
> I dug into the bio submission code and I thought generic_make_request was 
> the best place to save the ioprio information. This is quite close in 
> the call stack to init_request_from bio. Bcache sets the bio priority before 
> the submission, so we would have to check to see if the bio priority was 
> valid on bio submission leaving us with the same problem. Leaving the 
> priority in the upper bits of bio->bi_rw is fine with me. It may help to 
> have the bio->bi_ioprio for clarity, but I think we will still face the 
> issue of having to check if this value is set when we submit the bio or 
> init the request so I'm leaning towards leaving it as is.

I see.  Thanks for looking into it.  It's icky that we don't have a
clear path of propagating ioprio but let's save that for another day.

-- 
tejun

^ permalink raw reply

* Re: [PATCH 1/3] block: Add iocontext priority to request
From: Adam Manzanares @ 2016-10-04 15:49 UTC (permalink / raw)
  To: Tejun Heo; +Cc: axboe, linux-block, linux-ide
In-Reply-To: <20161002085321.GA13648@mtj.duckdns.org>

Hello Tejun,

10/02/2016 10:53, Tejun Heo wrote:
> Hello, Adam.
> 
> On Fri, Sep 30, 2016 at 09:02:17AM -0700, Adam Manzanares wrote:
> > I'll start with the changes I made and work my way through a grep of            
> > ioprio. Please add or correct any of the assumptions I have made.               
> 
> Well, it looks like you're the one who's most familiar with ioprio
> handling at this point. :)
> 
> > In blk-core, the behavior before the patch is to get the ioprio for the request 
> > from the bio. The only references I found to bio_set_prio are in bcache. Both   
> > of these references are in low priority operations (gc, bg writeback) so the    
> > iopriority of the bio is set to IO_PRIOCLASS_IDLE in these cases.               
> >                                                                                 
> > A kernel thread is used to submit these bios so the ioprio is going to come     
> > from the current running task if the iocontext exists. This could be a problem  
> > if we have set a task with high priority and some background work ends up       
> > getting generated in the bcache layer. I propose that we check if the           
> > iopriority of the bio is valid and if so, then we keep the priorirty from the   
> > bio.                                                                            
> 
> I wonder whether the right thing to do is adding bio->bi_ioprio which
> is initialized on bio submission and carried through req->ioprio.
> 

I looked around and thought about this and I'm not sure if this will help. 
I dug into the bio submission code and I thought generic_make_request was 
the best place to save the ioprio information. This is quite close in 
the call stack to init_request_from bio. Bcache sets the bio priority before 
the submission, so we would have to check to see if the bio priority was 
valid on bio submission leaving us with the same problem. Leaving the 
priority in the upper bits of bio->bi_rw is fine with me. It may help to 
have the bio->bi_ioprio for clarity, but I think we will still face the 
issue of having to check if this value is set when we submit the bio or 
init the request so I'm leaning towards leaving it as is.


> > The second area that I see a potential problem is in the merging code code in   
> > blk-core when a bio is queued. If there is a request that is mergeable then     
> > the merge code takes the highest priority of the bio and the request. This      
> > could wipe out the values set by bio_set_prio. I think it would be              
> > best to set the request as non mergeable when we see that it is a high          
> > priority IO request.                                                            
> 
> The current behavior should be fine for most non-pathological cases
> but I have no objection to not merging ios with differing priorities.
> 
> > The third area that is of interest is in the CFQ scheduler and the ioprio is    
> > only used in the case of async IO and I found that the priority is only         
> > obtained from the task and not from the request. This leads me to believe that  
> > the changes made in the blk-core to add the priority to the request will not    
> > impact the CFQ scheduler. 
> >
> > The fourth area that might be concerning is the drivers. virtio_block copies    
> > the request priority into a virtual block request. I am assuming that this      
> > eventually makes it to another device driver so we don't need to worry about    
> > this. null block device driver also uses the ioprio, but this is also not a     
> > concern. lightnvm also sets the ioprio to build a request that is passed onto   
> > another driver. The last driver that uses the request ioprio is the fusion      
> > mptsas driver and I don't understand how it is using the ioprio. From what I    
> > can tell it is taking a request of IOPRIO_CLASS_NONE with data of 0x7 and       
> > calling this high priority IO. This could be impacted by the code I have        
> > proposed, but I believe the authors intended to treat this particular ioprio    
> > value as high priority. The driver will pass the request to the device          
> > with high priority if the appropriate ioprio values is seen on the request.     
> >                                                                                 
> > The fifth area that I noticed may be impacted is file systems. btrfs uses low   
> > priority IO for read ahead. Ext4 uses ioprio for journaling. Both of these      
> > issues are not a problem because the ioprio is set on the task and not on a     
> > bio.
> 
> Yeah, looks good to me.  Care to include a brief summary of expected
> (non)impacts in the patch description?
> 
I'm going to send out an updated series of patches summarizing some of this 
discussion and I'll also include some performance results that we have 
measured.

> Thanks.
> 
> -- 
> tejun

Take care,
Adam

^ permalink raw reply

* Problems with item delivery, n.000773940
From: FedEx International MailService @ 2016-10-03 18:24 UTC (permalink / raw)
  To: linux-ide

[-- Attachment #1: Type: text/plain, Size: 172 bytes --]

Dear Customer,

This is to confirm that one or more of your parcels has been shipped.
Shipment Label is attached to email.

Yours sincerely,
Ryan Stokes,
Station Manager.


[-- Attachment #2: FedEx_ID_000773940.zip --]
[-- Type: application/zip, Size: 7507 bytes --]

^ permalink raw reply

* Re: [PATCH 1/3] block: Add iocontext priority to request
From: Tejun Heo @ 2016-10-02  8:53 UTC (permalink / raw)
  To: Adam Manzanares; +Cc: axboe, linux-block, linux-ide
In-Reply-To: <20160930160216.GA13637@hgst.com>

Hello, Adam.

On Fri, Sep 30, 2016 at 09:02:17AM -0700, Adam Manzanares wrote:
> I'll start with the changes I made and work my way through a grep of            
> ioprio. Please add or correct any of the assumptions I have made.               

Well, it looks like you're the one who's most familiar with ioprio
handling at this point. :)

> In blk-core, the behavior before the patch is to get the ioprio for the request 
> from the bio. The only references I found to bio_set_prio are in bcache. Both   
> of these references are in low priority operations (gc, bg writeback) so the    
> iopriority of the bio is set to IO_PRIOCLASS_IDLE in these cases.               
>                                                                                 
> A kernel thread is used to submit these bios so the ioprio is going to come     
> from the current running task if the iocontext exists. This could be a problem  
> if we have set a task with high priority and some background work ends up       
> getting generated in the bcache layer. I propose that we check if the           
> iopriority of the bio is valid and if so, then we keep the priorirty from the   
> bio.                                                                            

I wonder whether the right thing to do is adding bio->bi_ioprio which
is initialized on bio submission and carried through req->ioprio.

> The second area that I see a potential problem is in the merging code code in   
> blk-core when a bio is queued. If there is a request that is mergeable then     
> the merge code takes the highest priority of the bio and the request. This      
> could wipe out the values set by bio_set_prio. I think it would be              
> best to set the request as non mergeable when we see that it is a high          
> priority IO request.                                                            

The current behavior should be fine for most non-pathological cases
but I have no objection to not merging ios with differing priorities.

> The third area that is of interest is in the CFQ scheduler and the ioprio is    
> only used in the case of async IO and I found that the priority is only         
> obtained from the task and not from the request. This leads me to believe that  
> the changes made in the blk-core to add the priority to the request will not    
> impact the CFQ scheduler. 
>
> The fourth area that might be concerning is the drivers. virtio_block copies    
> the request priority into a virtual block request. I am assuming that this      
> eventually makes it to another device driver so we don't need to worry about    
> this. null block device driver also uses the ioprio, but this is also not a     
> concern. lightnvm also sets the ioprio to build a request that is passed onto   
> another driver. The last driver that uses the request ioprio is the fusion      
> mptsas driver and I don't understand how it is using the ioprio. From what I    
> can tell it is taking a request of IOPRIO_CLASS_NONE with data of 0x7 and       
> calling this high priority IO. This could be impacted by the code I have        
> proposed, but I believe the authors intended to treat this particular ioprio    
> value as high priority. The driver will pass the request to the device          
> with high priority if the appropriate ioprio values is seen on the request.     
>                                                                                 
> The fifth area that I noticed may be impacted is file systems. btrfs uses low   
> priority IO for read ahead. Ext4 uses ioprio for journaling. Both of these      
> issues are not a problem because the ioprio is set on the task and not on a     
> bio.

Yeah, looks good to me.  Care to include a brief summary of expected
(non)impacts in the patch description?

Thanks.

-- 
tejun

^ permalink raw reply

* RE: [PATCH] ahci: qoriq: Revert "ahci: qoriq: Disable NCQ on ls2080a SoC"
From: Y.T. Tang @ 2016-09-30  6:29 UTC (permalink / raw)
  To: Y.T. Tang, tj@kernel.org, linux-ide@vger.kernel.org
In-Reply-To: <1475215954-48395-1-git-send-email-yuantian.tang@nxp.com>

Please ignore this, it is a duplicated patch.

Regards,
Yuantian

> -----Original Message-----
> From: yuantian.tang@nxp.com [mailto:yuantian.tang@nxp.com]
> Sent: Friday, September 30, 2016 2:13 PM
> To: tj@kernel.org; linux-ide@vger.kernel.org
> Cc: Y.T. Tang <yuantian.tang@nxp.com>; Y.T. Tang <yuantian.tang@nxp.com>
> Subject: [PATCH] ahci: qoriq: Revert "ahci: qoriq: Disable NCQ on ls2080a SoC"
> 
> From: Tang Yuantian <Yuantian.Tang@nxp.com>
> 
> This reverts commit 640847298e2b7f19 ("ahci: qoriq: Disable NCQ on ls2080a
> SoC")
> 
> The erratum has been fixed in ls2080a v2.0 and later soc.
> In reality, customer will not get any ls2080a v1.0 soc. Neither apply to any
> products. So reverting this commit won't create any side effect.
> 
> Blacklisting v2.0 could also be a option, but that needs to check the soc
> version which is not suitable in the driver.
> 
> Signed-off-by: Tang Yuantian <yuantian.tang@nxp.com>
> ---
>  drivers/ata/ahci_qoriq.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/ata/ahci_qoriq.c b/drivers/ata/ahci_qoriq.c index
> 925c4b6..1eba8df 100644
> --- a/drivers/ata/ahci_qoriq.c
> +++ b/drivers/ata/ahci_qoriq.c
> @@ -136,7 +136,7 @@ static struct ata_port_operations ahci_qoriq_ops = {
>  	.hardreset	= ahci_qoriq_hardreset,
>  };
> 
> -static struct ata_port_info ahci_qoriq_port_info = {
> +static const struct ata_port_info ahci_qoriq_port_info = {
>  	.flags		= AHCI_FLAG_COMMON | ATA_FLAG_NCQ,
>  	.pio_mask	= ATA_PIO4,
>  	.udma_mask	= ATA_UDMA6,
> @@ -221,12 +221,6 @@ static int ahci_qoriq_probe(struct platform_device
> *pdev)
>  	if (rc)
>  		goto disable_resources;
> 
> -	/* Workaround for ls2080a */
> -	if (qoriq_priv->type == AHCI_LS2080A) {
> -		hpriv->flags |= AHCI_HFLAG_NO_NCQ;
> -		ahci_qoriq_port_info.flags &= ~ATA_FLAG_NCQ;
> -	}
> -
>  	rc = ahci_platform_init_host(pdev, hpriv, &ahci_qoriq_port_info,
>  				     &ahci_qoriq_sht);
>  	if (rc)
> --
> 2.1.0.27.g96db324


^ permalink raw reply

* Re: [PATCH 2/3] ata: Enabling ATA Command Priorities
From: Adam Manzanares @ 2016-09-30 16:04 UTC (permalink / raw)
  To: Tejun Heo; +Cc: axboe, linux-block, linux-ide
In-Reply-To: <20160929084529.GC11087@mtj.duckdns.org>

The 09/29/2016 10:45, Tejun Heo wrote:
> Hello,
> 
> On Tue, Sep 27, 2016 at 11:14:55AM -0700, Adam Manzanares wrote:
> > +/**
> > + *	ata_ncq_prio_enabled - Test whether NCQ prio is enabled
> > + *	@dev: ATA device to test for
> > + *
> > + *	LOCKING:
> > + *	spin_lock_irqsave(host lock)
> > + *
> > + *	RETURNS:
> > + *	1 if NCQ prio is enabled for @dev, 0 otherwise.
> > + */
> > +static inline int ata_ncq_prio_enabled(struct ata_device *dev)
> > +{
> > +	return (dev->flags & (ATA_DFLAG_PIO | ATA_DFLAG_NCQ_OFF |
> > +			      ATA_DFLAG_NCQ_PRIO)) == ATA_DFLAG_NCQ_PRIO;
> 
> I'm not sure this needs to test PIO and NCQ_OFF.  This functions
> pretty much can assume that it'd be only called in NCQ context, no?
>

This should only be called in the NCQ context so these checks are redundant
I'll clean this up in the next version of the patches.

> Thanks.
> 
> -- 
> tejun

Take care,
Adam

^ permalink raw reply

* Re: [PATCH 1/3] block: Add iocontext priority to request
From: Adam Manzanares @ 2016-09-30 16:02 UTC (permalink / raw)
  To: Tejun Heo; +Cc: axboe, linux-block, linux-ide
In-Reply-To: <20160929084059.GB11087@mtj.duckdns.org>

Hello Tejun,

The 09/29/2016 10:40, Tejun Heo wrote:
> Hello,
> 
> On Tue, Sep 27, 2016 at 11:14:54AM -0700, Adam Manzanares wrote:
> > Patch adds association between iocontext and a request.
> > 
> > Signed-off-by: Adam Manzanares <adam.manzanares@hgst.com>
> 
> Can you please describe how this may impact existing usages?
>
I'll start with the changes I made and work my way through a grep of            
ioprio. Please add or correct any of the assumptions I have made.               
                                                                                
In blk-core, the behavior before the patch is to get the ioprio for the request 
from the bio. The only references I found to bio_set_prio are in bcache. Both   
of these references are in low priority operations (gc, bg writeback) so the    
iopriority of the bio is set to IO_PRIOCLASS_IDLE in these cases.               
                                                                                
A kernel thread is used to submit these bios so the ioprio is going to come     
from the current running task if the iocontext exists. This could be a problem  
if we have set a task with high priority and some background work ends up       
getting generated in the bcache layer. I propose that we check if the           
iopriority of the bio is valid and if so, then we keep the priorirty from the   
bio.                                                                            
                                                                                
The second area that I see a potential problem is in the merging code code in   
blk-core when a bio is queued. If there is a request that is mergeable then     
the merge code takes the highest priority of the bio and the request. This      
could wipe out the values set by bio_set_prio. I think it would be              
best to set the request as non mergeable when we see that it is a high          
priority IO request.                                                            
                                                                                
The third area that is of interest is in the CFQ scheduler and the ioprio is    
only used in the case of async IO and I found that the priority is only         
obtained from the task and not from the request. This leads me to believe that  
the changes made in the blk-core to add the priority to the request will not    
impact the CFQ scheduler. 

The fourth area that might be concerning is the drivers. virtio_block copies    
the request priority into a virtual block request. I am assuming that this      
eventually makes it to another device driver so we don't need to worry about    
this. null block device driver also uses the ioprio, but this is also not a     
concern. lightnvm also sets the ioprio to build a request that is passed onto   
another driver. The last driver that uses the request ioprio is the fusion      
mptsas driver and I don't understand how it is using the ioprio. From what I    
can tell it is taking a request of IOPRIO_CLASS_NONE with data of 0x7 and       
calling this high priority IO. This could be impacted by the code I have        
proposed, but I believe the authors intended to treat this particular ioprio    
value as high priority. The driver will pass the request to the device          
with high priority if the appropriate ioprio values is seen on the request.     
                                                                                
The fifth area that I noticed may be impacted is file systems. btrfs uses low   
priority IO for read ahead. Ext4 uses ioprio for journaling. Both of these      
issues are not a problem because the ioprio is set on the task and not on a     
bio.

> Thanks.
> 
> -- 
> tejun

Take care,
Adam

^ permalink raw reply

* [PATCH] ahci: qoriq: Revert "ahci: qoriq: Disable NCQ on ls2080a SoC"
From: yuantian.tang @ 2016-09-30  6:12 UTC (permalink / raw)
  To: tj, linux-ide; +Cc: Tang Yuantian, Tang Yuantian

From: Tang Yuantian <Yuantian.Tang@nxp.com>

This reverts commit 640847298e2b7f19 ("ahci: qoriq: Disable NCQ
on ls2080a SoC")

The erratum has been fixed in ls2080a v2.0 and later soc.
In reality, customer will not get any ls2080a v1.0 soc. Neither apply
to any products. So reverting this commit won't create any side effect.

Blacklisting v2.0 could also be a option, but that needs to check the
soc version which is not suitable in the driver.

Signed-off-by: Tang Yuantian <yuantian.tang@nxp.com>
---
 drivers/ata/ahci_qoriq.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/ata/ahci_qoriq.c b/drivers/ata/ahci_qoriq.c
index 925c4b6..1eba8df 100644
--- a/drivers/ata/ahci_qoriq.c
+++ b/drivers/ata/ahci_qoriq.c
@@ -136,7 +136,7 @@ static struct ata_port_operations ahci_qoriq_ops = {
 	.hardreset	= ahci_qoriq_hardreset,
 };
 
-static struct ata_port_info ahci_qoriq_port_info = {
+static const struct ata_port_info ahci_qoriq_port_info = {
 	.flags		= AHCI_FLAG_COMMON | ATA_FLAG_NCQ,
 	.pio_mask	= ATA_PIO4,
 	.udma_mask	= ATA_UDMA6,
@@ -221,12 +221,6 @@ static int ahci_qoriq_probe(struct platform_device *pdev)
 	if (rc)
 		goto disable_resources;
 
-	/* Workaround for ls2080a */
-	if (qoriq_priv->type == AHCI_LS2080A) {
-		hpriv->flags |= AHCI_HFLAG_NO_NCQ;
-		ahci_qoriq_port_info.flags &= ~ATA_FLAG_NCQ;
-	}
-
 	rc = ahci_platform_init_host(pdev, hpriv, &ahci_qoriq_port_info,
 				     &ahci_qoriq_sht);
 	if (rc)
-- 
2.1.0.27.g96db324


^ permalink raw reply related

* Re: [PATCH v2] ahci: qoriq: Revert "ahci: qoriq: Disable NCQ on ls2080a SoC"
From: Tejun Heo @ 2016-09-30  8:30 UTC (permalink / raw)
  To: yuantian.tang; +Cc: linux-ide
In-Reply-To: <1475215993-329-1-git-send-email-yuantian.tang@nxp.com>

On Fri, Sep 30, 2016 at 02:13:13PM +0800, yuantian.tang@nxp.com wrote:
> From: Tang Yuantian <Yuantian.Tang@nxp.com>
> 
> This reverts commit 640847298e2b7f19 ("ahci: qoriq: Disable NCQ
> on ls2080a SoC")
> 
> The erratum has been fixed in ls2080a v2.0 and later soc.
> In reality, customer will not get any ls2080a v1.0 soc. Neither apply
> to any products. So reverting this commit won't create any side effect.
> 
> Blacklisting v2.0 could also be a option, but that needs to check the
> soc version which is not suitable in the driver.
>
> Signed-off-by: Tang Yuantian <yuantian.tang@nxp.com>

Applied to libata/for-4.9.

Thanks.

-- 
tejun

^ permalink raw reply

* [PATCH v2] ahci: qoriq: Revert "ahci: qoriq: Disable NCQ on ls2080a SoC"
From: yuantian.tang @ 2016-09-30  6:13 UTC (permalink / raw)
  To: tj, linux-ide; +Cc: Tang Yuantian, Tang Yuantian

From: Tang Yuantian <Yuantian.Tang@nxp.com>

This reverts commit 640847298e2b7f19 ("ahci: qoriq: Disable NCQ
on ls2080a SoC")

The erratum has been fixed in ls2080a v2.0 and later soc.
In reality, customer will not get any ls2080a v1.0 soc. Neither apply
to any products. So reverting this commit won't create any side effect.

Blacklisting v2.0 could also be a option, but that needs to check the
soc version which is not suitable in the driver.

Signed-off-by: Tang Yuantian <yuantian.tang@nxp.com>
---
v2:
	- refined the commit message

 drivers/ata/ahci_qoriq.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/ata/ahci_qoriq.c b/drivers/ata/ahci_qoriq.c
index 925c4b6..1eba8df 100644
--- a/drivers/ata/ahci_qoriq.c
+++ b/drivers/ata/ahci_qoriq.c
@@ -136,7 +136,7 @@ static struct ata_port_operations ahci_qoriq_ops = {
 	.hardreset	= ahci_qoriq_hardreset,
 };
 
-static struct ata_port_info ahci_qoriq_port_info = {
+static const struct ata_port_info ahci_qoriq_port_info = {
 	.flags		= AHCI_FLAG_COMMON | ATA_FLAG_NCQ,
 	.pio_mask	= ATA_PIO4,
 	.udma_mask	= ATA_UDMA6,
@@ -221,12 +221,6 @@ static int ahci_qoriq_probe(struct platform_device *pdev)
 	if (rc)
 		goto disable_resources;
 
-	/* Workaround for ls2080a */
-	if (qoriq_priv->type == AHCI_LS2080A) {
-		hpriv->flags |= AHCI_HFLAG_NO_NCQ;
-		ahci_qoriq_port_info.flags &= ~ATA_FLAG_NCQ;
-	}
-
 	rc = ahci_platform_init_host(pdev, hpriv, &ahci_qoriq_port_info,
 				     &ahci_qoriq_sht);
 	if (rc)
-- 
2.1.0.27.g96db324


^ permalink raw reply related

* Re: ATA failure regression
From: tj @ 2016-09-29 11:33 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Shah, Nehal-bakulchandra, Bharat Kumar Gogada, Deucher, Alexander,
	linux-pci@vger.kernel.org, holler@ahsoftware.de,
	linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org,
	Dan Williams
In-Reply-To: <20160929112558.GA24350@khazad-dum.debian.net>

Hello,

(cc'ing Dan, hi!)

On Thu, Sep 29, 2016 at 08:25:58AM -0300, Henrique de Moraes Holschuh wrote:
> Actually, according to Shah's post from the 26th to this thread,
> everything works just fine even with the IOMMU enabled, as long as the
> system is configured to NOT auto-shutdown unused SATA ports.
> 
> That seems to give some extra avenues at fixing/working around the
> issue, maybe?

Hmm... it could be that the multiple msix routing gets screwed when
the bios disables unoccupied ports.  Dan, can you please look into
this?  Shah is seeing ahci failing during boot on an AMD platform with
msi enabled.  If disabling of unoccupied ports is turned off in bios,
the problem goes away.  Maybe we're mapping the irqs incorrectly in
such cases?

Thanks.

-- 
tejun

^ permalink raw reply

* Re: ATA failure regression
From: Henrique de Moraes Holschuh @ 2016-09-29 11:25 UTC (permalink / raw)
  To: tj@kernel.org
  Cc: Shah, Nehal-bakulchandra, Bharat Kumar Gogada, Deucher, Alexander,
	linux-pci@vger.kernel.org, holler@ahsoftware.de,
	linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20160929082229.GA11087@mtj.duckdns.org>

On Thu, 29 Sep 2016, tj@kernel.org wrote:
> On Wed, Sep 28, 2016 at 05:45:08AM +0000, Shah, Nehal-bakulchandra wrote:
> > Can someone please help me to debug this issue?
> 
> The only thing I can do from libata side is disbling msi on the
> affected platform, but the problem doesn't seem confined to ahci, so
> probably the right thing to do for now is disabling msi on the
> platform?

Actually, according to Shah's post from the 26th to this thread,
everything works just fine even with the IOMMU enabled, as long as the
system is configured to NOT auto-shutdown unused SATA ports.

That seems to give some extra avenues at fixing/working around the
issue, maybe?

-- 
  Henrique Holschuh

^ permalink raw reply

* Re: [PATCH] ahci: qoriq: Revert "ahci: qoriq: Disable NCQ on ls2080a SoC"
From: Tejun Heo @ 2016-09-29  9:58 UTC (permalink / raw)
  To: Y.T. Tang; +Cc: linux-ide@vger.kernel.org
In-Reply-To: <DB6PR0402MB2837BA24F4CB8D137FA16574F0CE0@DB6PR0402MB2837.eurprd04.prod.outlook.com>

Hello,

On Thu, Sep 29, 2016 at 09:07:18AM +0000, Y.T. Tang wrote:
> In reality, customer will not get any ls2080a v1.0 soc. Neither
> apply to any products. If blacklisting v2.0, we need to check the
> soc version which is not suitable in the driver.  Anyway, reverting
> this commit won't create any side effect.

Can you please include this in the commit description?

Thanks.

-- 
tejun

^ permalink raw reply

* RE: [PATCH] ahci: qoriq: Revert "ahci: qoriq: Disable NCQ on ls2080a SoC"
From: Y.T. Tang @ 2016-09-29  9:07 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-ide@vger.kernel.org
In-Reply-To: <20160929085525.GE11087@mtj.duckdns.org>



> -----Original Message-----
> From: Tejun Heo [mailto:htejun@gmail.com] On Behalf Of Tejun Heo
> Sent: Thursday, September 29, 2016 4:55 PM
> To: Y.T. Tang <yuantian.tang@nxp.com>
> Cc: linux-ide@vger.kernel.org
> Subject: Re: [PATCH] ahci: qoriq: Revert "ahci: qoriq: Disable NCQ on ls2080a
> SoC"
> 
> Hello,
> 
> On Wed, Sep 28, 2016 at 09:46:12AM +0800, yuantian.tang@nxp.com wrote:
> > From: Tang Yuantian <Yuantian.Tang@nxp.com>
> >
> > This reverts commit 640847298e2b7f19 ("ahci: qoriq: Disable NCQ on
> > ls2080a SoC")
> >
> > The erratum has been fixed in ls2080a v2.0 and later soc.
> > Since ls2080a v1.0 has rarely been released, it can be safely
> > reverted.
> 
> Hmm... going by the description, I'm not sure whether this is a good idea.
> What's "rarely been released"?  Are there chips out in the wild?  Can't we
> blacklist v2.0+ only?
> 
In reality, customer will not get any ls2080a v1.0 soc. Neither apply to any products. If blacklisting v2.0, we need to check the soc version which is not suitable in the driver.
Anyway, reverting this commit won't create any side effect.

Regards,
Yuantian

> Thanks.
> 
> --
> tejun

^ permalink raw reply

* Re: [PATCH] ahci: qoriq: Revert "ahci: qoriq: Disable NCQ on ls2080a SoC"
From: Tejun Heo @ 2016-09-29  8:55 UTC (permalink / raw)
  To: yuantian.tang; +Cc: linux-ide
In-Reply-To: <1475027172-21012-1-git-send-email-yuantian.tang@nxp.com>

Hello,

On Wed, Sep 28, 2016 at 09:46:12AM +0800, yuantian.tang@nxp.com wrote:
> From: Tang Yuantian <Yuantian.Tang@nxp.com>
> 
> This reverts commit 640847298e2b7f19 ("ahci: qoriq: Disable NCQ
> on ls2080a SoC")
> 
> The erratum has been fixed in ls2080a v2.0 and later soc.
> Since ls2080a v1.0 has rarely been released, it can be safely
> reverted.

Hmm... going by the description, I'm not sure whether this is a good
idea.  What's "rarely been released"?  Are there chips out in the
wild?  Can't we blacklist v2.0+ only?

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH 0/3] Enabling ATA Command Priorities
From: tj @ 2016-09-29  8:48 UTC (permalink / raw)
  To: Adam Manzanares
  Cc: Christoph Hellwig, axboe@kernel.dk, linux-block@vger.kernel.org,
	linux-ide@vger.kernel.org
In-Reply-To: <5C8FF242-7C7B-442E-A6FF-89878366F154@hgst.com>

Hello,

On Wed, Sep 28, 2016 at 03:43:32AM +0000, Adam Manzanares wrote:
> I prefer having the feature conditional so you can use the CFQ
> scheduler with I/O priorities as is. If you decide to enable the
> feature then the priorities will be passed down to the drive in
> addition to the work that the CFQ scheduler does. Since this feature
> may change the user perceived performance of the device I want to
> make sure they know what they are getting into.

Yeah, I prefer to have it behind an explicit flag given the history of
optional ATA features.  The feature is unlikely to matter to a lot of
people and is almost bound to break existing RT prio usages.

Thanks.

-- 
tejun

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox