From: Yu Kuai <[email protected]> Throttle currently uses queue_lock for both blkcg topology and its own runtime state. This blocks moving blkg topology protection to blkcg_mutex cleanly.
Add a throttle-private spinlock and use it for throttle service queues, pending timers, runtime counters and config updates. Keep queue_lock only where the current intermediate code still walks blkcg topology. Signed-off-by: Yu Kuai <[email protected]> --- block/blk-throttle.c | 87 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 67 insertions(+), 20 deletions(-) diff --git a/block/blk-throttle.c b/block/blk-throttle.c index ffc3b70065d4..7bca2805404f 100644 --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -30,6 +30,9 @@ static struct workqueue_struct *kthrotld_workqueue; struct throtl_data { + /* protects throttle service queues and group runtime state */ + spinlock_t lock; + /* service tree for active throtl groups */ struct throtl_service_queue service_queue; @@ -346,11 +349,16 @@ static void tg_update_has_rules(struct throtl_grp *tg) static void throtl_pd_online(struct blkg_policy_data *pd) { struct throtl_grp *tg = pd_to_tg(pd); + struct throtl_data *td = tg->td; + unsigned long flags; + + spin_lock_irqsave(&td->lock, flags); /* * We don't want new groups to escape the limits of its ancestors. * Update has_rules[] after a new group is brought online. */ tg_update_has_rules(tg); + spin_unlock_irqrestore(&td->lock, flags); } static void tg_release(struct rcu_head *rcu) @@ -368,7 +376,7 @@ static void throtl_pd_free(struct blkg_policy_data *pd) { struct throtl_grp *tg = pd_to_tg(pd); - timer_delete_sync(&tg->service_queue.pending_timer); + timer_shutdown_sync(&tg->service_queue.pending_timer); call_rcu(&pd->rcu_head, tg_release); } @@ -1142,9 +1150,9 @@ static void throtl_pending_timer_fn(struct timer_list *t) else q = td->queue; - spin_lock_irq(&q->queue_lock); + spin_lock_irq(&td->lock); - if (!q->root_blkg) + if (!READ_ONCE(q->root_blkg)) goto out_unlock; again: @@ -1168,9 +1176,9 @@ static void throtl_pending_timer_fn(struct timer_list *t) break; /* this dispatch windows is still open, relax and repeat */ - spin_unlock_irq(&q->queue_lock); + spin_unlock_irq(&td->lock); cpu_relax(); - spin_lock_irq(&q->queue_lock); + spin_lock_irq(&td->lock); } if (!dispatched) @@ -1193,7 +1201,7 @@ static void throtl_pending_timer_fn(struct timer_list *t) queue_work(kthrotld_workqueue, &td->dispatch_work); } out_unlock: - spin_unlock_irq(&q->queue_lock); + spin_unlock_irq(&td->lock); } /** @@ -1209,7 +1217,6 @@ static void blk_throtl_dispatch_work_fn(struct work_struct *work) struct throtl_data *td = container_of(work, struct throtl_data, dispatch_work); struct throtl_service_queue *td_sq = &td->service_queue; - struct request_queue *q = td->queue; struct bio_list bio_list_on_stack; struct bio *bio; struct blk_plug plug; @@ -1217,11 +1224,11 @@ static void blk_throtl_dispatch_work_fn(struct work_struct *work) bio_list_init(&bio_list_on_stack); - spin_lock_irq(&q->queue_lock); + spin_lock_irq(&td->lock); for (rw = READ; rw <= WRITE; rw++) while ((bio = throtl_pop_queued(td_sq, NULL, rw))) bio_list_add(&bio_list_on_stack, bio); - spin_unlock_irq(&q->queue_lock); + spin_unlock_irq(&td->lock); if (!bio_list_empty(&bio_list_on_stack)) { blk_start_plug(&plug); @@ -1299,7 +1306,7 @@ static void tg_conf_updated(struct throtl_grp *tg, bool global) rcu_read_unlock(); /* - * We're already holding queue_lock and know @tg is valid. Let's + * We're already holding td->lock and know @tg is valid. Let's * apply the new config directly. * * Restart the slices for both READ and WRITES. It might happen @@ -1327,6 +1334,7 @@ static int blk_throtl_init(struct gendisk *disk) return -ENOMEM; INIT_WORK(&td->dispatch_work, blk_throtl_dispatch_work_fn); + spin_lock_init(&td->lock); throtl_service_queue_init(&td->service_queue); memflags = blk_mq_freeze_queue(disk->queue); @@ -1381,6 +1389,7 @@ static ssize_t tg_set_conf(struct kernfs_open_file *of, v = U64_MAX; tg = blkg_to_tg(ctx.blkg); + spin_lock_irq(&tg->td->lock); tg_update_carryover(tg); if (is_u64) @@ -1389,6 +1398,7 @@ static ssize_t tg_set_conf(struct kernfs_open_file *of, *(unsigned int *)((void *)tg + of_cft(of)->private) = v; tg_conf_updated(tg, false); + spin_unlock_irq(&tg->td->lock); ret = 0; unprep: @@ -1563,6 +1573,7 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of, goto close_bdev; tg = blkg_to_tg(ctx.blkg); + spin_lock_irq(&tg->td->lock); tg_update_carryover(tg); v[0] = tg->bps[READ]; @@ -1586,11 +1597,11 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of, p = tok; strsep(&p, "="); if (!p || (sscanf(p, "%llu", &val) != 1 && strcmp(p, "max"))) - goto unprep; + goto unlock; ret = -ERANGE; if (!val) - goto unprep; + goto unlock; ret = -EINVAL; if (!strcmp(tok, "rbps")) @@ -1602,7 +1613,7 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of, else if (!strcmp(tok, "wiops")) v[3] = min_t(u64, val, UINT_MAX); else - goto unprep; + goto unlock; } tg->bps[READ] = v[0]; @@ -1611,7 +1622,11 @@ static ssize_t tg_set_limit(struct kernfs_open_file *of, tg->iops[WRITE] = v[3]; tg_conf_updated(tg, false); + spin_unlock_irq(&tg->td->lock); ret = 0; + goto unprep; +unlock: + spin_unlock_irq(&tg->td->lock); unprep: blkg_conf_unprep(&ctx); close_bdev: @@ -1636,6 +1651,28 @@ static void throtl_shutdown_wq(struct request_queue *q) cancel_work_sync(&td->dispatch_work); } +static void throtl_shutdown_timers(struct request_queue *q) +{ + struct throtl_data *td = q->td; + struct blkcg_gq *blkg; + + /* + * blkg_destroy_all() has already offlined the policy, but blkg policy + * data is freed asynchronously. Shut down per-group timers before + * freeing td, as their callbacks still dereference tg->td. + */ + mutex_lock(&q->blkcg_mutex); + list_for_each_entry(blkg, &q->blkg_list, q_node) { + struct throtl_grp *tg = blkg_to_tg(blkg); + + if (tg) + timer_shutdown_sync(&tg->service_queue.pending_timer); + } + mutex_unlock(&q->blkcg_mutex); + + timer_shutdown_sync(&td->service_queue.pending_timer); +} + static void tg_flush_bios(struct throtl_grp *tg) { struct throtl_service_queue *sq = &tg->service_queue; @@ -1669,7 +1706,13 @@ static void tg_flush_bios(struct throtl_grp *tg) static void throtl_pd_offline(struct blkg_policy_data *pd) { - tg_flush_bios(pd_to_tg(pd)); + struct throtl_grp *tg = pd_to_tg(pd); + struct throtl_data *td = tg->td; + unsigned long flags; + + spin_lock_irqsave(&td->lock, flags); + tg_flush_bios(tg); + spin_unlock_irqrestore(&td->lock, flags); } struct blkcg_policy blkcg_policy_throtl = { @@ -1725,6 +1768,7 @@ static void tg_cancel_writeback_bios(struct throtl_grp *tg, void blk_throtl_cancel_bios(struct gendisk *disk) { struct request_queue *q = disk->queue; + struct throtl_data *td = q->td; struct cgroup_subsys_state *pos_css; struct blkcg_gq *blkg; struct bio_list cancel_bios[2] = { }; @@ -1734,6 +1778,7 @@ void blk_throtl_cancel_bios(struct gendisk *disk) return; spin_lock_irq(&q->queue_lock); + spin_lock(&td->lock); /* * queue_lock is held, rcu lock is not needed here technically. * However, rcu lock is still held to emphasize that following @@ -1752,6 +1797,7 @@ void blk_throtl_cancel_bios(struct gendisk *disk) tg_cancel_writeback_bios(blkg_to_tg(blkg), cancel_bios); } rcu_read_unlock(); + spin_unlock(&td->lock); spin_unlock_irq(&q->queue_lock); for (rw = READ; rw <= WRITE; rw++) { @@ -1791,7 +1837,6 @@ static bool tg_within_limit(struct throtl_grp *tg, struct bio *bio, bool rw) bool __blk_throtl_bio(struct bio *bio) { - struct request_queue *q = bdev_get_queue(bio->bi_bdev); struct blkcg_gq *blkg = bio->bi_blkg; struct throtl_qnode *qn = NULL; struct throtl_grp *tg = blkg_to_tg(blkg); @@ -1801,7 +1846,7 @@ bool __blk_throtl_bio(struct bio *bio) struct throtl_data *td = tg->td; rcu_read_lock(); - spin_lock_irq(&q->queue_lock); + spin_lock_irq(&td->lock); sq = &tg->service_queue; while (true) { @@ -1877,7 +1922,7 @@ bool __blk_throtl_bio(struct bio *bio) } out_unlock: - spin_unlock_irq(&q->queue_lock); + spin_unlock_irq(&td->lock); rcu_read_unlock(); return throttled; @@ -1886,17 +1931,19 @@ bool __blk_throtl_bio(struct bio *bio) void blk_throtl_exit(struct gendisk *disk) { struct request_queue *q = disk->queue; + struct throtl_data *td = q->td; /* * blkg_destroy_all() already deactivate throtl policy, just check and * free throtl data. */ - if (!q->td) + if (!td) return; - timer_delete_sync(&q->td->service_queue.pending_timer); + throtl_shutdown_timers(q); throtl_shutdown_wq(q); - kfree(q->td); + q->td = NULL; + kfree(td); } static int __init throtl_init(void) -- 2.51.0
