Linux block layer
 help / color / mirror / Atom feed
* [PATCH 0/9] convert genericirq.tmpl and kernel-api.tmpl to DocBook
From: Mauro Carvalho Chehab @ 2017-03-30 20:11 UTC (permalink / raw)
  To: Linux Media Mailing List, Linux Doc Mailing List
  Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, Noam Camus,
	James Morris, zijun_hu, Markus Heiser, linux-clk, Jani Nikula,
	Andrew Morton, Jens Axboe, Nicholas Piggin, Russell King,
	linux-block, Kirill A. Shutemov, Mauro Carvalho Chehab,
	Joonsoo Kim, Ingo Molnar, Bjorn Helgaas, Serge E. Hallyn,
	Michal Hocko, Ross Zwisler, Chris Wilson, linux-mm,
	linux-security-module, Silvio Fricke, Takashi Iwai,
	Sebastian Andrzej Siewior, Jan Kara, Vlastimil Babka, linux-pci,
	Matt Fleming, Johannes Weiner, Andrey Ryabinin, Andy Lutomirski,
	Mel Gorman, Andy Shevchenko, Jonathan Corbet, Alexey Dobriyan,
	Hillf Danton

Jani proposed to batch-convert the remaining DocBooks for us
to get rid of it.

Well, I tried ;) 

The conversion itself can easily done, but the problem is that
it hits several errors/warnings when parsing kernel-doc tags.

It ends that it takes some time to fix those.

Also, it seems that the "!I" and "!E" tags at the DocBook
template are not quite right. So, despite being properly
converted to the corresponding kernel-doc tags at ReST, they
didn't produce all that it was needed. I manually fixed a
few, but I guess there are more to be fixed there. Anyway,
this is something that the subsystem maintainers can fix later,
as they understand better what functions they want exported at
the public API documentation, and what functions they want to
hide.

This series converts just two documents, adding them to the
core-api.rst book. It addresses the errors/warnings that popup
after the conversion.

I had to add two fixes to scripts/kernel-doc, in order to solve
some of the issues.

If I have some time during this weekend, I may try to convert
some additional documents to DocBook.


Mauro Carvalho Chehab (9):
  scripts/kernel-doc: fix parser for apostrophes
  scripts/kernel-doc: fix handling of parameters with parenthesis
  genericirq.tmpl: convert it to ReST
  genericirq.rst: add cross-reference links and use monospaced fonts
  kernel-api.tmpl: convert it to ReST
  kernel-api.rst: fix output of the vsnprintf() documentation
  kernel-api.rst: make it handle lib/crc32.c
  kernel-api.rst: fix some complex tags at lib/bitmap.c
  kernel-api.rst: fix a series of errors when parsing C files

 Documentation/DocBook/Makefile        |   4 +-
 Documentation/DocBook/genericirq.tmpl | 520 ----------------------------------
 Documentation/DocBook/kernel-api.tmpl | 331 ----------------------
 Documentation/core-api/genericirq.rst | 440 ++++++++++++++++++++++++++++
 Documentation/core-api/index.rst      |   2 +
 Documentation/core-api/kernel-api.rst | 418 +++++++++++++++++++++++++++
 block/genhd.c                         |   7 +-
 drivers/pci/irq.c                     |   2 +-
 include/linux/clk.h                   |   4 +-
 ipc/util.c                            |  12 +-
 lib/bitmap.c                          |  28 +-
 lib/string.c                          |   2 +-
 lib/vsprintf.c                        |   6 +-
 mm/filemap.c                          |  18 +-
 mm/page_alloc.c                       |   3 +-
 mm/vmalloc.c                          |   2 +-
 scripts/kernel-doc                    |  19 +-
 security/security.c                   |  12 +-
 18 files changed, 932 insertions(+), 898 deletions(-)
 delete mode 100644 Documentation/DocBook/genericirq.tmpl
 delete mode 100644 Documentation/DocBook/kernel-api.tmpl
 create mode 100644 Documentation/core-api/genericirq.rst
 create mode 100644 Documentation/core-api/kernel-api.rst

-- 
2.9.3

^ permalink raw reply

* Re: null_blk: add blocking mode
From: Bart Van Assche @ 2017-03-30 19:33 UTC (permalink / raw)
  To: Jens Axboe, linux-block@vger.kernel.org; +Cc: Christoph Hellwig
