linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Jens Axboe <axboe@kernel.dk>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>,
	YangYang <yang.yang@vivo.com>,
	linux-block@vger.kernel.org
Subject: [PATCH 1/2] block: also mark disk-owned queues as dying in __blk_mark_disk_dead
Date: Tue,  8 Oct 2024 13:57:50 +0200	[thread overview]
Message-ID: <20241008115756.355936-2-hch@lst.de> (raw)
In-Reply-To: <20241008115756.355936-1-hch@lst.de>

When del_gendisk shuts down access to a gendisk, it could lead to a
deadlock with sd or, which try to submit passthrough SCSI commands from
their ->release method under open_mutex.  The submission can be blocked
in blk_enter_queue while del_gendisk can't get to actually telling them
top stop and wake them up.

As the disk is going away there is no real point in sending these
commands, but we have no really good way to distinguish between the
cases.  For now mark even standalone (aka SCSI queues) as dying in
del_gendisk to avoid this deadlock, but the real fix will be to split
freeing a disk from freezing a queue for not disk associated requests.

Reported-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/genhd.c          | 14 ++++++++++++--
 include/linux/blkdev.h |  1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 1c05dd4c6980b5..ac1c496ad43343 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -589,8 +589,16 @@ static void __blk_mark_disk_dead(struct gendisk *disk)
 	if (test_and_set_bit(GD_DEAD, &disk->state))
 		return;
 
-	if (test_bit(GD_OWNS_QUEUE, &disk->state))
-		blk_queue_flag_set(QUEUE_FLAG_DYING, disk->queue);
+	/*
+	 * Also mark the disk dead if it is not owned by the gendisk.  This
+	 * means we can't allow /dev/sg passthrough or SCSI internal commands
+	 * while unbinding a ULP.  That is more than just a bit ugly, but until
+	 * we untangle q_usage_counter into one owned by the disk and one owned
+	 * by the queue this is as good as it gets.  The flag will be cleared
+	 * at the end of del_gendisk if it wasn't set before.
+	 */
+	if (!test_and_set_bit(QUEUE_FLAG_DYING, &disk->queue->queue_flags))
+		set_bit(QUEUE_FLAG_RESURRECT, &disk->queue->queue_flags);
 
 	/*
 	 * Stop buffered writers from dirtying pages that can't be written out.
@@ -719,6 +727,8 @@ void del_gendisk(struct gendisk *disk)
 	 * again.  Else leave the queue frozen to fail all I/O.
 	 */
 	if (!test_bit(GD_OWNS_QUEUE, &disk->state)) {
+		if (test_bit(QUEUE_FLAG_RESURRECT, &q->queue_flags))
+			clear_bit(QUEUE_FLAG_DYING, &q->queue_flags);
 		blk_queue_flag_clear(QUEUE_FLAG_INIT_DONE, q);
 		__blk_mq_unfreeze_queue(q, true);
 	} else {
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 50c3b959da2816..391e3eb3bb5e61 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -590,6 +590,7 @@ struct request_queue {
 /* Keep blk_queue_flag_name[] in sync with the definitions below */
 enum {
 	QUEUE_FLAG_DYING,		/* queue being torn down */
+	QUEUE_FLAG_RESURRECT,		/* temporarily dying */
 	QUEUE_FLAG_NOMERGES,		/* disable merge attempts */
 	QUEUE_FLAG_SAME_COMP,		/* complete on same CPU-group */
 	QUEUE_FLAG_FAIL_IO,		/* fake timeout */
-- 
2.45.2


  reply	other threads:[~2024-10-08 11:58 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-08 11:57 RFC: try to avoid del_gendisk vs passthrough from ->release deadlocks Christoph Hellwig
2024-10-08 11:57 ` Christoph Hellwig [this message]
2024-10-09  5:06   ` [PATCH 1/2] block: also mark disk-owned queues as dying in __blk_mark_disk_dead Sergey Senozhatsky
2024-10-09  7:34     ` Christoph Hellwig
2024-10-08 11:57 ` [PATCH 2/2] block: mark the disk dead before taking open_mutx in del_gendisk Christoph Hellwig
2024-10-09  5:04   ` Sergey Senozhatsky
2024-10-08 15:23 ` RFC: try to avoid del_gendisk vs passthrough from ->release deadlocks Sergey Senozhatsky
  -- strict thread matches above, loose matches on Subject: below --
2024-10-09 11:38 try to avoid del_gendisk vs passthrough from ->release deadlocks v2 Christoph Hellwig
2024-10-09 11:38 ` [PATCH 1/2] block: also mark disk-owned queues as dying in __blk_mark_disk_dead Christoph Hellwig
2024-10-09 12:31   ` Sergey Senozhatsky
2024-10-09 12:41     ` Christoph Hellwig
2024-10-09 12:43       ` Sergey Senozhatsky
2024-10-09 13:49       ` Jens Axboe
2024-10-16  4:14   ` YangYang
2024-10-16 11:09   ` Ming Lei
2024-10-16 12:32     ` Christoph Hellwig
2024-10-16 12:49       ` Ming Lei
2024-10-16 13:35   ` Ming Lei
2024-10-19  1:25     ` Sergey Senozhatsky
2024-10-19 12:32       ` Ming Lei
2024-10-19 12:37         ` Sergey Senozhatsky
2024-10-19 12:50           ` Ming Lei
2024-10-19 12:58             ` Sergey Senozhatsky
2024-10-19 13:09               ` Ming Lei
2024-10-19 13:50                 ` Sergey Senozhatsky
2024-10-19 15:03                   ` Ming Lei
2024-10-19 15:11                     ` Sergey Senozhatsky
2024-10-19 15:40                     ` Sergey Senozhatsky
2024-10-28  5:44                     ` Sergey Senozhatsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241008115756.355936-2-hch@lst.de \
    --to=hch@lst.de \
    --cc=axboe@kernel.dk \
    --cc=linux-block@vger.kernel.org \
    --cc=senozhatsky@chromium.org \
    --cc=yang.yang@vivo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).