[PATCH v2 0/4] blk-mq-debugfs patches for kernel v4.13

[PATCH v2 0/4] blk-mq-debugfs patches for kernel v4.13

[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

[PATCH v2 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>
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

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

[Ming Lei]

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

[PATCH v2 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>
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

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

[Ming Lei]

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

[PATCH v2 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>
---
 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, &params);
+
+	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

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

[Eduardo Valentin]

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, &params);
> +
> +	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

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

[Bart Van Assche]

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, &params=
);
> > +
> > +	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.=

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

[Eduardo Valentin]

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, &params);
> > > +
> > > +	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

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

[Bart Van Assche]

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.=

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

[Eduardo Valentin]

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, &params);
> > > > > +
> > > > > +	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

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

[Ming Lei]

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

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

[Hannes Reinecke]

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)

[PATCH v2 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>
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

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

[Ming Lei]

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

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

[Hannes Reinecke]

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)