On Mon, Aug 15, 2016 at 03:39:49PM +0200, Peter Zijlstra wrote:
> On Mon, Jul 25, 2016 at 02:34:26PM +0100, Morten Rasmussen wrote:
>
>
> Because I forgot _again_, I added:
>
> /*
> * Disable WAKE_AFFINE in the case where task @p doesn't fit in the
> * capacity of either the waking CPU @cpu or the previous CPU @prev_cpu.
> *
> * In that case WAKE_AFFINE doesn't make sense and we'll let
> * BALANCE_WAKE sort things out.
> */
Thanks.
>
> > +static int wake_cap(struct task_struct *p, int cpu, int prev_cpu)
> > +{
> > + long min_cap, max_cap;
> > +
> > + min_cap = min(capacity_orig_of(prev_cpu), capacity_orig_of(cpu));
> > + max_cap = cpu_rq(cpu)->rd->max_cpu_capacity;
>
> There's a tiny hole here, which I'm fairly sure we don't care about. If
> @p last ran on @prev_cpu before @prev_cpu was split from @rd this
> doesn't 'work' right.
I hadn't considered that. What is 'working right' in this scenario?
Ignoring @prev_cpu as it isn't a valid option anymore?
In that case, since @prev_cpu is only used as part the min() it should
only cause min_cap to be potentially smaller than it should be, not
larger. It could lead us to let BALANCE_WAKE take over in scenarios
where select_idle_sibling() would have been sufficient, but it should
harm.
However, as you say, I'm not sure if we care that much.
Talking about @rd, I discussed with Juri and Dietmar the other week
whether the root_domain is RCU protected, and if we therefore have to
move the call to wake_cap() after the rcu_read_lock() below. I haven't
yet done thorough investigation to find the answer. Should it be
protected?
>
> > + /* Minimum capacity is close to max, no need to abort wake_affine */
> > + if (max_cap - min_cap < max_cap >> 3)
> > + return 0;
> > +
> > + return min_cap * 1024 < task_util(p) * capacity_margin;
> > +}
> > +
> > /*
> > * select_task_rq_fair: Select target runqueue for the waking task in
> > domains
> > * that have the 'sd_flag' flag set. In practice, this is SD_BALANCE_WAKE,
> > @@ -5389,7 +5414,8 @@ select_task_rq_fair(struct task_struct *p, int
> > prev_cpu, int sd_flag, int wake_f
> >
> > if (sd_flag & SD_BALANCE_WAKE) {
> > record_wakee(p);
> > - want_affine = !wake_wide(p) && cpumask_test_cpu(cpu,
> > tsk_cpus_allowed(p));
> > + want_affine = !wake_wide(p) && !wake_cap(p, cpu, prev_cpu)
> > + && cpumask_test_cpu(cpu, tsk_cpus_allowed(p));
> > }
> >
> > rcu_read_lock();
> > --
> > 1.9.1
> >
