linux-kernel - Re: [PATCH v6 02/23] PM: EM: Refactor em_cpufreq_update

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: <CAJZ5v0jCtS1XHczORnZFhwO7N=FommFNbQC7NdaM5SHKXUnTog@mail.gmail.com>
Date: Thu, 4 Jan 2024 20:07:59 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Lukasz Luba <lukasz.luba@....com>
Cc: linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org, rafael@...nel.org, 
	dietmar.eggemann@....com, rui.zhang@...el.com, amit.kucheria@...durent.com, 
	amit.kachhap@...il.com, daniel.lezcano@...aro.org, viresh.kumar@...aro.org, 
	len.brown@...el.com, pavel@....cz, mhiramat@...nel.org, qyousef@...alina.io, 
	wvw@...gle.com
Subject: Re: [PATCH v6 02/23] PM: EM: Refactor em_cpufreq_update_efficiencies()
 arguments

The word "refactor" appears to be quite loaded in your patch
descriptions, but it is not always the best one to use IMV.

For instance, this patch simply extends the argument list of
em_cpufreq_update_efficiencies(), so I would say just that in the
subject: "Extend em_cpufreq_update_efficiencies() argument list"

On Thu, Jan 4, 2024 at 6:14 PM Lukasz Luba <lukasz.luba@....com> wrote:
>
> In order to prepare the code for the modifiable EM perf_state table,
> refactor existing function em_cpufreq_update_efficiencies().

"make em_cpufreq_update_efficiencies() take a pointer to the EM table
as its second argument and modify it to use that new argument instead
of the "table" member of dev->em_pd"

or something like this.

> The function now takes the ptr to the EM table as its argument.
>
> No functional impact.
>
> Signed-off-by: Lukasz Luba <lukasz.luba@....com>
> ---
>  kernel/power/energy_model.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
> index 8b9dd4a39f63..42486674b834 100644
> --- a/kernel/power/energy_model.c
> +++ b/kernel/power/energy_model.c
> @@ -237,10 +237,10 @@ static int em_create_pd(struct device *dev, int nr_states,
>         return 0;
>  }
>
> -static void em_cpufreq_update_efficiencies(struct device *dev)
> +static void
> +em_cpufreq_update_efficiencies(struct device *dev, struct em_perf_state *table)
>  {
>         struct em_perf_domain *pd = dev->em_pd;
> -       struct em_perf_state *table;
>         struct cpufreq_policy *policy;
>         int found = 0;
>         int i;
> @@ -254,8 +254,6 @@ static void em_cpufreq_update_efficiencies(struct device *dev)
>                 return;
>         }
>
> -       table = pd->table;
> -
>         for (i = 0; i < pd->nr_perf_states; i++) {
>                 if (!(table[i].flags & EM_PERF_STATE_INEFFICIENT))
>                         continue;
> @@ -397,7 +395,7 @@ int em_dev_register_perf_domain(struct device *dev, unsigned int nr_states,
>
>         dev->em_pd->flags |= flags;
>
> -       em_cpufreq_update_efficiencies(dev);
> +       em_cpufreq_update_efficiencies(dev, dev->em_pd->table);
>
>         em_debug_create_pd(dev);
>         dev_info(dev, "EM: created perf domain\n");
> --

The code change itself LGTM.