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;
 
-- 
2.34.1

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;
>

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!