If del_gendisk() is done when some io are still throttled, such io
will not be handled until the throttle is done, which is not
necessary.
Changes in v2:
- move WARN_ON_ONCE() from throtl_rb_first() to it's caller
- merge some patches into one.
Yu Kuai (2):
blk-throtl: move WARN_ON_ONCE() from throtl_rb_first() to it's caller
block: cancel all throttled bios in del_gendisk()
block/blk-throttle.c | 68 ++++++++++++++++++++++++++++++++++++++++++--
block/blk-throttle.h | 2 ++
block/genhd.c | 2 ++
3 files changed, 69 insertions(+), 3 deletions(-)
--
2.31.1
Throttled bios can't be issued after del_gendisk() is done, thus
it's better to cancel them immediately rather than waiting for
throttle is done.
For example, if user thread is throttled with low bps while it's
issuing large io, and the device is deleted. The user thread will
wait for a long time for io to return.
Noted this patch is mainly from revertion of commit 32e3374304c7
("blk-throttle: remove tg_drain_bios") and commit b77412372b68
("blk-throttle: remove blk_throtl_drain").
Signed-off-by: Yu Kuai <redacted>
---
block/blk-throttle.c | 59 ++++++++++++++++++++++++++++++++++++++++++++
block/blk-throttle.h | 2 ++
block/genhd.c | 2 ++
3 files changed, 63 insertions(+)
@@ -2260,6 +2260,65 @@ void blk_throtl_bio_endio(struct bio *bio)}#endif+/*+*Dispatchallbiosfromallchildrentg'squeuedon@parent_sq.On+*return,@parent_sqisguaranteedtonothaveanyactivechildrentg's+*andallbiosfrompreviouslyactivetg'sareon@parent_sq->bio_lists[].+*/+staticvoidtg_drain_bios(structthrotl_service_queue*parent_sq)+{+structthrotl_grp*tg;++while((tg=throtl_rb_first(parent_sq))){+structthrotl_service_queue*sq=&tg->service_queue;+structbio*bio;++throtl_dequeue_tg(tg);++while((bio=throtl_peek_queued(&sq->queued[READ])))+tg_dispatch_one_bio(tg,bio_data_dir(bio));+while((bio=throtl_peek_queued(&sq->queued[WRITE])))+tg_dispatch_one_bio(tg,bio_data_dir(bio));+}+}++/**+*blk_throtl_cancel_bios-cancelthrottledbios+*@q:request_queuetocancelthrottledbiosfor+*+*Thisfunctioniscalledtoerrorallcurrentlythrottledbioson@q.+*/+voidblk_throtl_cancel_bios(structrequest_queue*q)+{+structthrotl_data*td=q->td;+structblkcg_gq*blkg;+structcgroup_subsys_state*pos_css;+structbio*bio;+intrw;++rcu_read_lock();++/*+*Draineachtgwhiledoingpost-orderwalkontheblkgtree,so+*thatallbiosarepropagatedtotd->service_queue.It'dbe+*bettertowalkservice_queuetreedirectlybutblkgwalkis+*easier.+*/+blkg_for_each_descendant_post(blkg,pos_css,td->queue->root_blkg)+tg_drain_bios(&blkg_to_tg(blkg)->service_queue);++/* finally, transfer bios from top-level tg's into the td */+tg_drain_bios(&td->service_queue);++rcu_read_unlock();++/* all bios now should be in td->service_queue, cancel them */+for(rw=READ;rw<=WRITE;rw++)+while((bio=throtl_pop_queued(&td->service_queue.queued[rw],+NULL)))+bio_io_error(bio);+}+intblk_throtl_init(structrequest_queue*q){structthrotl_data*td;
So, all of the draining is being performed without holding the q lock, which
*might* be okay given that we're in the del_gendisk path but is likely risky
- ie. there can still be timers or whatever racing against it.
Thanks.
--
tejun
So, all of the draining is being performed without holding the q lock, which
*might* be okay given that we're in the del_gendisk path but is likely risky
- ie. there can still be timers or whatever racing against it.
I'll hold queue_lock to draining bios in next iteration,
Thanks,
Kuai