[PATCH 1/2] loop: fix LO_FLAGS_PARTSCAN hang

[PATCH 1/2] loop: fix LO_FLAGS_PARTSCAN hang

[Omar Sandoval]

From: Omar Sandoval <osandov@fb.com>

loop_reread_partitions() needs to do I/O, but we just froze the queue,
so we end up waiting forever. This can easily be reproduced with losetup
-P. Fix it by moving the reread to after we unfreeze the queue.

Fixes: ecdd09597a57 ("block/loop: fix race between I/O and set_status")
Reported-by: Tejun Heo <tj@kernel.org>
Cc: Ming Lei <tom.leiming@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 drivers/block/loop.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 4b52a1690329..132c9f371dce 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1142,13 +1142,6 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 	     (info->lo_flags & LO_FLAGS_AUTOCLEAR))
 		lo->lo_flags ^= LO_FLAGS_AUTOCLEAR;
 
-	if ((info->lo_flags & LO_FLAGS_PARTSCAN) &&
-	     !(lo->lo_flags & LO_FLAGS_PARTSCAN)) {
-		lo->lo_flags |= LO_FLAGS_PARTSCAN;
-		lo->lo_disk->flags &= ~GENHD_FL_NO_PART_SCAN;
-		loop_reread_partitions(lo, lo->lo_device);
-	}
-
 	lo->lo_encrypt_key_size = info->lo_encrypt_key_size;
 	lo->lo_init[0] = info->lo_init[0];
 	lo->lo_init[1] = info->lo_init[1];
@@ -1163,6 +1156,14 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 
  exit:
 	blk_mq_unfreeze_queue(lo->lo_queue);
+
+	if (!err && (info->lo_flags & LO_FLAGS_PARTSCAN) &&
+	     !(lo->lo_flags & LO_FLAGS_PARTSCAN)) {
+		lo->lo_flags |= LO_FLAGS_PARTSCAN;
+		lo->lo_disk->flags &= ~GENHD_FL_NO_PART_SCAN;
+		loop_reread_partitions(lo, lo->lo_device);
+	}
+
 	return err;
 }
 
-- 
2.12.0

[PATCH 2/2] blk-mq-debugfs: add q->mq_freeze_depth output

[Omar Sandoval]

From: Omar Sandoval <osandov@fb.com>

This can be used to diagnose freeze-related hangs.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 block/blk-mq-debugfs.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
index f6d917977b33..1459546788da 100644
--- a/block/blk-mq-debugfs.c
+++ b/block/blk-mq-debugfs.c
@@ -29,6 +29,26 @@ struct blk_mq_debugfs_attr {
 	const struct file_operations *fops;
 };
 
+static int queue_freeze_depth_show(struct seq_file *m, void *v)
+{
+	struct request_queue *q = m->private;
+
+	seq_printf(m, "%d\n", atomic_read(&q->mq_freeze_depth));
+	return 0;
+}
+
+static int queue_freeze_depth_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, queue_freeze_depth_show, inode->i_private);
+}
+
+static const struct file_operations queue_freeze_depth_fops = {
+	.open		= queue_freeze_depth_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
 static int blk_mq_debugfs_seq_open(struct inode *inode, struct file *file,
 				   const struct seq_operations *ops)
 {
@@ -636,6 +656,11 @@ static const struct file_operations ctx_completed_fops = {
 	.release	= single_release,
 };
 
+static const struct blk_mq_debugfs_attr blk_mq_debugfs_queue_attrs[] = {
+	{"freeze_depth", 0400, &queue_freeze_depth_fops},
+	{},
+};
+
 static const struct blk_mq_debugfs_attr blk_mq_debugfs_hctx_attrs[] = {
 	{"state", 0400, &hctx_state_fops},
 	{"flags", 0400, &hctx_flags_fops},
@@ -753,6 +778,9 @@ int blk_mq_debugfs_register_hctxs(struct request_queue *q)
 	if (!q->mq_debugfs_dir)
 		goto err;
 
+	if (!debugfs_create_files(q->mq_debugfs_dir, q, blk_mq_debugfs_queue_attrs))
+		return -ENOMEM;
+
 	queue_for_each_hw_ctx(q, hctx, i) {
 		if (blk_mq_debugfs_register_hctx(q, hctx))
 			goto err;
-- 
2.12.0

Re: [PATCH 1/2] loop: fix LO_FLAGS_PARTSCAN hang

[Ming Lei]

On Thu, Mar 2, 2017 at 2:42 AM, Omar Sandoval <osandov@osandov.com> wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> loop_reread_partitions() needs to do I/O, but we just froze the queue,
> so we end up waiting forever. This can easily be reproduced with losetup
> -P. Fix it by moving the reread to after we unfreeze the queue.
>
> Fixes: ecdd09597a57 ("block/loop: fix race between I/O and set_status")
> Reported-by: Tejun Heo <tj@kernel.org>
> Cc: Ming Lei <tom.leiming@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Omar Sandoval <osandov@fb.com>

Reviewed-by: Ming Lei <tom.leiming@gmail.com>

Thanks,
Ming

Re: [PATCH 2/2] blk-mq-debugfs: add q->mq_freeze_depth output

[Jens Axboe]

On 03/01/2017 11:42 AM, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> This can be used to diagnose freeze-related hangs.

Do we really need this? For the loop case, it was painfully
obvious that it was a count issue. And I can't think of any
cases where it would have helped us, outside of rewriting
core parts (like the first edition of the shadow requests
for the scheduler).

Not strongly opposed to it, feel free to convince me.

-- 
Jens Axboe

Re: [PATCH 1/2] loop: fix LO_FLAGS_PARTSCAN hang

[Jens Axboe]

On 03/01/2017 11:42 AM, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> loop_reread_partitions() needs to do I/O, but we just froze the queue,
> so we end up waiting forever. This can easily be reproduced with losetup
> -P. Fix it by moving the reread to after we unfreeze the queue.

Thanks Omar, applied. That's a brown paper bag bug.

-- 
Jens Axboe

Re: [PATCH 2/2] blk-mq-debugfs: add q->mq_freeze_depth output

[Omar Sandoval]

On Wed, Mar 01, 2017 at 07:25:59PM -0700, Jens Axboe wrote:
> On 03/01/2017 11:42 AM, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > This can be used to diagnose freeze-related hangs.
> 
> Do we really need this? For the loop case, it was painfully
> obvious that it was a count issue. And I can't think of any
> cases where it would have helped us, outside of rewriting
> core parts (like the first edition of the shadow requests
> for the scheduler).
> 
> Not strongly opposed to it, feel free to convince me.

I don't feel a burning need for it, let's just drop it.