diff mbox series

sched/cpufreq: Ensure sd is rebuilt for EAS check

Message ID 35e572d9-1152-406a-9e34-2525f7548af9@arm.com (mailing list archive)
State In Next
Delegated to: Rafael Wysocki
Headers show
Series sched/cpufreq: Ensure sd is rebuilt for EAS check | expand

Commit Message

Christian Loehle Nov. 9, 2024, 12:24 a.m. UTC
Ensure sugov_eas_rebuild_sd() is always called when sugov_init()
succeeds. The out goto initialized sugov without forcing the rebuild.

Previously the missing call to sugov_eas_rebuild_sd() could lead to EAS
not being enabled on boot when it should have been, because it requires
all policies to be controlled by schedutil while they might not have
been initialized yet.

Fixes: e7a1b32e43b1 ("cpufreq: Rebuild sched-domains when removing cpufreq driver")
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 kernel/sched/cpufreq_schedutil.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Rafael J. Wysocki Nov. 12, 2024, 8:37 p.m. UTC | #1
On Sat, Nov 9, 2024 at 1:24 AM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> Ensure sugov_eas_rebuild_sd() is always called when sugov_init()
> succeeds. The out goto initialized sugov without forcing the rebuild.
>
> Previously the missing call to sugov_eas_rebuild_sd() could lead to EAS
> not being enabled on boot when it should have been, because it requires
> all policies to be controlled by schedutil while they might not have
> been initialized yet.
>
> Fixes: e7a1b32e43b1 ("cpufreq: Rebuild sched-domains when removing cpufreq driver")
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---
>  kernel/sched/cpufreq_schedutil.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> index c6ba15388ea7..28c77904ea74 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -783,9 +783,8 @@ static int sugov_init(struct cpufreq_policy *policy)
>         if (ret)
>                 goto fail;
>
> -       sugov_eas_rebuild_sd();
> -
>  out:
> +       sugov_eas_rebuild_sd();
>         mutex_unlock(&global_tunables_lock);
>         return 0;
>
> --

Applied as 6.13 material, thanks!
Pierre Gondois Nov. 13, 2024, 11:53 a.m. UTC | #2
I could reproduce the issue on an ACPI/CPPC based platform.

It seems the reason it doesn't work is that for CPPC,
CPUFREQ_HAVE_GOVERNOR_PER_POLICY is not set. So
   kernel/sched/cpufreq_schedutil.c::global_tunables
is set and we don't bail out in sugov_init().

On a system with CPUFREQ_HAVE_GOVERNOR_PER_POLICY set,
the issue is not triggered. sugov_eas_rebuild_sd() is always called.

The patch effectively solved the issue for me.

Regards,
Pierre

On 11/9/24 01:24, Christian Loehle wrote:
> Ensure sugov_eas_rebuild_sd() is always called when sugov_init()
> succeeds. The out goto initialized sugov without forcing the rebuild.
> 
> Previously the missing call to sugov_eas_rebuild_sd() could lead to EAS
> not being enabled on boot when it should have been, because it requires
> all policies to be controlled by schedutil while they might not have
> been initialized yet.
> 
> Fixes: e7a1b32e43b1 ("cpufreq: Rebuild sched-domains when removing cpufreq driver")
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---
>   kernel/sched/cpufreq_schedutil.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
> index c6ba15388ea7..28c77904ea74 100644
> --- a/kernel/sched/cpufreq_schedutil.c
> +++ b/kernel/sched/cpufreq_schedutil.c
> @@ -783,9 +783,8 @@ static int sugov_init(struct cpufreq_policy *policy)
>   	if (ret)
>   		goto fail;
>   
> -	sugov_eas_rebuild_sd();
> -
>   out:
> +	sugov_eas_rebuild_sd();
>   	mutex_unlock(&global_tunables_lock);
>   	return 0;
>
diff mbox series

Patch

diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index c6ba15388ea7..28c77904ea74 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -783,9 +783,8 @@  static int sugov_init(struct cpufreq_policy *policy)
 	if (ret)
 		goto fail;
 
-	sugov_eas_rebuild_sd();
-
 out:
+	sugov_eas_rebuild_sd();
 	mutex_unlock(&global_tunables_lock);
 	return 0;