In-Reply-To: <9a2b8d7e-1b04-4c94-2c79-c16fa8134f9d@kernel.dk>

On 03/30/2017 11:53 AM, Jens Axboe wrote:=0A=
> This adds a new module parameter to null_blk, blocking. If set, null_blk=
=0A=
> will set the BLK_MQ_F_BLOCKING flag, indicating that it sometimes/always=
=0A=
> needs to block in its ->queue_rq() function.  The intent is to help find=
=0A=
> regressions in blocking drivers, since not many of them exist.=0A=
> =0A=
> If null_blk is loaded with submit_queues > 1 and blocking=3D1, this=0A=
> shows the regression recently fixed by bf4907c05e61.=0A=
=0A=
Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>=0A=

^ permalink raw reply

* null_blk: add blocking mode
From: Jens Axboe @ 2017-03-30 18:53 UTC (permalink / raw)
  To: linux-block@vger.kernel.org; +Cc: Christoph Hellwig

This adds a new module parameter to null_blk, blocking. If set, null_blk
will set the BLK_MQ_F_BLOCKING flag, indicating that it sometimes/always
needs to block in its ->queue_rq() function.  The intent is to help find
regressions in blocking drivers, since not many of them exist.

If null_blk is loaded with submit_queues > 1 and blocking=1, this
shows the regression recently fixed by bf4907c05e61.

Signed-off-by: Jens Axboe <axboe@fb.com>

diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c
index 6f2e565..d95635f 100644
--- a/drivers/block/null_blk.c
+++ b/drivers/block/null_blk.c
@@ -117,6 +117,10 @@ static bool use_lightnvm;
 module_param(use_lightnvm, bool, S_IRUGO);
 MODULE_PARM_DESC(use_lightnvm, "Register as a LightNVM device");
 
+static bool blocking;
+module_param(blocking, bool, S_IRUGO);
+MODULE_PARM_DESC(blocking, "Register as a blocking blk-mq driver device");
+
 static int irqmode = NULL_IRQ_SOFTIRQ;
 
 static int null_set_irqmode(const char *str, const struct kernel_param *kp)
