linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] cpufreq: Avoid get_governor() for first policy
@ 2025-07-25  4:14 Zihuan Zhang
  2025-07-25  5:49 ` Viresh Kumar
  2025-07-25  9:11 ` zhenglifeng (A)
  0 siblings, 2 replies; 4+ messages in thread
From: Zihuan Zhang @ 2025-07-25  4:14 UTC (permalink / raw)
  To: rafael J . wysocki, Viresh Kumar
  Cc: zhenglifeng, linux-pm, linux-kernel, Zihuan Zhang

When a cpufreq driver registers the first policy, it may attempt to
initialize the policy governor from `last_governor`. However, this is
meaningless for the first policy instance, because `last_governor` is
only updated when policies are removed (e.g. during CPU offline).

The `last_governor` mechanism is intended to restore the previously
used governor across CPU hotplug events. For the very first policy,
there is no "previous governor" to restore, so calling
get_governor(last_governor) is unnecessary and potentially confusing.

This patch skips looking up `last_governor` when registering the first
policy. Instead, it directly uses the default governor after all
governors have been registered and are available.

This avoids meaningless lookups, reduces unnecessary module reference
handling, and simplifies the initial policy path.

Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>

---
Changes in v2:
 - Fix the case where the governor is NULL.
---
---
 drivers/cpufreq/cpufreq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index d7426e1d8bdd..1aa559f53479 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1122,7 +1122,8 @@ static int cpufreq_init_policy(struct cpufreq_policy *policy)
 
 	if (has_target()) {
 		/* Update policy governor to the one used before hotplug. */
-		gov = get_governor(policy->last_governor);
+		if (policy->last_governor[0] != '\0')
+			gov = get_governor(policy->last_governor);
 		if (gov) {
 			pr_debug("Restoring governor %s for cpu %d\n",
 				 gov->name, policy->cpu);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] cpufreq: Avoid get_governor() for first policy
  2025-07-25  4:14 [PATCH v2] cpufreq: Avoid get_governor() for first policy Zihuan Zhang
@ 2025-07-25  5:49 ` Viresh Kumar
  2025-07-25  9:11 ` zhenglifeng (A)
  1 sibling, 0 replies; 4+ messages in thread
From: Viresh Kumar @ 2025-07-25  5:49 UTC (permalink / raw)
  To: Zihuan Zhang; +Cc: rafael J . wysocki, zhenglifeng, linux-pm, linux-kernel

On 25-07-25, 12:14, Zihuan Zhang wrote:
> When a cpufreq driver registers the first policy, it may attempt to
> initialize the policy governor from `last_governor`. However, this is
> meaningless for the first policy instance, because `last_governor` is
> only updated when policies are removed (e.g. during CPU offline).
> 
> The `last_governor` mechanism is intended to restore the previously
> used governor across CPU hotplug events. For the very first policy,
> there is no "previous governor" to restore, so calling
> get_governor(last_governor) is unnecessary and potentially confusing.
> 
> This patch skips looking up `last_governor` when registering the first
> policy. Instead, it directly uses the default governor after all
> governors have been registered and are available.
> 
> This avoids meaningless lookups, reduces unnecessary module reference
> handling, and simplifies the initial policy path.
> 
> Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>
> 
> ---
> Changes in v2:
>  - Fix the case where the governor is NULL.
> ---
> ---
>  drivers/cpufreq/cpufreq.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index d7426e1d8bdd..1aa559f53479 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1122,7 +1122,8 @@ static int cpufreq_init_policy(struct cpufreq_policy *policy)
>  
>  	if (has_target()) {
>  		/* Update policy governor to the one used before hotplug. */
> -		gov = get_governor(policy->last_governor);
> +		if (policy->last_governor[0] != '\0')
> +			gov = get_governor(policy->last_governor);
>  		if (gov) {
>  			pr_debug("Restoring governor %s for cpu %d\n",
>  				 gov->name, policy->cpu);

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] cpufreq: Avoid get_governor() for first policy
  2025-07-25  4:14 [PATCH v2] cpufreq: Avoid get_governor() for first policy Zihuan Zhang
  2025-07-25  5:49 ` Viresh Kumar
@ 2025-07-25  9:11 ` zhenglifeng (A)
  2025-08-19 19:14   ` Rafael J. Wysocki
  1 sibling, 1 reply; 4+ messages in thread
From: zhenglifeng (A) @ 2025-07-25  9:11 UTC (permalink / raw)
  To: Zihuan Zhang, rafael J . wysocki, Viresh Kumar; +Cc: linux-pm, linux-kernel

On 2025/7/25 12:14, Zihuan Zhang wrote:

> When a cpufreq driver registers the first policy, it may attempt to
> initialize the policy governor from `last_governor`. However, this is
> meaningless for the first policy instance, because `last_governor` is
> only updated when policies are removed (e.g. during CPU offline).
> 
> The `last_governor` mechanism is intended to restore the previously
> used governor across CPU hotplug events. For the very first policy,
> there is no "previous governor" to restore, so calling
> get_governor(last_governor) is unnecessary and potentially confusing.
> 
> This patch skips looking up `last_governor` when registering the first
> policy. Instead, it directly uses the default governor after all
> governors have been registered and are available.
> 
> This avoids meaningless lookups, reduces unnecessary module reference
> handling, and simplifies the initial policy path.
> 
> Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>
> 
> ---
> Changes in v2:
>  - Fix the case where the governor is NULL.
> ---
> ---
>  drivers/cpufreq/cpufreq.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index d7426e1d8bdd..1aa559f53479 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -1122,7 +1122,8 @@ static int cpufreq_init_policy(struct cpufreq_policy *policy)
>  
>  	if (has_target()) {
>  		/* Update policy governor to the one used before hotplug. */
> -		gov = get_governor(policy->last_governor);
> +		if (policy->last_governor[0] != '\0')
> +			gov = get_governor(policy->last_governor);
>  		if (gov) {
>  			pr_debug("Restoring governor %s for cpu %d\n",
>  				 gov->name, policy->cpu);

Reviewed-by: Lifeng Zheng <zhenglifeng1@huawei.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] cpufreq: Avoid get_governor() for first policy
  2025-07-25  9:11 ` zhenglifeng (A)
@ 2025-08-19 19:14   ` Rafael J. Wysocki
  0 siblings, 0 replies; 4+ messages in thread
From: Rafael J. Wysocki @ 2025-08-19 19:14 UTC (permalink / raw)
  To: zhenglifeng (A), Zihuan Zhang, Viresh Kumar; +Cc: linux-pm, linux-kernel

On Fri, Jul 25, 2025 at 11:11 AM zhenglifeng (A)
<zhenglifeng1@huawei.com> wrote:
>
> On 2025/7/25 12:14, Zihuan Zhang wrote:
>
> > When a cpufreq driver registers the first policy, it may attempt to
> > initialize the policy governor from `last_governor`. However, this is
> > meaningless for the first policy instance, because `last_governor` is
> > only updated when policies are removed (e.g. during CPU offline).
> >
> > The `last_governor` mechanism is intended to restore the previously
> > used governor across CPU hotplug events. For the very first policy,
> > there is no "previous governor" to restore, so calling
> > get_governor(last_governor) is unnecessary and potentially confusing.
> >
> > This patch skips looking up `last_governor` when registering the first
> > policy. Instead, it directly uses the default governor after all
> > governors have been registered and are available.
> >
> > This avoids meaningless lookups, reduces unnecessary module reference
> > handling, and simplifies the initial policy path.
> >
> > Signed-off-by: Zihuan Zhang <zhangzihuan@kylinos.cn>
> >
> > ---
> > Changes in v2:
> >  - Fix the case where the governor is NULL.
> > ---
> > ---
> >  drivers/cpufreq/cpufreq.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index d7426e1d8bdd..1aa559f53479 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -1122,7 +1122,8 @@ static int cpufreq_init_policy(struct cpufreq_policy *policy)
> >
> >       if (has_target()) {
> >               /* Update policy governor to the one used before hotplug. */
> > -             gov = get_governor(policy->last_governor);
> > +             if (policy->last_governor[0] != '\0')
> > +                     gov = get_governor(policy->last_governor);
> >               if (gov) {
> >                       pr_debug("Restoring governor %s for cpu %d\n",
> >                                gov->name, policy->cpu);
>
> Reviewed-by: Lifeng Zheng <zhenglifeng1@huawei.com>

Applied as 6.18 material, thanks!

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-08-19 19:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-25  4:14 [PATCH v2] cpufreq: Avoid get_governor() for first policy Zihuan Zhang
2025-07-25  5:49 ` Viresh Kumar
2025-07-25  9:11 ` zhenglifeng (A)
2025-08-19 19:14   ` Rafael J. Wysocki

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 NNTP newsgroup(s).