linux-kernel - Re: [PATCH v2 2/2] cpufreq: schedutil: consolidate capacity margin calculation

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]

Date:   Sun, 1 Oct 2017 18:02:24 -0700
From:   Joel Fernandes <joelaf@...gle.com>
To:     Leo Yan <leo.yan@...aro.org>
Cc:     Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Morten Rasmussen <morten.rasmussen@....com>,
        Chris Redpath <Chris.Redpath@....com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Patrick Bellasi <patrick.bellasi@....com>,
        "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>
Subject: Re: [PATCH v2 2/2] cpufreq: schedutil: consolidate capacity margin calculation

Hi Leo,

On Sun, Oct 1, 2017 at 5:30 PM, Leo Yan <leo.yan@...aro.org> wrote:
> Scheduler CFS class has variable 'capacity_margin' to calculate the

s/calculate/represent/ ?

> capacity margin, and schedutil governor also needs to compensate the
> same margin for frequency tipping point. Below are formulas used in
> CFS class and schedutil governor separately:
>
> CFS:       U` = U * capacity_margin / 1024 = U * 1.25

You should mention in the commit message, at the moment
capacity_margin is 1280 which makes U` = 1.25.

> Schedutil: U` = U + U >> 2 = U + U * 0.25  = U * 1.25
>
> This patch consolidates the capacity margin calculation so let
> schedutil to use same formula with CFS class. As result this can avoid

As a result.

> the mismatch issue between schedutil and CFS class after change
> 'capacity_margin' to other values.

This didn't make sense to me. May be you meant:

This patch consolidates the usage of the capacity margin value and
lets schedutil use the same formula as the CFS class. Thus we can
avoid the mismatch between schedutil and CFS class if
'capacity_margin' is changed to other values in the future.

> Cc: Dietmar Eggemann <dietmar.eggemann@....com>
> Cc: Morten Rasmussen <morten.rasmussen@....com>
> Cc: Chris Redpath <Chris.Redpath@....com>
> Cc: Joel Fernandes <joelaf@...gle.com>
> Cc: Vincent Guittot <vincent.guittot@...aro.org>
> Cc: Patrick Bellasi <patrick.bellasi@....com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> Signed-off-by: Leo Yan <leo.yan@...aro.org>
> ---
>  kernel/sched/cpufreq_schedutil.c | 6 ++++--
>  kernel/sched/sched.h             | 1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> index 9209d83..13cc243 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -155,7 +155,8 @@ static void sugov_update_commit(struct sugov_policy *sg_policy, u64 time,
>   *
>   * next_freq = C * curr_freq * util_raw / max
>   *
> - * Take C = 1.25 for the frequency tipping point at (util / max) = 0.8.
> + * Take C = capacity_margin / 1024 = 1.25, so it's for the frequency tipping
> + * point at (util / max) = 0.8.

The above comment assumes capacity_margin is 1280. If for any reason
the capacity_margin is changed to something else, then the comment
wont make sense anymore.

thanks,

- Joel