Re: [PATCH v7 2/2] sched/fair: update scale invariance of PELT

[Patrick Bellasi]

On 29-Nov 13:53, Peter Zijlstra wrote:
> On Wed, Nov 28, 2018 at 11:53:36AM +0000, Patrick Bellasi wrote:
> 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index ac855b2f4774..93e0cf5d8a76 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -3661,6 +3661,10 @@ util_est_dequeue(struct cfs_rq *cfs_rq, struct task_struct *p, bool task_sleep)
> >  	if (!task_sleep)
> >  		return;
> > 
> > +	/* Skip samples which do not represent an actual utilization */
> > +	if (unlikely(task_util(p) > capacity_of(task_cpu(p))))
> > +		return;
> > +
> >  	/*
> >  	 * If the PELT values haven't changed since enqueue time,
> >  	 * skip the util_est update.
> 
> Would you not want something like:
> 
> 	min(task_util(p), capacity_of(task_cpu(p)))
> 
> And is this the only place where we need this?

Mmm... even this could be an over-estimation:

I've just posted an example in my last reply to Vincent, end of:

   Message-ID: <20181129150020.GF23094@e110439-lin>
   20181129150020.GF23094@e110439-lin/">https://lore.kernel.org/lkml/20181129150020.GF23094@e110439-lin/

> OTOH, if the task is always running, it will be always running
> irrespective of where it runs.

That's not what I'm concerned about. I'm concerned about small tasks
which are running on limited capacity (e.g. due to thermal capping)
without idle time. In this case, the new "utilization" signal could
overestimate the real task needs.

> Not storing these samples seems weird though; this is the exact
> condition you want to record -- the task is very active, if we skip
> these, we'll come back at a low frequency on the next wakeup.

When there is not idle time, we don't know if the reported
utilization, above the cpu capacity, is due to the task being bigger...
or just the new utilization signal converging towards:

    100% / RUNNABLE_TASKS_COUNT

-- 
#include <best/regards.h>

Patrick Bellasi