linux-kernel - Re: [PATCH] thermal: intel_powerclamp: Use first online CPU as control

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] [day] [month] [year] [list]

Message-ID: <Y0gXkhZ8gA+8vhWO@chenyu5-mobl1>
Date:   Thu, 13 Oct 2022 21:50:10 +0800
From:   Chen Yu <yu.c.chen@...el.com>
To:     "Rafael J. Wysocki" <rafael@...nel.org>
CC:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        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 15:27:30 +0200, Rafael J. Wysocki wrote:
> On Thu, Oct 13, 2022 at 3:10 PM Chen Yu <yu.c.chen@...el.com> wrote:
> >
> > 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?
> 
> Well, why not?  If CPU0 is the BSP, it is still preferred as before.
>
I see. Got it.

thanks,
Chenyu 
> > Reviewed-by: Chen Yu <yu.c.chen@...el.com>
> 
> Thanks!
> 
> > > -     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);
> > >
> > >
> > >