From: Lukasz Luba <lukasz.luba@arm.com>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Amit Daniel Kachhap <amit.kachhap@gmail.com>,
Daniel Lezcano <daniel.lezcano@linaro.org>,
Viresh Kumar <viresh.kumar@linaro.org>,
"Rafael J. Wysocki" <rafael@kernel.org>,
Zhang Rui <rui.zhang@intel.com>,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] thermal/cpufreq: increment i in cpufreq_get_requested_power()
Date: Mon, 6 May 2024 10:41:52 +0100 [thread overview]
Message-ID: <67159a18-3923-4345-bff8-ade49cc769ba@arm.com> (raw)
In-Reply-To: <a7c1fe73-b40e-437c-8ccb-7b3baad04df7@moroto.mountain>
Hi Dan,
On 5/4/24 12:25, Dan Carpenter wrote:
> We accidentally deleted the "i++" as part of a cleanup. Restore it.
>
> Fixes: 3f7ced7ac9af ("drivers/thermal/cpufreq_cooling : Refactor thermal_power_cpu_get_power tracing")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> This is based on static analysis and not tested.
Thank you for the patch. I have analyzed the code and why it
haven't trigger an issue when I was testing it.
I looks like the function get_load() which is called above that 'i++'
and takes the 'i' as the last argument is compiled in 2 versions:
1. for SMP system and the last argument 'cpu_idx' is ignored
2. for !SMP where we use the last argument 'cpu_idx' which is 'i'
value. Although, for !SMP system we only have 1 cpu, thus the
initialized 'int i = 0' at the beginning of that
cpufreq_get_requested_power() is used correctly.
The loop for !SMP goes only once.
>
> drivers/thermal/cpufreq_cooling.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c
> index 280071be30b1..a074192896de 100644
> --- a/drivers/thermal/cpufreq_cooling.c
> +++ b/drivers/thermal/cpufreq_cooling.c
> @@ -249,6 +249,7 @@ static int cpufreq_get_requested_power(struct thermal_cooling_device *cdev,
> load = 0;
>
> total_load += load;
> + i++;
> }
>
> cpufreq_cdev->last_load = total_load;
Would you agree that I will keep you as 'Reported-by' and send a
separate patch to change that !SMP code completely in that
get_load() function and get rid of the 'cpu_idx' argument?
Or I'm happy that you can develop such code and I can review it.
It's up to you.
Regards,
Lukasz
next prev parent reply other threads:[~2024-05-06 9:41 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-04 11:25 [PATCH] thermal/cpufreq: increment i in cpufreq_get_requested_power() Dan Carpenter
2024-05-06 9:41 ` Lukasz Luba [this message]
2024-05-06 11:26 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=67159a18-3923-4345-bff8-ade49cc769ba@arm.com \
--to=lukasz.luba@arm.com \
--cc=amit.kachhap@gmail.com \
--cc=dan.carpenter@linaro.org \
--cc=daniel.lezcano@linaro.org \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rafael@kernel.org \
--cc=rui.zhang@intel.com \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).