From: Ming Lei <ming.lei@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: axboe@kernel.dk, linux-block@vger.kernel.org,
syzbot+3e3f419f4a7816471838@syzkaller.appspotmail.com
Subject: Re: [PATCH] block: disable the elevator int del_gendisk
Date: Wed, 1 Jun 2022 15:09:26 +0800 [thread overview]
Message-ID: <YpcQpjUlX/CTORmp@T590> (raw)
In-Reply-To: <20220601064329.GB22915@lst.de>
On Wed, Jun 01, 2022 at 08:43:29AM +0200, Christoph Hellwig wrote:
> On Wed, Jun 01, 2022 at 08:54:30AM +0800, Ming Lei wrote:
> > This way can't be safe, who can guarantee that all sync submission
> > activities are gone after queue is frozen? We had lots of reports on
> > blk_mq_sched_has_work() which triggers UAF.
>
> Yes, we probably need a blk_mq_quiesce_queue call like in the incremental
> patch below. Do you have any good reproducer, though?
blktests block/027 should cover this.
>
> diff --git a/block/genhd.c b/block/genhd.c
> index 9914d0f24fecd..155b64ff991f6 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -652,9 +652,13 @@ void del_gendisk(struct gendisk *disk)
> blk_mq_cancel_work_sync(q);
>
> if (q->elevator) {
> + blk_mq_quiesce_queue(q);
> +
> mutex_lock(&q->sysfs_lock);
> elevator_exit(q);
> mutex_unlock(&q->sysfs_lock);
> +
> + blk_mq_unquiesce_queue(q);
> }
>
I am afraid the above way may slow down disk shutdown a lot, see
the following commit, that is also the reason why I moved it into disk
release handler, when any sync io submission are done.
commit 1311326cf4755c7ffefd20f576144ecf46d9906b
Author: Ming Lei <ming.lei@redhat.com>
Date: Mon Jun 25 19:31:49 2018 +0800
blk-mq: avoid to synchronize rcu inside blk_cleanup_queue()
SCSI probing may synchronously create and destroy a lot of request_queues
for non-existent devices. Any synchronize_rcu() in queue creation or
destroy path may introduce long latency during booting, see detailed
description in comment of blk_register_queue().
This patch removes one synchronize_rcu() inside blk_cleanup_queue()
for this case, commit c2856ae2f315d75(blk-mq: quiesce queue before freeing queue)
needs synchronize_rcu() for implementing blk_mq_quiesce_queue(), but
when queue isn't initialized, it isn't necessary to do that since
only pass-through requests are involved, no original issue in
scsi_execute() at all.
Without this patch and previous one, it may take more 20+ seconds for
virtio-scsi to complete disk probe. With the two patches, the time becomes
less than 100ms.
Thanks,
Ming
next prev parent reply other threads:[~2022-06-01 7:09 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-31 16:05 [PATCH] block: disable the elevator int del_gendisk Christoph Hellwig
2022-06-01 0:54 ` Ming Lei
2022-06-01 6:43 ` Christoph Hellwig
2022-06-01 7:09 ` Ming Lei [this message]
2022-06-01 7:14 ` Christoph Hellwig
2022-06-01 9:07 ` Ming Lei
2022-06-01 12:01 ` Christoph Hellwig
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=YpcQpjUlX/CTORmp@T590 \
--to=ming.lei@redhat.com \
--cc=axboe@kernel.dk \
--cc=hch@lst.de \
--cc=linux-block@vger.kernel.org \
--cc=syzbot+3e3f419f4a7816471838@syzkaller.appspotmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.