linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/4] Enabling ATA Command Priorities
@ 2016-12-06 17:18 Adam Manzanares
  2016-12-06 17:18 ` [PATCH v7 1/4] block: Add iocontext priority to request Adam Manzanares
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Adam Manzanares @ 2016-12-06 17:18 UTC (permalink / raw)
  To: axboe, hare, mchristi, dan.j.williams, martin.petersen,
	toshi.kani, damien.lemoal, ming.lei, tj
  Cc: linux-block, linux-kernel, linux-ide, Adam Manzanares

From: Adam Manzanares <adam.manzanares@wdc.com>

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 requires setting the iocontext 
ioprio on the request when it is initialized

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 for ATA devices by setting the ata ncq_prio_enable 
device attribute to 1. An ATA device is also checked to see if the device 
supports per command priority.

v7:
 - Run ncq prio support check when sysfs variable set (zero day bug fix)
 - Fixes from TJ merged in
 - Merge fix for linux-next incorporated

v6:
 - Removed fusion mpt sas prio related code deletion
 - Renamed ata device attribute that enables priority cmds to the device

v5:
 - Updated block patch commit message to indicate the why and not the how
v4:
 - Added blk_rq_set_prio function to associate request prio with ioc prio
 - In init_request_from_bio use bio_prio if it is valid
 - Added ata enable_prio dev attribute to sysfs to enable prioritized commands

v3:
 - Removed null dereference issue in blk-core
 - Renamed queue sysfs entries for clarity
 - Added documentation for sysfs queue entry

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 (4):
  block: Add iocontext priority to request
  ata: Enabling ATA Command Priorities
  ata: ATA Command Priority Disabled By Default
  ata: set ncq_prio_enabled iff device has support

 block/blk-core.c          |  4 ++-
 drivers/ata/libahci.c     |  1 +
 drivers/ata/libata-core.c | 33 ++++++++++++++++++-
 drivers/ata/libata-scsi.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++-
 drivers/ata/libata.h      |  3 +-
 include/linux/ata.h       |  6 ++++
 include/linux/blkdev.h    | 14 ++++++++
 include/linux/libata.h    |  5 +++
 8 files changed, 146 insertions(+), 4 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v7 1/4] block: Add iocontext priority to request
  2016-12-06 17:18 [PATCH v7 0/4] Enabling ATA Command Priorities Adam Manzanares
@ 2016-12-06 17:18 ` Adam Manzanares
  2016-12-06 17:18 ` [PATCH v7 2/4] ata: Enabling ATA Command Priorities Adam Manzanares
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Adam Manzanares @ 2016-12-06 17:18 UTC (permalink / raw)
  To: axboe, hare, mchristi, dan.j.williams, martin.petersen,
	toshi.kani, damien.lemoal, ming.lei, tj
  Cc: linux-block, linux-kernel, linux-ide, Adam Manzanares

From: Adam Manzanares <adam.manzanares@wdc.com>

Patch adds an association between iocontext ioprio and the ioprio of a
request. This is done to enable request based drivers the ability to
act on priority information stored in the request. An example being
ATA devices that support command priorities. If the ATA driver discovers
that the device supports command priorities and the request has valid
priority information indicating the request is high priority, then a high
priority command can be sent to the device. This should improve tail
latencies for high priority IO on any device that queues requests
internally and can make use of the priority information stored in the
request.

The ioprio of the request is set in blk_rq_set_prio which takes the
request and the ioc as arguments. If the ioc is valid in blk_rq_set_prio
then the iopriority of the request is set as the iopriority of the ioc.
In init_request_from_bio a check is made to see if the ioprio of the bio
is valid and if so then the request prio comes from the bio.

Signed-off-by: Adam Manzananares <adam.manzanares@wdc.com>
---
 block/blk-core.c       |  4 +++-
 include/linux/blkdev.h | 14 ++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 6c4a425..4d1d1f9 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1154,6 +1154,7 @@ static struct request *__get_request(struct request_list *rl, unsigned int op,
 
 	blk_rq_init(q, rq);
 	blk_rq_set_rl(rq, rl);
+	blk_rq_set_prio(rq, ioc);
 	rq->cmd_flags = op;
 	rq->rq_flags = rq_flags;
 
@@ -1658,7 +1659,8 @@ 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 (ioprio_valid(bio_prio(bio)))
+		req->ioprio = bio_prio(bio);
 	blk_rq_bio_prep(req->q, req, bio);
 }
 
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 541fdd8..8c2a1ef 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1050,6 +1050,20 @@ static inline unsigned int blk_rq_count_bios(struct request *rq)
 }
 
 /*
+ * blk_rq_set_prio - associate a request with prio from ioc
+ * @rq: request of interest
+ * @ioc: target iocontext
+ *
+ * Assocate request prio with ioc prio so request based drivers
+ * can leverage priority information.
+ */
+static inline void blk_rq_set_prio(struct request *rq, struct io_context *ioc)
+{
+	if (ioc)
+		rq->ioprio = ioc->ioprio;
+}
+
+/*
  * Request issue related functions.
  */
 extern struct request *blk_peek_request(struct request_queue *q);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v7 2/4] ata: Enabling ATA Command Priorities
  2016-12-06 17:18 [PATCH v7 0/4] Enabling ATA Command Priorities Adam Manzanares
  2016-12-06 17:18 ` [PATCH v7 1/4] block: Add iocontext priority to request Adam Manzanares
