linux-kernel - Re: [PATCH 2/5] energy model: Add a get previous state function

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: <e2b7e637-3977-4c06-9a95-88fc53bf4d82@arm.com>
Date: Thu, 5 Sep 2024 10:21:38 +0100
From: Lukasz Luba <lukasz.luba@....com>
To: Vincent Guittot <vincent.guittot@...aro.org>
Cc: qyousef@...alina.io, hongyan.xia2@....com, mingo@...hat.com,
 mgorman@...e.de, peterz@...radead.org, dietmar.eggemann@....com,
 bsegall@...gle.com, vschneid@...hat.com, rostedt@...dmis.org,
 rafael.j.wysocki@...el.com, linux-kernel@...r.kernel.org,
 juri.lelli@...hat.com
Subject: Re: [PATCH 2/5] energy model: Add a get previous state function

Hi Vincent,

On 8/30/24 14:03, Vincent Guittot wrote:
> Instead of parsing all EM table everytime, add a function to get the
> previous state.
> 
> Will be used in the scheduler feec() function.
> 
> Signed-off-by: Vincent Guittot <vincent.guittot@...aro.org>
> ---
>   include/linux/energy_model.h | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
> index 1ff52020cf75..ea8ea7e031c0 100644
> --- a/include/linux/energy_model.h
> +++ b/include/linux/energy_model.h
> @@ -207,6 +207,24 @@ em_pd_get_efficient_state(struct em_perf_state *table, int nr_perf_states,
>   	return nr_perf_states - 1;
>   }
>   
> +static inline int
> +em_pd_get_previous_state(struct em_perf_state *table, int nr_perf_states,
> +			  int idx, unsigned long pd_flags)
> +{
> +	struct em_perf_state *ps;
> +	int i;
> +
> +	for (i = idx - 1; i >= 0; i--) {
> +		ps = &table[i];
> +		if (pd_flags & EM_PERF_DOMAIN_SKIP_INEFFICIENCIES &&
> +		    ps->flags & EM_PERF_STATE_INEFFICIENT)
> +			continue;
> +		return i;
> +	}

Would you mind to add a comment on top of that for loop?
Or maybe a bit more detail in the patch header what would you like to
find (e.g. 1st efficient OPP which is lower).

It's looking for a first OPP (don't forget it's ascending 'table') which
is lower or equal to the 'idx' state.

If uclamp_max is set and that OPP is inefficient, don't we choose
a higher OPP which is efficient?

I'm not against this function.

BTW, I wonder if this design is still valid with the uclamp_max.

Regards,
Lukasz