On 2/22/22 06:14, Christoph Hellwig wrote:
From: Ming Lei <ming.lei@xxxxxxxxxx>
There can't be FS IO in disk_release(), so move blk_exit_queue() there.
We still need to freeze queue here since the request is freed after the
bio is completed and passthrough request rely on scheduler tags as well.
The disk can be released before or after queue is cleaned up, and we have
to free the scheduler request pool before blk_cleanup_queue returns,
while the static request pool has to be freed before exiting the
I/O scheduler.
This patch looks dubious to me because:
- The blk_freeze_queue() call in blk_cleanup_queue() waits for pending
requests to finish, so why to move blk_exit_queue() from
blk_cleanup_queue() into disk_release()?
- I'm concerned that this patch will break user space, e.g. scripts that
try to unload an I/O scheduler kernel module immediately after having
removed a request queue.
+static void blk_mq_release_queue(struct request_queue *q)
+{
+ blk_mq_cancel_work_sync(q);
+
+ /*
+ * There can't be any non non-passthrough bios in flight here, but
+ * requests stay around longer, including passthrough ones so we
+ * still need to freeze the queue here.
+ */
+ blk_mq_freeze_queue(q);
The above comment should be elaborated since what matters in this
context is not whether or not any bios are still in flight but what
happens with the request structures. As you know blk_queue_enter() fails
after the DYING flag has been set, a flag that is set by
blk_cleanup_queue(). blk_cleanup_queue() already freezes the queue. So
why is it necessary to call blk_mq_freeze_queue() from
blk_mq_release_queue()?
Bart.
