[PATCH 0/2] Export more queue state information through debugfs

[PATCH 0/2] Export more queue state information through debugfs

[Bart Van Assche]

Hello Jens,

This is a short patch series with one patch that exports the queue state
and another patch that shows symbolic names for hctx state and flags
instead of a numerical bitmask.

Please consider these patches for kernel v4.12.

Thanks,

Bart.

Changes compared to v2:
- Introduced function blk_flags_show() in patch 1.
- Modified error message in patch 1 such that it shows the names of the
  valid commands.
- Added patch "blk-mq: Show symbolic names for hctx state and flags"

Changes compared to v1:
- Constified blk_queue_flag_name.
- Left out QUEUE_FLAG_VIRT because it is a synonym of QUEUE_FLAG_NONROT.
- Check array size before reading from blk_queue_flag_name[].
- Add functionality to restart a block layer queue.

Bart Van Assche (2):
  blk-mq: Export queue state through /sys/kernel/debug/block/*/state
  blk-mq: Show symbolic names for hctx state and flags

 block/blk-mq-debugfs.c | 143 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 140 insertions(+), 3 deletions(-)

-- 
2.12.0

[PATCH 1/2] blk-mq: Export queue state through /sys/kernel/debug/block/*/state

[Bart Van Assche]

Make it possible to check whether or not a block layer queue has
been stopped. Make it possible to start and to run a blk-mq queue
from user space.

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

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 4b3f962a9c7a..9e86056d47cd 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -43,6 +43,110 @@ static int blk_mq_debugfs_seq_open(struct inode *inode, struct file *file,
 	return ret;
 }
 
