linux-kernel - Re: [PATCH] sched/topology: Remove EM_MAX

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] [day] [month] [year] [list]

Message-ID: <62a7cafc-13d6-5341-0128-420db7d5da8c@arm.com>
Date:   Wed, 26 Oct 2022 13:23:03 +0100
From:   Lukasz Luba <lukasz.luba@....com>
To:     Pierre Gondois <pierre.gondois@....com>
Cc:     Ionela.Voinescu@....com, Jonathan Corbet <corbet@....net>,
        Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Juri Lelli <juri.lelli@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>,
        Daniel Bristot de Oliveira <bristot@...hat.com>,
        Valentin Schneider <vschneid@...hat.com>,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sched/topology: Remove EM_MAX_COMPLEXITY limit

Hi Pierre,

On 8/12/22 11:16, Pierre Gondois wrote:
> From: Pierre Gondois <Pierre.Gondois@....com>
> 
> The Energy Aware Scheduler (EAS) estimates the energy consumption
> of placing a task on different CPUs. The goal is to minimize this
> energy consumption. Estimating the energy of different task placements
> is increasingly complex with the size of the platform. To avoid having
> a slow wake-up path, EAS is only enabled if this complexity is low
> enough.
> 
> The current complexity limit was set in:
> commit b68a4c0dba3b1 ("sched/topology: Disable EAS on inappropriate
> platforms").
> base on the first implementation of EAS, which was re-computing
> the power of the whole platform for each task placement scenario, cf:
> commit 390031e4c309 ("sched/fair: Introduce an energy estimation helper
> function").
> but the complexity of EAS was reduced in:
> commit eb92692b2544d ("sched/fair: Speed-up energy-aware wake-ups")
> and find_energy_efficient_cpu() (feec) algorithm was updated in:
> commit 3e8c6c9aac42 ("sched/fair: Remove task_util from effective
> utilization in feec()")
> 
> find_energy_efficient_cpu() (feec) is now doing:
> feec()
> \_ for_each_pd(pd) [0]
>    // get max_spare_cap_cpu and compute_prev_delta
>    \_ for_each_cpu(pd) [1]
> 
>    \_ get_pd_busy_time(pd) [2]
>      \_ for_each_cpu(pd)
> 
>    // evaluate pd energy without the task
>    \_ get_pd_max_util(pd, -1) [3.0]
>      \_ for_each_cpu(pd)
>    \_ compute_energy(pd, -1)
>      \_ for_each_ps(pd)
> 
>    // evaluate pd energy with the task on prev_cpu
>    \_ get_pd_max_util(pd, prev_cpu) [3.1]
>      \_ for_each_cpu(pd)
>    \_ compute_energy(pd, prev_cpu)
>      \_ for_each_ps(pd)
> 
>    // evaluate pd energy with the task on max_spare_cap_cpu
>    \_ get_pd_max_util(pd, max_spare_cap_cpu) [3.2]
>      \_ for_each_cpu(pd)
>    \_ compute_energy(pd, max_spare_cap_cpu)
>      \_ for_each_ps(pd)
> 
> [3.1] happens only once since prev_cpu is unique. To have an upper
> bound of the complexity, [3.1] is taken into account for all pds.
> So with the same definitions for nr_pd, nr_cpus and nr_ps,
> the complexity is of:
> nr_pd * (2 * [nr_cpus in pd] + 3 * ([nr_cpus in pd] + [nr_ps in pd]))
>   [0]  * (     [1] + [2]      +       [3.0] + [3.1] + [3.2]          )
> = 5 * nr_cpus + 3 * nr_ps
> 
> The complexity limit was set to 2048 in:
> commit b68a4c0dba3b1 ("sched/topology: Disable EAS on inappropriate
> platforms")
> to make "EAS usable up to 16 CPUs with per-CPU DVFS and less than 8
> performance states each". For the same platform, the complexity would
> actually be of:
> 5 * 16 + 3 * 7 = 101
> 
> Since the EAS complexity was greatly reduced, bigger platforms can
> handle EAS. For instance, a platform with 256 CPUs with 256
> performance states each would reach it. To reflect this improvement,
> remove the EAS complexity check.
> 
> Signed-off-by: Pierre Gondois <Pierre.Gondois@....com>
> ---
>   Documentation/scheduler/sched-energy.rst | 37 ++--------------------
>   kernel/sched/topology.c                  | 39 ++----------------------
>   2 files changed, 6 insertions(+), 70 deletions(-)
> 

The patch looks good for both: documentation bit and code removal.

We have a new safety checks inside the Energy Model during the setup
of EM for perf domian, even a more strict and precised (32bit arch or
64bit arch) to no overflow in our calculations (when we estimate
energy). This is documented in the Energy Model, so IMO you can easily
drop this paragraph as the patch does. The same applies to the checks
in the code.

Reviewed-by: Lukasz Luba <lukasz.luba@....com>