@ 2016-12-06 17:18 ` Adam Manzanares
  2016-12-06 17:18 ` [PATCH v7 3/4] ata: ATA Command Priority Disabled By Default Adam Manzanares
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Adam Manzanares @ 2016-12-06 17:18 UTC (permalink / raw)
  To: axboe, hare, mchristi, dan.j.williams, martin.petersen,
	toshi.kani, damien.lemoal, ming.lei, tj
  Cc: linux-block, linux-kernel, linux-ide, Adam Manzanares

From: Adam Manzanares <adam.manzanares@wdc.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 then we build a tf with a high priority command.

This is done to improve the tail latency of commands that are high
priority by passing priority to the device.

tj: Removed trivial ata_ncq_prio_enabled() and open-coded the test.

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

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 223a770..5d16363 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 (dev->flags & ATA_DFLAG_NCQ_PRIO) {
+			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)
 	}
 }
 
+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)
 {
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 9cceb4a..2bccc3c 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"
@@ -1755,6 +1756,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 = IOPRIO_PRIO_CLASS(req_get_ioprio(rq));
 	unsigned int tf_flags = 0;
 	u64 block;
 	u32 n_block;
@@ -1821,7 +1824,8 @@ static unsigned int ata_scsi_rw_xlat(struct ata_queued_cmd *qc)
 	qc->nbytes = n_block * scmd->device->sector_size;
 
 	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..03d0908 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,
@@ -85,6 +85,7 @@ extern int ata_dev_reread_id(struct ata_device *dev, unsigned int readid_flags);
 extern int ata_dev_revalidate(struct ata_device *dev, unsigned int new_class,
 			      unsigned int readid_flags);
 extern int ata_dev_configure(struct ata_device *dev);
+extern void ata_dev_config_ncq_prio(struct ata_device *dev);
 extern int sata_down_spd_limit(struct ata_link *link, u32 spd_limit);
 extern int ata_down_xfermask_limit(struct ata_device *dev, unsigned int sel);
 extern unsigned int ata_dev_set_feature(struct ata_device *dev,
diff --git a/include/linux/ata.h b/include/linux/ata.h
index fdb1803..af6859b 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -348,6 +348,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,
@@ -940,6 +941,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 616eef4..90b69a6 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -166,6 +166,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),
@@ -342,7 +343,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,
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v7 3/4] ata: ATA Command Priority Disabled By Default
  2016-12-06 17:18 [PATCH v7 0/4] Enabling ATA Command Priorities Adam Manzanares
  2016-12-06 17:18 ` [PATCH v7 1/4] block: Add iocontext priority to request Adam Manzanares
  2016-12-06 17:18 ` [PATCH v7 2/4] ata: Enabling ATA Command Priorities Adam Manzanares
