[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y0gOI9hiP9NXsuJP@chenyu5-mobl1>
Date: Thu, 13 Oct 2022 21:09:55 +0800
From: Chen Yu <yu.c.chen@...el.com>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>
CC: Linux PM <linux-pm@...r.kernel.org>,
Bjorn Helgaas <helgaas@...nel.org>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
LKML <linux-kernel@...r.kernel.org>,
Jacob Pan <jacob.jun.pan@...ux.intel.com>,
Pavel Machek <pavel@....cz>
Subject: Re: [PATCH] thermal: intel_powerclamp: Use first online CPU as
control_cpu
On 2022-10-13 at 14:50:28 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> Commit 68b99e94a4a2 ("thermal: intel_powerclamp: Use get_cpu() instead
> of smp_processor_id() to avoid crash") fixed an issue related to using
> smp_processor_id() in preemptible context by replacing it with a pair
> of get_cpu()/put_cpu(), but what is needed there really is any online
> CPU and not necessarily the one currently running the code. Arguably,
> getting the one that's running the code in there is confusing.
>
> For this reason, simply give the control CPU role to the first online
> one which automatically will be CPU0 if it is online, so one check
> can be dropped from the code for an added benefit.
>
> Link: https://lore.kernel.org/linux-pm/20221011113646.GA12080@duo.ucw.cz/
> Fixes: 68b99e94a4a2 ("thermal: intel_powerclamp: Use get_cpu() instead of smp_processor_id() to avoid crash")
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
> drivers/thermal/intel/intel_powerclamp.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> Index: linux-pm/drivers/thermal/intel/intel_powerclamp.c
> ===================================================================
> --- linux-pm.orig/drivers/thermal/intel/intel_powerclamp.c
> +++ linux-pm/drivers/thermal/intel/intel_powerclamp.c
> @@ -516,11 +516,7 @@ static int start_power_clamp(void)
> cpus_read_lock();
>
> /* prefer BSP */
Above comment line is not true any more, might delete it as well?
Reviewed-by: Chen Yu <yu.c.chen@...el.com>
thanks,
Chenyu
> - control_cpu = 0;
> - if (!cpu_online(control_cpu)) {
> - control_cpu = get_cpu();
> - put_cpu();
> - }
> + control_cpu = cpumask_first(cpu_online_mask);
>
> clamping = true;
> schedule_delayed_work(&poll_pkg_cstate_work, 0);
>
>
>
Powered by blists - more mailing lists