* [PATCH 1/6] blk-mq: Do not invoke queue operations on a dead queue
2017-04-11 20:58 [PATCH 0/6] blk-mq debugfs patches for kernel v4.12 Bart Van Assche
@ 2017-04-11 20:58 ` Bart Van Assche
2017-04-13 23:01 ` Omar Sandoval
2017-04-11 20:58 ` [PATCH 2/6] blk-mq: Move the "state" debugfs attribute one level down Bart Van Assche
` (4 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Bart Van Assche @ 2017-04-11 20:58 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Omar Sandoval, Hannes Reinecke
The blk-mq debugfs attributes are removed after blk_cleanup_queue()
has finished. Since running a queue after a queue has entered the
"dead" state is not allowed, disallow this. This patch avoids that
an attempt to run a dead queue triggers a kernel crash.
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 | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index df9b688b877c..a1ce823578c7 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -111,6 +111,14 @@ static ssize_t blk_queue_flags_store(struct file *file, const char __user *ubuf,
struct request_queue *q = file_inode(file)->i_private;
char op[16] = { }, *s;
+ /*
+ * The debugfs attributes are removed after blk_cleanup_queue() has
+ * called blk_mq_free_queue(). Return if QUEUE_FLAG_DEAD has been set
+ * to avoid triggering a use-after-free.
+ */
+ if (blk_queue_dead(q))
+ return -ENOENT;
+
len = min(len, sizeof(op) - 1);
if (copy_from_user(op, ubuf, len))
return -EFAULT;
--
2.12.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] blk-mq: Do not invoke queue operations on a dead queue
2017-04-11 20:58 ` [PATCH 1/6] blk-mq: Do not invoke queue operations on a dead queue Bart Van Assche
@ 2017-04-13 23:01 ` Omar Sandoval
2017-04-13 23:03 ` Omar Sandoval
2017-04-13 23:05 ` Bart Van Assche
0 siblings, 2 replies; 20+ messages in thread
From: Omar Sandoval @ 2017-04-13 23:01 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, linux-block, Omar Sandoval, Hannes Reinecke
On Tue, Apr 11, 2017 at 01:58:37PM -0700, Bart Van Assche wrote:
> The blk-mq debugfs attributes are removed after blk_cleanup_queue()
> has finished. Since running a queue after a queue has entered the
> "dead" state is not allowed, disallow this. This patch avoids that
> an attempt to run a dead queue triggers a kernel crash.
>
> 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 | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> index df9b688b877c..a1ce823578c7 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -111,6 +111,14 @@ static ssize_t blk_queue_flags_store(struct file *file, const char __user *ubuf,
> struct request_queue *q = file_inode(file)->i_private;
> char op[16] = { }, *s;
>
> + /*
> + * The debugfs attributes are removed after blk_cleanup_queue() has
> + * called blk_mq_free_queue(). Return if QUEUE_FLAG_DEAD has been set
> + * to avoid triggering a use-after-free.
> + */
> + if (blk_queue_dead(q))
> + return -ENOENT;
> +
> len = min(len, sizeof(op) - 1);
> if (copy_from_user(op, ubuf, len))
> return -EFAULT;
> --
> 2.12.0
>
Hi, Bart,
Looking at this, I think we have similar issues with most of the other
debugfs files. Should we move the debugfs cleanup earlier?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] blk-mq: Do not invoke queue operations on a dead queue
2017-04-13 23:01 ` Omar Sandoval
@ 2017-04-13 23:03 ` Omar Sandoval
2017-04-13 23:05 ` Bart Van Assche
1 sibling, 0 replies; 20+ messages in thread
From: Omar Sandoval @ 2017-04-13 23:03 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, linux-block, Omar Sandoval, Hannes Reinecke
On Thu, Apr 13, 2017 at 04:01:02PM -0700, Omar Sandoval wrote:
> On Tue, Apr 11, 2017 at 01:58:37PM -0700, Bart Van Assche wrote:
> > The blk-mq debugfs attributes are removed after blk_cleanup_queue()
> > has finished. Since running a queue after a queue has entered the
> > "dead" state is not allowed, disallow this. This patch avoids that
> > an attempt to run a dead queue triggers a kernel crash.
> >
> > 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 | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> > index df9b688b877c..a1ce823578c7 100644
> > --- a/block/blk-mq-debugfs.c
> > +++ b/block/blk-mq-debugfs.c
> > @@ -111,6 +111,14 @@ static ssize_t blk_queue_flags_store(struct file *file, const char __user *ubuf,
> > struct request_queue *q = file_inode(file)->i_private;
> > char op[16] = { }, *s;
> >
> > + /*
> > + * The debugfs attributes are removed after blk_cleanup_queue() has
> > + * called blk_mq_free_queue(). Return if QUEUE_FLAG_DEAD has been set
> > + * to avoid triggering a use-after-free.
> > + */
> > + if (blk_queue_dead(q))
> > + return -ENOENT;
> > +
> > len = min(len, sizeof(op) - 1);
> > if (copy_from_user(op, ubuf, len))
> > return -EFAULT;
> > --
> > 2.12.0
> >
>
> Hi, Bart,
>
> Looking at this, I think we have similar issues with most of the other
> debugfs files. Should we move the debugfs cleanup earlier?
In particular, I think we can call blk_mq_debugfs_unregister_hctxs()
(which is somewhat poorly named, as it removes the whole mq directory)
before we call blk_mq_free_queue(). I was under the impression that
that's what it already did, but I think I was wrong.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] blk-mq: Do not invoke queue operations on a dead queue
2017-04-13 23:01 ` Omar Sandoval
2017-04-13 23:03 ` Omar Sandoval
@ 2017-04-13 23:05 ` Bart Van Assche
2017-04-14 7:40 ` Omar Sandoval
1 sibling, 1 reply; 20+ messages in thread
From: Bart Van Assche @ 2017-04-13 23:05 UTC (permalink / raw)
To: osandov; +Cc: hare, linux-block, osandov, axboe
On Thu, 2017-04-13 at 16:01 -0700, Omar Sandoval wrote:
> On Tue, Apr 11, 2017 at 01:58:37PM -0700, Bart Van Assche wrote:
> > The blk-mq debugfs attributes are removed after blk_cleanup_queue()
> > has finished. Since running a queue after a queue has entered the
> > "dead" state is not allowed, disallow this. This patch avoids that
> > an attempt to run a dead queue triggers a kernel crash.
> >=20
> > 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 | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >=20
> > diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> > index df9b688b877c..a1ce823578c7 100644
> > --- a/block/blk-mq-debugfs.c
> > +++ b/block/blk-mq-debugfs.c
> > @@ -111,6 +111,14 @@ static ssize_t blk_queue_flags_store(struct file *=
file, const char __user *ubuf,
> > struct request_queue *q =3D file_inode(file)->i_private;
> > char op[16] =3D { }, *s;
> > =20
> > + /*
> > + * The debugfs attributes are removed after blk_cleanup_queue() has
> > + * called blk_mq_free_queue(). Return if QUEUE_FLAG_DEAD has been set
> > + * to avoid triggering a use-after-free.
> > + */
> > + if (blk_queue_dead(q))
> > + return -ENOENT;
> > +
> > len =3D min(len, sizeof(op) - 1);
> > if (copy_from_user(op, ubuf, len))
> > return -EFAULT;
>=20
> Looking at this, I think we have similar issues with most of the other
> debugfs files. Should we move the debugfs cleanup earlier?
Hello Omar,
That's a good question. However, while I was debugging it was very convenie=
nt
to be able to access the queue state after it had reached the "dead" state.
Performing the cleanup earlier would be an alternative solution but would
make debugging a bit harder ...
Bart.=
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] blk-mq: Do not invoke queue operations on a dead queue
2017-04-13 23:05 ` Bart Van Assche
@ 2017-04-14 7:40 ` Omar Sandoval
2017-04-14 16:12 ` Bart Van Assche
0 siblings, 1 reply; 20+ messages in thread
From: Omar Sandoval @ 2017-04-14 7:40 UTC (permalink / raw)
To: Bart Van Assche; +Cc: hare, linux-block, osandov, axboe
On Thu, Apr 13, 2017 at 11:05:32PM +0000, Bart Van Assche wrote:
> On Thu, 2017-04-13 at 16:01 -0700, Omar Sandoval wrote:
> > On Tue, Apr 11, 2017 at 01:58:37PM -0700, Bart Van Assche wrote:
> > > The blk-mq debugfs attributes are removed after blk_cleanup_queue()
> > > has finished. Since running a queue after a queue has entered the
> > > "dead" state is not allowed, disallow this. This patch avoids that
> > > an attempt to run a dead queue triggers a kernel crash.
> > >
> > > 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 | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> > > index df9b688b877c..a1ce823578c7 100644
> > > --- a/block/blk-mq-debugfs.c
> > > +++ b/block/blk-mq-debugfs.c
> > > @@ -111,6 +111,14 @@ static ssize_t blk_queue_flags_store(struct file *file, const char __user *ubuf,
> > > struct request_queue *q = file_inode(file)->i_private;
> > > char op[16] = { }, *s;
> > >
> > > + /*
> > > + * The debugfs attributes are removed after blk_cleanup_queue() has
> > > + * called blk_mq_free_queue(). Return if QUEUE_FLAG_DEAD has been set
> > > + * to avoid triggering a use-after-free.
> > > + */
> > > + if (blk_queue_dead(q))
> > > + return -ENOENT;
> > > +
> > > len = min(len, sizeof(op) - 1);
> > > if (copy_from_user(op, ubuf, len))
> > > return -EFAULT;
> >
> > Looking at this, I think we have similar issues with most of the other
> > debugfs files. Should we move the debugfs cleanup earlier?
>
> Hello Omar,
>
> That's a good question. However, while I was debugging it was very convenient
> to be able to access the queue state after it had reached the "dead" state.
> Performing the cleanup earlier would be an alternative solution but would
> make debugging a bit harder ...
>
> Bart.
What useful information were you getting out of debugfs once the queue
was already dead? Wasn't the interesting stuff freed at that point?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] blk-mq: Do not invoke queue operations on a dead queue
2017-04-14 7:40 ` Omar Sandoval
@ 2017-04-14 16:12 ` Bart Van Assche
2017-04-14 17:13 ` Omar Sandoval
0 siblings, 1 reply; 20+ messages in thread
From: Bart Van Assche @ 2017-04-14 16:12 UTC (permalink / raw)
To: osandov; +Cc: hare, linux-block, osandov, axboe
On Fri, 2017-04-14 at 00:40 -0700, Omar Sandoval wrote:
> On Thu, Apr 13, 2017 at 11:05:32PM +0000, Bart Van Assche wrote:
> > On Thu, 2017-04-13 at 16:01 -0700, Omar Sandoval wrote:
> > > Looking at this, I think we have similar issues with most of the othe=
r
> > > debugfs files. Should we move the debugfs cleanup earlier?
> >=20
> > That's a good question. However, while I was debugging it was very conv=
enient
> > to be able to access the queue state after it had reached the "dead" st=
ate.
> > Performing the cleanup earlier would be an alternative solution but wou=
ld
> > make debugging a bit harder ...
>=20
> What useful information were you getting out of debugfs once the queue
> was already dead? Wasn't the interesting stuff freed at that point?
Hello Omar,
I'm currently chasing a stall of dm-rq + dm-mpath that occurs after the
queues below it have reached the "dead" state. I will look for another
way to obtain the information I need such that we can remove the block
layer queue debugfs information before these queues reach the "dead"
state.
Bart.=
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] blk-mq: Do not invoke queue operations on a dead queue
2017-04-14 16:12 ` Bart Van Assche
@ 2017-04-14 17:13 ` Omar Sandoval
2017-04-14 17:37 ` Bart Van Assche
0 siblings, 1 reply; 20+ messages in thread
From: Omar Sandoval @ 2017-04-14 17:13 UTC (permalink / raw)
To: Bart Van Assche; +Cc: hare, linux-block, osandov, axboe
On Fri, Apr 14, 2017 at 04:12:01PM +0000, Bart Van Assche wrote:
> On Fri, 2017-04-14 at 00:40 -0700, Omar Sandoval wrote:
> > On Thu, Apr 13, 2017 at 11:05:32PM +0000, Bart Van Assche wrote:
> > > On Thu, 2017-04-13 at 16:01 -0700, Omar Sandoval wrote:
> > > > Looking at this, I think we have similar issues with most of the other
> > > > debugfs files. Should we move the debugfs cleanup earlier?
> > >
> > > That's a good question. However, while I was debugging it was very convenient
> > > to be able to access the queue state after it had reached the "dead" state.
> > > Performing the cleanup earlier would be an alternative solution but would
> > > make debugging a bit harder ...
> >
> > What useful information were you getting out of debugfs once the queue
> > was already dead? Wasn't the interesting stuff freed at that point?
>
> Hello Omar,
>
> I'm currently chasing a stall of dm-rq + dm-mpath that occurs after the
> queues below it have reached the "dead" state. I will look for another
> way to obtain the information I need such that we can remove the block
> layer queue debugfs information before these queues reach the "dead"
> state.
>
> Bart.
Thanks, Bart. In this case, the absence of the "mq" directory should
tell you that the queue is dead. Will you move the cleanup in v2 or
should I submit a separate patch?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/6] blk-mq: Do not invoke queue operations on a dead queue
2017-04-14 17:13 ` Omar Sandoval
@ 2017-04-14 17:37 ` Bart Van Assche
0 siblings, 0 replies; 20+ messages in thread
From: Bart Van Assche @ 2017-04-14 17:37 UTC (permalink / raw)
To: osandov; +Cc: hare, linux-block, osandov, axboe
On Fri, 2017-04-14 at 10:13 -0700, Omar Sandoval wrote:
> Thanks, Bart. In this case, the absence of the "mq" directory should
> tell you that the queue is dead. Will you move the cleanup in v2 or
> should I submit a separate patch?
Hello Omar,
I will include a patch in v2 of this patch series that unregisters the
debugfs attributes earlier.
Bart.=
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/6] blk-mq: Move the "state" debugfs attribute one level down
2017-04-11 20:58 [PATCH 0/6] blk-mq debugfs patches for kernel v4.12 Bart Van Assche
2017-04-11 20:58 ` [PATCH 1/6] blk-mq: Do not invoke queue operations on a dead queue Bart Van Assche
@ 2017-04-11 20:58 ` Bart Van Assche
2017-04-13 23:01 ` Omar Sandoval
2017-04-11 20:58 ` [PATCH 3/6] blk-mq: Make blk_flags_show() callers append a newline character Bart Van Assche
` (3 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Bart Van Assche @ 2017-04-11 20:58 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Omar Sandoval, Hannes Reinecke
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>
Cc: 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 a1ce823578c7..564470d4af52 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -149,11 +149,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) {
@@ -762,6 +757,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},
{},
};
@@ -877,9 +873,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.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/6] blk-mq: Move the "state" debugfs attribute one level down
2017-04-11 20:58 ` [PATCH 2/6] blk-mq: Move the "state" debugfs attribute one level down Bart Van Assche
@ 2017-04-13 23:01 ` Omar Sandoval
0 siblings, 0 replies; 20+ messages in thread
From: Omar Sandoval @ 2017-04-13 23:01 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, linux-block, Omar Sandoval, Hannes Reinecke
On Tue, Apr 11, 2017 at 01:58:38PM -0700, Bart Van Assche wrote:
> 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>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Hannes Reinecke <hare@suse.com>
Thanks!
Reviewed-by: Omar Sandoval <osandov@fb.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 a1ce823578c7..564470d4af52 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -149,11 +149,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) {
> @@ -762,6 +757,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},
> {},
> };
>
> @@ -877,9 +873,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.0
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/6] blk-mq: Make blk_flags_show() callers append a newline character
2017-04-11 20:58 [PATCH 0/6] blk-mq debugfs patches for kernel v4.12 Bart Van Assche
2017-04-11 20:58 ` [PATCH 1/6] blk-mq: Do not invoke queue operations on a dead queue Bart Van Assche
2017-04-11 20:58 ` [PATCH 2/6] blk-mq: Move the "state" debugfs attribute one level down Bart Van Assche
@ 2017-04-11 20:58 ` Bart Van Assche
2017-04-13 23:08 ` Omar Sandoval
2017-04-11 20:58 ` [PATCH 4/6] blk-mq: Show operation, cmd_flags and rq_flags names Bart Van Assche
` (2 subsequent siblings)
5 siblings, 1 reply; 20+ messages in thread
From: Bart Van Assche @ 2017-04-11 20:58 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Omar Sandoval, Hannes Reinecke
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>
Cc: 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 564470d4af52..aae4b7c7b7b0 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;
}
@@ -198,6 +198,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;
}
@@ -241,6 +242,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.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/6] blk-mq: Show operation, cmd_flags and rq_flags names
2017-04-11 20:58 [PATCH 0/6] blk-mq debugfs patches for kernel v4.12 Bart Van Assche
` (2 preceding siblings ...)
2017-04-11 20:58 ` [PATCH 3/6] blk-mq: Make blk_flags_show() callers append a newline character Bart Van Assche
@ 2017-04-11 20:58 ` Bart Van Assche
2017-04-13 23:17 ` Omar Sandoval
2017-04-11 20:58 ` [PATCH 5/6] blk-mq: Add blk_mq_ops.show_rq() Bart Van Assche
2017-04-11 20:58 ` Bart Van Assche
5 siblings, 1 reply; 20+ messages in thread
From: Bart Van Assche @ 2017-04-11 20:58 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Omar Sandoval, Hannes Reinecke
Show the operation name, .cmd_flags and .rq_flags as names instead
of numbers.
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 | 72 +++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 69 insertions(+), 3 deletions(-)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index aae4b7c7b7b0..161f30fc236f 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -258,13 +258,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 ^ op, 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.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 4/6] blk-mq: Show operation, cmd_flags and rq_flags names
2017-04-11 20:58 ` [PATCH 4/6] blk-mq: Show operation, cmd_flags and rq_flags names Bart Van Assche
@ 2017-04-13 23:17 ` Omar Sandoval
0 siblings, 0 replies; 20+ messages in thread
From: Omar Sandoval @ 2017-04-13 23:17 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, linux-block, Omar Sandoval, Hannes Reinecke
On Tue, Apr 11, 2017 at 01:58:40PM -0700, Bart Van Assche wrote:
> Show the operation name, .cmd_flags and .rq_flags as names instead
> of numbers.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Hannes Reinecke <hare@suse.com>
I like this :) one minor nit below.
Reviewed-by: Omar Sandoval <osandov@fb.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 aae4b7c7b7b0..161f30fc236f 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
[snip]
> 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 ^ op, cmd_flag_name,
^^^^^^^^^^^^^^^^^^
I think rq->cmd_flags & ~REQ_OP_MASK is slightly clearer here, but I
don't feel that strongly about it, it's up to you.
> + 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.0
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 5/6] blk-mq: Add blk_mq_ops.show_rq()
2017-04-11 20:58 [PATCH 0/6] blk-mq debugfs patches for kernel v4.12 Bart Van Assche
` (3 preceding siblings ...)
2017-04-11 20:58 ` [PATCH 4/6] blk-mq: Show operation, cmd_flags and rq_flags names Bart Van Assche
@ 2017-04-11 20:58 ` Bart Van Assche
2017-04-13 23:21 ` Omar Sandoval
2017-04-11 20:58 ` Bart Van Assche
5 siblings, 1 reply; 20+ messages in thread
From: Bart Van Assche @ 2017-04-11 20:58 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Omar Sandoval, Hannes Reinecke
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 | 10 ++++++++--
include/linux/blk-mq.h | 6 ++++++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 161f30fc236f..6b28d92d4c0e 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -316,7 +316,9 @@ 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;
+ char drv_info[200];
seq_printf(m, "%p {.op=", rq);
if (op < ARRAY_SIZE(op_name) && op_name[op])
@@ -329,8 +331,12 @@ 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,
- rq->internal_tag);
+ if (mq_ops->show_rq)
+ mq_ops->show_rq(rq, drv_info, sizeof(drv_info));
+ else
+ drv_info[0] = '\0';
+ seq_printf(m, ", .tag=%d, .internal_tag=%d%s%s}\n", rq->tag,
+ rq->internal_tag, drv_info[0] ? ", " : "", drv_info);
return 0;
}
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index dea255dee359..de2a1c2ddd84 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -120,6 +120,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 request *rq, char *info, unsigned int info_sz);
+
+ /*
* 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.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 5/6] blk-mq: Add blk_mq_ops.show_rq()
2017-04-11 20:58 ` [PATCH 5/6] blk-mq: Add blk_mq_ops.show_rq() Bart Van Assche
@ 2017-04-13 23:21 ` Omar Sandoval
2017-04-14 16:03 ` Bart Van Assche
0 siblings, 1 reply; 20+ messages in thread
From: Omar Sandoval @ 2017-04-13 23:21 UTC (permalink / raw)
To: Bart Van Assche; +Cc: Jens Axboe, linux-block, Omar Sandoval, Hannes Reinecke
On Tue, Apr 11, 2017 at 01:58:41PM -0700, Bart Van Assche wrote:
> 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 | 10 ++++++++--
> include/linux/blk-mq.h | 6 ++++++
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> index 161f30fc236f..6b28d92d4c0e 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -316,7 +316,9 @@ 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;
> + char drv_info[200];
>
> seq_printf(m, "%p {.op=", rq);
> if (op < ARRAY_SIZE(op_name) && op_name[op])
> @@ -329,8 +331,12 @@ 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,
> - rq->internal_tag);
> + if (mq_ops->show_rq)
> + mq_ops->show_rq(rq, drv_info, sizeof(drv_info));
How about passing the seq_file to the callback instead of this
arbitrarily-sized on-stack buffer?
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 6/6] scsi: Implement blk_mq_ops.show_rq()
2017-04-11 20:58 [PATCH 0/6] blk-mq debugfs patches for kernel v4.12 Bart Van Assche
@ 2017-04-11 20:58 ` Bart Van Assche
2017-04-11 20:58 ` [PATCH 2/6] blk-mq: Move the "state" debugfs attribute one level down Bart Van Assche
` (4 subsequent siblings)
5 siblings, 0 replies; 20+ messages in thread
From: Bart Van Assche @ 2017-04-11 20:58 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Bart Van Assche, Martin K . Petersen,
James Bottomley, Omar Sandoval, Hannes Reinecke, linux-scsi
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 | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 7bc4513bf4e4..7d3efb8924ee 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2126,6 +2126,32 @@ 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 request *rq, char *info, unsigned int info_sz)
+{
+ char *p = info, *const end = info + info_sz;
+ struct scsi_cmnd *cmd = container_of(scsi_req(rq), typeof(*cmd), req);
+ unsigned int i;
+
+ p += scnprintf(p, end - p, ".cmd =");
+ for (i = 0; i < cmd->cmd_len; i++)
+ p += scnprintf(p, end - p, " %02x", cmd->cmnd[i]);
+ p += scnprintf(p, end - p, ", .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])
+ p += scnprintf(p, end - p, " %s", ehflag_name[i]);
+ else
+ p += scnprintf(p, end - p, " %d", i);
+ }
+ p += scnprintf(p, end - p, ", .result = %#06x", cmd->result);
+}
+
struct request_queue *scsi_alloc_queue(struct scsi_device *sdev)
{
struct Scsi_Host *shost = sdev->host;
@@ -2158,6 +2184,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.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 6/6] scsi: Implement blk_mq_ops.show_rq()
@ 2017-04-11 20:58 ` Bart Van Assche
0 siblings, 0 replies; 20+ messages in thread
From: Bart Van Assche @ 2017-04-11 20:58 UTC (permalink / raw)
To: Jens Axboe
Cc: linux-block, Bart Van Assche, Martin K . Petersen,
James Bottomley, Omar Sandoval, Hannes Reinecke, linux-scsi
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 | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 7bc4513bf4e4..7d3efb8924ee 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2126,6 +2126,32 @@ 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 request *rq, char *info, unsigned int info_sz)
+{
+ char *p = info, *const end = info + info_sz;
+ struct scsi_cmnd *cmd = container_of(scsi_req(rq), typeof(*cmd), req);
+ unsigned int i;
+
+ p += scnprintf(p, end - p, ".cmd =");
+ for (i = 0; i < cmd->cmd_len; i++)
+ p += scnprintf(p, end - p, " %02x", cmd->cmnd[i]);
+ p += scnprintf(p, end - p, ", .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])
+ p += scnprintf(p, end - p, " %s", ehflag_name[i]);
+ else
+ p += scnprintf(p, end - p, " %d", i);
+ }
+ p += scnprintf(p, end - p, ", .result = %#06x", cmd->result);
+}
+
struct request_queue *scsi_alloc_queue(struct scsi_device *sdev)
{
struct Scsi_Host *shost = sdev->host;
@@ -2158,6 +2184,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.0
^ permalink raw reply related [flat|nested] 20+ messages in thread