[PATCH v3 0/4] Extend blk-mq-debugfs functionality further

[PATCH v3 0/4] Extend blk-mq-debugfs functionality further

[Bart Van Assche]

Hello Jens,

This series contains four patches that add new features to the blk-mq-debugfs
code. Please consider these patches for kernel v4.13.

Thanks,

Bart.

Changes between v2 and v3:
- Renamed a function in the patch that shows busy requests. In the same patch,
  added a comment as requested by Ming.
- Added Reviewed-by tags to those patches that got an additional review.  

Changes compared to v1:
- 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().

The patches in this series are:

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 | 77 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 76 insertions(+), 1 deletion(-)

-- 
2.12.2

[PATCH v3 1/4] blk-mq-debugfs: Show atomic request flags

[Bart Van Assche]

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>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Christoph Hellwig <hch@lst.de>
---
 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

[PATCH v3 2/4] blk-mq-debugfs: Show requeue list

[Bart Van Assche]

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>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Omar Sandoval <osandov@fb.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

[PATCH v3 3/4] blk-mq-debugfs: Show busy requests

[Bart Van Assche]

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>
Cc: Eduardo Valentin <eduval@amazon.com>
---
 block/blk-mq-debugfs.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 8b06a12c1461..90c454bbaf92 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -370,6 +370,36 @@ 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;
+};
+
+/*
+ * Note: the state of a request may change while this function is in progress,
+ * e.g. due to a concurrent blk_mq_finish_request() call.
+ */
+static void hctx_show_busy_rq(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_rq,
+				&params);
+
+	return 0;
+}
+
 static int hctx_ctx_map_show(void *data, struct seq_file *m)
 {
 	struct blk_mq_hw_ctx *hctx = data;
@@ -705,6 +735,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

[PATCH v3 4/4] blk-mq-debugfs: Add 'kick' operation

[Bart Van Assche]

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>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Omar Sandoval <osandov@fb.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 90c454bbaf92..9edebbdce0bd 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

Re: [PATCH v3 0/4] Extend blk-mq-debugfs functionality further

[Eduardo Valentin]

On Thu, Jun 01, 2017 at 08:55:09AM -0700, Bart Van Assche wrote:
> Hello Jens,
> 
> This series contains four patches that add new features to the blk-mq-debugfs
> code. Please consider these patches for kernel v4.13.
> 
> Thanks,
> 
> Bart.
> 
> Changes between v2 and v3:
> - Renamed a function in the patch that shows busy requests. In the same patch,
>   added a comment as requested by Ming.
> - Added Reviewed-by tags to those patches that got an additional review.  
> 
> Changes compared to v1:
> - 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().
> 
> The patches in this series are:
> 
> 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

For the patchset, you can add my
Reviewed-by: Eduardo Valentin <eduval@amazon.com>

> 
>  block/blk-mq-debugfs.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 76 insertions(+), 1 deletion(-)
> 
> -- 
> 2.12.2
> 
> 

-- 
All the best,
Eduardo Valentin

Re: [PATCH v3 0/4] Extend blk-mq-debugfs functionality further

[Jens Axboe]

On 06/01/2017 08:55 AM, Bart Van Assche wrote:
> Hello Jens,
> 
> This series contains four patches that add new features to the blk-mq-debugfs
> code. Please consider these patches for kernel v4.13.
> 
> Thanks,
> 
> Bart.
> 
> Changes between v2 and v3:
> - Renamed a function in the patch that shows busy requests. In the same patch,
>   added a comment as requested by Ming.
> - Added Reviewed-by tags to those patches that got an additional review.  
> 
> Changes compared to v1:
> - 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().
> 
> The patches in this series are:
> 
> 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 | 77 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 76 insertions(+), 1 deletion(-)

Added for 4.13, thanks Bart.

-- 
Jens Axboe

Re: [PATCH v3 3/4] blk-mq-debugfs: Show busy requests

[Ming Lei]

On Thu, Jun 01, 2017 at 08:55:12AM -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>
> Cc: Eduardo Valentin <eduval@amazon.com>
> ---
>  block/blk-mq-debugfs.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> index 8b06a12c1461..90c454bbaf92 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -370,6 +370,36 @@ 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;
> +};
> +
> +/*
> + * Note: the state of a request may change while this function is in progress,
> + * e.g. due to a concurrent blk_mq_finish_request() call.
> + */
> +static void hctx_show_busy_rq(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_rq,
> +				&params);
> +
> +	return 0;
> +}
> +
>  static int hctx_ctx_map_show(void *data, struct seq_file *m)
>  {
>  	struct blk_mq_hw_ctx *hctx = data;
> @@ -705,6 +735,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
> 

Reviewed-by: Ming Lei <ming.lei@redhat.com>

Thanks,
Ming