On Tue, May 30, 2017 at 06:24:27PM +0200, Vincent Guittot wrote:
> On 30 May 2017 at 17:50, Morten Rasmussen <morten.rasmus...@arm.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.