@@ -357,6 +361,8 @@ static int null_queue_rq(struct blk_mq_hw_ctx *hctx,
 {
 	struct nullb_cmd *cmd = blk_mq_rq_to_pdu(bd->rq);
 
+	might_sleep_if(hctx->flags & BLK_MQ_F_BLOCKING);
+
 	if (irqmode == NULL_IRQ_TIMER) {
 		hrtimer_init(&cmd->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 		cmd->timer.function = null_cmd_timer_expired;
@@ -724,6 +730,9 @@ static int null_add_dev(void)
 		nullb->tag_set.flags = BLK_MQ_F_SHOULD_MERGE;
 		nullb->tag_set.driver_data = nullb;
 
+		if (blocking)
+			nullb->tag_set.flags |= BLK_MQ_F_BLOCKING;
+
 		rv = blk_mq_alloc_tag_set(&nullb->tag_set);
 		if (rv)
 			goto out_cleanup_queues;

^ permalink raw reply related

* Re: [PATCH] blk-mq: fix schedule-under-preempt for blocking drivers
From: Jens Axboe @ 2017-03-30 18:30 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-block@vger.kernel.org, Josef Bacik
In-Reply-To: <20170330182825.GA25277@lst.de>

On 03/30/2017 12:28 PM, Christoph Hellwig wrote:
> Oh, I need to add one of the blocking drivers to my test runs..

I was just thinking that too. The easiest would be to add a BLOCKING
mode to null_blk.

-- 
Jens Axboe

^ permalink raw reply

* Re: [PATCH] blk-mq: fix schedule-under-preempt for blocking drivers
From: Christoph Hellwig @ 2017-03-30 18:28 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block@vger.kernel.org, Christoph Hellwig, Josef Bacik
In-Reply-To: <b298fcf4-47bc-b2e1-7c8f-b4affe347501@kernel.dk>

Oh, I need to add one of the blocking drivers to my test runs..

Looks fine:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply

* [PATCH 2/2] blk-mq: Show symbolic names for hctx state and flags
From: Bart Van Assche @ 2017-03-30 18:21 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Omar Sandoval, Hannes Reinecke
In-Reply-To: <20170330182127.24288-1-bart.vanassche@sandisk.com>

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

^ permalink raw reply related

* [PATCH 1/2] blk-mq: Export queue state through /sys/kernel/debug/block/*/state
From: Bart Van Assche @ 2017-03-30 18:21 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, Bart Van Assche, Omar Sandoval, Hannes Reinecke
In-Reply-To: <20170330182127.24288-1-bart.vanassche@sandisk.com>

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

^ permalink raw reply related

* [PATCH 0/2] Export more queue state information through debugfs
From: Bart Van Assche @ 2017-03-30 18:21 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, 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

^ permalink raw reply

* Re: [PATCH] blk-mq: Export queue state through /sys/kernel/debug/block/*/state
From: Jens Axboe @ 2017-03-30 18:14 UTC (permalink / raw)
  To: Bart Van Assche; +Cc: osandov@fb.com, hare@suse.de, linux-block@vger.kernel.org
In-Reply-To: <1490897421.2753.12.camel@sandisk.com>

On 03/30/2017 12:10 PM, Bart Van Assche wrote:
> On Thu, 2017-03-30 at 09:27 -0600, Jens Axboe wrote:
>> On 03/29/2017 03:32 PM, 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.
>>
>> Looks good, minor nitpicking below.
>>
>>> +static int blk_queue_flags_show(struct seq_file *m, void *v)
>>> +{
>>> +	struct request_queue *q = m->private;
>>> +	bool sep = false;
>>> +	int i;
>>> +
>>> +	for (i = 0; i < sizeof(q->queue_flags) * BITS_PER_BYTE; i++) {
>>> +		if (!(q->queue_flags & BIT(i)))
>>> +			continue;
>>> +		if (sep)
>>> +			seq_puts(m, " ");
>>> +		sep = true;
>>
>> Put that sep = true in the else branch?
> 
> I can do that, but that would result in slightly less efficient assembler
> code (additional jump) while stores can be pipelined easily.

It's a sysfs show function, it's not like we care about branching. But we
can leave it as-is, I don't feel that strongly about it at all.

>>> +	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\n", __func__, op);
>>> +		return -EINVAL;
>>> +	}
>>
>> You are inconsistent with your braces. I'd prefer no braces for single
>> lines.
> 
> Sorry but if braces are used for one side of an if-then-else statement then
> checkpatch wants braces to be used for the other side too, even if that other
> side is only a single line. From the checkpatch source code:

Omar informs me that it's coding style mandated. The block layer generally
does NOT do braces, so I'd prefer to keep it consistent at least. Again, not
really a huge problem, and as I prefaced the original email with, totally
nitpicking.

-- 
Jens Axboe

^ permalink raw reply

* Re: [PATCH] blk-mq: Export queue state through /sys/kernel/debug/block/*/state
From: Bart Van Assche @ 2017-03-30 18:10 UTC (permalink / raw)
  To: axboe@fb.com; +Cc: osandov@fb.com, hare@suse.de, linux-block@vger.kernel.org
In-Reply-To: <f80153aa-3e7d-0a20-e426-7e58cd7c249d@fb.com>

On Thu, 2017-03-30 at 09:27 -0600, Jens Axboe wrote:
> On 03/29/2017 03:32 PM, 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.
>=20
> Looks good, minor nitpicking below.
>=20
> > +static int blk_queue_flags_show(struct seq_file *m, void *v)
> > +{
> > +	struct request_queue *q =3D m->private;
> > +	bool sep =3D false;
> > +	int i;
> > +
> > +	for (i =3D 0; i < sizeof(q->queue_flags) * BITS_PER_BYTE; i++) {
> > +		if (!(q->queue_flags & BIT(i)))
> > +			continue;
> > +		if (sep)
> > +			seq_puts(m, " ");
> > +		sep =3D true;
>=20
> Put that sep =3D true in the else branch?

I can do that, but that would result in slightly less efficient assembler
code (additional jump) while stores can be pipelined easily.

> > +	if (strcmp(op, "run") =3D=3D 0) {
> > +		blk_mq_run_hw_queues(q, true);
> > +	} else if (strcmp(op, "start") =3D=3D 0) {
> > +		blk_mq_start_stopped_hw_queues(q, true);
> > +	} else {
> > +		pr_err("%s: unsupported operation %s\n", __func__, op);
> > +		return -EINVAL;
> > +	}
>=20
> You are inconsistent with your braces. I'd prefer no braces for single
> lines.

Sorry but if braces are used for one side of an if-then-else statement then
checkpatch wants braces to be used for the other side too, even if that oth=
er
side is only a single line. From the checkpatch source code:

# check for single line unbalanced braces
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if ($sline =3D~ /^.\s*\}\s*=
else\s*$/ ||
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0$sline =3D~ /^.=
\s*else\s*\{\s*$/) {
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0CHK=
("BRACES", "Unbalanced braces around else statement\n" . $herecurr);
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0}

> Should the error case include valid commands? It'd be nice to have this
> available, and not have to resort to looking at the source to figure out
> what commands are available.

OK, I will update the error message.

Thanks for the review.

Bart.=

^ permalink raw reply

* [PATCH] blk-mq: fix schedule-under-preempt for blocking drivers
From: Jens Axboe @ 2017-03-30 18:00 UTC (permalink / raw)
  To: linux-block@vger.kernel.org; +Cc: Christoph Hellwig, Josef Bacik

Commit a4d907b6a33b unified the single and multi queue request handlers,
but in the process, it also screwed up the locking balance and calls
blk_mq_try_issue_directly() with the ctx preempt lock held. This is a
problem for drivers that have set BLK_MQ_F_BLOCKING, since now they
can't reliably sleep.

While in there, protect against similar issues in the future, by adding
a might_sleep() trigger in the BLOCKING path for direct issue or queue
run.

Reported-by: Josef Bacik <josef@toxicpanda.com>
Tested-by: Josef Bacik <josef@toxicpanda.com>
Fixes: a4d907b6a33b ("blk-mq: streamline blk_mq_make_request")
Signed-off-by: Jens Axboe <axboe@fb.com>

diff --git a/block/blk-mq.c b/block/blk-mq.c
index e6ac774b0e41..d4d6ed4e7250 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1122,6 +1122,8 @@ static void __blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx)
 		blk_mq_sched_dispatch_requests(hctx);
 		rcu_read_unlock();
 	} else {
+		might_sleep();
+
 		srcu_idx = srcu_read_lock(&hctx->queue_rq_srcu);
 		blk_mq_sched_dispatch_requests(hctx);
 		srcu_read_unlock(&hctx->queue_rq_srcu, srcu_idx);
@@ -1496,7 +1498,11 @@ static void blk_mq_try_issue_directly(struct blk_mq_hw_ctx *hctx,
 		__blk_mq_try_issue_directly(rq, cookie, false);
 		rcu_read_unlock();
 	} else {
-		unsigned int srcu_idx = srcu_read_lock(&hctx->queue_rq_srcu);
+		unsigned int srcu_idx;
+
+		might_sleep();
+
+		srcu_idx = srcu_read_lock(&hctx->queue_rq_srcu);
 		__blk_mq_try_issue_directly(rq, cookie, true);
 		srcu_read_unlock(&hctx->queue_rq_srcu, srcu_idx);
 	}
@@ -1596,18 +1602,23 @@ static blk_qc_t blk_mq_make_request(struct request_queue *q, struct bio *bio)
 			list_del_init(&same_queue_rq->queuelist);
 		list_add_tail(&rq->queuelist, &plug->mq_list);
 
+		blk_mq_put_ctx(data.ctx);
+
 		if (same_queue_rq)
 			blk_mq_try_issue_directly(data.hctx, same_queue_rq,
 					&cookie);
+
+		return cookie;
 	} else if (q->nr_hw_queues > 1 && is_sync) {
+		blk_mq_put_ctx(data.ctx);
 		blk_mq_bio_to_request(rq, bio);
 		blk_mq_try_issue_directly(data.hctx, rq, &cookie);
+		return cookie;
 	} else if (q->elevator) {
 		blk_mq_bio_to_request(rq, bio);
 		blk_mq_sched_insert_request(rq, false, true, true, true);
-	} else if (!blk_mq_merge_queue_io(data.hctx, data.ctx, rq, bio)) {
+	} else if (!blk_mq_merge_queue_io(data.hctx, data.ctx, rq, bio))
 		blk_mq_run_hw_queue(data.hctx, true);
-	}
 
 	blk_mq_put_ctx(data.ctx);
 	return cookie;

^ permalink raw reply related

* Re: [PATCH 12/23] sd: handle REQ_UNMAP
From: hch @ 2017-03-30 17:30 UTC (permalink / raw)
  To: Martin K. Petersen
  Cc: hch@lst.de, Bart Van Assche, agk@redhat.com,
	lars.ellenberg@linbit.com, snitzer@redhat.com,
	philipp.reisner@linbit.com, axboe@kernel.dk, shli@kernel.org,
	linux-scsi@vger.kernel.org, dm-devel@redhat.com,
	drbd-dev@lists.linbit.com, linux-block@vger.kernel.org,
	linux-raid@vger.kernel.org
In-Reply-To: <yq1h92a3hsv.fsf@oracle.com>

On Thu, Mar 30, 2017 at 11:28:32AM -0400, Martin K. Petersen wrote:
> "hch@lst.de" <hch@lst.de> writes:
> 
> Christoph,
> 
> > On Tue, Mar 28, 2017 at 04:48:55PM +0000, Bart Van Assche wrote:
> >> >  	if (sdp->no_write_same)
> >> >  		return BLKPREP_INVALID;
> >> >  	if (sdkp->ws16 || sector > 0xffffffff || nr_sectors > 0xffff)
> >> 
> >> Users can change the provisioning mode from user space from�SD_LBP_WS16 into
> >> SD_LBP_WS10 so I'm not sure it's safe to skip the (sdkp->ws16 || sector >
> >> 0xffffffff || nr_sectors > 0xffff) check if REQ_UNMAP is set.
> >
> > They can, and if the device has too many sectors that will already cause
> > discard to fail,
> 
> I'm not sure I understand what you mean by that?

If you manually change the provisioning mode to WS10 on a device that
must use WRITE SAME (16) to be able to address all blocks you're already
screwed right now, and with this patch you can screw yourself through
the WRITE_ZEROES path in addition to the DISCARD path.

^ permalink raw reply

* Re: [PATCH 23/23] block: remove the discard_zeroes_data flag
From: hch @ 2017-03-30 17:29 UTC (permalink / raw)
  To: Martin K. Petersen
  Cc: hch@lst.de, Bart Van Assche, agk@redhat.com,
	lars.ellenberg@linbit.com, snitzer@redhat.com,
	philipp.reisner@linbit.com, axboe@kernel.dk, shli@kernel.org,
	linux-scsi@vger.kernel.org, dm-devel@redhat.com,
	drbd-dev@lists.linbit.com, linux-block@vger.kernel.org,
	linux-raid@vger.kernel.org
In-Reply-To: <yq1d1cy3hra.fsf@oracle.com>

On Thu, Mar 30, 2017 at 11:29:29AM -0400, Martin K. Petersen wrote:
> "hch@lst.de" <hch@lst.de> writes:
> 
> > Jens, any opinion?  I'd like to remove it too, but I fear it might
> > break things.  We could deprecate it first with a warning when read
> > and then remove it a few releases down the road.
> 
> I know of several apps that check this variable (as opposed to the
> ioctl).

The above was in reference to both methods..

^ permalink raw reply

* Re: Outstanding MQ questions from MMC
From: Ulf Hansson @ 2017-03-30 16:39 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linus Walleij, linux-mmc@vger.kernel.org, linux-block, Jens Axboe,
	Christoph Hellwig, Adrian Hunter, Paolo Valente
In-Reply-To: <CAK8P3a1CGtj5VSVoMM34VWn781WpS8ZcHpq6n0UfNOWWYEq=QQ@mail.gmail.com>

On 30 March 2017 at 14:42, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wed, Mar 29, 2017 at 5:09 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> Hi folks,
>>
>> I earlier left some unanswered questions in my MMC to MQ conversion series
>> but I figured it is better if I collect them and ask the blk-mq
>> maintainers directly
>> how to deal with the following situations that occur in the MMC block layer:
>>
>>
>> 1. The current MMC code locks the host when the first request comes in
>> from blk_fetch_request() and unlocks it when blk_fetch_request() returns
>> NULL twice in a row. Then the polling thread terminated and is not restarted
>> until we get called by the mmc_request_fn.
>>
>> Host locking means that we will not send other commands to the MMC
>> card from i.e. userspace, which sometimes can send spurious stuff orthogonal
>> to the block layer. If the block layer has locked the host, userspace
>> has to wait
>> and vice versa. It is not a common contention point but it still happens.
>>
>> In MQ, I have simply locked the host on the first request and then I never
>> release it. Clearly this does not work. I am uncertain on how to handle this
>> and whether MQ has a way to tell us that the queue is empty so we may release
>> the host. I toyed with the idea to just set up a timer, but a "queue
>> empty" callback
>> from the block layer is what would be ideal.
>
> Would it be possible to change the userspace code to go through
> the block layer instead and queue a request there, to avoid having
> to lock the card at all?

That would be good from an I/O scheduling point of view, as it would
avoid one side being able to starve the other.

However, we would still need a lock, as we also have card detect work
queue, which also needs to claim the host when it polls for removable
cards.

Kind regards
Uffe

^ permalink raw reply

* Re: RFC: always use REQ_OP_WRITE_ZEROES for zeroing offload
From: Mike Snitzer @ 2017-03-30 15:38 UTC (permalink / raw)
  To: Martin K. Petersen
  Cc: Christoph Hellwig, axboe, agk, shli, philipp.reisner,
	lars.ellenberg, linux-block, linux-raid, dm-devel, linux-scsi,
	drbd-dev
In-Reply-To: <yq1lgrm3i36.fsf@oracle.com>

On Thu, Mar 30 2017 at 11:22am -0400,
Martin K. Petersen <martin.petersen@oracle.com> wrote:

> Mike Snitzer <snitzer@redhat.com> writes:
> 
> > Would be very useful, particularly for testing, if
> > drivers/scsi/scsi_debug.c were updated to support WRITE ZEROES.
> 
> There is no WRITE ZEROES in SCSI. You should be able to get the right
> behavior with lbpws=1 lbprz=1.

OK, thanks.

^ permalink raw reply

* Re: [PATCH] zram: set physical queue limits to avoid array out of bounds accesses
From: Jens Axboe @ 2017-03-30 15:35 UTC (permalink / raw)
  To: Minchan Kim, Johannes Thumshirn
  Cc: Hannes Reinecke, Nitin Gupta, Christoph Hellwig,
	Sergey Senozhatsky, yizhan, Linux Block Layer Mailinglist,
	Linux Kernel Mailinglist, Andrew Morton
In-Reply-To: <CAEwNFnBnEObVdjqNdvh_-Dc0ryDjeW3c1qVdfOCbbyQREaDdqA@mail.gmail.com>

On 03/30/2017 09:08 AM, Minchan Kim wrote:
> Hi Jens,
> 
> It seems you miss this.
> Could you handle this?

I can, but I'm a little confused. The comment talks about replacing
the one I merged with this one, I can't do that. I'm assuming you
are talking about this commit:

commit 0bc315381fe9ed9fb91db8b0e82171b645ac008f
Author: Johannes Thumshirn <jthumshirn@suse.de>
Date:   Mon Mar 6 11:23:35 2017 +0100

    zram: set physical queue limits to avoid array out of bounds accesses

which is in mainline. The patch still applies, though.

Do we really REALLY need this for 4.11, or can we queue for 4.12 and
mark it stable?

-- 
Jens Axboe

^ permalink raw reply

* Re: [PATCH 23/23] block: remove the discard_zeroes_data flag
From: Martin K. Petersen @ 2017-03-30 15:29 UTC (permalink / raw)
  To: hch@lst.de
  Cc: Bart Van Assche, agk@redhat.com, lars.ellenberg@linbit.com,
	snitzer@redhat.com, martin.petersen@oracle.com,
	philipp.reisner@linbit.com, axboe@kernel.dk, shli@kernel.org,
	linux-scsi@vger.kernel.org, dm-devel@redhat.com,
	drbd-dev@lists.linbit.com, linux-block@vger.kernel.org,
	linux-raid@vger.kernel.org
In-Reply-To: <20170330090655.GF12015@lst.de>

"hch@lst.de" <hch@lst.de> writes:

> Jens, any opinion?  I'd like to remove it too, but I fear it might
> break things.  We could deprecate it first with a warning when read
> and then remove it a few releases down the road.

I know of several apps that check this variable (as opposed to the
ioctl).

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply

* Re: [PATCH 12/23] sd: handle REQ_UNMAP
From: Martin K. Petersen @ 2017-03-30 15:28 UTC (permalink / raw)
  To: hch@lst.de
  Cc: Bart Van Assche, agk@redhat.com, lars.ellenberg@linbit.com,
	snitzer@redhat.com, martin.petersen@oracle.com,
	philipp.reisner@linbit.com, axboe@kernel.dk, shli@kernel.org,
	linux-scsi@vger.kernel.org, dm-devel@redhat.com,
	drbd-dev@lists.linbit.com, linux-block@vger.kernel.org,
	linux-raid@vger.kernel.org
In-Reply-To: <20170330090201.GD12015@lst.de>

"hch@lst.de" <hch@lst.de> writes:

Christoph,

> On Tue, Mar 28, 2017 at 04:48:55PM +0000, Bart Van Assche wrote:
>> >  	if (sdp->no_write_same)
>> >  		return BLKPREP_INVALID;
>> >  	if (sdkp->ws16 || sector > 0xffffffff || nr_sectors > 0xffff)
>>=20
>> Users can change the provisioning mode from user space from=C2=A0SD_LBP_=
WS16 into
>> SD_LBP_WS10 so I'm not sure it's safe to skip the (sdkp->ws16 || sector >
>> 0xffffffff || nr_sectors > 0xffff) check if REQ_UNMAP is set.
>
> They can, and if the device has too many sectors that will already cause
> discard to fail,

I'm not sure I understand what you mean by that?

--=20
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply

* Re: [PATCH] blk-mq: Export queue state through /sys/kernel/debug/block/*/state
From: Jens Axboe @ 2017-03-30 15:27 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: Omar Sandoval, Hannes Reinecke, linux-block@vger.kernel.org
In-Reply-To: <1D08B61A9CF0974AA09887BE32D889DA12BA96@ULS-OP-MBXIP03.sdcorp.global.sandisk.com>

On 03/29/2017 03:32 PM, 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.

Looks good, minor nitpicking below.

> +static int blk_queue_flags_show(struct seq_file *m, void *v)
> +{
> +	struct request_queue *q = m->private;
> +	bool sep = false;
> +	int i;
> +
> +	for (i = 0; i < sizeof(q->queue_flags) * BITS_PER_BYTE; i++) {
> +		if (!(q->queue_flags & BIT(i)))
> +			continue;
> +		if (sep)
> +			seq_puts(m, " ");
> +		sep = true;

Put that sep = true in the else branch?

> +	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\n", __func__, op);
> +		return -EINVAL;
> +	}

You are inconsistent with your braces. I'd prefer no braces for single
lines.

Should the error case include valid commands? It'd be nice to have this
available, and not have to resort to looking at the source to figure out
what commands are available.

-- 
Jens Axboe

^ permalink raw reply

* Re: [PATCH] blk-mq: Export queue state through /sys/kernel/debug/block/*/state
From: Bart Van Assche @ 2017-03-30 15:23 UTC (permalink / raw)
  To: hare@suse.de, axboe@fb.com; +Cc: osandov@fb.com, linux-block@vger.kernel.org
In-Reply-To: <fe04eba2-a67d-9e02-83f4-1f519f79ed4b@fb.com>

On Thu, 2017-03-30 at 09:19 -0600, Jens Axboe wrote:
> On 03/30/2017 09:16 AM, Bart Van Assche wrote:
> > On Thu, 2017-03-30 at 07:50 +0200, Hannes Reinecke wrote:
> > > On 03/29/2017 10:20 PM, Bart Van Assche wrote:
> > > > Make it possible to check whether or not a block layer queue has
> > > > been stopped. Make it possible to run a blk-mq queue from user
> > > > space.
> > > >=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 | 84 ++++++++++++++++++++++++++++++++++++++=
++++++++++++
> > > >  1 file changed, 84 insertions(+)
> > > >=20
> > >=20
> > > About bloody time :-)
> > >=20
> > > Reviewed-by: Hannes Reinecke <hare@suse.com>
> >=20
> > Hello Hannes,
> >=20
> > Thanks for the review :-) However, had you noticed that I had already
> > posted a v2 of this patch? Anyway, since I have improved v2 further
> > after I had posted it, I will post a v3 today.
>=20
> I didn't see a v2 posting?

Hello Jens,

The label "v2" was missing from the subject, maybe that's why it didn't get
noticed. Anyway, v2 is available e.g. at URL
http://marc.info/?l=3Dlinux-block&m=3D149082319230648.

Bart.=

^ permalink raw reply

* Re: [PATCH] block/sed-opal: fix spelling mistake: "Lifcycle" -> "Lifecycle"
From: Jens Axboe @ 2017-03-30 15:23 UTC (permalink / raw)
  To: Colin King, Scott Bauer, Jonathan Derrick, Rafael Antognolli,
	linux-block
  Cc: kernel-janitors, linux-kernel
In-Reply-To: <20170330095808.19574-1-colin.king@canonical.com>

On 03/30/2017 03:58 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> trivial fix to spelling mistake in pr_err error message

Added for 4.12, thanks.

-- 
Jens Axboe

^ permalink raw reply

* Re: RFC: always use REQ_OP_WRITE_ZEROES for zeroing offload
From: Martin K. Petersen @ 2017-03-30 15:22 UTC (permalink / raw)
  To: Mike Snitzer
  Cc: Christoph Hellwig, axboe, martin.petersen, agk, shli,
	philipp.reisner, lars.ellenberg, linux-block, linux-raid,
	dm-devel, linux-scsi, drbd-dev
In-Reply-To: <20170330151257.GA910@redhat.com>

Mike Snitzer <snitzer@redhat.com> writes:

> Would be very useful, particularly for testing, if
> drivers/scsi/scsi_debug.c were updated to support WRITE ZEROES.

There is no WRITE ZEROES in SCSI. You should be able to get the right
behavior with lbpws=1 lbprz=1.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply

* Re: [PATCH 22/23] drbd: implement REQ_OP_WRITE_ZEROES
From: Martin K. Petersen @ 2017-03-30 15:20 UTC (permalink / raw)
  To: Mike Snitzer
  Cc: Christoph Hellwig, axboe, martin.petersen, agk, shli,
	philipp.reisner, linux-block, linux-scsi, drbd-dev, dm-devel,
	linux-raid
In-Reply-To: <20170330134957.GA508@redhat.com>

Mike Snitzer <snitzer@redhat.com> writes:

> I can work on this now.  Only question I have is: should DM thinp take
> care to zero any misaligned head and tail?  (I assume so but with all
> the back and forth between Bart, Paolo and Martin I figured I'd ask
> explicitly).

Yep, let's make sure our semantics match the hardware ditto.

 - So write zeroes should behave deterministically and explicitly handle
   any blocks that can't be cleared via deprovisioning.

 - And discard can work at the discard granularity in a
   non-deterministic fashion.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply

* Re: [PATCH] blk-mq: Export queue state through /sys/kernel/debug/block/*/state
From: Jens Axboe @ 2017-03-30 15:19 UTC (permalink / raw)
  To: Bart Van Assche, hare@suse.de; +Cc: osandov@fb.com, linux-block@vger.kernel.org
In-Reply-To: <1490886955.2753.3.camel@sandisk.com>

On 03/30/2017 09:16 AM, Bart Van Assche wrote:
> On Thu, 2017-03-30 at 07:50 +0200, Hannes Reinecke wrote:
>> On 03/29/2017 10:20 PM, Bart Van Assche wrote:
>>> Make it possible to check whether or not a block layer queue has
>>> been stopped. Make it possible 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 | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 84 insertions(+)
>>>
>>
>> About bloody time :-)
>>
>> Reviewed-by: Hannes Reinecke <hare@suse.com>
> 
> Hello Hannes,
> 
> Thanks for the review :-) However, had you noticed that I had already
> posted a v2 of this patch? Anyway, since I have improved v2 further
> after I had posted it, I will post a v3 today.

I didn't see a v2 posting?

-- 
Jens Axboe

^ permalink raw reply

* Re: [PATCH] blk-mq: Export queue state through /sys/kernel/debug/block/*/state
From: Bart Van Assche @ 2017-03-30 15:16 UTC (permalink / raw)
  To: hare@suse.de, axboe@fb.com; +Cc: osandov@fb.com, linux-block@vger.kernel.org
In-Reply-To: <824697ce-3fe4-f7ab-e23b-237b280196fb@suse.de>

On Thu, 2017-03-30 at 07:50 +0200, Hannes Reinecke wrote:
> On 03/29/2017 10:20 PM, Bart Van Assche wrote:
> > Make it possible to check whether or not a block layer queue has
> > been stopped. Make it possible to run a blk-mq queue from user
> > space.
> >=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 | 84 ++++++++++++++++++++++++++++++++++++++++++=
++++++++
> >  1 file changed, 84 insertions(+)
> >=20
>=20
> About bloody time :-)
>=20
> Reviewed-by: Hannes Reinecke <hare@suse.com>

Hello Hannes,

Thanks for the review :-) However, had you noticed that I had already
posted a v2 of this patch? Anyway, since I have improved v2 further
after I had posted it, I will post a v3 today.

Bart.=

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox