* Re: block: add a error_count field to struct request
From: Bart Van Assche @ 2017-04-18 22:57 UTC (permalink / raw)
To: hch@bombadil.infradead.org, axboe@kernel.dk
Cc: linux-block@vger.kernel.org, hch@lst.de, konrad.wilk@oracle.com,
roger.pau@citrix.com, linux-scsi@vger.kernel.org,
linux-nvme@lists.infradead.org, jbacik@fb.com,
james.smart@broadcom.com, dm-devel@redhat.com
In-Reply-To: <20170418155229.5977-18-hch@bombadil.infradead.org>
On Tue, 2017-04-18 at 08:52 -0700, Christoph Hellwig wrote:
> From: Christoph Hellwig <hch@lst.de>
>=20
> This is for the legacy floppy and ataflop drivers that currently abuse
> ->errors for this purpose. It's stashed away in a union to not grow
> the struct size, the other fields are either used by modern drivers
> for different purposes or the I/O scheduler before queing the I/O
> to drivers.
>=20
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> include/linux/blkdev.h | 1 +
> 1 file changed, 1 insertion(+)
>=20
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 5986e1250d7d..e618164462e8 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -175,6 +175,7 @@ struct request {
> struct rb_node rb_node; /* sort/lookup */
> struct bio_vec special_vec;
> void *completion_data;
> + int error_count; /* for legacy drivers, don't use */
> };
> =20
> /*
Hello Christoph,
Both blk-mq and the traditional block layer support a .cmd_size field to
make the block layer core allocate driver-specific data at the end of struc=
t
request. Could that mechanism have been used for the error_count field?
Thanks,
Bart.=
^ permalink raw reply
* Re: pd: remove bogus check for req->errors
From: Bart Van Assche @ 2017-04-18 22:59 UTC (permalink / raw)
To: hch@bombadil.infradead.org, axboe@kernel.dk
Cc: linux-block@vger.kernel.org, hch@lst.de, konrad.wilk@oracle.com,
roger.pau@citrix.com, linux-scsi@vger.kernel.org,
linux-nvme@lists.infradead.org, jbacik@fb.com,
james.smart@broadcom.com, dm-devel@redhat.com
In-Reply-To: <20170418155229.5977-22-hch@bombadil.infradead.org>
On Tue, 2017-04-18 at 08:52 -0700, Christoph Hellwig wrote:
> The driver never sets req->errors
Reviewed-by: Bart Van Assche <Bart.VanAssche@sandisk.com>=
^ permalink raw reply
* [PATCH 0/5] Reduce code duplication
From: Bart Van Assche @ 2017-04-18 23:10 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche
Hello Jens,
This series contains three patches intended to reduce code duplication
and two patches that simplify / optimize I/O priority handling. Please
consider these patches for kernel v4.12.
Thanks,
Bart.
Bart Van Assche (5):
block: Export blk_init_request_from_bio()
null_blk: Use blk_init_request_from_bio() instead of open-coding it
lightnvm: Use blk_init_request_from_bio() instead of open-coding it
block: Inline blk_rq_set_prio()
block: Optimize ioprio_best()
block/blk-core.c | 12 +++++++-----
block/blk-mq.c | 2 +-
block/blk.h | 1 -
block/ioprio.c | 12 +-----------
drivers/block/null_blk.c | 9 +--------
drivers/nvme/host/lightnvm.c | 6 +-----
include/linux/blkdev.h | 15 +--------------
7 files changed, 12 insertions(+), 45 deletions(-)
--
2.12.2
^ permalink raw reply
* [PATCH 1/5] block: Export blk_init_request_from_bio()
From: Bart Van Assche @ 2017-04-18 23:10 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Bart Van Assche, Christoph Hellwig,
Matias Bjørling, Adam Manzanares
In-Reply-To: <20170418231037.3968-1-bart.vanassche@sandisk.com>
Export this function such that it becomes available to block
drivers.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Matias Bjørling <m@bjorling.me>
Cc: Adam Manzanares <adam.manzanares@wdc.com>
---
block/blk-core.c | 5 +++--
block/blk-mq.c | 2 +-
block/blk.h | 1 -
include/linux/blkdev.h | 1 +
4 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index ab1ecf71cb27..c274aed2ca3f 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1633,7 +1633,7 @@ unsigned int blk_plug_queued_count(struct request_queue *q)
return ret;
}
-void init_request_from_bio(struct request *req, struct bio *bio)
+void blk_init_request_from_bio(struct request *req, struct bio *bio)
{
if (bio->bi_opf & REQ_RAHEAD)
req->cmd_flags |= REQ_FAILFAST_MASK;
@@ -1645,6 +1645,7 @@ void init_request_from_bio(struct request *req, struct bio *bio)
req->ioprio = bio_prio(bio);
blk_rq_bio_prep(req->q, req, bio);
}
+EXPORT_SYMBOL(blk_init_request_from_bio);
static blk_qc_t blk_queue_bio(struct request_queue *q, struct bio *bio)
{
@@ -1735,7 +1736,7 @@ static blk_qc_t blk_queue_bio(struct request_queue *q, struct bio *bio)
* We don't worry about that case for efficiency. It won't happen
* often, and the elevators are able to handle it.
*/
- init_request_from_bio(req, bio);
+ blk_init_request_from_bio(req, bio);
if (test_bit(QUEUE_FLAG_SAME_COMP, &q->queue_flags))
req->cpu = raw_smp_processor_id();
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 4d8368f657ac..a3a2e9c40453 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1410,7 +1410,7 @@ void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule)
static void blk_mq_bio_to_request(struct request *rq, struct bio *bio)
{
- init_request_from_bio(rq, bio);
+ blk_init_request_from_bio(rq, bio);
blk_account_io_start(rq, true);
}
diff --git a/block/blk.h b/block/blk.h
index 07d375183f31..cc8e61cdc229 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -60,7 +60,6 @@ void blk_free_flush_queue(struct blk_flush_queue *q);
int blk_init_rl(struct request_list *rl, struct request_queue *q,
gfp_t gfp_mask);
void blk_exit_rl(struct request_list *rl);
-void init_request_from_bio(struct request *req, struct bio *bio);
void blk_rq_bio_prep(struct request_queue *q, struct request *rq,
struct bio *bio);
void blk_queue_bypass_start(struct request_queue *q);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 7cadaa0cc0b9..e1ea875ec048 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -924,6 +924,7 @@ extern int blk_register_queue(struct gendisk *disk);
extern void blk_unregister_queue(struct gendisk *disk);
extern blk_qc_t generic_make_request(struct bio *bio);
extern void blk_rq_init(struct request_queue *q, struct request *rq);
+extern void blk_init_request_from_bio(struct request *req, struct bio *bio);
extern void blk_put_request(struct request *);
extern void __blk_put_request(struct request_queue *, struct request *);
extern struct request *blk_get_request(struct request_queue *, int, gfp_t);
--
2.12.2
^ permalink raw reply related
* [PATCH 2/5] null_blk: Use blk_init_request_from_bio() instead of open-coding it
From: Bart Van Assche @ 2017-04-18 23:10 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Bart Van Assche, Christoph Hellwig,
Matias Bjørling, Adam Manzanares
In-Reply-To: <20170418231037.3968-1-bart.vanassche@sandisk.com>
This patch changes the behavior of the null_blk driver for the
LightNVM mode as follows:
* REQ_FAILFAST_MASK is set for read-ahead requests.
* If no I/O priority has been set in the bio, the I/O priority is
copied from the I/O context.
* The rq_disk member is initialized if bio->bi_bdev != NULL.
* req->errors is initialized to zero.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Matias Bjørling <m@bjorling.me>
Cc: Adam Manzanares <adam.manzanares@wdc.com>
---
drivers/block/null_blk.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c
index f93906ff31e8..e79e3d24e229 100644
--- a/drivers/block/null_blk.c
+++ b/drivers/block/null_blk.c
@@ -443,14 +443,7 @@ static int null_lnvm_submit_io(struct nvm_dev *dev, struct nvm_rq *rqd)
if (IS_ERR(rq))
return -ENOMEM;
- rq->__sector = bio->bi_iter.bi_sector;
- rq->ioprio = bio_prio(bio);
-
- if (bio_has_data(bio))
- rq->nr_phys_segments = bio_phys_segments(q, bio);
-
- rq->__data_len = bio->bi_iter.bi_size;
- rq->bio = rq->biotail = bio;
+ blk_init_request_from_bio(rq, bio);
rq->end_io_data = rqd;
--
2.12.2
^ permalink raw reply related
* [PATCH 3/5] lightnvm: Use blk_init_request_from_bio() instead of open-coding it
From: Bart Van Assche @ 2017-04-18 23:10 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Bart Van Assche, Christoph Hellwig,
Matias Bjørling, Adam Manzanares
In-Reply-To: <20170418231037.3968-1-bart.vanassche@sandisk.com>
This patch changes the behavior of the lightnvm driver as follows:
* REQ_FAILFAST_MASK is set for read-ahead requests.
* If no I/O priority has been set in the bio, the I/O priority is
copied from the I/O context.
* The rq_disk member is initialized if bio->bi_bdev != NULL.
* The bio sector offset is copied into req->__sector instead of
retaining the value -1 set by blk_mq_alloc_request().
* req->errors is initialized to zero.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Matias Bjørling <m@bjorling.me>
Cc: Adam Manzanares <adam.manzanares@wdc.com>
---
drivers/nvme/host/lightnvm.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c
index 4b78090518e1..b76e2e36fef4 100644
--- a/drivers/nvme/host/lightnvm.c
+++ b/drivers/nvme/host/lightnvm.c
@@ -511,11 +511,7 @@ static int nvme_nvm_submit_io(struct nvm_dev *dev, struct nvm_rq *rqd)
rq->cmd_flags &= ~REQ_FAILFAST_DRIVER;
if (bio) {
- rq->ioprio = bio_prio(bio);
- rq->__data_len = bio->bi_iter.bi_size;
- rq->bio = rq->biotail = bio;
- if (bio_has_data(bio))
- rq->nr_phys_segments = bio_phys_segments(q, bio);
+ blk_init_request_from_bio(rq, bio);
} else {
rq->ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_NORM);
rq->__data_len = 0;
--
2.12.2
^ permalink raw reply related
* [PATCH 4/5] block: Inline blk_rq_set_prio()
From: Bart Van Assche @ 2017-04-18 23:10 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Bart Van Assche, Christoph Hellwig,
Matias Bjørling
In-Reply-To: <20170418231037.3968-1-bart.vanassche@sandisk.com>
Since only a single caller remains, inline blk_rq_set_prio(). Initialize
req->ioprio even if no I/O priority has been set in the bio nor in the
I/O context.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Adam Manzanares <adam.manzanares@wdc.com>
Tested-by: Adam Manzanares <adam.manzanares@wdc.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Matias Bjørling <m@bjorling.me>
---
block/blk-core.c | 7 ++++---
include/linux/blkdev.h | 14 --------------
2 files changed, 4 insertions(+), 17 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index c274aed2ca3f..7374b02370fa 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1635,14 +1635,15 @@ unsigned int blk_plug_queued_count(struct request_queue *q)
void blk_init_request_from_bio(struct request *req, struct bio *bio)
{
+ struct io_context *ioc = rq_ioc(bio);
+
if (bio->bi_opf & REQ_RAHEAD)
req->cmd_flags |= REQ_FAILFAST_MASK;
req->errors = 0;
req->__sector = bio->bi_iter.bi_sector;
- blk_rq_set_prio(req, rq_ioc(bio));
- if (ioprio_valid(bio_prio(bio)))
- req->ioprio = bio_prio(bio);
+ req->ioprio = ioprio_valid(bio_prio(bio)) ? bio_prio(bio) : ioc ?
+ ioc->ioprio : IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0);
blk_rq_bio_prep(req->q, req, bio);
}
EXPORT_SYMBOL(blk_init_request_from_bio);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index e1ea875ec048..28f713803871 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1088,20 +1088,6 @@ 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.12.2
^ permalink raw reply related
* [PATCH 5/5] block: Optimize ioprio_best()
From: Bart Van Assche @ 2017-04-18 23:10 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Bart Van Assche, Christoph Hellwig,
Matias Bjørling
In-Reply-To: <20170418231037.3968-1-bart.vanassche@sandisk.com>
Since ioprio_best() translates IOPRIO_CLASS_NONE into IOPRIO_CLASS_BE
and since lower numerical priority values represent a higher priority
a simple numerical comparison is sufficient.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Adam Manzanares <adam.manzanares@wdc.com>
Tested-by: Adam Manzanares <adam.manzanares@wdc.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Matias Bjørling <m@bjorling.me>
---
block/ioprio.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/block/ioprio.c b/block/ioprio.c
index 0c47a00f92a8..4b120c9cf7e8 100644
--- a/block/ioprio.c
+++ b/block/ioprio.c
@@ -163,22 +163,12 @@ static int get_task_ioprio(struct task_struct *p)
int ioprio_best(unsigned short aprio, unsigned short bprio)
{
- unsigned short aclass;
- unsigned short bclass;
-
if (!ioprio_valid(aprio))
aprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_NORM);
if (!ioprio_valid(bprio))
bprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, IOPRIO_NORM);
- aclass = IOPRIO_PRIO_CLASS(aprio);
- bclass = IOPRIO_PRIO_CLASS(bprio);
- if (aclass == bclass)
- return min(aprio, bprio);
- if (aclass > bclass)
- return bprio;
- else
- return aprio;
+ return min(aprio, bprio);
}
SYSCALL_DEFINE2(ioprio_get, int, which, int, who)
--
2.12.2
^ permalink raw reply related
* [PATCH v3 0/8] blk-mq debugfs patches for kernel v4.12
From: Bart Van Assche @ 2017-04-18 23:29 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche
Hello Jens,
Please consider the eight patches in this series for kernel v4.12.
These patches improve blk-mq debugfs support.
Thanks,
Bart.
Changes compared to v2:
- Changed the mutex_lock() calls in registration methods into
mutex_lock_interruptible() since these functions can be called from
the context of a user space process.
- Avoid that the blk_mq_register_dev() changes in patch 1/8 cause a
deadlock.
Changes compared to v1:
- Added two patches and replaced patch 1/6 such that debugfs
attributes are now unregistered before freeing of a blk-mq queue
starts instead of checking the "dead" queue flag.
- Changed "rq->cmd_flags ^ op" into "rq->cmd_flags & ~REQ_OP_MASK" as
proposed by Omar.
- A seq_file pointer is now passed to the new queue_rq callback function
instead of a fixed-size char buffer.
Bart Van Assche (8):
blk-mq: Register <dev>/queue/mq after having registered <dev>/queue
blk-mq: Let blk_mq_debugfs_register() look up the queue name
blk-mq: Unregister debugfs attributes earlier
blk-mq: Move the "state" debugfs attribute one level down
blk-mq: Make blk_flags_show() callers append a newline character
blk-mq: Show operation, cmd_flags and rq_flags names
blk-mq: Add blk_mq_ops.show_rq()
scsi: Implement blk_mq_ops.show_rq()
block/blk-mq-debugfs.c | 94 +++++++++++++++++++++++++++++++++++++++++--------
block/blk-mq-sysfs.c | 66 +++++++++++++++++++---------------
block/blk-mq.h | 6 ++--
block/blk-sysfs.c | 9 +++--
drivers/scsi/scsi_lib.c | 26 ++++++++++++++
include/linux/blk-mq.h | 6 ++++
6 files changed, 156 insertions(+), 51 deletions(-)
--
2.12.2
^ permalink raw reply
* [PATCH v3 1/8] blk-mq: Register <dev>/queue/mq after having registered <dev>/queue
From: Bart Van Assche @ 2017-04-18 23:29 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Omar Sandoval, Hannes Reinecke
In-Reply-To: <20170418232949.5228-1-bart.vanassche@sandisk.com>
A later patch in this series will modify blk_mq_debugfs_register()
such that it uses q->kobj.parent to determine the name of a
request queue. Hence make sure that that pointer is initialized
before blk_mq_debugfs_register() is called. To avoid lock inversion,
protect sysfs / debugfs registration with the queue sysfs_lock
instead of the global mutex all_q_mutex.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Hannes Reinecke <hare@suse.com>
---
block/blk-mq-sysfs.c | 43 +++++++++++++++++++++++++++++++++++--------
block/blk-mq.h | 1 +
block/blk-sysfs.c | 6 +++---
3 files changed, 39 insertions(+), 11 deletions(-)
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index d745ab81033a..54ef9402914c 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -253,6 +253,8 @@ static void __blk_mq_unregister_dev(struct device *dev, struct request_queue *q)
struct blk_mq_hw_ctx *hctx;
int i;
+ lockdep_assert_held(&q->sysfs_lock);
+
queue_for_each_hw_ctx(q, hctx, i)
blk_mq_unregister_hctx(hctx);
@@ -267,9 +269,9 @@ static void __blk_mq_unregister_dev(struct device *dev, struct request_queue *q)
void blk_mq_unregister_dev(struct device *dev, struct request_queue *q)
{
- blk_mq_disable_hotplug();
+ mutex_lock(&q->sysfs_lock);
__blk_mq_unregister_dev(dev, q);
- blk_mq_enable_hotplug();
+ mutex_unlock(&q->sysfs_lock);
}
void blk_mq_hctx_kobj_init(struct blk_mq_hw_ctx *hctx)
@@ -302,12 +304,13 @@ void blk_mq_sysfs_init(struct request_queue *q)
}
}
-int blk_mq_register_dev(struct device *dev, struct request_queue *q)
+int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
int ret, i;
- blk_mq_disable_hotplug();
+ WARN_ON_ONCE(!q->kobj.parent);
+ lockdep_assert_held(&q->sysfs_lock);
ret = kobject_add(&q->mq_kobj, kobject_get(&dev->kobj), "%s", "mq");
if (ret < 0)
@@ -327,8 +330,20 @@ int blk_mq_register_dev(struct device *dev, struct request_queue *q)
__blk_mq_unregister_dev(dev, q);
else
q->mq_sysfs_init_done = true;
+
out:
- blk_mq_enable_hotplug();
+ return ret;
+}
+
+int blk_mq_register_dev(struct device *dev, struct request_queue *q)
+{
+ int ret;
+
+ ret = mutex_lock_interruptible(&q->sysfs_lock);
+ if (ret < 0)
+ return ret;
+ ret = __blk_mq_register_dev(dev, q);
+ mutex_unlock(&q->sysfs_lock);
return ret;
}
@@ -339,23 +354,32 @@ void blk_mq_sysfs_unregister(struct request_queue *q)
struct blk_mq_hw_ctx *hctx;
int i;
+ mutex_lock(&q->sysfs_lock);
+
if (!q->mq_sysfs_init_done)
- return;
+ goto unlock;
blk_mq_debugfs_unregister_hctxs(q);
queue_for_each_hw_ctx(q, hctx, i)
blk_mq_unregister_hctx(hctx);
+
+unlock:
+ mutex_unlock(&q->sysfs_lock);
}
int blk_mq_sysfs_register(struct request_queue *q)
{
struct blk_mq_hw_ctx *hctx;
- int i, ret = 0;
+ int i, ret;
- if (!q->mq_sysfs_init_done)
+ ret = mutex_lock_interruptible(&q->sysfs_lock);
+ if (ret < 0)
return ret;
+ if (!q->mq_sysfs_init_done)
+ goto unlock;
+
blk_mq_debugfs_register_hctxs(q);
queue_for_each_hw_ctx(q, hctx, i) {
@@ -364,5 +388,8 @@ int blk_mq_sysfs_register(struct request_queue *q)
break;
}
+unlock:
+ mutex_unlock(&q->sysfs_lock);
+
return ret;
}
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 524f44742816..7d955c756810 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -78,6 +78,7 @@ static inline struct blk_mq_hw_ctx *blk_mq_map_queue(struct request_queue *q,
*/
extern void blk_mq_sysfs_init(struct request_queue *q);
extern void blk_mq_sysfs_deinit(struct request_queue *q);
+extern int __blk_mq_register_dev(struct device *dev, struct request_queue *q);
extern int blk_mq_sysfs_register(struct request_queue *q);
extern void blk_mq_sysfs_unregister(struct request_queue *q);
extern void blk_mq_hctx_kobj_init(struct blk_mq_hw_ctx *hctx);
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index fc20489f0d2b..726ca28584dc 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -894,9 +894,6 @@ int blk_register_queue(struct gendisk *disk)
if (ret)
return ret;
- if (q->mq_ops)
- blk_mq_register_dev(dev, q);
-
/* Prevent changes through sysfs until registration is completed. */
mutex_lock(&q->sysfs_lock);
@@ -906,6 +903,9 @@ int blk_register_queue(struct gendisk *disk)
goto unlock;
}
+ if (q->mq_ops)
+ __blk_mq_register_dev(dev, q);
+
kobject_uevent(&q->kobj, KOBJ_ADD);
blk_wb_init(q);
--
2.12.2
^ permalink raw reply related
* [PATCH v3 2/8] blk-mq: Let blk_mq_debugfs_register() look up the queue name
From: Bart Van Assche @ 2017-04-18 23:29 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Omar Sandoval, Hannes Reinecke
In-Reply-To: <20170418232949.5228-1-bart.vanassche@sandisk.com>
A later patch will move the call of blk_mq_debugfs_register() to
a function to which the queue name is not passed as an argument.
To avoid having to add a 'name' argument to multiple callers, let
blk_mq_debugfs_register() look up the queue name.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Hannes Reinecke <hare@suse.com>
---
block/blk-mq-debugfs.c | 5 +++--
block/blk-mq-sysfs.c | 2 +-
block/blk-mq.h | 5 ++---
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index df9b688b877c..2a5d6d83d57c 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -782,12 +782,13 @@ static const struct blk_mq_debugfs_attr blk_mq_debugfs_ctx_attrs[] = {
{},
};
-int blk_mq_debugfs_register(struct request_queue *q, const char *name)
+int blk_mq_debugfs_register(struct request_queue *q)
{
if (!blk_debugfs_root)
return -ENOENT;
- q->debugfs_dir = debugfs_create_dir(name, blk_debugfs_root);
+ q->debugfs_dir = debugfs_create_dir(kobject_name(q->kobj.parent),
+ blk_debugfs_root);
if (!q->debugfs_dir)
goto err;
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 54ef9402914c..727e3b675130 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -318,7 +318,7 @@ int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
kobject_uevent(&q->mq_kobj, KOBJ_ADD);
- blk_mq_debugfs_register(q, kobject_name(&dev->kobj));
+ blk_mq_debugfs_register(q);
queue_for_each_hw_ctx(q, hctx, i) {
ret = blk_mq_register_hctx(hctx);
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 7d955c756810..9049c0f11505 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -87,13 +87,12 @@ extern void blk_mq_hctx_kobj_init(struct blk_mq_hw_ctx *hctx);
* debugfs helpers
*/
#ifdef CONFIG_BLK_DEBUG_FS
-int blk_mq_debugfs_register(struct request_queue *q, const char *name);
+int blk_mq_debugfs_register(struct request_queue *q);
void blk_mq_debugfs_unregister(struct request_queue *q);
int blk_mq_debugfs_register_hctxs(struct request_queue *q);
void blk_mq_debugfs_unregister_hctxs(struct request_queue *q);
#else
-static inline int blk_mq_debugfs_register(struct request_queue *q,
- const char *name)
+static inline int blk_mq_debugfs_register(struct request_queue *q)
{
return 0;
}
--
2.12.2
^ permalink raw reply related
* [PATCH v3 3/8] blk-mq: Unregister debugfs attributes earlier
From: Bart Van Assche @ 2017-04-18 23:29 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Omar Sandoval, Hannes Reinecke
In-Reply-To: <20170418232949.5228-1-bart.vanassche@sandisk.com>
One of the debugfs attributes allows to run a queue. Since running
a queue after a queue has entered the "dead" state is not allowed
and even can cause a kernel crash, unregister the debugfs attributes
before a queue reaches the "dead" state.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Hannes Reinecke <hare@suse.com>
---
block/blk-mq-sysfs.c | 31 ++++++-------------------------
block/blk-sysfs.c | 3 +--
2 files changed, 7 insertions(+), 27 deletions(-)
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 727e3b675130..1b2107f229ee 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -258,7 +258,7 @@ static void __blk_mq_unregister_dev(struct device *dev, struct request_queue *q)
queue_for_each_hw_ctx(q, hctx, i)
blk_mq_unregister_hctx(hctx);
- blk_mq_debugfs_unregister_hctxs(q);
+ blk_mq_debugfs_unregister(q);
kobject_uevent(&q->mq_kobj, KOBJ_REMOVE);
kobject_del(&q->mq_kobj);
@@ -306,8 +306,7 @@ void blk_mq_sysfs_init(struct request_queue *q)
int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
{
- struct blk_mq_hw_ctx *hctx;
- int ret, i;
+ int ret;
WARN_ON_ONCE(!q->kobj.parent);
lockdep_assert_held(&q->sysfs_lock);
@@ -318,14 +317,7 @@ int __blk_mq_register_dev(struct device *dev, struct request_queue *q)
kobject_uevent(&q->mq_kobj, KOBJ_ADD);
- blk_mq_debugfs_register(q);
-
- queue_for_each_hw_ctx(q, hctx, i) {
- ret = blk_mq_register_hctx(hctx);
- if (ret)
- break;
- }
-
+ ret = blk_mq_debugfs_register(q);
if (ret)
__blk_mq_unregister_dev(dev, q);
else
@@ -351,20 +343,9 @@ EXPORT_SYMBOL_GPL(blk_mq_register_dev);
void blk_mq_sysfs_unregister(struct request_queue *q)
{
- struct blk_mq_hw_ctx *hctx;
- int i;
-
mutex_lock(&q->sysfs_lock);
-
- if (!q->mq_sysfs_init_done)
- goto unlock;
-
- blk_mq_debugfs_unregister_hctxs(q);
-
- queue_for_each_hw_ctx(q, hctx, i)
- blk_mq_unregister_hctx(hctx);
-
-unlock:
+ if (q->mq_sysfs_init_done)
+ blk_mq_debugfs_unregister(q);
mutex_unlock(&q->sysfs_lock);
}
@@ -380,7 +361,7 @@ int blk_mq_sysfs_register(struct request_queue *q)
if (!q->mq_sysfs_init_done)
goto unlock;
- blk_mq_debugfs_register_hctxs(q);
+ blk_mq_debugfs_register(q);
queue_for_each_hw_ctx(q, hctx, i) {
ret = blk_mq_register_hctx(hctx);
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 726ca28584dc..3b6eca07b7a4 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -823,8 +823,7 @@ static void blk_release_queue(struct kobject *kobj)
blk_trace_shutdown(q);
- if (q->mq_ops)
- blk_mq_debugfs_unregister(q);
+ WARN_ON_ONCE(q->debugfs_dir);
if (q->bio_split)
bioset_free(q->bio_split);
--
2.12.2
^ permalink raw reply related
* [PATCH v3 4/8] blk-mq: Move the "state" debugfs attribute one level down
From: Bart Van Assche @ 2017-04-18 23:29 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Hannes Reinecke
In-Reply-To: <20170418232949.5228-1-bart.vanassche@sandisk.com>
Move the "state" attribute from the top level to the "mq" directory
as requested by Omar.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Omar Sandoval <osandov@fb.com>
Cc: Hannes Reinecke <hare@suse.com>
---
block/blk-mq-debugfs.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 2a5d6d83d57c..34cac9a5fd28 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -141,11 +141,6 @@ static const struct file_operations blk_queue_flags_fops = {
.write = blk_queue_flags_store,
};
-static const struct blk_mq_debugfs_attr blk_queue_attrs[] = {
- {"state", 0600, &blk_queue_flags_fops},
- {},
-};
-
static void print_stat(struct seq_file *m, struct blk_rq_stat *stat)
{
if (stat->nr_samples) {
@@ -754,6 +749,7 @@ static const struct file_operations ctx_completed_fops = {
static const struct blk_mq_debugfs_attr blk_mq_debugfs_queue_attrs[] = {
{"poll_stat", 0400, &queue_poll_stat_fops},
+ {"state", 0600, &blk_queue_flags_fops},
{},
};
@@ -870,9 +866,6 @@ int blk_mq_debugfs_register_hctxs(struct request_queue *q)
if (!q->debugfs_dir)
return -ENOENT;
- if (!debugfs_create_files(q->debugfs_dir, q, blk_queue_attrs))
- goto err;
-
q->mq_debugfs_dir = debugfs_create_dir("mq", q->debugfs_dir);
if (!q->mq_debugfs_dir)
goto err;
--
2.12.2
^ permalink raw reply related
* [PATCH v3 5/8] blk-mq: Make blk_flags_show() callers append a newline character
From: Bart Van Assche @ 2017-04-18 23:29 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Hannes Reinecke
In-Reply-To: <20170418232949.5228-1-bart.vanassche@sandisk.com>
This patch does not change any functionality but makes it possible
to produce a single line of output with multiple flag-to-name
translations.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Omar Sandoval <osandov@fb.com>
Cc: Hannes Reinecke <hare@suse.com>
---
block/blk-mq-debugfs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 34cac9a5fd28..27054293b37b 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -60,7 +60,6 @@ static int blk_flags_show(struct seq_file *m, const unsigned long flags,
else
seq_printf(m, "%d", i);
}
- seq_puts(m, "\n");
return 0;
}
@@ -102,6 +101,7 @@ static int blk_queue_flags_show(struct seq_file *m, void *v)
blk_flags_show(m, q->queue_flags, blk_queue_flag_name,
ARRAY_SIZE(blk_queue_flag_name));
+ seq_puts(m, "\n");
return 0;
}
@@ -190,6 +190,7 @@ static int hctx_state_show(struct seq_file *m, void *v)
blk_flags_show(m, hctx->state, hctx_state_name,
ARRAY_SIZE(hctx_state_name));
+ seq_puts(m, "\n");
return 0;
}
@@ -233,6 +234,7 @@ static int hctx_flags_show(struct seq_file *m, void *v)
blk_flags_show(m,
hctx->flags ^ BLK_ALLOC_POLICY_TO_MQ_FLAG(alloc_policy),
hctx_flag_name, ARRAY_SIZE(hctx_flag_name));
+ seq_puts(m, "\n");
return 0;
}
--
2.12.2
^ permalink raw reply related
* [PATCH v3 6/8] blk-mq: Show operation, cmd_flags and rq_flags names
From: Bart Van Assche @ 2017-04-18 23:29 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Hannes Reinecke
In-Reply-To: <20170418232949.5228-1-bart.vanassche@sandisk.com>
Show the operation name, .cmd_flags and .rq_flags as names instead
of numbers.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Omar Sandoval <osandov@fb.com>
Cc: Hannes Reinecke <hare@suse.com>
---
block/blk-mq-debugfs.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 69 insertions(+), 3 deletions(-)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 27054293b37b..64b584ba576a 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -250,13 +250,79 @@ static const struct file_operations hctx_flags_fops = {
.release = single_release,
};
+static const char *const op_name[] = {
+ [REQ_OP_READ] = "READ",
+ [REQ_OP_WRITE] = "WRITE",
+ [REQ_OP_FLUSH] = "FLUSH",
+ [REQ_OP_DISCARD] = "DISCARD",
+ [REQ_OP_ZONE_REPORT] = "ZONE_REPORT",
+ [REQ_OP_SECURE_ERASE] = "SECURE_ERASE",
+ [REQ_OP_ZONE_RESET] = "ZONE_RESET",
+ [REQ_OP_WRITE_SAME] = "WRITE_SAME",
+ [REQ_OP_WRITE_ZEROES] = "WRITE_ZEROES",
+ [REQ_OP_SCSI_IN] = "SCSI_IN",
+ [REQ_OP_SCSI_OUT] = "SCSI_OUT",
+ [REQ_OP_DRV_IN] = "DRV_IN",
+ [REQ_OP_DRV_OUT] = "DRV_OUT",
+};
+
+static const char *const cmd_flag_name[] = {
+ [__REQ_FAILFAST_DEV] = "FAILFAST_DEV",
+ [__REQ_FAILFAST_TRANSPORT] = "FAILFAST_TRANSPORT",
+ [__REQ_FAILFAST_DRIVER] = "FAILFAST_DRIVER",
+ [__REQ_SYNC] = "SYNC",
+ [__REQ_META] = "META",
+ [__REQ_PRIO] = "PRIO",
+ [__REQ_NOMERGE] = "NOMERGE",
+ [__REQ_IDLE] = "IDLE",
+ [__REQ_INTEGRITY] = "INTEGRITY",
+ [__REQ_FUA] = "FUA",
+ [__REQ_PREFLUSH] = "PREFLUSH",
+ [__REQ_RAHEAD] = "RAHEAD",
+ [__REQ_BACKGROUND] = "BACKGROUND",
+ [__REQ_NR_BITS] = "NR_BITS",
+};
+
+static const char *const rqf_name[] = {
+ [ilog2(RQF_SORTED)] = "SORTED",
+ [ilog2(RQF_STARTED)] = "STARTED",
+ [ilog2(RQF_QUEUED)] = "QUEUED",
+ [ilog2(RQF_SOFTBARRIER)] = "SOFTBARRIER",
+ [ilog2(RQF_FLUSH_SEQ)] = "FLUSH_SEQ",
+ [ilog2(RQF_MIXED_MERGE)] = "MIXED_MERGE",
+ [ilog2(RQF_MQ_INFLIGHT)] = "MQ_INFLIGHT",
+ [ilog2(RQF_DONTPREP)] = "DONTPREP",
+ [ilog2(RQF_PREEMPT)] = "PREEMPT",
+ [ilog2(RQF_COPY_USER)] = "COPY_USER",
+ [ilog2(RQF_FAILED)] = "FAILED",
+ [ilog2(RQF_QUIET)] = "QUIET",
+ [ilog2(RQF_ELVPRIV)] = "ELVPRIV",
+ [ilog2(RQF_IO_STAT)] = "IO_STAT",
+ [ilog2(RQF_ALLOCED)] = "ALLOCED",
+ [ilog2(RQF_PM)] = "PM",
+ [ilog2(RQF_HASHED)] = "HASHED",
+ [ilog2(RQF_STATS)] = "STATS",
+ [ilog2(RQF_SPECIAL_PAYLOAD)] = "SPECIAL_PAYLOAD",
+};
+
static int blk_mq_debugfs_rq_show(struct seq_file *m, void *v)
{
struct request *rq = list_entry_rq(v);
+ const unsigned int op = rq->cmd_flags & REQ_OP_MASK;
- seq_printf(m, "%p {.cmd_flags=0x%x, .rq_flags=0x%x, .tag=%d, .internal_tag=%d}\n",
- rq, rq->cmd_flags, (__force unsigned int)rq->rq_flags,
- rq->tag, rq->internal_tag);
+ seq_printf(m, "%p {.op=", rq);
+ if (op < ARRAY_SIZE(op_name) && op_name[op])
+ seq_printf(m, "%s", op_name[op]);
+ else
+ seq_printf(m, "%d", op);
+ seq_puts(m, ", .cmd_flags=");
+ blk_flags_show(m, rq->cmd_flags & ~REQ_OP_MASK, cmd_flag_name,
+ ARRAY_SIZE(cmd_flag_name));
+ seq_puts(m, ", .rq_flags=");
+ blk_flags_show(m, (__force unsigned int)rq->rq_flags, rqf_name,
+ ARRAY_SIZE(rqf_name));
+ seq_printf(m, ", .tag=%d, .internal_tag=%d}\n", rq->tag,
+ rq->internal_tag);
return 0;
}
--
2.12.2
^ permalink raw reply related
* [PATCH v3 7/8] blk-mq: Add blk_mq_ops.show_rq()
From: Bart Van Assche @ 2017-04-18 23:29 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Omar Sandoval, Hannes Reinecke
In-Reply-To: <20170418232949.5228-1-bart.vanassche@sandisk.com>
This new callback function will be used in the next patch to show
more information about SCSI requests.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Hannes Reinecke <hare@suse.com>
---
block/blk-mq-debugfs.c | 6 +++++-
include/linux/blk-mq.h | 6 ++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 64b584ba576a..b1b669f98ea0 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -308,6 +308,7 @@ static const char *const rqf_name[] = {
static int blk_mq_debugfs_rq_show(struct seq_file *m, void *v)
{
struct request *rq = list_entry_rq(v);
+ const struct blk_mq_ops *const mq_ops = rq->q->mq_ops;
const unsigned int op = rq->cmd_flags & REQ_OP_MASK;
seq_printf(m, "%p {.op=", rq);
@@ -321,8 +322,11 @@ static int blk_mq_debugfs_rq_show(struct seq_file *m, void *v)
seq_puts(m, ", .rq_flags=");
blk_flags_show(m, (__force unsigned int)rq->rq_flags, rqf_name,
ARRAY_SIZE(rqf_name));
- seq_printf(m, ", .tag=%d, .internal_tag=%d}\n", rq->tag,
+ seq_printf(m, ", .tag=%d, .internal_tag=%d", rq->tag,
rq->internal_tag);
+ if (mq_ops->show_rq)
+ mq_ops->show_rq(m, rq);
+ seq_puts(m, "}\n");
return 0;
}
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index d75de612845d..a761d275cb44 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -121,6 +121,12 @@ struct blk_mq_ops {
softirq_done_fn *complete;
/*
+ * Used by the debugfs implementation to show driver-specific
+ * information about a request.
+ */
+ void (*show_rq)(struct seq_file *m, struct request *rq);
+
+ /*
* Called when the block layer side of a hardware queue has been
* set up, allowing the driver to allocate/init matching structures.
* Ditto for exit/teardown.
--
2.12.2
^ permalink raw reply related
* [PATCH v3 8/8] scsi: Implement blk_mq_ops.show_rq()
From: Bart Van Assche @ 2017-04-18 23:29 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Bart Van Assche, Martin K . Petersen,
James Bottomley, Omar Sandoval, Hannes Reinecke, linux-scsi
In-Reply-To: <20170418232949.5228-1-bart.vanassche@sandisk.com>
Show the SCSI CDB, .eh_eflags and .result for pending SCSI commands
in /sys/kernel/debug/block/*/mq/*/dispatch and */rq_list.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Martin K. Petersen <martin.petersen@oracle.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: <linux-scsi@vger.kernel.org>
---
drivers/scsi/scsi_lib.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 7bc4513bf4e4..52604573e4b6 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2126,6 +2126,31 @@ static void scsi_exit_rq(struct request_queue *q, struct request *rq)
scsi_free_sense_buffer(shost, cmd->sense_buffer);
}
+static const char *const ehflag_name[] = {
+ [ilog2(SCSI_EH_CANCEL_CMD)] = "CANCEL_CMD",
+ [ilog2(SCSI_EH_ABORT_SCHEDULED)] = "ABORT_SCHEDULED",
+};
+
+static void scsi_show_rq(struct seq_file *m, struct request *rq)
+{
+ struct scsi_cmnd *cmd = container_of(scsi_req(rq), typeof(*cmd), req);
+ unsigned int i;
+
+ seq_puts(m, ", .cmd =");
+ for (i = 0; i < cmd->cmd_len; i++)
+ seq_printf(m, " %02x", cmd->cmnd[i]);
+ seq_puts(m, ", .eh_eflags =");
+ for (i = 0; i < sizeof(cmd->eh_eflags) * BITS_PER_BYTE; i++) {
+ if (!(cmd->eh_eflags & BIT(i)))
+ continue;
+ if (i < ARRAY_SIZE(ehflag_name) && ehflag_name[i])
+ seq_printf(m, " %s", ehflag_name[i]);
+ else
+ seq_printf(m, " %d", i);
+ }
+ seq_printf(m, ", .result = %#06x", cmd->result);
+}
+
struct request_queue *scsi_alloc_queue(struct scsi_device *sdev)
{
struct Scsi_Host *shost = sdev->host;
@@ -2158,6 +2183,7 @@ static const struct blk_mq_ops scsi_mq_ops = {
.queue_rq = scsi_queue_rq,
.complete = scsi_softirq_done,
.timeout = scsi_timeout,
+ .show_rq = scsi_show_rq,
.init_request = scsi_init_request,
.exit_request = scsi_exit_request,
.map_queues = scsi_map_queues,
--
2.12.2
^ permalink raw reply related
* [PATCH 0/3] mtip32xx: make it working with mq scheduler
From: Ming Lei @ 2017-04-19 0:31 UTC (permalink / raw)
To: Jens Axboe, linux-block
Cc: Christoph Hellwig, Omar Sandoval, Jozef Mikovic, Ming Lei
Hi,
This patchset fixes two issues on mtip32xx when mq schedluer is
used, and the issue is introduced in v4.11-rc:
1) the 1st two patches fixes one kernel oops trigered in blk_mq_tag_to_rq()
2) the 3rd patch fixes hardware failure and system panic which is caused
by misusing request index as request tag in .init_request()
Please consider this patchset for v4.11.
Sorry for being a bit late because my test system is just recovered.
Thanks,
Ming
Ming Lei (3):
blk-mq: export blk_mq_get_driver_tag
mtip32xx: assign driver tag
mtip32xx: use runtime tag to initialize command header
block/blk-mq.c | 1 +
drivers/block/mtip32xx/mtip32xx.c | 38 ++++++++++++++++++++++++++------------
include/linux/blk-mq.h | 3 +++
3 files changed, 30 insertions(+), 12 deletions(-)
--
2.9.3
^ permalink raw reply
* [PATCH 1/3] blk-mq: export blk_mq_get_driver_tag
From: Ming Lei @ 2017-04-19 0:31 UTC (permalink / raw)
To: Jens Axboe, linux-block
Cc: Christoph Hellwig, Omar Sandoval, Jozef Mikovic, Ming Lei
In-Reply-To: <20170419003142.9581-1-ming.lei@redhat.com>
Before blk-mq iosched is introduced, drivers may suppose
the the allocated request includes one valid tag number,
but it becomes not true any more after we bring mq iosched in.
So introduces this helper to make driver get req's hw tag.
Mtip32xx needs this helper for sending internal command via
one request.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
block/blk-mq.c | 1 +
include/linux/blk-mq.h | 3 +++
2 files changed, 4 insertions(+)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 572966f49596..4614ab8b941e 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -865,6 +865,7 @@ bool blk_mq_get_driver_tag(struct request *rq, struct blk_mq_hw_ctx **hctx,
*hctx = data.hctx;
return rq->tag != -1;
}
+EXPORT_SYMBOL(blk_mq_get_driver_tag);
static void __blk_mq_put_driver_tag(struct blk_mq_hw_ctx *hctx,
struct request *rq)
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 9382c5da7a2e..ecb7b681aa7c 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -255,6 +255,9 @@ int blk_mq_reinit_tagset(struct blk_mq_tag_set *set);
int blk_mq_map_queues(struct blk_mq_tag_set *set);
void blk_mq_update_nr_hw_queues(struct blk_mq_tag_set *set, int nr_hw_queues);
+bool blk_mq_get_driver_tag(struct request *rq, struct blk_mq_hw_ctx **hctx,
+ bool wait);
+
/*
* Driver command data is immediately after the request. So subtract request
* size to get back to the original request, add request size to get the PDU.
--
2.9.3
^ permalink raw reply related
* [PATCH 2/3] mtip32xx: assign driver tag
From: Ming Lei @ 2017-04-19 0:31 UTC (permalink / raw)
To: Jens Axboe, linux-block
Cc: Christoph Hellwig, Omar Sandoval, Jozef Mikovic, Ming Lei
In-Reply-To: <20170419003142.9581-1-ming.lei@redhat.com>
Assign driver tag for internal command, so that we
can avoid kernel oops[1] trigered in blk_mq_tag_to_rq().
[1] https://bugzilla.kernel.org/show_bug.cgi?id=195429
Reported-by: Jozef Mikovic <jmikovic@redhat.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/block/mtip32xx/mtip32xx.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index f96ab717534c..8cfe5a6edb05 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -180,6 +180,8 @@ static struct mtip_cmd *mtip_get_int_command(struct driver_data *dd)
if (IS_ERR(rq))
return NULL;
+ WARN_ON(!blk_mq_get_driver_tag(rq, NULL, true));
+
return blk_mq_rq_to_pdu(rq);
}
--
2.9.3
^ permalink raw reply related
* [PATCH 3/3] mtip32xx: use runtime tag to initialize command header
From: Ming Lei @ 2017-04-19 0:31 UTC (permalink / raw)
To: Jens Axboe, linux-block
Cc: Christoph Hellwig, Omar Sandoval, Jozef Mikovic, Ming Lei
In-Reply-To: <20170419003142.9581-1-ming.lei@redhat.com>
mtip32xx supposes that 'request_idx' passed to .init_request()
is tag of the request, and use that as request's tag to initialize
command header.
After MQ IO scheduler is in, request tag assigned isn't same with
the request index anymore, so cause strange hardware failure on
mtip32xx, even whole system panic is triggered.
This patch fixes the issue by initializing command header via
request's real tag.
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/block/mtip32xx/mtip32xx.c | 36 ++++++++++++++++++++++++------------
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index 8cfe5a6edb05..b122dc0368f4 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -169,6 +169,25 @@ static bool mtip_check_surprise_removal(struct pci_dev *pdev)
return false; /* device present */
}
+/* we have to use runtime tag to setup command header */
+static void mtip_init_cmd_header(struct request *rq)
+{
+ struct driver_data *dd = rq->q->queuedata;
+ struct mtip_cmd *cmd = blk_mq_rq_to_pdu(rq);
+ u32 host_cap_64 = readl(dd->mmio + HOST_CAP) & HOST_CAP_64;
+
+ /* Point the command headers at the command tables. */
+ cmd->command_header = dd->port->command_list +
+ (sizeof(struct mtip_cmd_hdr) * rq->tag);
+ cmd->command_header_dma = dd->port->command_list_dma +
+ (sizeof(struct mtip_cmd_hdr) * rq->tag);
+
+ if (host_cap_64)
+ cmd->command_header->ctbau = __force_bit2int cpu_to_le32((cmd->command_dma >> 16) >> 16);
+
+ cmd->command_header->ctba = __force_bit2int cpu_to_le32(cmd->command_dma & 0xFFFFFFFF);
+}
+
static struct mtip_cmd *mtip_get_int_command(struct driver_data *dd)
{
struct request *rq;
@@ -182,6 +201,9 @@ static struct mtip_cmd *mtip_get_int_command(struct driver_data *dd)
WARN_ON(!blk_mq_get_driver_tag(rq, NULL, true));
+ /* Internal cmd isn't submitted via .queue_rq */
+ mtip_init_cmd_header(rq);
+
return blk_mq_rq_to_pdu(rq);
}
@@ -3809,6 +3831,8 @@ static int mtip_queue_rq(struct blk_mq_hw_ctx *hctx,
struct request *rq = bd->rq;
int ret;
+ mtip_init_cmd_header(rq);
+
if (unlikely(mtip_check_unal_depth(hctx, rq)))
return BLK_MQ_RQ_QUEUE_BUSY;
@@ -3840,7 +3864,6 @@ static int mtip_init_cmd(void *data, struct request *rq, unsigned int hctx_idx,
{
struct driver_data *dd = data;
struct mtip_cmd *cmd = blk_mq_rq_to_pdu(rq);
- u32 host_cap_64 = readl(dd->mmio + HOST_CAP) & HOST_CAP_64;
/*
* For flush requests, request_idx starts at the end of the
@@ -3857,17 +3880,6 @@ static int mtip_init_cmd(void *data, struct request *rq, unsigned int hctx_idx,
memset(cmd->command, 0, CMD_DMA_ALLOC_SZ);
- /* Point the command headers at the command tables. */
- cmd->command_header = dd->port->command_list +
- (sizeof(struct mtip_cmd_hdr) * request_idx);
- cmd->command_header_dma = dd->port->command_list_dma +
- (sizeof(struct mtip_cmd_hdr) * request_idx);
-
- if (host_cap_64)
- cmd->command_header->ctbau = __force_bit2int cpu_to_le32((cmd->command_dma >> 16) >> 16);
-
- cmd->command_header->ctba = __force_bit2int cpu_to_le32(cmd->command_dma & 0xFFFFFFFF);
-
sg_init_table(cmd->sg, MTIP_MAX_SG);
return 0;
}
--
2.9.3
^ permalink raw reply related
* Re: [PATCH] nbd: set the max segment size to UINT_MAX
From: Ming Lei @ 2017-04-19 0:54 UTC (permalink / raw)
To: Josef Bacik
Cc: Jens Axboe, nbd-general@lists.sourceforge.net, linux-block,
FB Kernel Team
In-Reply-To: <1492546971-2682-1-git-send-email-jbacik@fb.com>
On Wed, Apr 19, 2017 at 4:22 AM, Josef Bacik <josef@toxicpanda.com> wrote:
> NBD doesn't care about limiting the segment size, let the user push the
> largest bio's they want. This allows us to control the request size
> solely through max_sectors_kb.
>
> Signed-off-by: Josef Bacik <jbacik@fb.com>
> ---
> drivers/block/nbd.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
> index b78f23c..6e592c2 100644
> --- a/drivers/block/nbd.c
> +++ b/drivers/block/nbd.c
> @@ -1469,6 +1469,7 @@ static int nbd_dev_add(int index)
> queue_flag_clear_unlocked(QUEUE_FLAG_ADD_RANDOM, disk->queue);
> disk->queue->limits.discard_granularity = 512;
> blk_queue_max_discard_sectors(disk->queue, UINT_MAX);
> + blk_queue_max_segment_size(disk->queue, UINT_MAX);
> blk_queue_max_hw_sectors(disk->queue, 65536);
> disk->queue->limits.max_sectors = 256;
The change is fine, since the segment size limit should be from hw itself.
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Thanks,
Ming Lei
^ permalink raw reply
* Re: [PATCH 0/4] blk-mq-sched: allow to use hw tag for sched
From: Ming Lei @ 2017-04-19 1:10 UTC (permalink / raw)
To: Omar Sandoval
Cc: Jens Axboe, linux-block, Christoph Hellwig, Omar Sandoval,
Jozef Mikovic
In-Reply-To: <20170417173046.GA27028@vader.DHCP.thefacebook.com>
On Mon, Apr 17, 2017 at 10:30:46AM -0700, Omar Sandoval wrote:
> On Mon, Apr 17, 2017 at 12:03:53AM +0800, Ming Lei wrote:
> > On Sat, Apr 15, 2017 at 08:38:21PM +0800, Ming Lei wrote:
> > > The 1st patch enhances BLK_MQ_F_NO_SCHED so that we can't change/
> > > show available io schedulers on devices which don't support io
> > > scheduler.
> > >
> > > The 2nd patch passes BLK_MQ_F_NO_SCHED for avoiding one regression
> > > on mtip32xx, which is introduced by blk-mq io scheduler.
> > >
> > > The last two patches introduce BLK_MQ_F_SCHED_USE_HW_TAG so that
> > > we can allow to use hardware tag for scheduler, then mq-deadline
> > > can work well on mtip32xx. Even though other devices with enough
> > > hardware tag space can benefit from this feature too.
> > >
> > > The 1st two patches aims on v4.11, and the last two are for
> > > v4.12.
> >
> > Please ignore this patchset, and I will post another serial for
> > mtip32xx fix.
> >
> > thanks,
> > Ming
>
> Regardless of the mtip32xx fix, I actually wanted to skip the scheduler
> tags when possible just for performance reasons. I don't think that
> should be a device-specific setting. We can let the I/O scheduler decide
> how many tags it wants, and if the device has at least that many tags,
> just use the hardware tags directly. I'll probably look at doing that
> for 4.13, we'll see if it actually makes a performance difference.
Yes, I thought about that too, the following policy may be applied at
default:
- if queue depth is not less than q->nr_requests
and
- the tag space isn't shared
We still can keep the flag of BLK_MQ_F_SCHED_USE_HW_TAG so that
drivers can decide if they want to do that, since I guess it may
help some shared tag cases too.
Now the mtip32xx fix has been sent out already, I will post this
patchset of using hw tag for scheduler out soon for review.
Thanks,
Ming
^ permalink raw reply
* Re: bfq-mq performance comparison to cfq
From: Bart Van Assche @ 2017-04-19 5:01 UTC (permalink / raw)
To: Paolo Valente
Cc: aherrmann@suse.com, linux-kernel@vger.kernel.org,
linux-block@vger.kernel.org, axboe@kernel.dk
In-Reply-To: <B7819549-81C3-4952-A31D-5E4A0732AB14@linaro.org>
On 04/11/17 00:29, Paolo Valente wrote:=0A=
>=0A=
>> Il giorno 10 apr 2017, alle ore 17:15, Bart Van Assche <bart.vanassche@s=
andisk.com> ha scritto:=0A=
>>=0A=
>> On Mon, 2017-04-10 at 11:55 +0200, Paolo Valente wrote:=0A=
>>> That said, if you do always want maximum throughput, even at the=0A=
>>> expense of latency, then just switch off low-latency heuristics, i.e.,=
=0A=
>>> set low_latency to 0. Depending on the device, setting slice_ilde to=
=0A=
>>> 0 may help a lot too (as well as with CFQ). If the throughput is=0A=
>>> still low also after forcing BFQ to an only-throughput mode, then you=
=0A=
>>> hit some bug, and I'll have a little more work to do ...=0A=
>>=0A=
>> Has it been considered to make applications tell the I/O scheduler=0A=
>> whether to optimize for latency or for throughput? It shouldn't be that=
=0A=
>> hard for window managers and shells to figure out whether or not a new=
=0A=
>> application that is being started is interactive or not. This would=0A=
>> require a mechanism that allows applications to provide such information=
=0A=
>> to the I/O scheduler. Wouldn't that be a better approach than the I/O=0A=
>> scheduler trying to guess whether or not an application is an interactiv=
e=0A=
>> application?=0A=
>=0A=
> IMO that would be an (or maybe the) optimal solution, in terms of both=0A=
> throughput and latency. We have even developed a prototype doing what=0A=
> you propose, for Android. Unfortunately, I have not yet succeeded in=0A=
> getting support, to turn it into candidate production code, or to make=0A=
> a similar solution for lsb-compliant systems.=0A=
=0A=
Hello Paolo,=0A=
=0A=
What API was used by the Android application to tell the I/O scheduler =0A=
to optimize for latency? Do you think that it would be sufficient if the =
=0A=
application uses the ioprio_set() system call to set the I/O priority to =
=0A=
IOPRIO_CLASS_RT?=0A=
=0A=
Thanks,=0A=
=0A=
Bart.=0A=
^ permalink raw reply
* Re: [PATCH V3 02/16] block, bfq: add full hierarchical scheduling and cgroups support
From: Paolo Valente @ 2017-04-19 5:33 UTC (permalink / raw)
To: Tejun Heo
Cc: Jens Axboe, Fabio Checconi, Arianna Avanzini, linux-block,
Linux-Kernal, Ulf Hansson, Linus Walleij, broonie
In-Reply-To: <20170418070435.GB3899@wtj.duckdns.org>
> Il giorno 18 apr 2017, alle ore 09:04, Tejun Heo <tj@kernel.org> ha =
scritto:
>=20
> Hello, Paolo.
>=20
> On Wed, Apr 12, 2017 at 07:22:03AM +0200, Paolo Valente wrote:
>> could you elaborate a bit more on this? I mean, cgroups support has
>> been in BFQ (and CFQ) for almost ten years, perfectly working as far
>> as I know. Of course it is perfectly working in terms of I/O and not
>> of CPU bandwidth distribution; and, for the moment, it is effective
>> only for devices below 30-50KIOPS. What's the point in throwing
>> (momentarily?) away such a fundamental feature? What am I missing?
>=20
> I've been trying to track down latency issues with the CPU controller
> which basically takes the same approach and I'm not sure nesting
> scheduler timelines is a good approach. It intuitively feels elegant
> but seems to have some fundamental issues. IIUC, bfq isn't quite the
> same in that it doesn't need load balancer across multiple queues and
> it could be that bfq is close enough to the basic model that the
> nested behavior maps to the correct scheduling behavior.
>=20
> However, for example, in the CPU controller, the nested timelines
> break sleeper boost. The boost is implemented by considering the
> thread to have woken up upto some duration prior to the current time;
> however, it only affects the timeline inside the cgroup and there's no
> good way to propagate it upwards. The final result is two threads in
> a cgroup with the double weight can behave significantly worse in
> terms of latency compared to two threads with the weight of 1 in the
> root.
>=20
Hi Tejun,
I don't know in detail the specific multiple-queue issues you report,
but bfq implements the upward propagation you mention: if a process in
a group is to be privileged, i.e., if the process has basically to be
provided with a higher weight (in addition to other important forms of
help), then this weight boost is propagated upward through the path
from the process to the root node in the group hierarchy.
> Given that the nested scheduling ends up pretty expensive, I'm not
> sure how good a model this nesting approach is. Especially if there
> can be multiple queues, the weight distribution across cgroup
> instances across multiple queues has to be coordinated globally
> anyway,
To get perfect global service guarantees, yes. But you can settle
with tradeoffs that, according to my experience with storage and
packet I/O, are so good to be probably indistinguishable from an
ideal, but too costly solution. I mean, with a well-done approximated
scheduling solution, the deviation with respect to an ideal service
can be in the same order of the noise caused by unavoidable latencies
of other sw and hw components than the scheduler.
> so the weight / cost adjustment part can't happen
> automatically anyway as in single queue case. If we're going there,
> we might as well implement cgroup support by actively modulating the
> combined weights, which will make individual scheduling operations
> cheaper and it easier to think about and guarantee latency behaviors.
>=20
Yes. Anyway, I didn't quite understand what is or could be the
alternative, w.r.t. hierarchical scheduling, for guaranteeing
bandwidth distribution of shared resources in a complex setting. If
you think I could be of any help on this, just put me somehow in the
loop.
> If you think that bfq will stay single queue and won't need timeline
> modifying heuristics (for responsiveness or whatever), the current
> approach could be fine, but I'm a bit awry about committing to the
> current approach if we're gonna encounter the same problems.
>=20
As of now, bfq is targeted at not too fast devices (< 30-50KIOPS),
which happen to be single queue. In particular, bfq is currently
agnostic w.r.t. to the number of downstream queues.
Thanks,
Paolo
> Thanks.
>=20
> --=20
> tejun
^ permalink raw reply
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