Commit 9d230c09 authored by Waiman Long's avatar Waiman Long Committed by Jens Axboe

blk-cgroup: Properly propagate the iostat update up the hierarchy

During a cgroup_rstat_flush() call, the lowest level of nodes are flushed
first before their parents. Since commit 3b8cc629 ("blk-cgroup:
Optimize blkcg_rstat_flush()"), iostat propagation was still done to
the parent. Grandparent, however, may not get the iostat update if the
parent has no blkg_iostat_set queued in its lhead lockless list.

Fix this iostat propagation problem by queuing the parent's global
blkg->iostat into one of its percpu lockless lists to make sure that
the delta will always be propagated up to the grandparent and so on
toward the root blkcg.

Note that successive calls to __blkcg_rstat_flush() are serialized by
the cgroup_rstat_lock. So no special barrier is used in the reading
and writing of blkg->iostat.lqueued.

Fixes: 3b8cc629 ("blk-cgroup: Optimize blkcg_rstat_flush()")
Reported-by: default avatarDan Schatzberg <schatzberg.dan@gmail.com>
Closes: https://lore.kernel.org/lkml/ZkO6l%2FODzadSgdhC@dschatzberg-fedora-PF3DHTBV/Signed-off-by: default avatarWaiman Long <longman@redhat.com>
Reviewed-by: default avatarMing Lei <ming.lei@redhat.com>
Acked-by: default avatarTejun Heo <tj@kernel.org>
Link: https://lore.kernel.org/r/20240515143059.276677-1-longman@redhat.comSigned-off-by: default avatarJens Axboe <axboe@kernel.dk>
parent d0aac236
...@@ -322,6 +322,7 @@ static struct blkcg_gq *blkg_alloc(struct blkcg *blkcg, struct gendisk *disk, ...@@ -322,6 +322,7 @@ static struct blkcg_gq *blkg_alloc(struct blkcg *blkcg, struct gendisk *disk,
blkg->q = disk->queue; blkg->q = disk->queue;
INIT_LIST_HEAD(&blkg->q_node); INIT_LIST_HEAD(&blkg->q_node);
blkg->blkcg = blkcg; blkg->blkcg = blkcg;
blkg->iostat.blkg = blkg;
#ifdef CONFIG_BLK_CGROUP_PUNT_BIO #ifdef CONFIG_BLK_CGROUP_PUNT_BIO
spin_lock_init(&blkg->async_bio_lock); spin_lock_init(&blkg->async_bio_lock);
bio_list_init(&blkg->async_bios); bio_list_init(&blkg->async_bios);
...@@ -1046,6 +1047,8 @@ static void __blkcg_rstat_flush(struct blkcg *blkcg, int cpu) ...@@ -1046,6 +1047,8 @@ static void __blkcg_rstat_flush(struct blkcg *blkcg, int cpu)
smp_mb(); smp_mb();
WRITE_ONCE(bisc->lqueued, false); WRITE_ONCE(bisc->lqueued, false);
if (bisc == &blkg->iostat)
goto propagate_up; /* propagate up to parent only */
/* fetch the current per-cpu values */ /* fetch the current per-cpu values */
do { do {
...@@ -1055,10 +1058,24 @@ static void __blkcg_rstat_flush(struct blkcg *blkcg, int cpu) ...@@ -1055,10 +1058,24 @@ static void __blkcg_rstat_flush(struct blkcg *blkcg, int cpu)
blkcg_iostat_update(blkg, &cur, &bisc->last); blkcg_iostat_update(blkg, &cur, &bisc->last);
propagate_up:
/* propagate global delta to parent (unless that's root) */ /* propagate global delta to parent (unless that's root) */
if (parent && parent->parent) if (parent && parent->parent) {
blkcg_iostat_update(parent, &blkg->iostat.cur, blkcg_iostat_update(parent, &blkg->iostat.cur,
&blkg->iostat.last); &blkg->iostat.last);
/*
* Queue parent->iostat to its blkcg's lockless
* list to propagate up to the grandparent if the
* iostat hasn't been queued yet.
*/
if (!parent->iostat.lqueued) {
struct llist_head *plhead;
plhead = per_cpu_ptr(parent->blkcg->lhead, cpu);
llist_add(&parent->iostat.lnode, plhead);
parent->iostat.lqueued = true;
}
}
} }
raw_spin_unlock_irqrestore(&blkg_stat_lock, flags); raw_spin_unlock_irqrestore(&blkg_stat_lock, flags);
out: out:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment