Hi Tejun, On 2019/11/12 上午12:25, Tejun Heo wrote: > Hello, Jiufei. > > On Mon, Nov 11, 2019 at 03:37:18PM +0800, Jiufei Xue wrote: >> Internal nodes that issued IOs are treated as root when leaves are >> activated. However, leaf nodes can still be activated when internal >> nodes are active, leaving the sum of hweights exceeds HWEIGHT_WHOLE. >> >> I think we should also treat the leaf nodes as root while leaf-only >> constraint broken. > > Hmm... I'm not sure this description makes sense. > Should I change the description to something like this? "we should treat the leaf nodes as root while the parent are already activated". >> @@ -1057,8 +1057,8 @@ 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)) >> + for (i = iocg->level - 1; i > 0; i--) >> + if (!list_empty(&iocg->ancestors[i]->active_list)) > > But there's an obvious bug there as it's checking the same active_list > over and over again. Shouldn't it be sth like the following instead? > > if (!list_empty(&iocg->active_list)) > goto succeed_unlock; iocg has already checked before, do you mean we should check it again after ioc->lock? > for (i = iocg->level - 1; i > 0; i--) > if (!list_empty(&iocg->ancestors[i]->active_list)) > goto fail_unlock; > > Thanks. > Thanks, Jiufei
