iocost: check active_list of all the ancestors in iocg_activate()
authorJiufei Xue <jiufei.xue@linux.alibaba.com>
Wed, 13 Nov 2019 07:21:31 +0000 (15:21 +0800)
committerJens Axboe <axboe@kernel.dk>
Thu, 14 Nov 2019 20:56:54 +0000 (13:56 -0700)
There is a bug that checking the same active_list over and over again
in iocg_activate(). The intention of the code was checking whether all
the ancestors and self have already been activated. So fix it.

Fixes: 7caa47151ab2 ("blkcg: implement blk-iocost")
Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Jiufei Xue <jiufei.xue@linux.alibaba.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
block/blk-iocost.c

index a7ed434..e01267f 100644 (file)
@@ -1057,9 +1057,12 @@ static bool iocg_activate(struct ioc_gq *iocg, struct ioc_now *now)
        atomic64_set(&iocg->active_period, cur_period);
 
        /* already activated or breaking leaf-only constraint? */
-       for (i = iocg->level; i > 0; i--)
-               if (!list_empty(&iocg->active_list))
+       if (!list_empty(&iocg->active_list))
+               goto succeed_unlock;
+       for (i = iocg->level - 1; i > 0; i--)
+               if (!list_empty(&iocg->ancestors[i]->active_list))
                        goto fail_unlock;
+
        if (iocg->child_active_sum)
                goto fail_unlock;
 
@@ -1101,6 +1104,7 @@ static bool iocg_activate(struct ioc_gq *iocg, struct ioc_now *now)
                ioc_start_period(ioc, now);
        }
 
+succeed_unlock:
        spin_unlock_irq(&ioc->lock);
        return true;