Messages in this thread | | | From | Vincent Guittot <> | Date | Tue, 19 Jan 2021 15:19:30 +0100 | Subject | Re: [PATCH] sched/eas: Don't update misfit status if the task is pinned |
| |
On Tue, 19 Jan 2021 at 14:54, Valentin Schneider <valentin.schneider@arm.com> wrote: > > On 19/01/21 14:34, Vincent Guittot wrote: > > On Tue, 19 Jan 2021 at 13:08, Qais Yousef <qais.yousef@arm.com> wrote: > >> > >> If the task is pinned to a cpu, setting the misfit status means that > >> we'll unnecessarily continuously attempt to migrate the task but fail. > >> > >> This continuous failure will cause the balance_interval to increase to > >> a high value, and eventually cause unnecessary significant delays in > >> balancing the system when real imbalance happens. > >> > >> Caught while testing uclamp where rt-app calibration loop was pinned to > >> cpu 0, shortly after which we spawn another task with high util_clamp > >> value. The task was failing to migrate after over 40ms of runtime due to > >> balance_interval unnecessary expanded to a very high value from the > >> calibration loop. > >> > >> Not done here, but it could be useful to extend the check for pinning to > >> verify that the affinity of the task has a cpu that fits. We could end > >> up in a similar situation otherwise. > >> > >> Fixes: 3b1baa6496e6 ("sched/fair: Add 'group_misfit_task' load-balance type") > >> Signed-off-by: Qais Yousef <qais.yousef@arm.com> > >> --- > >> kernel/sched/fair.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > >> index 197a51473e0c..9379a481dd8c 100644 > >> --- a/kernel/sched/fair.c > >> +++ b/kernel/sched/fair.c > >> @@ -4060,7 +4060,7 @@ static inline void update_misfit_status(struct task_struct *p, struct rq *rq) > >> if (!static_branch_unlikely(&sched_asym_cpucapacity)) > >> return; > >> > >> - 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
> 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 ?
> misfit task to "accelerate" its upmigration; this might need to be > revisited... > > >> rq->misfit_task_load = 0; > >> return; > >> } > >> -- > >> 2.25.1 > >>
| |