Messages in this thread | | | Subject | Re: [PATCH] sched: Call update_group_power only for local_group | From | Peter Zijlstra <> | Date | Fri, 02 Jul 2010 18:40:16 +0200 |
| |
On Fri, 2010-07-02 at 09:20 -0700, Venkatesh Pallipadi wrote: > > Hrm,.. so Gautham removed that because for things like the NO_HZ > > balancer the initial balance_cpu == this_cpu constraint doesn't hold. > > > > Not I don't think the local_group constraint holds for that either, so > > the below would again break that.. > > > > Should we perhaps have a conditional on this_rq->nohz_balance_kick or > > so? > > The thing is that update_group_power is only updating the power of > local group (sd->groups).
Not quite, see nohz_idle_balance(), that iterates idle_cpus_mask, and calls rebalance_domains(balance_cpu, CPU_IDLE), which then does for_each_domain(balance_cpu, sd)
So sd need not be local at all, and sd->group will be the group of which balance_cpu is part.
> It is getting called multiple times however for each group as > update_sd_lb_stats loops > through groups->next calling update_sg_lb_stats.
Sure I see how that's happening and why you would want to avoid that, no argument there.
> If we really want to update the power of non-local groups, > update_cpu_power has to change > to take a groups parameter and non this_cpu as arguments and may have > to access non-local > rq etc.
No, see above. All we need is to somehow allow nohz_idle_balance() to update cpu_power as well.
So I think we want something like:
if (local_group) { balance_cpu = group_first_cpu(group); if (balance_cpu == this_cpu || nohz_balance) update_group_power(sd, this_cpu); }
Or am I totally missing something here?
| |