Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 24 Jan 2019 10:07:55 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Patrick Bellasi <patrick.bellasi@arm.com>
Cc:     Vincent Guittot <vincent.guittot@linaro.org>,
        Ingo Molnar <mingo@kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Morten Rasmussen <Morten.Rasmussen@arm.com>,
        Paul Turner <pjt@google.com>, Ben Segall <bsegall@google.com>,
        Thara Gopinath <thara.gopinath@linaro.org>,
        pkondeti@codeaurora.org, Quentin Perret <quentin.perret@arm.com>,
        Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Subject: Re: [PATCH v7 2/2] sched/fair: update scale invariance of PELT
Message-ID: <20190124090755.GC13536@hirez.programming.kicks-ass.net>
References: <1542711308-25256-1-git-send-email-vincent.guittot@linaro.org>
 <1542711308-25256-3-git-send-email-vincent.guittot@linaro.org>
 <CAKfTPtD=sV3zJiZMfCFi92_f6j-jTO9D5RsEBAXHVa6VN3Urwg@mail.gmail.com>
 <20181128100241.GA2131@hirez.programming.kicks-ass.net>
 <20181128115336.GB23094@e110439-lin>
 <20181129125348.GL2131@hirez.programming.kicks-ass.net>
 <20181129151316.GG23094@e110439-lin>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20181129151316.GG23094@e110439-lin>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


Sorry; trying to get back to this and re-reading the old conversations.

On Thu, Nov 29, 2018 at 03:13:16PM +0000, Patrick Bellasi wrote:
> 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>
>    https://lore.kernel.org/lkml/20181129150020.GF23094@e110439-lin/

In particular this bit:

 | Seems we agree that, when there is no idle time:
 | - the two 15% tasks will be overestimated
 | - their utilization will reach 50% after a while

Right?

> > 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

So if I'm not mistaken we then have 3 cases:

 1) runnable == util <= capacity

    no contention, idle

 2) runnable == util > capacity

    no contention, no idle

 3) runnable > util

    contention, no idle

For 1) we can use: 'util'
For 2) we can use: 'capacity'
For 3) we can use: 'util * capacity >> 10'

(note that 2 is a special case of 3 when u=1)

This should work right?

Now, instead of doing complicated things like that, you instead figure
that when there's no idle there's also no dequeue happening and we can
simply short-cut by skipping the entire thing, forgetting everything
about 2,3.

Did I get that right?