Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 13 Oct 2022 21:50:10 +0800
From:   Chen Yu <yu.c.chen@intel.com>
To:     "Rafael J. Wysocki" <rafael@kernel.org>
CC:     "Rafael J. Wysocki" <rjw@rjwysocki.net>,
        Linux PM <linux-pm@vger.kernel.org>,
        Bjorn Helgaas <helgaas@kernel.org>,
        "Srinivas Pandruvada" <srinivas.pandruvada@linux.intel.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Jacob Pan <jacob.jun.pan@linux.intel.com>,
        Pavel Machek <pavel@ucw.cz>
Subject: Re: [PATCH] thermal: intel_powerclamp: Use first online CPU as
 control_cpu
Message-ID: <Y0gXkhZ8gA+8vhWO@chenyu5-mobl1>
References: <5633735.DvuYhMxLoT@kreacher>
 <Y0gOI9hiP9NXsuJP@chenyu5-mobl1>
 <CAJZ5v0jyYB7PVQm-q3UQAaCu15LbMpfZ+8bCckZWS93w3y2XRA@mail.gmail.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <CAJZ5v0jyYB7PVQm-q3UQAaCu15LbMpfZ+8bCckZWS93w3y2XRA@mail.gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?zm/hLnasQZhSBK6cclmEqfOumomUtthOL9b4de/0rXqAqozF1gxa9dDvU0iq?=
 =?us-ascii?Q?llbD9mNaJsvNh8Fq+VA64Np0WL2CdUYvRtNtEvXs98cQZSeML+/fFe0IHh1q?=
 =?us-ascii?Q?6lxyBKJ1UQlPG7GsY3DaUkiBwUma05xdyR/iZDNHkAkD5vAzkZKcX2FLNmxr?=
 =?us-ascii?Q?oe0IEsUqLw2WEJZWB8WmnUc9M60uTrAgRSj01nVQV/ipxs36EKNEILAl+LMb?=
 =?us-ascii?Q?9hzOtUIkNXuaaZ995c5WMBt+PpOJzYawu27NiEzy+psGaFcfoXf5Z6BM5uRT?=
 =?us-ascii?Q?2EfMIv0j3lLlz1IvSx+I/JuulwMasYYXHU79NSvGZ8PytO8rHmVo4rfUwJhO?=
 =?us-ascii?Q?tkFHUWXXAchOLdRXPVMWsU/1rAp+pZ8Q5aqjE4JdLCXZjsvp72Ye+MxIxwCf?=
 =?us-ascii?Q?nY6h8T2E7kJRdeP/YN+ES/LElHHmovpvE54lUswESOHVvc2K0DMxvciBzM+Q?=
 =?us-ascii?Q?KDlBSLhHyyS6/0Fxaa0TvepcUS2pr45H4ddsw/8ZIAZgAYlUiL9mJNMpRt95?=
 =?us-ascii?Q?p2be1cZbdRJLHoKCzNED/we4C7/6aIOhQ6DJ/16/5b00PsZ/KDHUbhTAClil?=
 =?us-ascii?Q?PngcHPSHenyDKQXjjz3PyNg66caD83DCXuANnSld6pGXtoYiB1l2Fu8odTNs?=
 =?us-ascii?Q?vy3rSy3Air7NASnMl7wbqXTF4PcB3lnnonesz2ujQqJO6LtB60SnQ6A+VVHu?=
 =?us-ascii?Q?Qp3MAmlrIgXqf4+Kr8qN3+IKoTO7/pRxB0+VrcEBBEMLjnnMg/wKLqkayjDo?=
 =?us-ascii?Q?F5ZUp0SSZm5Xwk5QS156JdGrbzRj3B+hAoSAdxDMWNrq/3TORowVstlX8wrY?=
 =?us-ascii?Q?Vgygndi0fUn76uzzKbMxmBaWGln1Yh3tDoV6F43aH28mCdET7wNyC7aMHC3T?=
 =?us-ascii?Q?7wlY9yVfYM1H0U9a6T4IugJS3kl77rbqS/0u8zCIytaCp0iRvRhbMNAKYOwD?=
 =?us-ascii?Q?F+hXuQQmNYQ5SOkTeKxxwm2oVv5dlIm8OGQmRaidKH01K8SR2KShLG14QOHc?=
 =?us-ascii?Q?IftjnZAbruJKK93CqWGz2ZiQKFAgbxnjBxI5FeziTeCATLAhthieemjwSXL/?=
 =?us-ascii?Q?XQIlOPjRAvsDjGGTbUmuuMlp+GDYJvfTjbm7cWiW6iOG/ZH0eDZpdePAHmHW?=
 =?us-ascii?Q?Oj/76IplhXtizF/mC4MjheAE7cGyvgc44DZT3qO1UNWv+psY0XgHdyhnuIIC?=
 =?us-ascii?Q?CZM6t+pFY/+ZGJCUwQYQr+cHv0Hx/ZPS/bEH3JAsxGJEbYK6rz+snoNrPkXl?=
 =?us-ascii?Q?9AtoQ0fmo1Yy/yv+yw4XN/llJ7yhUeQOqtmxRq/8j3wE16HTIyd89xH9HM5N?=
 =?us-ascii?Q?wHhyAiBYhjsTr695Fh56bcv+HnEKXOiO8j4idpwPXXElJNNh6gwigIFaBQTw?=
 =?us-ascii?Q?6IPeK8Id1lwlRvaqTgUyZuNEJ8Mu7ZGEpy2LaODmKUSpfhiMv6swD/P9YezC?=
 =?us-ascii?Q?FlRSYDwX8NokAPUYRMLKtOTYAoGHynknai4XZelSAPndZw9mtH1joS2Gx1ez?=
 =?us-ascii?Q?7nQEWkCf0SCdzheYZ0shKuH/YyBE0E/D3dfMEehK8pUrXsOCGHsPcqGf/Gcl?=
 =?us-ascii?Q?L7wIB4rv3qIiQDdUpj1Aiz7Ip0JS4wlNX3943AKC?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 97c6794d-d414-459d-8f5e-08daad21df15
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6206.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Oct 2022 13:50:21.6033
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: tBknffGKY3gwLAqeK4a8Xw6/IDJhWqHvA75aFuJq26oncLLQQUdYkAa7qkc5tJj4PtQsWkWd9ZulXTVH7ioX2w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5205
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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@intel.com> wrote:
> >
> > On 2022-10-13 at 14:50:28 +0200, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.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@intel.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@intel.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);
> > >
> > >
> > >