| Message ID | 20220216113212.83000-1-jianchao.wan9@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | blk: do rq_qos_exit in blk_cleanup_queue | expand |
On 2/16/22 03:32, Wang Jianchao (Kuaishou) wrote: > From: Wang Jianchao <wangjianchao@kuaishou.com> > > When __alloc_disk_node() failed, there will not not del_gendisk() Please use the present tense for patch descriptions (failed -> fails). Anyway: Reviewed-by: Bart Van Assche <bvanassche@acm.org>
On Wed, 16 Feb 2022 19:32:12 +0800, Wang Jianchao (Kuaishou) wrote: > From: Wang Jianchao <wangjianchao@kuaishou.com> > > When __alloc_disk_node() failed, there will not not del_gendisk() > any more, then resource in rqos policies is leaked. Add rq_qos_exit() > into blk_cleanup_queue(). rqos is removed from the list, so needn't > to worry .exit is called twice. > > [...] Applied, thanks! [1/1] blk: do rq_qos_exit in blk_cleanup_queue commit: 20d41d9e993735b996175d087148d9de1fc94ac0 Best regards,
On Wed, Feb 16, 2022 at 07:32:12PM +0800, Wang Jianchao (Kuaishou) wrote: > From: Wang Jianchao <wangjianchao@kuaishou.com> > > When __alloc_disk_node() failed, there will not not del_gendisk() > any more, then resource in rqos policies is leaked. Add rq_qos_exit() > into blk_cleanup_queue(). rqos is removed from the list, so needn't > to worry .exit is called twice. > > Fixes: commit 8e141f9eb803 ("block: drain file system I/O on del_gendisk") > Suggested-by: Bart Van Assche <bart.vanassche@wdc.com> > Signed-off-by: Wang Jianchao <wangjianchao@kuaishou.com> Ming had a pending patch to move it into disk_release instead, which I think is the right place.
On 2/17/22 12:48 AM, Christoph Hellwig wrote: > On Wed, Feb 16, 2022 at 07:32:12PM +0800, Wang Jianchao (Kuaishou) wrote: >> From: Wang Jianchao <wangjianchao@kuaishou.com> >> >> When __alloc_disk_node() failed, there will not not del_gendisk() >> any more, then resource in rqos policies is leaked. Add rq_qos_exit() >> into blk_cleanup_queue(). rqos is removed from the list, so needn't >> to worry .exit is called twice. >> >> Fixes: commit 8e141f9eb803 ("block: drain file system I/O on del_gendisk") >> Suggested-by: Bart Van Assche <bart.vanassche@wdc.com> >> Signed-off-by: Wang Jianchao <wangjianchao@kuaishou.com> > > Ming had a pending patch to move it into disk_release instead, which > I think is the right place. I missed that patch and can't seem to find it, do you have a link?
On Thu, Feb 17, 2022 at 06:34:02AM -0700, Jens Axboe wrote: > On 2/17/22 12:48 AM, Christoph Hellwig wrote: > > On Wed, Feb 16, 2022 at 07:32:12PM +0800, Wang Jianchao (Kuaishou) wrote: > >> From: Wang Jianchao <wangjianchao@kuaishou.com> > >> > >> When __alloc_disk_node() failed, there will not not del_gendisk() > >> any more, then resource in rqos policies is leaked. Add rq_qos_exit() > >> into blk_cleanup_queue(). rqos is removed from the list, so needn't > >> to worry .exit is called twice. > >> > >> Fixes: commit 8e141f9eb803 ("block: drain file system I/O on del_gendisk") > >> Suggested-by: Bart Van Assche <bart.vanassche@wdc.com> > >> Signed-off-by: Wang Jianchao <wangjianchao@kuaishou.com> > > > > Ming had a pending patch to move it into disk_release instead, which > > I think is the right place. > > I missed that patch and can't seem to find it, do you have a link? [PATCH V2 12/13] block: move rq_qos_exit() into disk_release() from Jan 22. Although it would need a rebase so it can be applied without the preceding patches.
On 2/17/22 7:03 AM, Christoph Hellwig wrote: > On Thu, Feb 17, 2022 at 06:34:02AM -0700, Jens Axboe wrote: >> On 2/17/22 12:48 AM, Christoph Hellwig wrote: >>> On Wed, Feb 16, 2022 at 07:32:12PM +0800, Wang Jianchao (Kuaishou) wrote: >>>> From: Wang Jianchao <wangjianchao@kuaishou.com> >>>> >>>> When __alloc_disk_node() failed, there will not not del_gendisk() >>>> any more, then resource in rqos policies is leaked. Add rq_qos_exit() >>>> into blk_cleanup_queue(). rqos is removed from the list, so needn't >>>> to worry .exit is called twice. >>>> >>>> Fixes: commit 8e141f9eb803 ("block: drain file system I/O on del_gendisk") >>>> Suggested-by: Bart Van Assche <bart.vanassche@wdc.com> >>>> Signed-off-by: Wang Jianchao <wangjianchao@kuaishou.com> >>> >>> Ming had a pending patch to move it into disk_release instead, which >>> I think is the right place. >> >> I missed that patch and can't seem to find it, do you have a link? > > [PATCH V2 12/13] block: move rq_qos_exit() into disk_release() > > from Jan 22. Although it would need a rebase so it can be applied > without the preceding patches. Can someone respin that for 5.17 then?
On Thu, Feb 17, 2022 at 07:55:16AM -0700, Jens Axboe wrote: > > from Jan 22. Although it would need a rebase so it can be applied > > without the preceding patches. > > Can someone respin that for 5.17 then? I looked at it and it I don't think we can do that without a lot of the prep patches. That being said I think this version of the patch also is buggy, we want the policies shut down in del_gendisk with the queue frozen for normal operation. I guess until we can move the initialization and teardown entirely to the gendisk as in Ming's more complex series we need to keep the call in del_gendisk and also do it in blk_cleanup_queue. For the normal shutdown on disk that were life del_gendisk does the all the work on the frozen queue, while for queues that never had a disk blk_cleanup_queue will clean up the unused rq_qos.
diff --git a/block/blk-core.c b/block/blk-core.c index d93e3bb9a769..108c7207d048 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -50,6 +50,7 @@ #include "blk-mq-sched.h" #include "blk-pm.h" #include "blk-throttle.h" +#include "blk-rq-qos.h" struct dentry *blk_debugfs_root; @@ -322,6 +323,8 @@ void blk_cleanup_queue(struct request_queue *q) blk_queue_flag_set(QUEUE_FLAG_DEAD, q); + rq_qos_exit(q); + blk_sync_queue(q); if (queue_is_mq(q)) { blk_mq_cancel_work_sync(q);