* [PATCH v2 0/4] blk-mq-debugfs patches for kernel v4.13
@ 2017-05-31 21:30 Bart Van Assche
2017-05-31 21:30 ` [PATCH v2 1/4] blk-mq-debugfs: Show atomic request flags Bart Van Assche
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Bart Van Assche @ 2017-05-31 21:30 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Christoph Hellwig, Bart Van Assche
Hello Jens,
This series contains four patches that add new features to the blk-mq-debugfs.
The changes compared to v1 of this series are:
- Left out the first patch of this series because it's already upstream.
- Renamed "ctx" into "params" in the patch that shows the busy requests.
- In the same patch, switched from blk_mq_debugfs_rq_show() to
__blk_mq_debugfs_rq_show().
Please consider these patches for kernel v4.13.
Thanks,
Bart.
Bart Van Assche (4):
blk-mq-debugfs: Show atomic request flags
blk-mq-debugfs: Show requeue list
blk-mq-debugfs: Show busy requests
blk-mq-debugfs: Add 'kick' operation
block/blk-mq-debugfs.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 71 insertions(+), 1 deletion(-)
--
2.12.2
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/4] blk-mq-debugfs: Show atomic request flags
2017-05-31 21:30 [PATCH v2 0/4] blk-mq-debugfs patches for kernel v4.13 Bart Van Assche
@ 2017-05-31 21:30 ` Bart Van Assche
2017-06-01 2:37 ` Ming Lei
2017-05-31 21:30 ` [PATCH v2 2/4] blk-mq-debugfs: Show requeue list Bart Van Assche
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2017-05-31 21:30 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche, Omar Sandoval, Ming Lei
When analyzing e.g. queue lockups it is important to know whether
or not a request has already been started. Hence also show the
atomic request flags.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Ming Lei <ming.lei@redhat.com>
---
block/blk-mq-debugfs.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 803aed4d7221..d56ddd7a1285 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -267,6 +267,14 @@ static const char *const rqf_name[] = {
};
#undef RQF_NAME
+#define RQAF_NAME(name) [REQ_ATOM_##name] = #name
+static const char *const rqaf_name[] = {
+ RQAF_NAME(COMPLETE),
+ RQAF_NAME(STARTED),
+ RQAF_NAME(POLL_SLEPT),
+};
+#undef RQAF_NAME
+
int __blk_mq_debugfs_rq_show(struct seq_file *m, struct request *rq)
{
const struct blk_mq_ops *const mq_ops = rq->q->mq_ops;
@@ -283,6 +291,8 @@ int __blk_mq_debugfs_rq_show(struct seq_file *m, struct request *rq)
seq_puts(m, ", .rq_flags=");
blk_flags_show(m, (__force unsigned int)rq->rq_flags, rqf_name,
ARRAY_SIZE(rqf_name));
+ seq_puts(m, ", .atomic_flags=");
+ blk_flags_show(m, rq->atomic_flags, rqaf_name, ARRAY_SIZE(rqaf_name));
seq_printf(m, ", .tag=%d, .internal_tag=%d", rq->tag,
rq->internal_tag);
if (mq_ops->show_rq)
--
2.12.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/4] blk-mq-debugfs: Show requeue list
2017-05-31 21:30 [PATCH v2 0/4] blk-mq-debugfs patches for kernel v4.13 Bart Van Assche
2017-05-31 21:30 ` [PATCH v2 1/4] blk-mq-debugfs: Show atomic request flags Bart Van Assche
@ 2017-05-31 21:30 ` Bart Van Assche
2017-06-01 2:41 ` Ming Lei
2017-05-31 21:30 ` [PATCH v2 3/4] blk-mq-debugfs: Show busy requests Bart Van Assche
2017-05-31 21:30 ` [PATCH v2 4/4] blk-mq-debugfs: Add 'kick' operation Bart Van Assche
3 siblings, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2017-05-31 21:30 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche, Omar Sandoval, Ming Lei
When verifying whether or not a blk-mq driver forgot to kick the
requeue list after having requeued a request it is important to
be able to verify the contents of the requeue list. Hence export
that list through debugfs.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Ming Lei <ming.lei@redhat.com>
---
block/blk-mq-debugfs.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index d56ddd7a1285..8b06a12c1461 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -308,6 +308,37 @@ int blk_mq_debugfs_rq_show(struct seq_file *m, void *v)
}
EXPORT_SYMBOL_GPL(blk_mq_debugfs_rq_show);
+static void *queue_requeue_list_start(struct seq_file *m, loff_t *pos)
+ __acquires(&q->requeue_lock)
+{
+ struct request_queue *q = m->private;
+
+ spin_lock_irq(&q->requeue_lock);
+ return seq_list_start(&q->requeue_list, *pos);
+}
+
+static void *queue_requeue_list_next(struct seq_file *m, void *v, loff_t *pos)
+{
+ struct request_queue *q = m->private;
+
+ return seq_list_next(v, &q->requeue_list, pos);
+}
+
+static void queue_requeue_list_stop(struct seq_file *m, void *v)
+ __releases(&q->requeue_lock)
+{
+ struct request_queue *q = m->private;
+
+ spin_unlock_irq(&q->requeue_lock);
+}
+
+static const struct seq_operations queue_requeue_list_seq_ops = {
+ .start = queue_requeue_list_start,
+ .next = queue_requeue_list_next,
+ .stop = queue_requeue_list_stop,
+ .show = blk_mq_debugfs_rq_show,
+};
+
static void *hctx_dispatch_start(struct seq_file *m, loff_t *pos)
__acquires(&hctx->lock)
{
@@ -665,6 +696,7 @@ const struct file_operations blk_mq_debugfs_fops = {
static const struct blk_mq_debugfs_attr blk_mq_debugfs_queue_attrs[] = {
{"poll_stat", 0400, queue_poll_stat_show},
+ {"requeue_list", 0400, .seq_ops = &queue_requeue_list_seq_ops},
{"state", 0600, queue_state_show, queue_state_write},
{},
};
--
2.12.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 3/4] blk-mq-debugfs: Show busy requests
2017-05-31 21:30 [PATCH v2 0/4] blk-mq-debugfs patches for kernel v4.13 Bart Van Assche
2017-05-31 21:30 ` [PATCH v2 1/4] blk-mq-debugfs: Show atomic request flags Bart Van Assche
2017-05-31 21:30 ` [PATCH v2 2/4] blk-mq-debugfs: Show requeue list Bart Van Assche
@ 2017-05-31 21:30 ` Bart Van Assche
2017-05-31 21:43 ` Eduardo Valentin
` (2 more replies)
2017-05-31 21:30 ` [PATCH v2 4/4] blk-mq-debugfs: Add 'kick' operation Bart Van Assche
3 siblings, 3 replies; 16+ messages in thread
From: Bart Van Assche @ 2017-05-31 21:30 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche, Hannes Reinecke,
Omar Sandoval, Ming Lei
Requests that got stuck in a block driver are neither on
blk_mq_ctx.rq_list nor on any hw dispatch queue. Make these
visible in debugfs through the "busy" attribute.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Ming Lei <ming.lei@redhat.com>
---
block/blk-mq-debugfs.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 8b06a12c1461..fa0f624dfccd 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -370,6 +370,31 @@ static const struct seq_operations hctx_dispatch_seq_ops = {
.show = blk_mq_debugfs_rq_show,
};
+struct show_busy_params {
+ struct seq_file *m;
+ struct blk_mq_hw_ctx *hctx;
+};
+
+static void hctx_show_busy(struct request *rq, void *data, bool reserved)
+{
+ const struct show_busy_params *params = data;
+
+ if (blk_mq_map_queue(rq->q, rq->mq_ctx->cpu) == params->hctx &&
+ test_bit(REQ_ATOM_STARTED, &rq->atomic_flags))
+ __blk_mq_debugfs_rq_show(params->m,
+ list_entry_rq(&rq->queuelist));
+}
+
+static int hctx_busy_show(void *data, struct seq_file *m)
+{
+ struct blk_mq_hw_ctx *hctx = data;
+ struct show_busy_params params = { .m = m, .hctx = hctx };
+
+ blk_mq_tagset_busy_iter(hctx->queue->tag_set, hctx_show_busy, ¶ms);
+
+ return 0;
+}
+
static int hctx_ctx_map_show(void *data, struct seq_file *m)
{
struct blk_mq_hw_ctx *hctx = data;
@@ -705,6 +730,7 @@ static const struct blk_mq_debugfs_attr blk_mq_debugfs_hctx_attrs[] = {
{"state", 0400, hctx_state_show},
{"flags", 0400, hctx_flags_show},
{"dispatch", 0400, .seq_ops = &hctx_dispatch_seq_ops},
+ {"busy", 0400, hctx_busy_show},
{"ctx_map", 0400, hctx_ctx_map_show},
{"tags", 0400, hctx_tags_show},
{"tags_bitmap", 0400, hctx_tags_bitmap_show},
--
2.12.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 4/4] blk-mq-debugfs: Add 'kick' operation
2017-05-31 21:30 [PATCH v2 0/4] blk-mq-debugfs patches for kernel v4.13 Bart Van Assche
` (2 preceding siblings ...)
2017-05-31 21:30 ` [PATCH v2 3/4] blk-mq-debugfs: Show busy requests Bart Van Assche
@ 2017-05-31 21:30 ` Bart Van Assche
2017-06-01 2:50 ` Ming Lei
2017-06-01 5:48 ` Hannes Reinecke
3 siblings, 2 replies; 16+ messages in thread
From: Bart Van Assche @ 2017-05-31 21:30 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Christoph Hellwig, Bart Van Assche, Hannes Reinecke,
Omar Sandoval, Ming Lei
Running a queue causes the block layer to examine the per-CPU and
hw queues but not the requeue list. Hence add a 'kick' operation
that also examines the requeue list.
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Ming Lei <ming.lei@redhat.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 fa0f624dfccd..962c8417809d 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -114,10 +114,12 @@ static ssize_t queue_state_write(void *data, const char __user *buf,
blk_mq_run_hw_queues(q, true);
} else if (strcmp(op, "start") == 0) {
blk_mq_start_stopped_hw_queues(q, true);
+ } else if (strcmp(op, "kick") == 0) {
+ blk_mq_kick_requeue_list(q);
} else {
pr_err("%s: unsupported operation '%s'\n", __func__, op);
inval:
- pr_err("%s: use either 'run' or 'start'\n", __func__);
+ pr_err("%s: use 'run', 'start' or 'kick'\n", __func__);
return -EINVAL;
}
return count;
--
2.12.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/4] blk-mq-debugfs: Show busy requests
2017-05-31 21:30 ` [PATCH v2 3/4] blk-mq-debugfs: Show busy requests Bart Van Assche
@ 2017-05-31 21:43 ` Eduardo Valentin
2017-05-31 21:45 ` Bart Van Assche
2017-06-01 2:47 ` Ming Lei
2017-06-01 5:48 ` Hannes Reinecke
2 siblings, 1 reply; 16+ messages in thread
From: Eduardo Valentin @ 2017-05-31 21:43 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Hannes Reinecke,
Omar Sandoval, Ming Lei
Hello,
On Wed, May 31, 2017 at 02:30:49PM -0700, Bart Van Assche wrote:
> Requests that got stuck in a block driver are neither on
> blk_mq_ctx.rq_list nor on any hw dispatch queue. Make these
> visible in debugfs through the "busy" attribute.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Ming Lei <ming.lei@redhat.com>
> ---
> block/blk-mq-debugfs.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> index 8b06a12c1461..fa0f624dfccd 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -370,6 +370,31 @@ static const struct seq_operations hctx_dispatch_seq_ops = {
> .show = blk_mq_debugfs_rq_show,
> };
>
> +struct show_busy_params {
> + struct seq_file *m;
> + struct blk_mq_hw_ctx *hctx;
> +};
> +
> +static void hctx_show_busy(struct request *rq, void *data, bool reserved)
> +{
> + const struct show_busy_params *params = data;
> +
> + if (blk_mq_map_queue(rq->q, rq->mq_ctx->cpu) == params->hctx &&
> + test_bit(REQ_ATOM_STARTED, &rq->atomic_flags))
> + __blk_mq_debugfs_rq_show(params->m,
> + list_entry_rq(&rq->queuelist));
> +}
> +
> +static int hctx_busy_show(void *data, struct seq_file *m)
> +{
> + struct blk_mq_hw_ctx *hctx = data;
> + struct show_busy_params params = { .m = m, .hctx = hctx };
> +
> + blk_mq_tagset_busy_iter(hctx->queue->tag_set, hctx_show_busy, ¶ms);
> +
> + return 0;
> +}
Why not making the two above one single function?
hctx_busy_show vs. hctx_show_busy seams a bit confusing, and I could not see
where they get reused in your patch set..
> +
> static int hctx_ctx_map_show(void *data, struct seq_file *m)
> {
> struct blk_mq_hw_ctx *hctx = data;
> @@ -705,6 +730,7 @@ static const struct blk_mq_debugfs_attr blk_mq_debugfs_hctx_attrs[] = {
> {"state", 0400, hctx_state_show},
> {"flags", 0400, hctx_flags_show},
> {"dispatch", 0400, .seq_ops = &hctx_dispatch_seq_ops},
> + {"busy", 0400, hctx_busy_show},
> {"ctx_map", 0400, hctx_ctx_map_show},
> {"tags", 0400, hctx_tags_show},
> {"tags_bitmap", 0400, hctx_tags_bitmap_show},
> --
> 2.12.2
>
>
--
All the best,
Eduardo Valentin
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/4] blk-mq-debugfs: Show busy requests
2017-05-31 21:43 ` Eduardo Valentin
@ 2017-05-31 21:45 ` Bart Van Assche
2017-05-31 21:49 ` Eduardo Valentin
0 siblings, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2017-05-31 21:45 UTC (permalink / raw)
To: eduval; +Cc: hch, osandov, linux-block, hare, axboe, ming.lei
On Wed, 2017-05-31 at 14:43 -0700, Eduardo Valentin wrote:
> On Wed, May 31, 2017 at 02:30:49PM -0700, Bart Van Assche wrote:
> > +static void hctx_show_busy(struct request *rq, void *data, bool reserv=
ed)
> > +{
> > + const struct show_busy_params *params =3D data;
> > +
> > + if (blk_mq_map_queue(rq->q, rq->mq_ctx->cpu) =3D=3D params->hctx &&
> > + test_bit(REQ_ATOM_STARTED, &rq->atomic_flags))
> > + __blk_mq_debugfs_rq_show(params->m,
> > + list_entry_rq(&rq->queuelist));
> > +}
> > +
> > +static int hctx_busy_show(void *data, struct seq_file *m)
> > +{
> > + struct blk_mq_hw_ctx *hctx =3D data;
> > + struct show_busy_params params =3D { .m =3D m, .hctx =3D hctx };
> > +
> > + blk_mq_tagset_busy_iter(hctx->queue->tag_set, hctx_show_busy, ¶ms=
);
> > +
> > + return 0;
> > +}
>=20
> Why not making the two above one single function?
> hctx_busy_show vs. hctx_show_busy seams a bit confusing, and I could not =
see
> where they get reused in your patch set..
Hello Eduardo,
If I would open-code blk_mq_tagset_busy_iter() then I would be able to impl=
ement
the above two functions as a single function. However, blk_mq_tagset_busy_i=
ter()
expects a function pointer as third argument. That's why the above function=
ality
has been split over two functions.
Bart.=
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/4] blk-mq-debugfs: Show busy requests
2017-05-31 21:45 ` Bart Van Assche
@ 2017-05-31 21:49 ` Eduardo Valentin
2017-05-31 21:54 ` Bart Van Assche
0 siblings, 1 reply; 16+ messages in thread
From: Eduardo Valentin @ 2017-05-31 21:49 UTC (permalink / raw)
To: Bart Van Assche; +Cc: eduval, hch, osandov, linux-block, hare, axboe, ming.lei
On Wed, May 31, 2017 at 09:45:54PM +0000, Bart Van Assche wrote:
> On Wed, 2017-05-31 at 14:43 -0700, Eduardo Valentin wrote:
> > On Wed, May 31, 2017 at 02:30:49PM -0700, Bart Van Assche wrote:
> > > +static void hctx_show_busy(struct request *rq, void *data, bool reserved)
> > > +{
> > > + const struct show_busy_params *params = data;
> > > +
> > > + if (blk_mq_map_queue(rq->q, rq->mq_ctx->cpu) == params->hctx &&
> > > + test_bit(REQ_ATOM_STARTED, &rq->atomic_flags))
> > > + __blk_mq_debugfs_rq_show(params->m,
> > > + list_entry_rq(&rq->queuelist));
> > > +}
> > > +
> > > +static int hctx_busy_show(void *data, struct seq_file *m)
> > > +{
> > > + struct blk_mq_hw_ctx *hctx = data;
> > > + struct show_busy_params params = { .m = m, .hctx = hctx };
> > > +
> > > + blk_mq_tagset_busy_iter(hctx->queue->tag_set, hctx_show_busy, ¶ms);
> > > +
> > > + return 0;
> > > +}
> >
> > Why not making the two above one single function?
> > hctx_busy_show vs. hctx_show_busy seams a bit confusing, and I could not see
> > where they get reused in your patch set..
>
> Hello Eduardo,
>
> If I would open-code blk_mq_tagset_busy_iter() then I would be able to implement
> the above two functions as a single function. However, blk_mq_tagset_busy_iter()
> expects a function pointer as third argument. That's why the above functionality
> has been split over two functions.
Yeah, my bad here. I misread the functions. But still the naming doesnt seam
too suggestive? how about s/hctx_show_busy/hctx_busy_entry/g?
>
> Bart.
--
All the best,
Eduardo Valentin
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/4] blk-mq-debugfs: Show busy requests
2017-05-31 21:49 ` Eduardo Valentin
@ 2017-05-31 21:54 ` Bart Van Assche
2017-05-31 23:27 ` Eduardo Valentin
0 siblings, 1 reply; 16+ messages in thread
From: Bart Van Assche @ 2017-05-31 21:54 UTC (permalink / raw)
To: eduval; +Cc: hch, ming.lei, osandov, linux-block, hare, axboe
On Wed, 2017-05-31 at 14:49 -0700, Eduardo Valentin wrote:
> On Wed, May 31, 2017 at 09:45:54PM +0000, Bart Van Assche wrote:
> > On Wed, 2017-05-31 at 14:43 -0700, Eduardo Valentin wrote:
> > > On Wed, May 31, 2017 at 02:30:49PM -0700, Bart Van Assche wrote:
> > > > +static void hctx_show_busy(struct request *rq, void *data, bool re=
served)
> > > > +{
> > > > + const struct show_busy_params *params =3D data;
> > > > +
> > > > + if (blk_mq_map_queue(rq->q, rq->mq_ctx->cpu) =3D=3D params->hctx =
&&
> > > > + test_bit(REQ_ATOM_STARTED, &rq->atomic_flags))
> > > > + __blk_mq_debugfs_rq_show(params->m,
> > > > + list_entry_rq(&rq->queuelist));
> > > > +}
> > > > +
> > > > +static int hctx_busy_show(void *data, struct seq_file *m)
> > > > +{
> > > > + struct blk_mq_hw_ctx *hctx =3D data;
> > > > + struct show_busy_params params =3D { .m =3D m, .hctx =3D hctx };
> > > > +
> > > > + blk_mq_tagset_busy_iter(hctx->queue->tag_set, hctx_show_busy, &pa=
rams);
> > > > +
> > > > + return 0;
> > > > +}
> > >=20
> > > Why not making the two above one single function?
> > > hctx_busy_show vs. hctx_show_busy seams a bit confusing, and I could =
not see
> > > where they get reused in your patch set..
> >=20
> > Hello Eduardo,
> >=20
> > If I would open-code blk_mq_tagset_busy_iter() then I would be able to =
implement
> > the above two functions as a single function. However, blk_mq_tagset_bu=
sy_iter()
> > expects a function pointer as third argument. That's why the above func=
tionality
> > has been split over two functions.
>=20
> Yeah, my bad here. I misread the functions. But still the naming doesnt s=
eam
> too suggestive? how about s/hctx_show_busy/hctx_busy_entry/g?
Hello Eduardo,
Since that function shows information about a single request, how about
hctx_show_busy_rq()?
Bart.=
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/4] blk-mq-debugfs: Show busy requests
2017-05-31 21:54 ` Bart Van Assche
@ 2017-05-31 23:27 ` Eduardo Valentin
0 siblings, 0 replies; 16+ messages in thread
From: Eduardo Valentin @ 2017-05-31 23:27 UTC (permalink / raw)
To: Bart Van Assche; +Cc: eduval, hch, ming.lei, osandov, linux-block, hare, axboe
On Wed, May 31, 2017 at 09:54:11PM +0000, Bart Van Assche wrote:
> On Wed, 2017-05-31 at 14:49 -0700, Eduardo Valentin wrote:
> > On Wed, May 31, 2017 at 09:45:54PM +0000, Bart Van Assche wrote:
> > > On Wed, 2017-05-31 at 14:43 -0700, Eduardo Valentin wrote:
> > > > On Wed, May 31, 2017 at 02:30:49PM -0700, Bart Van Assche wrote:
> > > > > +static void hctx_show_busy(struct request *rq, void *data, bool reserved)
> > > > > +{
> > > > > + const struct show_busy_params *params = data;
> > > > > +
> > > > > + if (blk_mq_map_queue(rq->q, rq->mq_ctx->cpu) == params->hctx &&
> > > > > + test_bit(REQ_ATOM_STARTED, &rq->atomic_flags))
> > > > > + __blk_mq_debugfs_rq_show(params->m,
> > > > > + list_entry_rq(&rq->queuelist));
> > > > > +}
> > > > > +
> > > > > +static int hctx_busy_show(void *data, struct seq_file *m)
> > > > > +{
> > > > > + struct blk_mq_hw_ctx *hctx = data;
> > > > > + struct show_busy_params params = { .m = m, .hctx = hctx };
> > > > > +
> > > > > + blk_mq_tagset_busy_iter(hctx->queue->tag_set, hctx_show_busy, ¶ms);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > >
> > > > Why not making the two above one single function?
> > > > hctx_busy_show vs. hctx_show_busy seams a bit confusing, and I could not see
> > > > where they get reused in your patch set..
> > >
> > > Hello Eduardo,
> > >
> > > If I would open-code blk_mq_tagset_busy_iter() then I would be able to implement
> > > the above two functions as a single function. However, blk_mq_tagset_busy_iter()
> > > expects a function pointer as third argument. That's why the above functionality
> > > has been split over two functions.
> >
> > Yeah, my bad here. I misread the functions. But still the naming doesnt seam
> > too suggestive? how about s/hctx_show_busy/hctx_busy_entry/g?
>
> Hello Eduardo,
>
> Since that function shows information about a single request, how about
> hctx_show_busy_rq()?
Sounds good to me.
>
> Bart.
--
All the best,
Eduardo Valentin
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/4] blk-mq-debugfs: Show atomic request flags
2017-05-31 21:30 ` [PATCH v2 1/4] blk-mq-debugfs: Show atomic request flags Bart Van Assche
@ 2017-06-01 2:37 ` Ming Lei
0 siblings, 0 replies; 16+ messages in thread
From: Ming Lei @ 2017-06-01 2:37 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, linux-block, Christoph Hellwig, Omar Sandoval
On Wed, May 31, 2017 at 02:30:47PM -0700, Bart Van Assche wrote:
> When analyzing e.g. queue lockups it is important to know whether
> or not a request has already been started. Hence also show the
> atomic request flags.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Reviewed-by: Hannes Reinecke <hare@suse.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Ming Lei <ming.lei@redhat.com>
> ---
> block/blk-mq-debugfs.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> index 803aed4d7221..d56ddd7a1285 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -267,6 +267,14 @@ static const char *const rqf_name[] = {
> };
> #undef RQF_NAME
>
> +#define RQAF_NAME(name) [REQ_ATOM_##name] = #name
> +static const char *const rqaf_name[] = {
> + RQAF_NAME(COMPLETE),
> + RQAF_NAME(STARTED),
> + RQAF_NAME(POLL_SLEPT),
> +};
> +#undef RQAF_NAME
> +
> int __blk_mq_debugfs_rq_show(struct seq_file *m, struct request *rq)
> {
> const struct blk_mq_ops *const mq_ops = rq->q->mq_ops;
> @@ -283,6 +291,8 @@ int __blk_mq_debugfs_rq_show(struct seq_file *m, struct request *rq)
> seq_puts(m, ", .rq_flags=");
> blk_flags_show(m, (__force unsigned int)rq->rq_flags, rqf_name,
> ARRAY_SIZE(rqf_name));
> + seq_puts(m, ", .atomic_flags=");
> + blk_flags_show(m, rq->atomic_flags, rqaf_name, ARRAY_SIZE(rqaf_name));
> seq_printf(m, ", .tag=%d, .internal_tag=%d", rq->tag,
> rq->internal_tag);
> if (mq_ops->show_rq)
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Thanks,
Ming
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/4] blk-mq-debugfs: Show requeue list
2017-05-31 21:30 ` [PATCH v2 2/4] blk-mq-debugfs: Show requeue list Bart Van Assche
@ 2017-06-01 2:41 ` Ming Lei
0 siblings, 0 replies; 16+ messages in thread
From: Ming Lei @ 2017-06-01 2:41 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, linux-block, Christoph Hellwig, Omar Sandoval
On Wed, May 31, 2017 at 02:30:48PM -0700, Bart Van Assche wrote:
> When verifying whether or not a blk-mq driver forgot to kick the
> requeue list after having requeued a request it is important to
> be able to verify the contents of the requeue list. Hence export
> that list through debugfs.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Reviewed-by: Hannes Reinecke <hare@suse.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Ming Lei <ming.lei@redhat.com>
> ---
> block/blk-mq-debugfs.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> index d56ddd7a1285..8b06a12c1461 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -308,6 +308,37 @@ int blk_mq_debugfs_rq_show(struct seq_file *m, void *v)
> }
> EXPORT_SYMBOL_GPL(blk_mq_debugfs_rq_show);
>
> +static void *queue_requeue_list_start(struct seq_file *m, loff_t *pos)
> + __acquires(&q->requeue_lock)
> +{
> + struct request_queue *q = m->private;
> +
> + spin_lock_irq(&q->requeue_lock);
> + return seq_list_start(&q->requeue_list, *pos);
> +}
> +
> +static void *queue_requeue_list_next(struct seq_file *m, void *v, loff_t *pos)
> +{
> + struct request_queue *q = m->private;
> +
> + return seq_list_next(v, &q->requeue_list, pos);
> +}
> +
> +static void queue_requeue_list_stop(struct seq_file *m, void *v)
> + __releases(&q->requeue_lock)
> +{
> + struct request_queue *q = m->private;
> +
> + spin_unlock_irq(&q->requeue_lock);
> +}
> +
> +static const struct seq_operations queue_requeue_list_seq_ops = {
> + .start = queue_requeue_list_start,
> + .next = queue_requeue_list_next,
> + .stop = queue_requeue_list_stop,
> + .show = blk_mq_debugfs_rq_show,
> +};
> +
> static void *hctx_dispatch_start(struct seq_file *m, loff_t *pos)
> __acquires(&hctx->lock)
> {
> @@ -665,6 +696,7 @@ const struct file_operations blk_mq_debugfs_fops = {
>
> static const struct blk_mq_debugfs_attr blk_mq_debugfs_queue_attrs[] = {
> {"poll_stat", 0400, queue_poll_stat_show},
> + {"requeue_list", 0400, .seq_ops = &queue_requeue_list_seq_ops},
> {"state", 0600, queue_state_show, queue_state_write},
> {},
> };
> --
> 2.12.2
>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
thanks,
Ming
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/4] blk-mq-debugfs: Show busy requests
2017-05-31 21:30 ` [PATCH v2 3/4] blk-mq-debugfs: Show busy requests Bart Van Assche
2017-05-31 21:43 ` Eduardo Valentin
@ 2017-06-01 2:47 ` Ming Lei
2017-06-01 5:48 ` Hannes Reinecke
2 siblings, 0 replies; 16+ messages in thread
From: Ming Lei @ 2017-06-01 2:47 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Hannes Reinecke,
Omar Sandoval
On Wed, May 31, 2017 at 02:30:49PM -0700, Bart Van Assche wrote:
> Requests that got stuck in a block driver are neither on
> blk_mq_ctx.rq_list nor on any hw dispatch queue. Make these
> visible in debugfs through the "busy" attribute.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Ming Lei <ming.lei@redhat.com>
> ---
> block/blk-mq-debugfs.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> index 8b06a12c1461..fa0f624dfccd 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -370,6 +370,31 @@ static const struct seq_operations hctx_dispatch_seq_ops = {
> .show = blk_mq_debugfs_rq_show,
> };
>
> +struct show_busy_params {
> + struct seq_file *m;
> + struct blk_mq_hw_ctx *hctx;
> +};
> +
> +static void hctx_show_busy(struct request *rq, void *data, bool reserved)
> +{
> + const struct show_busy_params *params = data;
> +
> + if (blk_mq_map_queue(rq->q, rq->mq_ctx->cpu) == params->hctx &&
> + test_bit(REQ_ATOM_STARTED, &rq->atomic_flags))
> + __blk_mq_debugfs_rq_show(params->m,
> + list_entry_rq(&rq->queuelist));
> +}
Not like dumping requests in ctx and requeue list, the dumped requests
here may have been released and the result may not be 100% reliable,
so suggest to add comment for this fact.
Otherwise, looks fine for me.
Thanks,
Ming
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 4/4] blk-mq-debugfs: Add 'kick' operation
2017-05-31 21:30 ` [PATCH v2 4/4] blk-mq-debugfs: Add 'kick' operation Bart Van Assche
@ 2017-06-01 2:50 ` Ming Lei
2017-06-01 5:48 ` Hannes Reinecke
1 sibling, 0 replies; 16+ messages in thread
From: Ming Lei @ 2017-06-01 2:50 UTC (permalink / raw)
To: Bart Van Assche
Cc: Jens Axboe, linux-block, Christoph Hellwig, Hannes Reinecke,
Omar Sandoval
On Wed, May 31, 2017 at 02:30:50PM -0700, Bart Van Assche wrote:
> Running a queue causes the block layer to examine the per-CPU and
> hw queues but not the requeue list. Hence add a 'kick' operation
> that also examines the requeue list.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Ming Lei <ming.lei@redhat.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 fa0f624dfccd..962c8417809d 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -114,10 +114,12 @@ static ssize_t queue_state_write(void *data, const char __user *buf,
> blk_mq_run_hw_queues(q, true);
> } else if (strcmp(op, "start") == 0) {
> blk_mq_start_stopped_hw_queues(q, true);
> + } else if (strcmp(op, "kick") == 0) {
> + blk_mq_kick_requeue_list(q);
> } else {
> pr_err("%s: unsupported operation '%s'\n", __func__, op);
> inval:
> - pr_err("%s: use either 'run' or 'start'\n", __func__);
> + pr_err("%s: use 'run', 'start' or 'kick'\n", __func__);
> return -EINVAL;
> }
> return count;
> --
> 2.12.2
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Thanks,
Ming
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/4] blk-mq-debugfs: Show busy requests
2017-05-31 21:30 ` [PATCH v2 3/4] blk-mq-debugfs: Show busy requests Bart Van Assche
2017-05-31 21:43 ` Eduardo Valentin
2017-06-01 2:47 ` Ming Lei
@ 2017-06-01 5:48 ` Hannes Reinecke
2 siblings, 0 replies; 16+ messages in thread
From: Hannes Reinecke @ 2017-06-01 5:48 UTC (permalink / raw)
To: Bart Van Assche, Jens Axboe
Cc: linux-block, Christoph Hellwig, Hannes Reinecke, Omar Sandoval, Ming Lei
On 05/31/2017 11:30 PM, Bart Van Assche wrote:
> Requests that got stuck in a block driver are neither on
> blk_mq_ctx.rq_list nor on any hw dispatch queue. Make these
> visible in debugfs through the "busy" attribute.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Ming Lei <ming.lei@redhat.com>
> ---
> block/blk-mq-debugfs.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 4/4] blk-mq-debugfs: Add 'kick' operation
2017-05-31 21:30 ` [PATCH v2 4/4] blk-mq-debugfs: Add 'kick' operation Bart Van Assche
2017-06-01 2:50 ` Ming Lei
@ 2017-06-01 5:48 ` Hannes Reinecke
1 sibling, 0 replies; 16+ messages in thread
From: Hannes Reinecke @ 2017-06-01 5:48 UTC (permalink / raw)
To: Bart Van Assche, Jens Axboe
Cc: linux-block, Christoph Hellwig, Hannes Reinecke, Omar Sandoval, Ming Lei
On 05/31/2017 11:30 PM, Bart Van Assche wrote:
> Running a queue causes the block layer to examine the per-CPU and
> hw queues but not the requeue list. Hence add a 'kick' operation
> that also examines the requeue list.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Ming Lei <ming.lei@redhat.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 fa0f624dfccd..962c8417809d 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -114,10 +114,12 @@ static ssize_t queue_state_write(void *data, const char __user *buf,
> blk_mq_run_hw_queues(q, true);
> } else if (strcmp(op, "start") == 0) {
> blk_mq_start_stopped_hw_queues(q, true);
> + } else if (strcmp(op, "kick") == 0) {
> + blk_mq_kick_requeue_list(q);
> } else {
> pr_err("%s: unsupported operation '%s'\n", __func__, op);
> inval:
> - pr_err("%s: use either 'run' or 'start'\n", __func__);
> + pr_err("%s: use 'run', 'start' or 'kick'\n", __func__);
> return -EINVAL;
> }
> return count;
>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-06-01 5:48 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-31 21:30 [PATCH v2 0/4] blk-mq-debugfs patches for kernel v4.13 Bart Van Assche
2017-05-31 21:30 ` [PATCH v2 1/4] blk-mq-debugfs: Show atomic request flags Bart Van Assche
2017-06-01 2:37 ` Ming Lei
2017-05-31 21:30 ` [PATCH v2 2/4] blk-mq-debugfs: Show requeue list Bart Van Assche
2017-06-01 2:41 ` Ming Lei
2017-05-31 21:30 ` [PATCH v2 3/4] blk-mq-debugfs: Show busy requests Bart Van Assche
2017-05-31 21:43 ` Eduardo Valentin
2017-05-31 21:45 ` Bart Van Assche
2017-05-31 21:49 ` Eduardo Valentin
2017-05-31 21:54 ` Bart Van Assche
2017-05-31 23:27 ` Eduardo Valentin
2017-06-01 2:47 ` Ming Lei
2017-06-01 5:48 ` Hannes Reinecke
2017-05-31 21:30 ` [PATCH v2 4/4] blk-mq-debugfs: Add 'kick' operation Bart Van Assche
2017-06-01 2:50 ` Ming Lei
2017-06-01 5:48 ` Hannes Reinecke
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.