lkml.org 
[lkml]   [2021]   [Jan]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [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
    > >>

    \
     
     \ /
      Last update: 2021-01-20 00:00    [W:2.698 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site