@ 2016-12-06 17:18 ` Adam Manzanares
  2016-12-06 17:18 ` [PATCH v7 4/4] ata: set ncq_prio_enabled iff device has support Adam Manzanares
  2016-12-06 18:37 ` [PATCH v7 0/4] Enabling ATA Command Priorities Tejun Heo
  4 siblings, 0 replies; 6+ messages in thread
From: Adam Manzanares @ 2016-12-06 17:18 UTC (permalink / raw)
  To: axboe, hare, mchristi, dan.j.williams, martin.petersen,
	toshi.kani, damien.lemoal, ming.lei, tj
  Cc: linux-block, linux-kernel, linux-ide, Adam Manzanares

From: Adam Manzanares <adam.manzanares@wdc.com>

Add a sysfs entry to turn on priority information being passed
to a ATA device. By default this feature is turned off.

This patch depends on ata: Enabling ATA Command Priorities

tj: Renamed ncq_prio_on to ncq_prio_enable and removed trivial
    ata_ncq_prio_on() and open-coded the test.

Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
---
 drivers/ata/libahci.c     |  1 +
 drivers/ata/libata-core.c |  3 ++-
 drivers/ata/libata-scsi.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/libata.h    |  2 ++
 4 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index 0d028ea..ee7db31 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -140,6 +140,7 @@ EXPORT_SYMBOL_GPL(ahci_shost_attrs);
 struct device_attribute *ahci_sdev_attrs[] = {
 	&dev_attr_sw_activity,
 	&dev_attr_unload_heads,
+	&dev_attr_ncq_prio_enable,
 	NULL
 };
 EXPORT_SYMBOL_GPL(ahci_sdev_attrs);
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 5d16363..f6a631a 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -787,7 +787,8 @@ int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,
 		if (tf->flags & ATA_TFLAG_FUA)
 			tf->device |= 1 << 7;
 
-		if (dev->flags & ATA_DFLAG_NCQ_PRIO) {
+		if ((dev->flags & ATA_DFLAG_NCQ_PRIO) &&
+		    (dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLE)) {
 			if (class == IOPRIO_CLASS_RT)
 				tf->hob_nsect |= ATA_PRIO_HIGH <<
 						 ATA_SHIFT_PRIO;
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 2bccc3c..328a601 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -271,6 +271,74 @@ DEVICE_ATTR(unload_heads, S_IRUGO | S_IWUSR,
 	    ata_scsi_park_show, ata_scsi_park_store);
 EXPORT_SYMBOL_GPL(dev_attr_unload_heads);
 
+static ssize_t ata_ncq_prio_enable_show(struct device *device,
+					struct device_attribute *attr,
+					char *buf)
+{
+	struct scsi_device *sdev = to_scsi_device(device);
+	struct ata_port *ap;
+	struct ata_device *dev;
+	bool ncq_prio_enable;
+	int rc = 0;
+
+	ap = ata_shost_to_port(sdev->host);
+
+	spin_lock_irq(ap->lock);
+	dev = ata_scsi_find_dev(ap, sdev);
+	if (!dev) {
+		rc = -ENODEV;
+		goto unlock;
+	}
+
+	ncq_prio_enable = dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLE;
+
+unlock:
+	spin_unlock_irq(ap->lock);
+
+	return rc ? rc : snprintf(buf, 20, "%u\n", ncq_prio_enable);
+}
+
+static ssize_t ata_ncq_prio_enable_store(struct device *device,
+					 struct device_attribute *attr,
+					 const char *buf, size_t len)
+{
+	struct scsi_device *sdev = to_scsi_device(device);
+	struct ata_port *ap;
+	struct ata_device *dev;
+	long int input;
+	unsigned long flags;
+	int rc;
+
+	rc = kstrtol(buf, 10, &input);
+	if (rc)
+		return rc;
+	if ((input < 0) || (input > 1))
+		return -EINVAL;
+
+	ap = ata_shost_to_port(sdev->host);
+
+	spin_lock_irqsave(ap->lock, flags);
+	dev = ata_scsi_find_dev(ap, sdev);
+	if (unlikely(!dev)) {
+		rc = -ENODEV;
+		goto unlock;
+	}
+
+	if (input)
+		dev->flags |= ATA_DFLAG_NCQ_PRIO_ENABLE;
+	else
+		dev->flags &= ~ATA_DFLAG_NCQ_PRIO_ENABLE;
+
+unlock:
+	spin_unlock_irqrestore(ap->lock, flags);
+
+	return rc ? rc : len;
+}
+
+DEVICE_ATTR(ncq_prio_enable, S_IRUGO | S_IWUSR,
+	    ata_ncq_prio_enable_show, ata_ncq_prio_enable_store);
+EXPORT_SYMBOL_GPL(dev_attr_ncq_prio_enable);
+
 void ata_scsi_set_sense(struct ata_device *dev, struct scsi_cmnd *cmd,
 			u8 sk, u8 asc, u8 ascq)
 {
@@ -402,6 +470,7 @@ EXPORT_SYMBOL_GPL(dev_attr_sw_activity);
 
 struct device_attribute *ata_common_sdev_attrs[] = {
 	&dev_attr_unload_heads,
+	&dev_attr_ncq_prio_enable,
 	NULL
 };
 EXPORT_SYMBOL_GPL(ata_common_sdev_attrs);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 90b69a6..c170be5 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -167,6 +167,7 @@ enum {
 	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_NCQ_PRIO_ENABLE = (1 << 21), /* Priority cmds sent to dev */
 	ATA_DFLAG_INIT_MASK	= (1 << 24) - 1,
 
 	ATA_DFLAG_DETACH	= (1 << 24),
@@ -545,6 +546,7 @@ typedef void (*ata_postreset_fn_t)(struct ata_link *link, unsigned int *classes)
 
 extern struct device_attribute dev_attr_link_power_management_policy;
 extern struct device_attribute dev_attr_unload_heads;
+extern struct device_attribute dev_attr_ncq_prio_enable;
 extern struct device_attribute dev_attr_em_message_type;
 extern struct device_attribute dev_attr_em_message;
 extern struct device_attribute dev_attr_sw_activity;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v7 4/4] ata: set ncq_prio_enabled iff device has support
  2016-12-06 17:18 [PATCH v7 0/4] Enabling ATA Command Priorities Adam Manzanares
                   ` (2 preceding siblings ...)
  2016-12-06 17:18 ` [PATCH v7 3/4] ata: ATA Command Priority Disabled By Default Adam Manzanares
@ 2016-12-06 17:18 ` Adam Manzanares
  2016-12-06 18:37 ` [PATCH v7 0/4] Enabling ATA Command Priorities Tejun Heo
  4 siblings, 0 replies; 6+ messages in thread
From: Adam Manzanares @ 2016-12-06 17:18 UTC (permalink / raw)
  To: axboe, hare, mchristi, dan.j.williams, martin.petersen,
	toshi.kani, damien.lemoal, ming.lei, tj
  Cc: linux-block, linux-kernel, linux-ide, Adam Manzanares

From: Adam Manzanares <adam.manzanares@wdc.com>

We previously had a check to see if the device has support for
prioritized ncq commands and a check to see if a device flag
is set, through a sysfs variable, in order to send a prioritized
command.

This patch only allows the sysfs variable to be set if the device
supports prioritized commands enabling one check in ata_build_rw_tf
in order to determine whether or not to send a prioritized command.

This patch depends on ata: ATA Command Priority Disabled By Default

tj: Minor subject and formatting updates.

Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
---
 drivers/ata/libata-core.c |  3 +--
 drivers/ata/libata-scsi.c | 21 +++++++++++++++------
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index f6a631a..324e76e 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -787,8 +787,7 @@ int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,
 		if (tf->flags & ATA_TFLAG_FUA)
 			tf->device |= 1 << 7;
 
-		if ((dev->flags & ATA_DFLAG_NCQ_PRIO) &&
-		    (dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLE)) {
+		if (dev->flags & ATA_DFLAG_NCQ_PRIO_ENABLE) {
 			if (class == IOPRIO_CLASS_RT)
 				tf->hob_nsect |= ATA_PRIO_HIGH <<
 						 ATA_SHIFT_PRIO;
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 328a601..c24cbf1 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -317,17 +317,26 @@ static ssize_t ata_ncq_prio_enable_store(struct device *device,
 
 	ap = ata_shost_to_port(sdev->host);
 
-	spin_lock_irqsave(ap->lock, flags);
 	dev = ata_scsi_find_dev(ap, sdev);
-	if (unlikely(!dev)) {
-		rc = -ENODEV;
-		goto unlock;
+	if (unlikely(!dev))
+		return  -ENODEV;
+
+	if (!(dev->flags & ATA_DFLAG_NCQ_PRIO)) {
+		if (ata_id_has_ncq_prio(dev->id))
+			ata_dev_config_ncq_prio(dev);
 	}
 
-	if (input)
+	spin_lock_irqsave(ap->lock, flags);
+	if (input) {
+		if (!(dev->flags & ATA_DFLAG_NCQ_PRIO)) {
+			rc = -EOPNOTSUPP;
+			goto unlock;
+		}
+
 		dev->flags |= ATA_DFLAG_NCQ_PRIO_ENABLE;
-	else
+	} else {
 		dev->flags &= ~ATA_DFLAG_NCQ_PRIO_ENABLE;
+	}
 
 unlock:
 	spin_unlock_irqrestore(ap->lock, flags);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v7 0/4] Enabling ATA Command Priorities
  2016-12-06 17:18 [PATCH v7 0/4] Enabling ATA Command Priorities Adam Manzanares
                   ` (3 preceding siblings ...)
  2016-12-06 17:18 ` [PATCH v7 4/4] ata: set ncq_prio_enabled iff device has support Adam Manzanares
@ 2016-12-06 18:37 ` Tejun Heo
  4 siblings, 0 replies; 6+ messages in thread
