* 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
* 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: 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
* 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
* [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
* [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 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
* 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
* 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 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
* [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
* [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
* [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-10 13:04 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
Tejun Heo <tj@kernel.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |tj@kernel.org
--- Comment #1 from Tejun Heo <tj@kernel.org> ---
Does booting with "irqpoll" boot option change anything?
--
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: Adam Manzanares @ 2016-10-10 20:37 UTC (permalink / raw)
To: Jeff Moyer; +Cc: axboe, tj, linux-block, linux-ide
In-Reply-To: <x49d1jdi7lz.fsf@segfault.boston.devel.redhat.com>
Hello Jeff,
The 10/06/2016 15:46, Jeff Moyer wrote:
> 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.
>
I have a strong preference for making this a tunable for the following
reason. I am concerned that this could negatively impact performance if this
feature is not properly implemented on a device. In addition, this feature
can make a dramatic difference in the performance of prioritized vs
non-prioritized IO. Priority IO is improved, but it comes at the cost of
non-prioritized IO. If someone has tuned a system in such a way that things
work well as is, I do not want to cause any surprises.
I can see the argument for not having the tunable in the block layer, but
then we need to add a tunable to all request based drivers that may leverage
the iopriority information. This has the potential to generate a lot more
code and documentation. I also would like to use the tunable when the
iopriority is set on the request so we can preserve the default behavior.
This can be a concern when we have drivers that use request iopriority
information, such as the fusion mptsas driver.
I will also document the tunable :) if we agree that it is necessary.
> > @@ -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.
>
Ouch, this will be cleaned up in the next revision.
> > @@ -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;
>
I caught this in the explanation of the first patch I sent out. I am still
assuming that this will be a tunable, but I will have the bio_prio take
precedence in the next patch.
> Finally, please re-order your series as Hannes suggested.
Will do.
>
> Thanks!
> Jeff
Take care,
Adam
^ permalink raw reply
* [PATCH v3 1/2] block: Add iocontext priority to request
From: Adam Manzanares @ 2016-10-11 20:40 UTC (permalink / raw)
To: axboe, tj; +Cc: linux-block, linux-ide, Adam Manzanares
In-Reply-To: <1476218427-4021-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>
---
Documentation/block/queue-sysfs.txt | 12 ++++++++++++
block/blk-core.c | 5 +++++
block/blk-sysfs.c | 32 ++++++++++++++++++++++++++++++++
include/linux/blkdev.h | 3 +++
4 files changed, 52 insertions(+)
diff --git a/Documentation/block/queue-sysfs.txt b/Documentation/block/queue-sysfs.txt
index 2a39040..3ca4e8f 100644
--- a/Documentation/block/queue-sysfs.txt
+++ b/Documentation/block/queue-sysfs.txt
@@ -144,6 +144,18 @@ For storage configurations that need to maximize distribution of completion
processing setting this option to '2' forces the completion to run on the
requesting cpu (bypassing the "group" aggregation logic).
+rq_ioc_prio (RW)
+----------------
+If this option is '1', and there is a valid iocontext associated with the
+issuing context, and the bio we are processing does not have a valid
+prio, then we save the prio value from the iocontext with the request.
+
+This feature can be combined with device drivers that are aware of prio
+values in order to handle prio accordingly. An example would be if the ata
+layer recognizes prio and creates ata commands with high priority and sends
+them to the device. If the hardware supports priorities for commands then
+this has the potential to speed up response times for high priority IO.
+
scheduler (RW)
--------------
When read, this file will display the current and available IO schedulers
diff --git a/block/blk-core.c b/block/blk-core.c
index 14d7c07..2e740c4 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 @@ unsigned int blk_plug_queued_count(struct request_queue *q)
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;
@@ -1657,6 +1659,9 @@ 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_rq_ioc_prio(req->q) && !ioprio_valid(req->ioprio) && ioc)
+ req->ioprio = ioc->ioprio;
+
blk_rq_bio_prep(req->q, req, bio);
}
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 9cc8d7c..a9c5105 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_rq_ioc_prio_show(struct request_queue *q, char *page)
+{
+ return queue_var_show(blk_queue_rq_ioc_prio(q), page);
+}
+
+static ssize_t queue_rq_ioc_prio_store(struct request_queue *q,
+ const char *page, size_t count)
+{
+ unsigned long rq_ioc_prio_on;
+ ssize_t ret;
+
+ ret = queue_var_store(&rq_ioc_prio_on, page, count);
+ if (ret < 0)
+ return ret;
+
+ spin_lock_irq(q->queue_lock);
+ if (rq_ioc_prio_on)
+ queue_flag_set(QUEUE_FLAG_RQ_IOC_PRIO, q);
+ else
+ queue_flag_clear(QUEUE_FLAG_RQ_IOC_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_rq_ioc_prio_entry = {
+ .attr = {.name = "rq_ioc_prio", .mode = S_IRUGO | S_IWUSR },
+ .show = queue_rq_ioc_prio_show,
+ .store = queue_rq_ioc_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_rq_ioc_prio_entry.attr,
NULL,
};
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index c47c358..63b842a 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_RQ_IOC_PRIO 27 /* Use iocontext ioprio */
#define QUEUE_FLAG_DEFAULT ((1 << QUEUE_FLAG_IO_STAT) | \
(1 << QUEUE_FLAG_STACKABLE) | \
@@ -595,6 +596,8 @@ 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_rq_ioc_prio(q) \
+ test_bit(QUEUE_FLAG_RQ_IOC_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 v3 2/2] ata: Enabling ATA Command Priorities
From: Adam Manzanares @ 2016-10-11 20:40 UTC (permalink / raw)
To: axboe, tj; +Cc: linux-block, linux-ide, Adam Manzanares
In-Reply-To: <1476218427-4021-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..4304694 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_rq_ioc_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 v3 0/2] Enabling ATA Command Priorities
From: Adam Manzanares @ 2016-10-11 20:40 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 rq_ioc_prio. If this feature is enabled, and the submission
iocontext exists, and the bio_prio is not valid then the request ioprio is
set to the iocontext prio.
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 (2):
block: Add iocontext priority to request
ata: Enabling ATA Command Priorities
Documentation/block/queue-sysfs.txt | 12 ++++++++++++
block/blk-core.c | 5 +++++
block/blk-sysfs.c | 32 ++++++++++++++++++++++++++++++++
drivers/ata/libata-core.c | 35 ++++++++++++++++++++++++++++++++++-
drivers/ata/libata-scsi.c | 10 +++++++++-
drivers/ata/libata.h | 2 +-
include/linux/ata.h | 6 ++++++
include/linux/blkdev.h | 3 +++
include/linux/libata.h | 18 ++++++++++++++++++
9 files changed, 120 insertions(+), 3 deletions(-)
--
2.1.4
^ permalink raw reply
* [PATCH] Fix interface autodetection in legacy IDE driver (trial #2)
From: Luiz Carlos Ramos @ 2016-10-12 1:12 UTC (permalink / raw)
To: linux-ide@vger.kernel.org
Hello,
This humble patch was sent one or two months before, and had no actions,
except for a colleague reply which friendly pointed out some formatting
problems (which were solved in a second message).
It relates to an old code, the legacy IDE driver, but the bug it
addresses is real. The code, although rarely used, is
still there to be compiled if one chooses to do so (like me).
Also, the fix has a very low risk of present collateral effects IMHO.
It is already compiled and tested in some embedded machines.
So, again IMHO, it is worth be fixed.
This email is a second trial with it. I hope it can help the one or two
guys out there which are still running the legacy IDE driver and
haven't noticed the former email.
Best regards,
Signed-off-by: Luiz Carlos Ramos <lramos.prof@yahoo.com.br>
---
drivers/ide/ide-generic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
index 54d7c4685d23aa5e62ce606e7b994a57bb54b08a..419818a39c270d3ad219e8f7b5df56a9aea3d640 100644
--- a/drivers/ide/ide-generic.c
+++ b/drivers/ide/ide-generic.c
@@ -96,10 +96,10 @@ static int __init ide_generic_init(void)
printk(KERN_INFO DRV_NAME ": please use \"probe_mask=0x3f\" "
"module parameter for probing all legacy ISA IDE ports\n");
- if (primary == 0)
+ if (primary)
probe_mask |= 0x1;
- if (secondary == 0)
+ if (secondary)
probe_mask |= 0x2;
} else
printk(KERN_INFO DRV_NAME ": enforcing probing of I/O ports "
--
2.8.2
^ permalink raw reply related
* Re: [PATCH v3 1/2] block: Add iocontext priority to request
From: Jens Axboe @ 2016-10-12 14:49 UTC (permalink / raw)
To: Adam Manzanares, tj; +Cc: linux-block, linux-ide
In-Reply-To: <1476218427-4021-2-git-send-email-adam.manzanares@hgst.com>
On 10/11/2016 02:40 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.
Honestly, I don't get this patchset. For the normal file system path, we
inherit the iocontext priority into the bio. That in turns gets
inherited by the request. Why is this any different?
It'd be much cleaner to just have 'rq' inherit the IO priority from the
io context when the 'rq' is allocated, and ensuring that we inherit and
override this if the bio has one set instead. It should already work
like that.
And in no way should we add some sysfs file to control this, that is
nuts.
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH v3 1/2] block: Add iocontext priority to request
From: Adam Manzanares @ 2016-10-12 17:58 UTC (permalink / raw)
To: Jens Axboe; +Cc: Adam Manzanares, tj, linux-block, linux-ide
In-Reply-To: <4b60fe5d-113f-9361-bbae-7473002500ba@kernel.dk>
The 10/12/2016 08:49, Jens Axboe wrote:
> On 10/11/2016 02:40 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.
>
> Honestly, I don't get this patchset. For the normal file system path, we
> inherit the iocontext priority into the bio. That in turns gets
> inherited by the request. Why is this any different?
>
I was hoping this was true before I started looking into this, but the
iocontext priority is not set on the bio. I did look into setting the
iocontext priority on the bio, and this would have do be done close
in the call stack to init_request_from_bio so I chose to set it here.
If I missed where this occurs I would appreciate it if you pointed me to the
code where the bio gets the iopriority from the iocontext.
> It'd be much cleaner to just have 'rq' inherit the IO priority from the
> io context when the 'rq' is allocated, and ensuring that we inherit and
> override this if the bio has one set instead. It should already work
> like that.
I looked into the request allocation code and the only place I see where
the iocontext is associated with the request is through a icq. Looking at
the documentation of the icq it states that the icq_size of the elevator
has to be set in order for block core to manage this. The only scheduler
using this currently is the cfq scheduler and I think prioritized requests
should be independent of the scheduler used.
I agree that this should make it into the code where the rq is allocated.
Again if I have missed something please point me to the relevant code and
I will modify the patch as necessary.
>
> And in no way should we add some sysfs file to control this, that is
> nuts.
My concern is that we will now be exposing priority information to lower
layers and if there happens to be code that uses the priority now it will
actually make an impact. My example being the fusion mptsas driver.
The second issue I foresee is that lower level drivers will need a sysfs
file to control whether or not we send prioritized commands to the device.
We are wary of sending prioritized commands by default when we are unsure
of how the device will respond to these prioritized commands.
The reason I propose a sysfs file in the queue is that it solves these two
issues at the same time.
I would appreciate it if you could suggest an alternative fix for these issues
or an explanation of why these concerns are not valid.
>
> --
> Jens Axboe
>
Take care,
Adam
^ permalink raw reply
* Re: [PATCH v3 1/2] block: Add iocontext priority to request
From: Jens Axboe @ 2016-10-12 21:05 UTC (permalink / raw)
To: Adam Manzanares; +Cc: Adam Manzanares, tj, linux-block, linux-ide
In-Reply-To: <20161012175830.GA4072@hgst.com>
On 10/12/2016 11:58 AM, Adam Manzanares wrote:
> The 10/12/2016 08:49, Jens Axboe wrote:
>> On 10/11/2016 02:40 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.
>>
>> Honestly, I don't get this patchset. For the normal file system path, we
>> inherit the iocontext priority into the bio. That in turns gets
>> inherited by the request. Why is this any different?
>>
> I was hoping this was true before I started looking into this, but the
> iocontext priority is not set on the bio. I did look into setting the
> iocontext priority on the bio, and this would have do be done close
> in the call stack to init_request_from_bio so I chose to set it here.
>
> If I missed where this occurs I would appreciate it if you pointed me
> to the code where the bio gets the iopriority from the iocontext.
It currently happens in CFQ, since that is what deals with priorities.
But we should just make that the case, generically. The rule is that if
priority is set in a bio, that is what we'll use. But if not, we'll
use what the application has set in general, and that is available in
the io context.
>> It'd be much cleaner to just have 'rq' inherit the IO priority from the
>> io context when the 'rq' is allocated, and ensuring that we inherit and
>> override this if the bio has one set instead. It should already work
>> like that.
>
> I looked into the request allocation code and the only place I see where
> the iocontext is associated with the request is through a icq. Looking at
> the documentation of the icq it states that the icq_size of the elevator
> has to be set in order for block core to manage this. The only scheduler
> using this currently is the cfq scheduler and I think prioritized requests
> should be independent of the scheduler used.
Agree, see above, it should just happen generically.
>> And in no way should we add some sysfs file to control this, that is
>> nuts.
>
> My concern is that we will now be exposing priority information to lower
> layers and if there happens to be code that uses the priority now it will
> actually make an impact. My example being the fusion mptsas driver.
>
> The second issue I foresee is that lower level drivers will need a sysfs
> file to control whether or not we send prioritized commands to the device.
> We are wary of sending prioritized commands by default when we are unsure
> of how the device will respond to these prioritized commands.
>
> The reason I propose a sysfs file in the queue is that it solves these two
> issues at the same time.
>
> I would appreciate it if you could suggest an alternative fix for
> these issues or an explanation of why these concerns are not valid.
At that point it should be a driver knob, it's not something that should
be part of the generic block layer.
--
Jens Axboe
^ permalink raw reply
* [PATCH v4 0/4] Enabling ATA Command Priorities
From: Adam Manzanares @ 2016-10-13 19:53 UTC (permalink / raw)
To: axboe, tj, dan.j.williams, hare, martin.petersen, mchristi,
toshi.kani, ming.lei, sathya.prakash, chaitra.basappa,
suganath-prabu.subramani
Cc: linux-block, linux-ide, linux-kernel, MPT-FusionLinux.pdl,
linux-scsi, 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 required setting the iocontext ioprio on the request when
the request 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 enable_prio device
attribute to 1. An ATA device is also checked to see if the device supports per
command priority.
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
fusion: remove iopriority handling
ata: Enabling ATA Command Priorities
ata: ATA Command Priority Disabled By Default
block/blk-core.c | 4 ++-
drivers/ata/libahci.c | 1 +
drivers/ata/libata-core.c | 35 +++++++++++++++++-
drivers/ata/libata-scsi.c | 74 ++++++++++++++++++++++++++++++++++++++-
drivers/ata/libata.h | 2 +-
drivers/message/fusion/mptscsih.c | 5 ---
include/linux/ata.h | 6 ++++
include/linux/blkdev.h | 14 ++++++++
include/linux/libata.h | 26 ++++++++++++++
9 files changed, 158 insertions(+), 9 deletions(-)
--
2.1.4
^ permalink raw reply
* [PATCH v4 1/4] block: Add iocontext priority to request
From: Adam Manzanares @ 2016-10-13 19:53 UTC (permalink / raw)
To: axboe, tj, dan.j.williams, hare, martin.petersen, mchristi,
toshi.kani, ming.lei, sathya.prakash, chaitra.basappa,
suganath-prabu.subramani
Cc: linux-block, linux-ide, linux-kernel, MPT-FusionLinux.pdl,
linux-scsi, Adam Manzanares, Adam Manzananares
In-Reply-To: <1476388433-2539-1-git-send-email-adam.manzanares@hgst.com>
Patch adds an association between iocontext ioprio and the ioprio of a
request. This value 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 14d7c07..361b1b9 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1153,6 +1153,7 @@ static struct request *__get_request(struct request_list *rl, int op,
blk_rq_init(q, rq);
blk_rq_set_rl(rq, rl);
+ blk_rq_set_prio(rq, ioc);
req_set_op_attrs(rq, op, op_flags | REQ_ALLOCED);
/* init elvpriv */
@@ -1656,7 +1657,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 c47c358..9a0ceaa 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -934,6 +934,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.1.4
^ permalink raw reply related
* [PATCH v4 2/4] fusion: remove iopriority handling
From: Adam Manzanares @ 2016-10-13 19:53 UTC (permalink / raw)
To: axboe, tj, dan.j.williams, hare, martin.petersen, mchristi,
toshi.kani, ming.lei, sathya.prakash, chaitra.basappa,
suganath-prabu.subramani
Cc: linux-block, linux-ide, linux-kernel, MPT-FusionLinux.pdl,
linux-scsi, Adam Manzanares, Adam Manzanares
In-Reply-To: <1476388433-2539-1-git-send-email-adam.manzanares@hgst.com>
The request priority is now by default coming from the ioc. It was not
clear what this code was trying to do based upon the iopriority class or
data. The driver should check that a device supports priorities and use
them according to the specificiations of ioprio.
Signed-off-by: Adam Manzanares <adam.manzanares@wdc.com>
---
drivers/message/fusion/mptscsih.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/message/fusion/mptscsih.c b/drivers/message/fusion/mptscsih.c
index 6c9fc11..4740bb6 100644
--- a/drivers/message/fusion/mptscsih.c
+++ b/drivers/message/fusion/mptscsih.c
@@ -1369,11 +1369,6 @@ mptscsih_qcmd(struct scsi_cmnd *SCpnt)
if ((vdevice->vtarget->tflags & MPT_TARGET_FLAGS_Q_YES)
&& (SCpnt->device->tagged_supported)) {
scsictl = scsidir | MPI_SCSIIO_CONTROL_SIMPLEQ;
- if (SCpnt->request && SCpnt->request->ioprio) {
- if (((SCpnt->request->ioprio & 0x7) == 1) ||
- !(SCpnt->request->ioprio & 0x7))
- scsictl |= MPI_SCSIIO_CONTROL_HEADOFQ;
- }
} else
scsictl = scsidir | MPI_SCSIIO_CONTROL_UNTAGGED;
--
2.1.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox