Re: [PATCH 2/3] sched/fair: don't set LBF_ALL_PINNED unnecessarily

[Vincent Guittot]

On Wed, 6 Jan 2021 at 16:10, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> On Wed, Jan 06, 2021 at 02:34:18PM +0100, Vincent Guittot wrote:
> > Setting LBF_ALL_PINNED during active load balance is only valid when there
> > is only 1 running task on the rq otherwise this ends up increasing the
> > balance interval whereas other tasks could migrate after the next interval
> > once they become cache-cold as an example.
> >
> > Signed-off-by: Vincent Guittot <vincent.guittot@xxxxxxxxxx>
> > ---
> >  kernel/sched/fair.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 5428b8723e61..69a455113b10 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -9759,7 +9759,8 @@ static int load_balance(int this_cpu, struct rq *this_rq,
> >                       if (!cpumask_test_cpu(this_cpu, busiest->curr->cpus_ptr)) {
> >                               raw_spin_unlock_irqrestore(&busiest->lock,
> >                                                           flags);
> > -                             env.flags |= LBF_ALL_PINNED;
> > +                             if (busiest->nr_running == 1)
> > +                                     env.flags |= LBF_ALL_PINNED;
> >                               goto out_one_pinned;
> >                       }
>
> Hmm.. that wasn't the intention. It's entirely possible to have multiple
> tasks pinned.

But LBF_ALL_PINNED is already set in this case

>
> ISTR we set all-pinned and then clear it in can_migrate_task() when we
> actually find a task that can be moved to the destination CPU. That's a
> construct that works perfectly fine with multiple tasks.

I agree with you.

This case here is :
we have 2 tasks TA and TB on the rq.
The waiting one TB can't migrate for a reason other than the pinned case.
We decide to start the active migration on the running  TA task but TA
is pinned.
In this case we are not in the all pinned case.

We are in the all pinned case, only if the running task TA is the only
runnable task of the rq

>
>