Commit Message
Greg KH
Oct. 24, 2022, 11:31 a.m. UTC
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> commit 4bb7f6c2781e46fc5bd00475a66df2ea30ef330d upstream. 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@intel.com> Reviewed-by: Chen Yu <yu.c.chen@intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- drivers/thermal/intel_powerclamp.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
Comments
Hi! > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > commit 4bb7f6c2781e46fc5bd00475a66df2ea30ef330d upstream. > > 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. While this is nice cleanup (and I complained about original code being interesting) original code still looks okay and we don't really need this in stable. Thanks and best regards, Pavel > +++ b/drivers/thermal/intel_powerclamp.c > @@ -518,11 +518,7 @@ static int start_power_clamp(void) > get_online_cpus(); > > /* prefer BSP */ > - 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); >
--- a/drivers/thermal/intel_powerclamp.c +++ b/drivers/thermal/intel_powerclamp.c @@ -518,11 +518,7 @@ static int start_power_clamp(void) get_online_cpus(); /* prefer BSP */ - 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);