In presence of pushable tasks on the CPU, set it on the newly introduced
"overloaded+mask" in sched_domain_shared struct. This will be used by
the newidle balance to limit the scanning to these overloaded CPUs since
they contain tasks that could be run on the newly idle target.
Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
---
kernel/sched/fair.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 98d3ed2078cd..834fcdd15cac 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8559,6 +8559,24 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
return target;
}
+static inline void update_overloaded_mask(int cpu, bool contains_pushable)
+{
+ struct sched_domain_shared *sd_share = rcu_dereference(per_cpu(sd_llc_shared, cpu));
+ cpumask_var_t overloaded_mask;
+
+ if (!sd_share)
+ return;
+
+ overloaded_mask = sd_share->overloaded_mask;
+ if (!overloaded_mask)
+ return;
+
+ if (contains_pushable)
+ cpumask_set_cpu(cpu, overloaded_mask);
+ else
+ cpumask_clear_cpu(cpu, overloaded_mask);
+}
+
static inline bool fair_push_task(struct task_struct *p)
{
if (!task_on_rq_queued(p))
@@ -8606,11 +8624,17 @@ static inline void fair_queue_pushable_tasks(struct rq *rq)
static void fair_remove_pushable_task(struct rq *rq, struct task_struct *p)
{
plist_del(&p->pushable_tasks, &rq->cfs.pushable_tasks);
+
+ if (!has_pushable_tasks(rq))
+ update_overloaded_mask(rq->cpu, false);
}
static void fair_add_pushable_task(struct rq *rq, struct task_struct *p)
{
if (fair_push_task(p)) {
+ if (!has_pushable_tasks(rq))
+ update_overloaded_mask(rq->cpu, true);
+
plist_del(&p->pushable_tasks, &rq->cfs.pushable_tasks);
plist_node_init(&p->pushable_tasks, p->prio);
plist_add(&p->pushable_tasks, &rq->cfs.pushable_tasks);
--
2.34.1
On 4/9/25 16:45, K Prateek Nayak wrote:
Hi Prateek. Feel free to cc me in the future versions.
This seems interesting way if it can get us rid of newidle balance overheads.
> In presence of pushable tasks on the CPU, set it on the newly introduced
> "overloaded+mask" in sched_domain_shared struct. This will be used by
> the newidle balance to limit the scanning to these overloaded CPUs since
> they contain tasks that could be run on the newly idle target.
>
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
> ---
> kernel/sched/fair.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 98d3ed2078cd..834fcdd15cac 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8559,6 +8559,24 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
> return target;
> }
>
> +static inline void update_overloaded_mask(int cpu, bool contains_pushable)
> +{
> + struct sched_domain_shared *sd_share = rcu_dereference(per_cpu(sd_llc_shared, cpu));
> + cpumask_var_t overloaded_mask;
> +
> + if (!sd_share)
> + return;
> +
> + overloaded_mask = sd_share->overloaded_mask;
> + if (!overloaded_mask)
> + return;
> +
> + if (contains_pushable)
> + cpumask_set_cpu(cpu, overloaded_mask);
> + else
> + cpumask_clear_cpu(cpu, overloaded_mask);
> +}
> +
I was getting below error when compiling. Noticed that overloaded_mask is a local update and it wouldn't
update the actual overloaded_mask.
Compilation Error:
kernel/sched/fair.c: In function ‘update_overloaded_mask’:
kernel/sched/fair.c:8570:25: error: assignment to expression with array type
8570 | overloaded_mask = sd_share->overloaded_mask;
| ^
kernel/sched/fair.c:8571:13: warning: the address of ‘overloaded_mask’ will always evaluate as ‘true’ [-Waddress]
8571 | if (!overloaded_mask)
Made the below change. Also you would need rcu protection for sd_share i think.
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index bca3db5d0ac0..818d4769ec55 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8562,19 +8562,18 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
static inline void update_overloaded_mask(int cpu, bool contains_pushable)
{
struct sched_domain_shared *sd_share = rcu_dereference(per_cpu(sd_llc_shared, cpu));
- cpumask_var_t overloaded_mask;
if (!sd_share)
return;
- overloaded_mask = sd_share->overloaded_mask;
- if (!overloaded_mask)
+ if (!sd_share->overloaded_mask)
return;
+ guard(rcu)();
if (contains_pushable)
- cpumask_set_cpu(cpu, overloaded_mask);
+ cpumask_set_cpu(cpu, sd_share->overloaded_mask);
else
- cpumask_clear_cpu(cpu, overloaded_mask);
+ cpumask_clear_cpu(cpu, sd_share->overloaded_mask);
}
> static inline bool fair_push_task(struct task_struct *p)
> {
> if (!task_on_rq_queued(p))
> @@ -8606,11 +8624,17 @@ static inline void fair_queue_pushable_tasks(struct rq *rq)
> static void fair_remove_pushable_task(struct rq *rq, struct task_struct *p)
> {
> plist_del(&p->pushable_tasks, &rq->cfs.pushable_tasks);
> +
> + if (!has_pushable_tasks(rq))
> + update_overloaded_mask(rq->cpu, false);
> }
>
> static void fair_add_pushable_task(struct rq *rq, struct task_struct *p)
> {
> if (fair_push_task(p)) {
> + if (!has_pushable_tasks(rq))
> + update_overloaded_mask(rq->cpu, true);
> +
> plist_del(&p->pushable_tasks, &rq->cfs.pushable_tasks);
> plist_node_init(&p->pushable_tasks, p->prio);
> plist_add(&p->pushable_tasks, &rq->cfs.pushable_tasks);
Hello Shrikanth,
On 4/21/2025 10:50 AM, Shrikanth Hegde wrote:
>
>
> On 4/9/25 16:45, K Prateek Nayak wrote:
>
> Hi Prateek. Feel free to cc me in the future versions.
Will do! Thank you for taking a look at the series.
> This seems interesting way if it can get us rid of newidle balance overheads.
>
>> In presence of pushable tasks on the CPU, set it on the newly introduced
>> "overloaded+mask" in sched_domain_shared struct. This will be used by
>> the newidle balance to limit the scanning to these overloaded CPUs since
>> they contain tasks that could be run on the newly idle target.
>>
>> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
>> ---
>> kernel/sched/fair.c | 24 ++++++++++++++++++++++++
>> 1 file changed, 24 insertions(+)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 98d3ed2078cd..834fcdd15cac 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -8559,6 +8559,24 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
>> return target;
>> }
>> +static inline void update_overloaded_mask(int cpu, bool contains_pushable)
>> +{
>> + struct sched_domain_shared *sd_share = rcu_dereference(per_cpu(sd_llc_shared, cpu));
>> + cpumask_var_t overloaded_mask;
>> +
>> + if (!sd_share)
>> + return;
>> +
>> + overloaded_mask = sd_share->overloaded_mask;
>> + if (!overloaded_mask)
>> + return;
>> +
>> + if (contains_pushable)
>> + cpumask_set_cpu(cpu, overloaded_mask);
>> + else
>> + cpumask_clear_cpu(cpu, overloaded_mask);
>> +}
>> +
>
> I was getting below error when compiling. Noticed that overloaded_mask is a local update and it wouldn't
> update the actual overloaded_mask.
Interesting! Question: Do you have "CONFIG_CPUMASK_OFFSTACK" enabled in
your config? (me makes a note to test this too in the next iteration)
Looking at the arch specific Kconfigs, there is a slight difference in
how this is toggled on x86 vs powerpc and I'm wondering if that is why
I haven't seen this warning in my testing.
>
> Compilation Error:
> kernel/sched/fair.c: In function ‘update_overloaded_mask’:
> kernel/sched/fair.c:8570:25: error: assignment to expression with array type
> 8570 | overloaded_mask = sd_share->overloaded_mask;
> | ^
> kernel/sched/fair.c:8571:13: warning: the address of ‘overloaded_mask’ will always evaluate as ‘true’ [-Waddress]
> 8571 | if (!overloaded_mask)
>
>
>
> Made the below change. Also you would need rcu protection for sd_share i think.
You are right! Thank you for the pointers and spotting my oversight.
Aaron too pointed some shortcomings here. I'll make sure to test
these bits more next time (LOCKDEP, hotplug, and
!CONFIG_CPUMASK_OFFSTACK)
--
Thanks and Regards,
Prateek
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index bca3db5d0ac0..818d4769ec55 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8562,19 +8562,18 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
> static inline void update_overloaded_mask(int cpu, bool contains_pushable)
> {
> struct sched_domain_shared *sd_share = rcu_dereference(per_cpu(sd_llc_shared, cpu));
> - cpumask_var_t overloaded_mask;
>
> if (!sd_share)
> return;
>
> - overloaded_mask = sd_share->overloaded_mask;
> - if (!overloaded_mask)
> + if (!sd_share->overloaded_mask)
> return;
>
> + guard(rcu)();
> if (contains_pushable)
> - cpumask_set_cpu(cpu, overloaded_mask);
> + cpumask_set_cpu(cpu, sd_share->overloaded_mask);
> else
> - cpumask_clear_cpu(cpu, overloaded_mask);
> + cpumask_clear_cpu(cpu, sd_share->overloaded_mask);
> }
>
>> static inline bool fair_push_task(struct task_struct *p)
>> {
>> if (!task_on_rq_queued(p))
>> @@ -8606,11 +8624,17 @@ static inline void fair_queue_pushable_tasks(struct rq *rq)
>> static void fair_remove_pushable_task(struct rq *rq, struct task_struct *p)
>> {
>> plist_del(&p->pushable_tasks, &rq->cfs.pushable_tasks);
>> +
>> + if (!has_pushable_tasks(rq))
>> + update_overloaded_mask(rq->cpu, false);
>> }
>> static void fair_add_pushable_task(struct rq *rq, struct task_struct *p)
>> {
>> if (fair_push_task(p)) {
>> + if (!has_pushable_tasks(rq))
>> + update_overloaded_mask(rq->cpu, true);
>> +
>> plist_del(&p->pushable_tasks, &rq->cfs.pushable_tasks);
>> plist_node_init(&p->pushable_tasks, p->prio);
>> plist_add(&p->pushable_tasks, &rq->cfs.pushable_tasks);
>
On 4/21/25 11:24, K Prateek Nayak wrote:
> Hello Shrikanth,
>
> On 4/21/2025 10:50 AM, Shrikanth Hegde wrote:
>>
>>
>> On 4/9/25 16:45, K Prateek Nayak wrote:
>>
>> Hi Prateek. Feel free to cc me in the future versions.
>
>
> Will do! Thank you for taking a look at the series.
>
>> This seems interesting way if it can get us rid of newidle balance
>> overheads.
>>
>>> In presence of pushable tasks on the CPU, set it on the newly introduced
>>> "overloaded+mask" in sched_domain_shared struct. This will be used by
>>> the newidle balance to limit the scanning to these overloaded CPUs since
>>> they contain tasks that could be run on the newly idle target.
>>>
>>> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
>>> ---
>>> kernel/sched/fair.c | 24 ++++++++++++++++++++++++
>>> 1 file changed, 24 insertions(+)
>>>
>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>>> index 98d3ed2078cd..834fcdd15cac 100644
>>> --- a/kernel/sched/fair.c
>>> +++ b/kernel/sched/fair.c
>>> @@ -8559,6 +8559,24 @@ static int find_energy_efficient_cpu(struct
>>> task_struct *p, int prev_cpu)
>>> return target;
>>> }
>>> +static inline void update_overloaded_mask(int cpu, bool
>>> contains_pushable)
>>> +{
>>> + struct sched_domain_shared *sd_share =
>>> rcu_dereference(per_cpu(sd_llc_shared, cpu));
>>> + cpumask_var_t overloaded_mask;
>>> +
>>> + if (!sd_share)
>>> + return;
>>> +
>>> + overloaded_mask = sd_share->overloaded_mask;
>>> + if (!overloaded_mask)
>>> + return;
>>> +
>>> + if (contains_pushable)
>>> + cpumask_set_cpu(cpu, overloaded_mask);
>>> + else
>>> + cpumask_clear_cpu(cpu, overloaded_mask);
>>> +}
>>> +
>
>> I was getting below error when compiling. Noticed that overloaded_mask
>> is a local update and it wouldn't
>> update the actual overloaded_mask.
>
> Interesting! Question: Do you have "CONFIG_CPUMASK_OFFSTACK" enabled in
> your config? (me makes a note to test this too in the next iteration)
> Looking at the arch specific Kconfigs, there is a slight difference in
> how this is toggled on x86 vs powerpc and I'm wondering if that is why
> I haven't seen this warning in my testing.
>
Yes, that's the reason you didn't run into.
for me, CONFIG_CPUMASK_OFFSTACK is not set.
>>
>> Compilation Error:
>> kernel/sched/fair.c: In function ‘update_overloaded_mask’:
>> kernel/sched/fair.c:8570:25: error: assignment to expression with
>> array type
>> 8570 | overloaded_mask = sd_share->overloaded_mask;
>> | ^
>> kernel/sched/fair.c:8571:13: warning: the address of ‘overloaded_mask’
>> will always evaluate as ‘true’ [-Waddress]
>> 8571 | if (!overloaded_mask)
>>
>>
>>
>> Made the below change. Also you would need rcu protection for sd_share
>> i think.
Or you need to use like below. This also works (Not tested on x86)
struct cpumask* overloaded_mask;
>
> You are right! Thank you for the pointers and spotting my oversight.
> Aaron too pointed some shortcomings here. I'll make sure to test
> these bits more next time (LOCKDEP, hotplug, and
> !CONFIG_CPUMASK_OFFSTACK)
>
Hello Shrikanth, On 4/21/2025 1:33 PM, Shrikanth Hegde wrote: >>> I was getting below error when compiling. Noticed that overloaded_mask is a local update and it wouldn't >>> update the actual overloaded_mask. >> >> Interesting! Question: Do you have "CONFIG_CPUMASK_OFFSTACK" enabled in >> your config? (me makes a note to test this too in the next iteration) >> Looking at the arch specific Kconfigs, there is a slight difference in >> how this is toggled on x86 vs powerpc and I'm wondering if that is why >> I haven't seen this warning in my testing. >> > > Yes, that's the reason you didn't run into. > for me, CONFIG_CPUMASK_OFFSTACK is not set. Thank you for confirming! > >>> >>> Compilation Error: >>> kernel/sched/fair.c: In function ‘update_overloaded_mask’: >>> kernel/sched/fair.c:8570:25: error: assignment to expression with array type >>> 8570 | overloaded_mask = sd_share->overloaded_mask; >>> | ^ >>> kernel/sched/fair.c:8571:13: warning: the address of ‘overloaded_mask’ will always evaluate as ‘true’ [-Waddress] >>> 8571 | if (!overloaded_mask) >>> >>> >>> >>> Made the below change. Also you would need rcu protection for sd_share i think. > > Or you need to use like below. This also works (Not tested on x86) > > struct cpumask* overloaded_mask; Ack! Perhaps that is the way to go. I'll take some inspiration from other cpumask usage in kernel and adjust this accordingly. Thanks a ton for testing. > >> >> You are right! Thank you for the pointers and spotting my oversight. >> Aaron too pointed some shortcomings here. I'll make sure to test >> these bits more next time (LOCKDEP, hotplug, and >> !CONFIG_CPUMASK_OFFSTACK) -- Thanks and Regards, Prateek
Hi Prateek,
On Wed, Apr 09, 2025 at 11:15:37AM +0000, K Prateek Nayak wrote:
> In presence of pushable tasks on the CPU, set it on the newly introduced
> "overloaded+mask" in sched_domain_shared struct. This will be used by
> the newidle balance to limit the scanning to these overloaded CPUs since
> they contain tasks that could be run on the newly idle target.
>
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
> ---
> kernel/sched/fair.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 98d3ed2078cd..834fcdd15cac 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -8559,6 +8559,24 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
> return target;
> }
>
> +static inline void update_overloaded_mask(int cpu, bool contains_pushable)
> +{
> + struct sched_domain_shared *sd_share = rcu_dereference(per_cpu(sd_llc_shared, cpu));
I got a suspicious RCU usage warning for this line while testing your
series. Since rq lock is held here, rcu_dereference() should not be
necessary.
> + cpumask_var_t overloaded_mask;
> +
> + if (!sd_share)
> + return;
> +
> + overloaded_mask = sd_share->overloaded_mask;
> + if (!overloaded_mask)
> + return;
> +
> + if (contains_pushable)
> + cpumask_set_cpu(cpu, overloaded_mask);
> + else
> + cpumask_clear_cpu(cpu, overloaded_mask);
> +}
Hello Aaron,
On 4/14/2025 7:58 AM, Aaron Lu wrote:
> Hi Prateek,
>
> On Wed, Apr 09, 2025 at 11:15:37AM +0000, K Prateek Nayak wrote:
>> In presence of pushable tasks on the CPU, set it on the newly introduced
>> "overloaded+mask" in sched_domain_shared struct. This will be used by
>> the newidle balance to limit the scanning to these overloaded CPUs since
>> they contain tasks that could be run on the newly idle target.
>>
>> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
>> ---
>> kernel/sched/fair.c | 24 ++++++++++++++++++++++++
>> 1 file changed, 24 insertions(+)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 98d3ed2078cd..834fcdd15cac 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -8559,6 +8559,24 @@ static int find_energy_efficient_cpu(struct task_struct *p, int prev_cpu)
>> return target;
>> }
>>
>> +static inline void update_overloaded_mask(int cpu, bool contains_pushable)
>> +{
>> + struct sched_domain_shared *sd_share = rcu_dereference(per_cpu(sd_llc_shared, cpu));
>
> I got a suspicious RCU usage warning for this line while testing your
> series. Since rq lock is held here, rcu_dereference() should not be
> necessary.
Thank you for reporting this. I'll make sure to run with LOCKDEP next
time around. Note: The performance aspect is still quite bad with this
series an the intent for the RFC was to vet the idea and to understand
if I got the basic implementation details right.
>
>> + cpumask_var_t overloaded_mask;
>> +
>> + if (!sd_share)
>> + return;
>> +
>> + overloaded_mask = sd_share->overloaded_mask;
>> + if (!overloaded_mask)
>> + return;
>> +
>> + if (contains_pushable)
>> + cpumask_set_cpu(cpu, overloaded_mask);
>> + else
>> + cpumask_clear_cpu(cpu, overloaded_mask);
>> +}
--
Thanks and Regards,
Prateek
© 2016 - 2025 Red Hat, Inc.