+static int blk_flags_show(struct seq_file *m, const unsigned long flags,
+			  const char *const *flag_name, int flag_name_count)
+{
+	bool sep = false;
+	int i;
+
+	for (i = 0; i < sizeof(flags) * BITS_PER_BYTE; i++) {
+		if (!(flags & BIT(i)))
+			continue;
+		if (sep)
+			seq_puts(m, " ");
+		sep = true;
+		if (i < flag_name_count && flag_name[i])
+			seq_puts(m, flag_name[i]);
+		else
+			seq_printf(m, "%d", i);
+	}
+	seq_puts(m, "\n");
+	return 0;
+}
+
+static const char *const blk_queue_flag_name[] = {
+	[QUEUE_FLAG_QUEUED]	 = "QUEUED",
+	[QUEUE_FLAG_STOPPED]	 = "STOPPED",
+	[QUEUE_FLAG_SYNCFULL]	 = "SYNCFULL",
+	[QUEUE_FLAG_ASYNCFULL]	 = "ASYNCFULL",
+	[QUEUE_FLAG_DYING]	 = "DYING",
+	[QUEUE_FLAG_BYPASS]	 = "BYPASS",
+	[QUEUE_FLAG_BIDI]	 = "BIDI",
+	[QUEUE_FLAG_NOMERGES]	 = "NOMERGES",
+	[QUEUE_FLAG_SAME_COMP]	 = "SAME_COMP",
+	[QUEUE_FLAG_FAIL_IO]	 = "FAIL_IO",
+	[QUEUE_FLAG_STACKABLE]	 = "STACKABLE",
+	[QUEUE_FLAG_NONROT]	 = "NONROT",
+	[QUEUE_FLAG_IO_STAT]	 = "IO_STAT",
+	[QUEUE_FLAG_DISCARD]	 = "DISCARD",
+	[QUEUE_FLAG_NOXMERGES]	 = "NOXMERGES",
+	[QUEUE_FLAG_ADD_RANDOM]	 = "ADD_RANDOM",
+	[QUEUE_FLAG_SECERASE]	 = "SECERASE",
+	[QUEUE_FLAG_SAME_FORCE]	 = "SAME_FORCE",
+	[QUEUE_FLAG_DEAD]	 = "DEAD",
+	[QUEUE_FLAG_INIT_DONE]	 = "INIT_DONE",
+	[QUEUE_FLAG_NO_SG_MERGE] = "NO_SG_MERGE",
+	[QUEUE_FLAG_POLL]	 = "POLL",
+	[QUEUE_FLAG_WC]		 = "WC",
+	[QUEUE_FLAG_FUA]	 = "FUA",
+	[QUEUE_FLAG_FLUSH_NQ]	 = "FLUSH_NQ",
+	[QUEUE_FLAG_DAX]	 = "DAX",
+	[QUEUE_FLAG_STATS]	 = "STATS",
+	[QUEUE_FLAG_RESTART]	 = "RESTART",
+	[QUEUE_FLAG_POLL_STATS]	 = "POLL_STATS",
+	[QUEUE_FLAG_REGISTERED]	 = "REGISTERED",
+};
+
+static int blk_queue_flags_show(struct seq_file *m, void *v)
+{
+	struct request_queue *q = m->private;
+
+	blk_flags_show(m, q->queue_flags, blk_queue_flag_name,
+		       ARRAY_SIZE(blk_queue_flag_name));
+	return 0;
+}
+
+static ssize_t blk_queue_flags_store(struct file *file, const char __user *ubuf,
+				     size_t len, loff_t *offp)
+{
+	struct request_queue *q = file_inode(file)->i_private;
+	char op[16] = { }, *s;
+
+	len = min(len, sizeof(op) - 1);
+	if (copy_from_user(op, ubuf, len))
+		return -EFAULT;
+	s = op;
+	strsep(&s, " \t\n"); /* strip trailing whitespace */
+	if (strcmp(op, "run") == 0) {
+		blk_mq_run_hw_queues(q, true);
+	} else if (strcmp(op, "start") == 0) {
+		blk_mq_start_stopped_hw_queues(q, true);
+	} else {
+		pr_err("%s: unsupported operation %s. Use either 'run' or 'start'\n",
+		       __func__, op);
+		return -EINVAL;
+	}
+	return len;
+}
+
+static int blk_queue_flags_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, blk_queue_flags_show, inode->i_private);
+}
+
+static const struct file_operations blk_queue_flags_fops = {
+	.open		= blk_queue_flags_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+	.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) {
@@ -735,6 +839,9 @@ 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

[PATCH 2/2] blk-mq: Show symbolic names for hctx state and flags

[Bart Van Assche]

Instead of showing the hctx state and flags as numbers, show the
names of the flags.

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 | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 9e86056d47cd..91d09f58a596 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -183,11 +183,19 @@ static const struct file_operations queue_poll_stat_fops = {
 	.release	= single_release,
 };
 
+static const char *const hctx_state_name[] = {
+	[BLK_MQ_S_STOPPED]	 = "STOPPED",
+	[BLK_MQ_S_TAG_ACTIVE]	 = "TAG_ACTIVE",
+	[BLK_MQ_S_SCHED_RESTART] = "SCHED_RESTART",
+	[BLK_MQ_S_TAG_WAITING]	 = "TAG_WAITING",
+
+};
 static int hctx_state_show(struct seq_file *m, void *v)
 {
 	struct blk_mq_hw_ctx *hctx = m->private;
 
-	seq_printf(m, "0x%lx\n", hctx->state);
+	blk_flags_show(m, hctx->state, hctx_state_name,
+		       ARRAY_SIZE(hctx_state_name));
 	return 0;
 }
 
@@ -203,11 +211,34 @@ static const struct file_operations hctx_state_fops = {
 	.release	= single_release,
 };
 
+static const char *const alloc_policy_name[] = {
+	[BLK_TAG_ALLOC_FIFO]	= "fifo",
+	[BLK_TAG_ALLOC_RR]	= "rr",
+};
+
+static const char *const hctx_flag_name[] = {
+	[ilog2(BLK_MQ_F_SHOULD_MERGE)]	= "SHOULD_MERGE",
+	[ilog2(BLK_MQ_F_TAG_SHARED)]	= "TAG_SHARED",
+	[ilog2(BLK_MQ_F_SG_MERGE)]	= "SG_MERGE",
+	[ilog2(BLK_MQ_F_BLOCKING)]	= "BLOCKING",
+	[ilog2(BLK_MQ_F_NO_SCHED)]	= "NO_SCHED",
+};
+
 static int hctx_flags_show(struct seq_file *m, void *v)
 {
 	struct blk_mq_hw_ctx *hctx = m->private;
-
-	seq_printf(m, "0x%lx\n", hctx->flags);
+	const int alloc_policy = BLK_MQ_FLAG_TO_ALLOC_POLICY(hctx->flags);
+
+	seq_puts(m, "alloc_policy=");
+	if (alloc_policy < ARRAY_SIZE(alloc_policy_name) &&
+	    alloc_policy_name[alloc_policy])
+		seq_puts(m, alloc_policy_name[alloc_policy]);
+	else
+		seq_printf(m, "%d", alloc_policy);
+	seq_puts(m, " ");
+	blk_flags_show(m,
+		       hctx->flags ^ BLK_ALLOC_POLICY_TO_MQ_FLAG(alloc_policy),
+		       hctx_flag_name, ARRAY_SIZE(hctx_flag_name));
 	return 0;
 }
 
-- 
2.12.0

[PATCH v4 1/2] blk-mq: Export queue state through /sys/kernel/debug/block/*/state

[Bart Van Assche]

Make it possible to check whether or not a block layer queue has=0A=
been stopped. Make it possible to start and to run a blk-mq queue=0A=
from user space.=0A=
=0A=
Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>=0A=
Cc: Omar Sandoval <osandov@fb.com>=0A=
Cc: Hannes Reinecke <hare@suse.com>=0A=
=0A=
---=0A=
=0A=
Changes compared to v3:=0A=
- Return -ENOENT for attempts to run or start a queue after it has reached =
the=0A=
  state "dead". This is needed to avoid a use-after-free and potentially a =
kernel=0A=
  crash.=0A=
=0A=
---=0A=
=0A=
 block/blk-mq-debugfs.c | 114 +++++++++++++++++++++++++++++++++++++++++++++=
++++=0A=
 1 file changed, 114 insertions(+)=0A=
=0A=
diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c=0A=
index 4b3f962a9c7a..bd3afa4e1602 100644=0A=
--- a/block/blk-mq-debugfs.c=0A=
+++ b/block/blk-mq-debugfs.c=0A=
@@ -43,6 +43,117 @@ static int blk_mq_debugfs_seq_open(struct inode *inode,=
 struct file *file,=0A=
 	return ret;=0A=
 }=0A=
 =0A=
+static int blk_flags_show(struct seq_file *m, const unsigned long flags,=
=0A=
+			  const char *const *flag_name, int flag_name_count)=0A=
+{=0A=
+	bool sep =3D false;=0A=
+	int i;=0A=
+=0A=
+	for (i =3D 0; i < sizeof(flags) * BITS_PER_BYTE; i++) {=0A=
+		if (!(flags & BIT(i)))=0A=
+			continue;=0A=
+		if (sep)=0A=
+			seq_puts(m, " ");=0A=
+		sep =3D true;=0A=
+		if (i < flag_name_count && flag_name[i])=0A=
+			seq_puts(m, flag_name[i]);=0A=
+		else=0A=
+			seq_printf(m, "%d", i);=0A=
+	}=0A=
+	seq_puts(m, "\n");=0A=
+	return 0;=0A=
+}=0A=
+=0A=
+static const char *const blk_queue_flag_name[] =3D {=0A=
+	[QUEUE_FLAG_QUEUED]	 =3D "QUEUED",=0A=
+	[QUEUE_FLAG_STOPPED]	 =3D "STOPPED",=0A=
+	[QUEUE_FLAG_SYNCFULL]	 =3D "SYNCFULL",=0A=
+	[QUEUE_FLAG_ASYNCFULL]	 =3D "ASYNCFULL",=0A=
+	[QUEUE_FLAG_DYING]	 =3D "DYING",=0A=
+	[QUEUE_FLAG_BYPASS]	 =3D "BYPASS",=0A=
+	[QUEUE_FLAG_BIDI]	 =3D "BIDI",=0A=
+	[QUEUE_FLAG_NOMERGES]	 =3D "NOMERGES",=0A=
+	[QUEUE_FLAG_SAME_COMP]	 =3D "SAME_COMP",=0A=
+	[QUEUE_FLAG_FAIL_IO]	 =3D "FAIL_IO",=0A=
+	[QUEUE_FLAG_STACKABLE]	 =3D "STACKABLE",=0A=
+	[QUEUE_FLAG_NONROT]	 =3D "NONROT",=0A=
+	[QUEUE_FLAG_IO_STAT]	 =3D "IO_STAT",=0A=
+	[QUEUE_FLAG_DISCARD]	 =3D "DISCARD",=0A=
+	[QUEUE_FLAG_NOXMERGES]	 =3D "NOXMERGES",=0A=
+	[QUEUE_FLAG_ADD_RANDOM]	 =3D "ADD_RANDOM",=0A=
+	[QUEUE_FLAG_SECERASE]	 =3D "SECERASE",=0A=
+	[QUEUE_FLAG_SAME_FORCE]	 =3D "SAME_FORCE",=0A=
+	[QUEUE_FLAG_DEAD]	 =3D "DEAD",=0A=
+	[QUEUE_FLAG_INIT_DONE]	 =3D "INIT_DONE",=0A=
+	[QUEUE_FLAG_NO_SG_MERGE] =3D "NO_SG_MERGE",=0A=
+	[QUEUE_FLAG_POLL]	 =3D "POLL",=0A=
+	[QUEUE_FLAG_WC]		 =3D "WC",=0A=
+	[QUEUE_FLAG_FUA]	 =3D "FUA",=0A=
+	[QUEUE_FLAG_FLUSH_NQ]	 =3D "FLUSH_NQ",=0A=
+	[QUEUE_FLAG_DAX]	 =3D "DAX",=0A=
+	[QUEUE_FLAG_STATS]	 =3D "STATS",=0A=
+	[QUEUE_FLAG_POLL_STATS]	 =3D "POLL_STATS",=0A=
+	[QUEUE_FLAG_REGISTERED]	 =3D "REGISTERED",=0A=
+};=0A=
+=0A=
+static int blk_queue_flags_show(struct seq_file *m, void *v)=0A=
+{=0A=
+	struct request_queue *q =3D m->private;=0A=
+=0A=
+	blk_flags_show(m, q->queue_flags, blk_queue_flag_name,=0A=
+		       ARRAY_SIZE(blk_queue_flag_name));=0A=
+	return 0;=0A=
+}=0A=
+=0A=
+static ssize_t blk_queue_flags_store(struct file *file, const char __user =
*ubuf,=0A=
+				     size_t len, loff_t *offp)=0A=
+{=0A=
+	struct request_queue *q =3D file_inode(file)->i_private;=0A=
+	char op[16] =3D { }, *s;=0A=
+=0A=
+	/*=0A=
+	 * The debugfs attributes are removed after blk_cleanup_queue() has=0A=
+	 * called blk_mq_free_queue(). Return if QUEUE_FLAG_DEAD has been set=0A=
+	 * to avoid triggering a use-after-free.=0A=
+	 */=0A=
+	if (blk_queue_dead(q))=0A=
+		return -ENOENT;=0A=
+=0A=
+	len =3D min(len, sizeof(op) - 1);=0A=
+	if (copy_from_user(op, ubuf, len))=0A=
+		return -EFAULT;=0A=
+	s =3D op;=0A=
+	strsep(&s, " \t\n"); /* strip trailing whitespace */=0A=
+	if (strcmp(op, "run") =3D=3D 0) {=0A=
+		blk_mq_run_hw_queues(q, true);=0A=
+	} else if (strcmp(op, "start") =3D=3D 0) {=0A=
+		blk_mq_start_stopped_hw_queues(q, true);=0A=
+	} else {=0A=
+		pr_err("%s: unsupported operation %s. Use either 'run' or 'start'\n",=0A=
+		       __func__, op);=0A=
+		return -EINVAL;=0A=
+	}=0A=
+	return len;=0A=
+}=0A=
+=0A=
+static int blk_queue_flags_open(struct inode *inode, struct file *file)=0A=
+{=0A=
+	return single_open(file, blk_queue_flags_show, inode->i_private);=0A=
+}=0A=
+=0A=
+static const struct file_operations blk_queue_flags_fops =3D {=0A=
+	.open		=3D blk_queue_flags_open,=0A=
+	.read		=3D seq_read,=0A=
+	.llseek		=3D seq_lseek,=0A=
+	.release	=3D single_release,=0A=
+	.write		=3D blk_queue_flags_store,=0A=
+};=0A=
+=0A=
+static const struct blk_mq_debugfs_attr blk_queue_attrs[] =3D {=0A=
+	{"state", 0600, &blk_queue_flags_fops},=0A=
+	{},=0A=
+};=0A=
+=0A=
 static void print_stat(struct seq_file *m, struct blk_rq_stat *stat)=0A=
 {=0A=
 	if (stat->nr_samples) {=0A=
@@ -735,6 +846,9 @@ int blk_mq_debugfs_register_hctxs(struct request_queue =
*q)=0A=
 	if (!q->debugfs_dir)=0A=
 		return -ENOENT;=0A=
 =0A=
+	if (!debugfs_create_files(q->debugfs_dir, q, blk_queue_attrs))=0A=
+		goto err;=0A=
+=0A=
 	q->mq_debugfs_dir =3D debugfs_create_dir("mq", q->debugfs_dir);=0A=
 	if (!q->mq_debugfs_dir)=0A=
 		goto err;=0A=
-- =0A=
2.12.0=0A=
=0A=
=0A=

Re: [PATCH v4 1/2] blk-mq: Export queue state through /sys/kernel/debug/block/*/state

[Hannes Reinecke]

On 04/01/2017 01:23 AM, Bart Van Assche wrote:
> Make it possible to check whether or not a block layer queue has
> been stopped. Make it possible to start and to run a blk-mq queue
> from user space.
>
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Hannes Reinecke <hare@suse.com>
>
> ---
>
> Changes compared to v3:
> - Return -ENOENT for attempts to run or start a queue after it has reached the
>   state "dead". This is needed to avoid a use-after-free and potentially a kernel
>   crash.
>
> ---
>
>  block/blk-mq-debugfs.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 114 insertions(+)
>
> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> index 4b3f962a9c7a..bd3afa4e1602 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -43,6 +43,117 @@ static int blk_mq_debugfs_seq_open(struct inode *inode, struct file *file,
>  	return ret;
>  }
>
> +static int blk_flags_show(struct seq_file *m, const unsigned long flags,
> +			  const char *const *flag_name, int flag_name_count)
> +{
> +	bool sep = false;
> +	int i;
> +
> +	for (i = 0; i < sizeof(flags) * BITS_PER_BYTE; i++) {
> +		if (!(flags & BIT(i)))
> +			continue;
> +		if (sep)
> +			seq_puts(m, " ");
> +		sep = true;
> +		if (i < flag_name_count && flag_name[i])
> +			seq_puts(m, flag_name[i]);
> +		else
> +			seq_printf(m, "%d", i);
> +	}
> +	seq_puts(m, "\n");
> +	return 0;
> +}
> +
> +static const char *const blk_queue_flag_name[] = {
> +	[QUEUE_FLAG_QUEUED]	 = "QUEUED",
> +	[QUEUE_FLAG_STOPPED]	 = "STOPPED",
> +	[QUEUE_FLAG_SYNCFULL]	 = "SYNCFULL",
> +	[QUEUE_FLAG_ASYNCFULL]	 = "ASYNCFULL",
> +	[QUEUE_FLAG_DYING]	 = "DYING",
> +	[QUEUE_FLAG_BYPASS]	 = "BYPASS",
> +	[QUEUE_FLAG_BIDI]	 = "BIDI",
> +	[QUEUE_FLAG_NOMERGES]	 = "NOMERGES",
> +	[QUEUE_FLAG_SAME_COMP]	 = "SAME_COMP",
> +	[QUEUE_FLAG_FAIL_IO]	 = "FAIL_IO",
> +	[QUEUE_FLAG_STACKABLE]	 = "STACKABLE",
> +	[QUEUE_FLAG_NONROT]	 = "NONROT",
> +	[QUEUE_FLAG_IO_STAT]	 = "IO_STAT",
> +	[QUEUE_FLAG_DISCARD]	 = "DISCARD",
> +	[QUEUE_FLAG_NOXMERGES]	 = "NOXMERGES",
> +	[QUEUE_FLAG_ADD_RANDOM]	 = "ADD_RANDOM",
> +	[QUEUE_FLAG_SECERASE]	 = "SECERASE",
> +	[QUEUE_FLAG_SAME_FORCE]	 = "SAME_FORCE",
> +	[QUEUE_FLAG_DEAD]	 = "DEAD",
> +	[QUEUE_FLAG_INIT_DONE]	 = "INIT_DONE",
> +	[QUEUE_FLAG_NO_SG_MERGE] = "NO_SG_MERGE",
> +	[QUEUE_FLAG_POLL]	 = "POLL",
> +	[QUEUE_FLAG_WC]		 = "WC",
> +	[QUEUE_FLAG_FUA]	 = "FUA",
> +	[QUEUE_FLAG_FLUSH_NQ]	 = "FLUSH_NQ",
> +	[QUEUE_FLAG_DAX]	 = "DAX",
> +	[QUEUE_FLAG_STATS]	 = "STATS",
> +	[QUEUE_FLAG_POLL_STATS]	 = "POLL_STATS",
> +	[QUEUE_FLAG_REGISTERED]	 = "REGISTERED",
> +};
> +
> +static int blk_queue_flags_show(struct seq_file *m, void *v)
> +{
> +	struct request_queue *q = m->private;
> +
> +	blk_flags_show(m, q->queue_flags, blk_queue_flag_name,
> +		       ARRAY_SIZE(blk_queue_flag_name));
> +	return 0;
> +}
> +
> +static ssize_t blk_queue_flags_store(struct file *file, const char __user *ubuf,
> +				     size_t len, loff_t *offp)
> +{
> +	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;
> +	s = op;
> +	strsep(&s, " \t\n"); /* strip trailing whitespace */
> +	if (strcmp(op, "run") == 0) {
> +		blk_mq_run_hw_queues(q, true);
> +	} else if (strcmp(op, "start") == 0) {
> +		blk_mq_start_stopped_hw_queues(q, true);
> +	} else {
> +		pr_err("%s: unsupported operation %s. Use either 'run' or 'start'\n",
> +		       __func__, op);
> +		return -EINVAL;
> +	}
> +	return len;
> +}
> +
I would have added 'stop' for completeness, but that's probably for very 
specific cases only.

Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes

Re: [PATCH 0/2] Export more queue state information through debugfs

[Jens Axboe]

On 03/30/2017 12:21 PM, Bart Van Assche wrote:
> Hello Jens,
> 
> This is a short patch series with one patch that exports the queue state
> and another patch that shows symbolic names for hctx state and flags
> instead of a numerical bitmask.
> 
> Please consider these patches for kernel v4.12.

Thanks, added for 4.12.


-- 
Jens Axboe

Re: [PATCH 0/2] Export more queue state information through debugfs

[Bart Van Assche]

On 04/10/2017 11:28 AM, Jens Axboe wrote:
> On 03/30/2017 12:21 PM, Bart Van Assche wrote:
>> This is a short patch series with one patch that exports the queue state
>> and another patch that shows symbolic names for hctx state and flags
>> instead of a numerical bitmask.
>>
>> Please consider these patches for kernel v4.12.
> 
> Thanks, added for 4.12.

Hello Jens,

Thanks! This infrastructure was essential while analyzing queue stalls.

After I had posted this series I improved and extended the blk-mq debugfs
functionality further. Please consider including the patch below in v4.12.

Thanks,

Bart.


From: Bart Van Assche <bart.vanassche@sandisk.com>
Subject: [PATCH] blk-mq: Two fixes for the code that exports the queue state

Remove the array entry for QUEUE_FLAG_RESTART since that flag has
been removed after the blk-mq-debugfs patch that introduced this
array entry was posted.

Avoid that querying the queue state of a dead queue triggers a
kernel crash.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
---
 block/blk-mq-debugfs.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index 91d09f58a596..a1ce823578c7 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -92,7 +92,6 @@ static const char *const blk_queue_flag_name[] = {
 	[QUEUE_FLAG_FLUSH_NQ]	 = "FLUSH_NQ",
 	[QUEUE_FLAG_DAX]	 = "DAX",
 	[QUEUE_FLAG_STATS]	 = "STATS",
-	[QUEUE_FLAG_RESTART]	 = "RESTART",
 	[QUEUE_FLAG_POLL_STATS]	 = "POLL_STATS",
 	[QUEUE_FLAG_REGISTERED]	 = "REGISTERED",
 };
@@ -112,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

Re: [PATCH 0/2] Export more queue state information through debugfs

[Omar Sandoval]

On Mon, Apr 10, 2017 at 11:40:49AM -0700, Bart Van Assche wrote:
> On 04/10/2017 11:28 AM, Jens Axboe wrote:
> > On 03/30/2017 12:21 PM, Bart Van Assche wrote:
> >> This is a short patch series with one patch that exports the queue state
> >> and another patch that shows symbolic names for hctx state and flags
> >> instead of a numerical bitmask.
> >>
> >> Please consider these patches for kernel v4.12.
> > 
> > Thanks, added for 4.12.
> 
> Hello Jens,
> 
> Thanks! This infrastructure was essential while analyzing queue stalls.
> 
> After I had posted this series I improved and extended the blk-mq debugfs
> functionality further. Please consider including the patch below in v4.12.
> 
> Thanks,
> 
> Bart.
> 
> 
> From: Bart Van Assche <bart.vanassche@sandisk.com>
> Subject: [PATCH] blk-mq: Two fixes for the code that exports the queue state
> 
> Remove the array entry for QUEUE_FLAG_RESTART since that flag has
> been removed after the blk-mq-debugfs patch that introduced this
> array entry was posted.
> 
> Avoid that querying the queue state of a dead queue triggers a
> kernel crash.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> ---
>  block/blk-mq-debugfs.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> index 91d09f58a596..a1ce823578c7 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -92,7 +92,6 @@ static const char *const blk_queue_flag_name[] = {
>  	[QUEUE_FLAG_FLUSH_NQ]	 = "FLUSH_NQ",
>  	[QUEUE_FLAG_DAX]	 = "DAX",
>  	[QUEUE_FLAG_STATS]	 = "STATS",
> -	[QUEUE_FLAG_RESTART]	 = "RESTART",
>  	[QUEUE_FLAG_POLL_STATS]	 = "POLL_STATS",
>  	[QUEUE_FLAG_REGISTERED]	 = "REGISTERED",
>  };
> @@ -112,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;
> +

Can you just move blk_queue_flags_fops to blk_mq_debugfs_queue_attrs
instead of adding blk_queue_attrs?

Re: [PATCH 0/2] Export more queue state information through debugfs

[Bart Van Assche]

On Mon, 2017-04-10 at 13:00 -0700, Omar Sandoval wrote:
> Can you just move blk_queue_flags_fops to blk_mq_debugfs_queue_attrs
> instead of adding blk_queue_attrs?

Hello Omar,

Are you aware that that change will move the "state" attribute one level
down in the hierarchy? blk_mq_debugfs_queue_attrs attributes are created
under "mq" while blk_queue_flags_fops attributes are created at the same
level as the "mq" attribute. I had added blk_queue_flags_fops because the
"state" attribute is not related to blk-mq. That attribute is also relevant
for single-queue block layer queues.

Bart.

Re: [PATCH 0/2] Export more queue state information through debugfs

[Omar Sandoval]

On Mon, Apr 10, 2017 at 08:12:00PM +0000, Bart Van Assche wrote:
> On Mon, 2017-04-10 at 13:00 -0700, Omar Sandoval wrote:
> > Can you just move blk_queue_flags_fops to blk_mq_debugfs_queue_attrs
> > instead of adding blk_queue_attrs?
> 
> Hello Omar,
> 
> Are you aware that that change will move the "state" attribute one level
> down in the hierarchy? blk_mq_debugfs_queue_attrs attributes are created
> under "mq" while blk_queue_flags_fops attributes are created at the same
> level as the "mq" attribute. I had added blk_queue_flags_fops because the
> "state" attribute is not related to blk-mq. That attribute is also relevant
> for single-queue block layer queues.

Yes, I am aware of that. We don't set up debugfs for single-queue queues
anyways, so the top-level directory is really just for blktrace. It
simplifies the lifetime and cleanup to have everything under mq, so
please move it there.