diff mbox series

[14/14] block: move rq_qos_exit() into disk_release()

Message ID 20220304160331.399757-15-hch@lst.de
State Superseded
Headers show
Series [01/14] blk-mq: do not include passthrough requests in I/O accounting | expand

Commit Message

Christoph Hellwig March 4, 2022, 4:03 p.m. UTC 
From: Ming Lei <ming.lei@redhat.com>

There can't be FS IO in disk_release(), so it is safe to move rq_qos_exit()
there.

Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/genhd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Bart Van Assche March 6, 2022, 8:51 p.m. UTC | #1 
On 3/4/22 08:03, Christoph Hellwig wrote:
> From: Ming Lei <ming.lei@redhat.com>
> 
> There can't be FS IO in disk_release(), so it is safe to move rq_qos_exit()
> there.

The commit message only explains why it is safe to move rq_qos_exit() 
but not why moving that function call is useful. Please add an 
explanation of why moving that function call is useful and/or necessary.

> diff --git a/block/genhd.c b/block/genhd.c
> index 857e0a54da7dd..56f66c6fee943 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -627,7 +627,6 @@ void del_gendisk(struct gendisk *disk)
>   
>   	blk_mq_freeze_queue_wait(q);
>   
> -	rq_qos_exit(q);
>   	blk_sync_queue(q);
>   	blk_flush_integrity();
>   	/*
> @@ -1119,7 +1118,7 @@ static void disk_release_mq(struct request_queue *q)
>   		elevator_exit(q);
>   		mutex_unlock(&q->sysfs_lock);
>   	}
> -
> +	rq_qos_exit(q);
>   	__blk_mq_unfreeze_queue(q, true);
>   }

Commit 8e141f9eb803 ("block: drain file system I/O on del_gendisk") 
removed the rq_qos_exit() call from blk_cleanup_queue(). This patch 
series does not restore the rq_qos_exit() call in blk_cleanup_queue(). I 
think that call should be restored since rq_qos_add() can be called 
before add_disk() is called. I'm referring to the following call chain:

__alloc_disk_node()
   blkcg_init_queue()
     blk_iolatency_init()
       rq_qos_add()

sd_probe() is one of the functions that can take an error path after 
__alloc_disk_node() has returned and before device_add_disk() is called.

Thanks,

Bart.
diff mbox series

Patch

diff --git a/block/genhd.c b/block/genhd.c
index 857e0a54da7dd..56f66c6fee943 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -627,7 +627,6 @@  void del_gendisk(struct gendisk *disk)
 
 	blk_mq_freeze_queue_wait(q);
 
-	rq_qos_exit(q);
 	blk_sync_queue(q);
 	blk_flush_integrity();
 	/*
@@ -1119,7 +1118,7 @@  static void disk_release_mq(struct request_queue *q)
 		elevator_exit(q);
 		mutex_unlock(&q->sysfs_lock);
 	}
-
+	rq_qos_exit(q);
 	__blk_mq_unfreeze_queue(q, true);
 }