linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <senozhatsky@chromium.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>,
	Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org
Subject: Re: block: del_gendisk() vs blk_queue_enter() race condition
Date: Thu, 3 Oct 2024 23:00:51 +0900	[thread overview]
Message-ID: <20241003140051.GM11458@google.com> (raw)
In-Reply-To: <Zv6fbloZRg2xQ1Jf@infradead.org>

On (24/10/03 06:43), Christoph Hellwig wrote:
> .. actually, we still clear QUEUE_FLAG_DYING early.  Something like
> the pathc below (plus proper comments) should sort it out:
> 
> diff --git a/block/genhd.c b/block/genhd.c
> index 1c05dd4c6980b5..9a1e18fbb136cf 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -589,9 +589,6 @@ 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);
> -
>  	/*
>  	 * Stop buffered writers from dirtying pages that can't be written out.
>  	 */
> @@ -673,6 +670,9 @@ void del_gendisk(struct gendisk *disk)
>  		drop_partition(part);
>  	mutex_unlock(&disk->open_mutex);
>  
> +	if (test_bit(GD_OWNS_QUEUE, &disk->state))
> +		blk_queue_flag_set(QUEUE_FLAG_DYING, disk->queue);

So that mutex_lock(&disk->open_mutex) right before it potentially can
deadlock (I think it will).

My idea, thus far, was to

	if (test_bit(GD_OWNS_QUEUE, &disk->state)) }
		blk_queue_flag_set(QUEUE_FLAG_DYING, disk->queue);
		blk_kick_queue_enter(disk->queue);  // this one simply wake_up_all(&q->mq_freeze_wq);
											// if the queue has QUEUE_FLAG_DYING
	}

in del_gendisk() before the very first time del_gendisk() attempts to
mutex_lock(&disk->open_mutex), because that mutex is already locked
forever.

>  	if (!(disk->flags & GENHD_FL_HIDDEN)) {
>  		sysfs_remove_link(&disk_to_dev(disk)->kobj, "bdi");

  reply	other threads:[~2024-10-03 14:00 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-03  8:56 block: del_gendisk() vs blk_queue_enter() race condition Sergey Senozhatsky
2024-10-03 13:36 ` Christoph Hellwig
2024-10-03 13:43   ` Christoph Hellwig
2024-10-03 14:00     ` Sergey Senozhatsky [this message]
2024-10-03 14:17       ` Sergey Senozhatsky
2024-10-04  4:21         ` Sergey Senozhatsky
2024-10-04  6:45           ` Christoph Hellwig
2024-10-04  7:48             ` Sergey Senozhatsky
2024-10-04  7:49               ` Sergey Senozhatsky
2024-10-04 12:20               ` Christoph Hellwig
2024-10-04 14:32                 ` Sergey Senozhatsky
2024-10-07  6:10                   ` Christoph Hellwig
2024-10-07  9:45                     ` Sergey Senozhatsky
2024-10-08  5:31                       ` Sergey Senozhatsky
2024-10-04 14:41                 ` Sergey Senozhatsky
2024-10-03 13:55   ` Sergey Senozhatsky
2024-10-08  4:02 ` YangYang
2024-10-08  5:19   ` Sergey Senozhatsky
2024-10-08  5:26     ` Sergey Senozhatsky
2024-10-08  5:56       ` Christoph Hellwig
2024-10-08  6:04         ` Christoph Hellwig
2024-10-08  6:10         ` Sergey Senozhatsky
2024-10-08  8:13           ` Christoph Hellwig
2024-10-08  8:20             ` 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=20241003140051.GM11458@google.com \
    --to=senozhatsky@chromium.org \
    --cc=axboe@kernel.dk \
    --cc=hch@infradead.org \
    --cc=linux-block@vger.kernel.org \
    /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).