[PATCH] sched/fair: Use sched_energy_enabled()

Vincent Guittot posted 1 patch 6 days ago
kernel/sched/topology.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
[PATCH] sched/fair: Use sched_energy_enabled()
Posted by Vincent Guittot 6 days ago
Use helper sched_energy_enabled() everywhere we want to test if EAS is
enabled instead of mixing sched_energy_enabled() and direct call to
static_branch_unlikely().

No functional change

Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
---
 kernel/sched/topology.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 43150591914b..5847b83d9d55 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -273,7 +273,7 @@ void rebuild_sched_domains_energy(void)
 static int sched_energy_aware_handler(const struct ctl_table *table, int write,
 		void *buffer, size_t *lenp, loff_t *ppos)
 {
-	int ret, state;
+	int ret;
 
 	if (write && !capable(CAP_SYS_ADMIN))
 		return -EPERM;
@@ -289,8 +289,7 @@ static int sched_energy_aware_handler(const struct ctl_table *table, int write,
 
 	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
 	if (!ret && write) {
-		state = static_branch_unlikely(&sched_energy_present);
-		if (state != sysctl_sched_energy_aware)
+		if (sysctl_sched_energy_aware != sched_energy_enabled())
 			rebuild_sched_domains_energy();
 	}
 
@@ -388,11 +387,11 @@ static void destroy_perf_domain_rcu(struct rcu_head *rp)
 
 static void sched_energy_set(bool has_eas)
 {
-	if (!has_eas && static_branch_unlikely(&sched_energy_present)) {
+	if (!has_eas && sched_energy_enabled()) {
 		if (sched_debug())
 			pr_info("%s: stopping EAS\n", __func__);
 		static_branch_disable_cpuslocked(&sched_energy_present);
-	} else if (has_eas && !static_branch_unlikely(&sched_energy_present)) {
+	} else if (has_eas && !sched_energy_enabled()) {
 		if (sched_debug())
 			pr_info("%s: starting EAS\n", __func__);
 		static_branch_enable_cpuslocked(&sched_energy_present);
-- 
2.43.0
Re: [PATCH] sched/fair: Use sched_energy_enabled()
Posted by K Prateek Nayak 5 days, 21 hours ago
Hello Vincent,

On 3/27/2026 6:50 PM, Vincent Guittot wrote:
> Use helper sched_energy_enabled() everywhere we want to test if EAS is
> enabled instead of mixing sched_energy_enabled() and direct call to
> static_branch_unlikely().
> 
> No functional change
> 
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>

Feel free to include:

Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>
Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>

> ---
>  kernel/sched/topology.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 43150591914b..5847b83d9d55 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -273,7 +273,7 @@ void rebuild_sched_domains_energy(void)
>  static int sched_energy_aware_handler(const struct ctl_table *table, int write,
>  		void *buffer, size_t *lenp, loff_t *ppos)
>  {
> -	int ret, state;
> +	int ret;
>  
>  	if (write && !capable(CAP_SYS_ADMIN))
>  		return -EPERM;
> @@ -289,8 +289,7 @@ static int sched_energy_aware_handler(const struct ctl_table *table, int write,
>  
>  	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
>  	if (!ret && write) {
> -		state = static_branch_unlikely(&sched_energy_present);
> -		if (state != sysctl_sched_energy_aware)
> +		if (sysctl_sched_energy_aware != sched_energy_enabled())

Quick tangential question:

Since "sysctl_sched_energy_aware" is initialized to "1", if
build_perf_domains() returns false at boot, sysctl_sched_energy_aware
and sched_energy_enabled() will go out of sync and reading
"/proc/sys/kernel/sched_energy_aware" will still show "1" despite EAS
being disabled.

Is that okay?

Some sysctls like numa_balancing will show 0 if disabled (by user or
due to lack of NUMA) reflecting the real state of system. Should
sched_energy_aware do the same?

>  			rebuild_sched_domains_energy();
>  	}
>  
-- 
Thanks and Regards,
Prateek
Re: [PATCH] sched/fair: Use sched_energy_enabled()
Posted by Vincent Guittot 5 days, 21 hours ago
On Fri, 27 Mar 2026 at 17:18, K Prateek Nayak <kprateek.nayak@amd.com> wrote:
>
> Hello Vincent,
>
> On 3/27/2026 6:50 PM, Vincent Guittot wrote:
> > Use helper sched_energy_enabled() everywhere we want to test if EAS is
> > enabled instead of mixing sched_energy_enabled() and direct call to
> > static_branch_unlikely().
> >
> > No functional change
> >
> > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
>
> Feel free to include:
>
> Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>
> Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>
>
> > ---
> >  kernel/sched/topology.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> > index 43150591914b..5847b83d9d55 100644
> > --- a/kernel/sched/topology.c
> > +++ b/kernel/sched/topology.c
> > @@ -273,7 +273,7 @@ void rebuild_sched_domains_energy(void)
> >  static int sched_energy_aware_handler(const struct ctl_table *table, int write,
> >               void *buffer, size_t *lenp, loff_t *ppos)
> >  {
> > -     int ret, state;
> > +     int ret;
> >
> >       if (write && !capable(CAP_SYS_ADMIN))
> >               return -EPERM;
> > @@ -289,8 +289,7 @@ static int sched_energy_aware_handler(const struct ctl_table *table, int write,
> >
> >       ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
> >       if (!ret && write) {
> > -             state = static_branch_unlikely(&sched_energy_present);
> > -             if (state != sysctl_sched_energy_aware)
> > +             if (sysctl_sched_energy_aware != sched_energy_enabled())
>
> Quick tangential question:
>
> Since "sysctl_sched_energy_aware" is initialized to "1", if
> build_perf_domains() returns false at boot, sysctl_sched_energy_aware
> and sched_energy_enabled() will go out of sync and reading
> "/proc/sys/kernel/sched_energy_aware" will still show "1" despite EAS
> being disabled.
>
> Is that okay?

Isn't the call to sched_is_eas_possible() there to prevent this case ?

>
> Some sysctls like numa_balancing will show 0 if disabled (by user or
> due to lack of NUMA) reflecting the real state of system. Should
> sched_energy_aware do the same?
>
> >                       rebuild_sched_domains_energy();
> >       }
> >
> --
> Thanks and Regards,
> Prateek
Re: [PATCH] sched/fair: Use sched_energy_enabled()
Posted by Dietmar Eggemann 5 days, 20 hours ago




On 27.03.26 17:32, Vincent Guittot wrote:
> On Fri, 27 Mar 2026 at 17:18, K Prateek Nayak <kprateek.nayak@amd.com> wrote:
>>
>> Hello Vincent,
>>
>> On 3/27/2026 6:50 PM, Vincent Guittot wrote:
>>> Use helper sched_energy_enabled() everywhere we want to test if EAS is
>>> enabled instead of mixing sched_energy_enabled() and direct call to
>>> static_branch_unlikely().
>>>
>>> No functional change
>>>
>>> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
>>
>> Feel free to include:
>>
>> Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com>
>> Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>
>>
>>> ---
>>>  kernel/sched/topology.c | 9 ++++-----
>>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
>>> index 43150591914b..5847b83d9d55 100644
>>> --- a/kernel/sched/topology.c
>>> +++ b/kernel/sched/topology.c
>>> @@ -273,7 +273,7 @@ void rebuild_sched_domains_energy(void)
>>>  static int sched_energy_aware_handler(const struct ctl_table *table, int write,
>>>               void *buffer, size_t *lenp, loff_t *ppos)
>>>  {
>>> -     int ret, state;
>>> +     int ret;
>>>
>>>       if (write && !capable(CAP_SYS_ADMIN))
>>>               return -EPERM;
>>> @@ -289,8 +289,7 @@ static int sched_energy_aware_handler(const struct ctl_table *table, int write,
>>>
>>>       ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
>>>       if (!ret && write) {
>>> -             state = static_branch_unlikely(&sched_energy_present);
>>> -             if (state != sysctl_sched_energy_aware)
>>> +             if (sysctl_sched_energy_aware != sched_energy_enabled())
>>
>> Quick tangential question:
>>
>> Since "sysctl_sched_energy_aware" is initialized to "1", if
>> build_perf_domains() returns false at boot, sysctl_sched_energy_aware
>> and sched_energy_enabled() will go out of sync and reading
>> "/proc/sys/kernel/sched_energy_aware" will still show "1" despite EAS
>> being disabled.
>>
>> Is that okay?
> 
> Isn't the call to sched_is_eas_possible() there to prevent this case ?

Yes, I think so:

 300 static const struct ctl_table sched_energy_aware_sysctls[] = {
 ...
 302                 .procname       = "sched_energy_aware",
 ...
 306                 .proc_handler   = sched_energy_aware_handler,

 273 sched_energy_aware_handler()

 ...
 281         if (!sched_is_eas_possible(cpu_active_mask)) { <-- !!!
 282                 if (write) {
 283                         return -EOPNOTSUPP;
 284                 } else {
 285                         *lenp = 0;
 286                         return 0;
 287                 }
 ...


# cat /proc/sys/kernel/sched_energy_aware
1

# ./set_cpufreq_ondemand.sh

cat /proc/sys/kernel/sched_energy_aware
[  391.591167] rd 0-5: Checking EAS: cpufreq is not ready


Reviewed-by: Dietmar Eggemann <dietmar.eggemann@arm.com>

[...]
Re: [PATCH] sched/fair: Use sched_energy_enabled()
Posted by K Prateek Nayak 5 days, 20 hours ago
Hello Vincent,

On 3/27/2026 10:02 PM, Vincent Guittot wrote:
>>> +             if (sysctl_sched_energy_aware != sched_energy_enabled())
>>
>> Quick tangential question:
>>
>> Since "sysctl_sched_energy_aware" is initialized to "1", if
>> build_perf_domains() returns false at boot, sysctl_sched_energy_aware
>> and sched_energy_enabled() will go out of sync and reading
>> "/proc/sys/kernel/sched_energy_aware" will still show "1" despite EAS
>> being disabled.
>>
>> Is that okay?
> 
> Isn't the call to sched_is_eas_possible() there to prevent this case ?

Ah! pd_init() can never fail (expect for -ENOMEME)
I somehow thought there was more to it. Sorry for the noise.

>>
>> Some sysctls like numa_balancing will show 0 if disabled (by user or
>> due to lack of NUMA) reflecting the real state of system. Should
>> sched_energy_aware do the same?
-- 
Thanks and Regards,
Prateek
Re: [PATCH] sched/fair: Use sched_energy_enabled()
Posted by Shrikanth Hegde 6 days ago

On 3/27/26 6:50 PM, Vincent Guittot wrote:
> Use helper sched_energy_enabled() everywhere we want to test if EAS is
> enabled instead of mixing sched_energy_enabled() and direct call to
> static_branch_unlikely().
> 
> No functional change
> 

Makes sense.

> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
>   kernel/sched/topology.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 43150591914b..5847b83d9d55 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -273,7 +273,7 @@ void rebuild_sched_domains_energy(void)
>   static int sched_energy_aware_handler(const struct ctl_table *table, int write,
>   		void *buffer, size_t *lenp, loff_t *ppos)
>   {
> -	int ret, state;
> +	int ret;
>   
>   	if (write && !capable(CAP_SYS_ADMIN))
>   		return -EPERM;
> @@ -289,8 +289,7 @@ static int sched_energy_aware_handler(const struct ctl_table *table, int write,
>   
>   	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
>   	if (!ret && write) {
> -		state = static_branch_unlikely(&sched_energy_present);
> -		if (state != sysctl_sched_energy_aware)
> +		if (sysctl_sched_energy_aware != sched_energy_enabled())
>   			rebuild_sched_domains_energy();
>   	}
>   
> @@ -388,11 +387,11 @@ static void destroy_perf_domain_rcu(struct rcu_head *rp)
>   
>   static void sched_energy_set(bool has_eas)
>   {
> -	if (!has_eas && static_branch_unlikely(&sched_energy_present)) {
> +	if (!has_eas && sched_energy_enabled()) {
>   		if (sched_debug())
>   			pr_info("%s: stopping EAS\n", __func__);
>   		static_branch_disable_cpuslocked(&sched_energy_present);
> -	} else if (has_eas && !static_branch_unlikely(&sched_energy_present)) {
> +	} else if (has_eas && !sched_energy_enabled()) {
>   		if (sched_debug())
>   			pr_info("%s: starting EAS\n", __func__);
>   		static_branch_enable_cpuslocked(&sched_energy_present);

Acked-by: Shrikanth Hegde <sshegde@linux.ibm.com>