linux-kernel - Re: [RFC PATCH 1/2] sched/rt: add utilization tracking

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170531075659.GB13580@e105550-lin.cambridge.arm.com>
Date:   Wed, 31 May 2017 08:57:00 +0100
From:   Morten Rasmussen <morten.rasmussen@....com>
To:     Vincent Guittot <vincent.guittot@...aro.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        "rjw@...ysocki.net" <rjw@...ysocki.net>,
        Juri Lelli <juri.lelli@....com>,
        Dietmar Eggemann <dietmar.eggemann@....com>
Subject: Re: [RFC PATCH 1/2] sched/rt: add utilization tracking

On Tue, May 30, 2017 at 06:24:27PM +0200, Vincent Guittot wrote:
> On 30 May 2017 at 17:50, Morten Rasmussen <morten.rasmussen@....com> wrote:
> > On Wed, May 24, 2017 at 11:00:51AM +0200, Vincent Guittot wrote:
> >> @@ -1534,6 +1534,8 @@ static struct task_struct *_pick_next_task_rt(struct rq *rq)
> >>       return p;
> >>  }
> >>
> >> +extern int update_rt_rq_load_avg(u64 now, int cpu, struct rt_rq *rt_rq, int running);
> >> +
> >>  static struct task_struct *
> >>  pick_next_task_rt(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
> >>  {
> >> @@ -1579,6 +1581,10 @@ pick_next_task_rt(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
> >>
> >>       queue_push_tasks(rq);
> >>
> >> +     if (p)
> >> +             update_rt_rq_load_avg(rq_clock_task(rq), cpu_of(rq), rt_rq,
> >> +                             rq->curr->sched_class == &rt_sched_class);
> >> +
> >>       return p;
> >>  }
> >
> > I'm quite likely missing something, but why is this update need here?
> >
> > IIUC, put_prev_task() was called just a couple of lines further up which
> > also does an update (below). The only difference is that this update
> > depends on the current task being an RT task, which isn't always the
> > case. Is it supposed to decay the RT PELT average when we pick an RT
> > task and the previous task !RT? In that case, the "running" argument
> > needs a ! in front.
> 
> if prev task is not a rt task but a cfs task, the put_prev_task will
> apply on cfs class and cfs pelt but not on rt one
> so when the next task is a rt task, we have to update rt utilization
> either to decay the rt utilization if curr task (prev task) is not rt,
> or to add utilization if curr task is rt task.
> we have something similar with put_prev_task_rt

Yes. It was just me getting the logic wrong. rq->curr->sched_class ==
&rt_sched_class would of course be false if the previous task is CFS
meaning "running" is false and hence we will decay the rt rq PELT signal
as we correctly should.