From: Tejun Heo @ 2016-12-06 18:37 UTC (permalink / raw)
  To: Adam Manzanares
  Cc: axboe, hare, mchristi, dan.j.williams, martin.petersen,
	toshi.kani, damien.lemoal, ming.lei, linux-block, linux-kernel,
	linux-ide, Adam Manzanares

Adam,

On Tue, Dec 06, 2016 at 09:18:01AM -0800, Adam Manzanares wrote:
> From: Adam Manzanares <adam.manzanares@wdc.com>
> 
> 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 requires setting the iocontext 
> ioprio on the request when it is initialized
> 
> 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 for ATA devices by setting the ata ncq_prio_enable 
> device attribute to 1. An ATA device is also checked to see if the device 
> supports per command priority.
> 
> v7:
>  - Run ncq prio support check when sysfs variable set (zero day bug fix)
>  - Fixes from TJ merged in
>  - Merge fix for linux-next incorporated

The previous version has already been merged into libata/for-4.10.
Can you please send me an incremental patch on top of it?  The tree
can be fetched from

 git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata.git for-4.10

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-12-06 18:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-06 17:18 [PATCH v7 0/4] Enabling ATA Command Priorities Adam Manzanares
2016-12-06 17:18 ` [PATCH v7 1/4] block: Add iocontext priority to request Adam Manzanares
2016-12-06 17:18 ` [PATCH v7 2/4] ata: Enabling ATA Command Priorities Adam Manzanares
2016-12-06 17:18 ` [PATCH v7 3/4] ata: ATA Command Priority Disabled By Default Adam Manzanares
2016-12-06 17:18 ` [PATCH v7 4/4] ata: set ncq_prio_enabled iff device has support Adam Manzanares
2016-12-06 18:37 ` [PATCH v7 0/4] Enabling ATA Command Priorities Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).