Re: [PATCH] sched/eas: Don't update misfit status if the task is pinned

From: Valentin Schneider <valentin.schneider@arm.com>
To: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Qais Yousef <qais.yousef@arm.com>,
	"Peter Zijlstra \(Intel\)" <peterz@infradead.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Morten Rasmussen <morten.rasmussen@arm.com>
Subject: Re: [PATCH] sched/eas: Don't update misfit status if the task is pinned
Date: Tue, 19 Jan 2021 16:55:43 +0000	[thread overview]
Message-ID: <jhjim7s50j4.mognet@arm.com> (raw)
In-Reply-To: <CAKfTPtCRPViF7B1iqeNVDi31X2sgOWKDJSGCBSzbj0c_Zb=mzA@mail.gmail.com>

On 19/01/21 15:19, Vincent Guittot wrote:
> On Tue, 19 Jan 2021 at 14:54, Valentin Schneider
> <valentin.schneider@arm.com> wrote:
>> On 19/01/21 14:34, Vincent Guittot wrote:
>> >> -       if (!p) {
>> >> +       if (!p || p->nr_cpus_allowed == 1) {
>> >
>> > Side question: What happens if there is 2 misfit tasks and the current
>> > one is pinned but not the other waiting one
>> >
>>
>> update_misfit_status() is called either on the current task (at tick) or
>> on the task picked by pick_next_task_fair() - i.e. CFS current or
>> about-to-be-current.
>>
>> So if you have 2 CPU hogs enqueued on a single LITTLE, and one of them
>> is pinned, the other one will be moved away either via regular load
>
> This doesn't seem reliable because it uses load or nr_running
>

Right

>> balance, or via misfit balance sometime after it's picked as the next
>> task to run.
>>
>> Admittedly that second case suffers from unfortunate timing mostly
>> related to the load balance interval. There was an old patch in the
>> Android stack that would reduce the balance interval upon detecting a
>
> Shouldn't we keep track of enqueue misfit tasks instead ?
>

That might help. This being CFS however, the maintenance of it might
prove prohibitive. I faintly recall having concerns about expanding the
misfit logic to non-current tasks, but nothing comes to mind right
now...

Historically (before PELT time scaling) I think it wasn't possible to
have a steady state with more than one misfit task on a rq, as two
similar CPU hogs would end up with a utilization value of at most half
the CPU's capacity. If those were at e.g. opposite ends of the NICE
spectrum, if one would be flagged as misfit then the other wouldn't
(can't have two slices greater than 80%!)

I *think* that's still true with timescaling, but then we did add the
uclamp stuff to make tasks look bigger than they are...

>> misfit task to "accelerate" its upmigration; this might need to be
>> revisited...
>>
>> >>                 rq->misfit_task_load = 0;
>> >>                 return;
>> >>         }
>> >> --
>> >> 2.25.1
>> >>