linux-kernel - Re: [PATCH v2 3/3] PM / EM: Skip inefficient OPPs

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]

Message-ID: <b2a24d60-966e-7ee6-b3b8-9c1920881d76@arm.com>
Date:   Mon, 24 May 2021 13:55:44 +0100
From:   Lukasz Luba <lukasz.luba@....com>
To:     Vincent Donnefort <vincent.donnefort@....com>
Cc:     peterz@...radead.org, rjw@...ysocki.net, viresh.kumar@...aro.org,
        vincent.guittot@...aro.org, qperret@...gle.com,
        linux-kernel@...r.kernel.org, ionela.voinescu@....com,
        dietmar.eggemann@....com
Subject: Re: [PATCH v2 3/3] PM / EM: Skip inefficient OPPs



On 5/21/21 5:54 PM, Vincent Donnefort wrote:
> Some SoCs, such as the sd855 have OPPs within the same performance domain,
> whose cost is higher than others with a higher frequency. Even though
> those OPPs are interesting from a cooling perspective, it makes no sense
> to use them when the device can run at full capacity. Those OPPs handicap
> the performance domain, when choosing the most energy-efficient CPU and
> are wasting energy. They are inefficient.
> 
> Hence, add support for such OPPs to the Energy Model. The table can now
> be read skipping inefficient performance states (and by extension,
> inefficient OPPs).
> 
> Currently, the efficient table is used in two paths. Schedutil, and
> find_energy_efficient_cpu(). We have to modify both paths in the same
> patch so they stay synchronized. The thermal framework still relies on
> the full table.
> 
> Signed-off-by: Vincent Donnefort <vincent.donnefort@....com>
> 
> diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
> index 9be7bde..daaeccf 100644
> --- a/include/linux/energy_model.h
> +++ b/include/linux/energy_model.h
> @@ -17,13 +17,25 @@
>    *		device). It can be a total power: static and dynamic.
>    * @cost:	The cost coefficient associated with this level, used during
>    *		energy calculation. Equal to: power * max_frequency / frequency
> + * @flags:	see "em_perf_state flags" description below.
>    */
>   struct em_perf_state {
>   	unsigned long frequency;
>   	unsigned long power;
>   	unsigned long cost;
> +	unsigned long flags;

Maybe for now, we can have 'bool' here?
We would avoid *Num_opps of 'and' operations below

[snip]

> +static inline
> +struct em_perf_state *em_pd_get_efficient_state(struct em_perf_domain *pd,
> +						unsigned long freq)
> +{
> +	struct em_perf_state *ps;
> +	int i;
> +
> +	for (i = 0; i < pd->nr_perf_states; i++) {
> +		ps = &pd->table[i];
> +		if (ps->flags & EM_PERF_STATE_INEFFICIENT)

Here, we can avoid this *N of '&', when having a simple bool

> +			continue;
> +		if (ps->frequency >= freq)
> +			break;
> +	}
> +
> +	return ps;
> +}
> +

[snip

> +static inline unsigned long em_pd_get_efficient_freq(struct em_perf_domain *pd,
> +						     unsigned long freq)
> +{
> +	struct em_perf_state *ps;
> +
> +	if (!pd || !(pd->flags & EM_PERF_DOMAIN_INEFFICIENCIES))

This one is OK, since we have two features for this 'flags' now.

The rest looks good.

Regards,
Lukasz