* [PATCH v2 1/2 RESEND] sched/fair: take into account scheduling domain in select_idle_smt()
@ 2024-01-10 13:17 Keisuke Nishimura
2024-01-10 13:17 ` [PATCH v2 2/2 RESEND] sched/fair: take into account scheduling domain in select_idle_core() Keisuke Nishimura
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Keisuke Nishimura @ 2024-01-10 13:17 UTC (permalink / raw
To: Ingo Molnar, Peter Zijlstra, Vincent Guittot
Cc: linux-kernel, Dietmar Eggemann, Mel Gorman, Valentin Schneider,
Julia Lawall, Xunlei Pang, Abel Wu, Keisuke Nishimura
When picking out a CPU on a task wakeup, select_idle_smt() has to take
into account the scheduling domain of @target. This is because the
"isolcpus" kernel command line option can remove CPUs from the domain to
isolate them from other SMT siblings.
This fix checks if the candidate CPU is in the target scheduling domain.
The commit df3cb4ea1fb6 ("sched/fair: Fix wrong cpu selecting from isolated
domain") originally proposed this fix by adding the check of the scheduling
domain in the loop. However, the commit 3e6efe87cd5cc ("sched/fair: Remove
redundant check in select_idle_smt()") accidentally removed the check.
This commit brings the check back.
Fixes: 3e6efe87cd5c ("sched/fair: Remove redundant check in select_idle_smt()")
Signed-off-by: Keisuke Nishimura <keisuke.nishimura@inria.fr>
Signed-off-by: Julia Lawall <julia.lawall@inria.fr>
---
v2: - Changed the log message to mention only isolcpus
- Moved the check in the loop according to the original fix
kernel/sched/fair.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 533547e3c90a..66457d4b8965 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7311,13 +7311,19 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu
/*
* Scan the local SMT mask for idle CPUs.
*/
-static int select_idle_smt(struct task_struct *p, int target)
+static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target)
{
int cpu;
for_each_cpu_and(cpu, cpu_smt_mask(target), p->cpus_ptr) {
if (cpu == target)
continue;
+ /*
+ * Check if the CPU is in the LLC scheduling domain of @target.
+ * Due to isolcpus, there is no guarantee that all the siblings are in the domain.
+ */
+ if (!cpumask_test_cpu(cpu, sched_domain_span(sd)))
+ continue;
if (available_idle_cpu(cpu) || sched_idle_cpu(cpu))
return cpu;
}
@@ -7341,7 +7347,7 @@ static inline int select_idle_core(struct task_struct *p, int core, struct cpuma
return __select_idle_cpu(core, p);
}
-static inline int select_idle_smt(struct task_struct *p, int target)
+static inline int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target)
{
return -1;
}
@@ -7591,7 +7597,7 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
has_idle_core = test_idle_cores(target);
if (!has_idle_core && cpus_share_cache(prev, target)) {
- i = select_idle_smt(p, prev);
+ i = select_idle_smt(p, sd, prev);
if ((unsigned int)i < nr_cpumask_bits)
return i;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2 RESEND] sched/fair: take into account scheduling domain in select_idle_core()
2024-01-10 13:17 [PATCH v2 1/2 RESEND] sched/fair: take into account scheduling domain in select_idle_smt() Keisuke Nishimura
@ 2024-01-10 13:17 ` Keisuke Nishimura
2024-01-10 17:16 ` Vincent Guittot
2024-02-28 22:00 ` [tip: sched/core] sched/fair: Take the scheduling domain into account " tip-bot2 for Keisuke Nishimura
2024-01-10 17:21 ` [PATCH v2 1/2 RESEND] sched/fair: take into account scheduling domain in select_idle_smt() Vincent Guittot
2024-02-28 22:00 ` [tip: sched/core] sched/fair: Take the scheduling domain into account " tip-bot2 for Keisuke Nishimura
2 siblings, 2 replies; 8+ messages in thread
From: Keisuke Nishimura @ 2024-01-10 13:17 UTC (permalink / raw
To: Ingo Molnar, Peter Zijlstra, Vincent Guittot
Cc: linux-kernel, Dietmar Eggemann, Mel Gorman, Valentin Schneider,
Julia Lawall, Xunlei Pang, Abel Wu, Keisuke Nishimura
When picking out a CPU on a task wakeup, select_idle_core() has to take
into account the scheduling domain where the function looks for the CPU.
This is because the "isolcpus" kernel command line option can remove CPUs
from the domain to isolate them from other SMT siblings.
This change replaces the set of CPUs allowed to run the task from
p->cpus_ptr by the intersection of p->cpus_ptr and sched_domain_span(sd)
which is stored in the cpus argument provided by select_idle_cpu.
Fixes: 9fe1f127b913 ("sched/fair: Merge select_idle_core/cpu()")
Signed-off-by: Keisuke Nishimura <keisuke.nishimura@inria.fr>
Signed-off-by: Julia Lawall <julia.lawall@inria.fr>
---
v2: - Changed the log message to mention only isolcpus
kernel/sched/fair.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 66457d4b8965..e2b4e0396af8 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7289,7 +7289,7 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu
if (!available_idle_cpu(cpu)) {
idle = false;
if (*idle_cpu == -1) {
- if (sched_idle_cpu(cpu) && cpumask_test_cpu(cpu, p->cpus_ptr)) {
+ if (sched_idle_cpu(cpu) && cpumask_test_cpu(cpu, cpus)) {
*idle_cpu = cpu;
break;
}
@@ -7297,7 +7297,7 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu
}
break;
}
- if (*idle_cpu == -1 && cpumask_test_cpu(cpu, p->cpus_ptr))
+ if (*idle_cpu == -1 && cpumask_test_cpu(cpu, cpus))
*idle_cpu = cpu;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2 RESEND] sched/fair: take into account scheduling domain in select_idle_core()
2024-01-10 13:17 ` [PATCH v2 2/2 RESEND] sched/fair: take into account scheduling domain in select_idle_core() Keisuke Nishimura
@ 2024-01-10 17:16 ` Vincent Guittot
2024-02-28 22:00 ` [tip: sched/core] sched/fair: Take the scheduling domain into account " tip-bot2 for Keisuke Nishimura
1 sibling, 0 replies; 8+ messages in thread
From: Vincent Guittot @ 2024-01-10 17:16 UTC (permalink / raw
To: Keisuke Nishimura
Cc: Ingo Molnar, Peter Zijlstra, linux-kernel, Dietmar Eggemann,
Mel Gorman, Valentin Schneider, Julia Lawall, Xunlei Pang,
Abel Wu
On Wed, 10 Jan 2024 at 14:19, Keisuke Nishimura
<keisuke.nishimura@inria.fr> wrote:
>
> When picking out a CPU on a task wakeup, select_idle_core() has to take
> into account the scheduling domain where the function looks for the CPU.
> This is because the "isolcpus" kernel command line option can remove CPUs
> from the domain to isolate them from other SMT siblings.
>
> This change replaces the set of CPUs allowed to run the task from
> p->cpus_ptr by the intersection of p->cpus_ptr and sched_domain_span(sd)
> which is stored in the cpus argument provided by select_idle_cpu.
>
> Fixes: 9fe1f127b913 ("sched/fair: Merge select_idle_core/cpu()")
> Signed-off-by: Keisuke Nishimura <keisuke.nishimura@inria.fr>
> Signed-off-by: Julia Lawall <julia.lawall@inria.fr>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
> v2: - Changed the log message to mention only isolcpus
>
> kernel/sched/fair.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 66457d4b8965..e2b4e0396af8 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7289,7 +7289,7 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu
> if (!available_idle_cpu(cpu)) {
> idle = false;
> if (*idle_cpu == -1) {
> - if (sched_idle_cpu(cpu) && cpumask_test_cpu(cpu, p->cpus_ptr)) {
> + if (sched_idle_cpu(cpu) && cpumask_test_cpu(cpu, cpus)) {
> *idle_cpu = cpu;
> break;
> }
> @@ -7297,7 +7297,7 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu
> }
> break;
> }
> - if (*idle_cpu == -1 && cpumask_test_cpu(cpu, p->cpus_ptr))
> + if (*idle_cpu == -1 && cpumask_test_cpu(cpu, cpus))
> *idle_cpu = cpu;
> }
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2 RESEND] sched/fair: take into account scheduling domain in select_idle_smt()
2024-01-10 13:17 [PATCH v2 1/2 RESEND] sched/fair: take into account scheduling domain in select_idle_smt() Keisuke Nishimura
2024-01-10 13:17 ` [PATCH v2 2/2 RESEND] sched/fair: take into account scheduling domain in select_idle_core() Keisuke Nishimura
@ 2024-01-10 17:21 ` Vincent Guittot
2024-01-10 17:57 ` Keisuke Nishimura
2024-02-28 22:00 ` [tip: sched/core] sched/fair: Take the scheduling domain into account " tip-bot2 for Keisuke Nishimura
2 siblings, 1 reply; 8+ messages in thread
From: Vincent Guittot @ 2024-01-10 17:21 UTC (permalink / raw
To: Keisuke Nishimura
Cc: Ingo Molnar, Peter Zijlstra, linux-kernel, Dietmar Eggemann,
Mel Gorman, Valentin Schneider, Julia Lawall, Xunlei Pang,
Abel Wu
On Wed, 10 Jan 2024 at 14:19, Keisuke Nishimura
<keisuke.nishimura@inria.fr> wrote:
>
> When picking out a CPU on a task wakeup, select_idle_smt() has to take
> into account the scheduling domain of @target. This is because the
> "isolcpus" kernel command line option can remove CPUs from the domain to
> isolate them from other SMT siblings.
>
> This fix checks if the candidate CPU is in the target scheduling domain.
>
> The commit df3cb4ea1fb6 ("sched/fair: Fix wrong cpu selecting from isolated
> domain") originally proposed this fix by adding the check of the scheduling
> domain in the loop. However, the commit 3e6efe87cd5cc ("sched/fair: Remove
> redundant check in select_idle_smt()") accidentally removed the check.
> This commit brings the check back.
>
> Fixes: 3e6efe87cd5c ("sched/fair: Remove redundant check in select_idle_smt()")
> Signed-off-by: Keisuke Nishimura <keisuke.nishimura@inria.fr>
> Signed-off-by: Julia Lawall <julia.lawall@inria.fr>
> ---
> v2: - Changed the log message to mention only isolcpus
> - Moved the check in the loop according to the original fix
>
> kernel/sched/fair.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 533547e3c90a..66457d4b8965 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7311,13 +7311,19 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu
> /*
> * Scan the local SMT mask for idle CPUs.
> */
> -static int select_idle_smt(struct task_struct *p, int target)
> +static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target)
> {
> int cpu;
>
> for_each_cpu_and(cpu, cpu_smt_mask(target), p->cpus_ptr) {
> if (cpu == target)
> continue;
> + /*
> + * Check if the CPU is in the LLC scheduling domain of @target.
> + * Due to isolcpus, there is no guarantee that all the siblings are in the domain.
> + */
> + if (!cpumask_test_cpu(cpu, sched_domain_span(sd)))
commit df3cb4ea1fb6 ("sched/fair: Fix wrong cpu selecting from isolated domain")
also checked if (!cpumask_test_cpu(cpu, p->cpus_ptr) ||
Why didn't you also re-add this test ?
> + continue;
> if (available_idle_cpu(cpu) || sched_idle_cpu(cpu))
> return cpu;
> }
> @@ -7341,7 +7347,7 @@ static inline int select_idle_core(struct task_struct *p, int core, struct cpuma
> return __select_idle_cpu(core, p);
> }
>
> -static inline int select_idle_smt(struct task_struct *p, int target)
> +static inline int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target)
> {
> return -1;
> }
> @@ -7591,7 +7597,7 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
> has_idle_core = test_idle_cores(target);
>
> if (!has_idle_core && cpus_share_cache(prev, target)) {
> - i = select_idle_smt(p, prev);
> + i = select_idle_smt(p, sd, prev);
> if ((unsigned int)i < nr_cpumask_bits)
> return i;
> }
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2 RESEND] sched/fair: take into account scheduling domain in select_idle_smt()
2024-01-10 17:21 ` [PATCH v2 1/2 RESEND] sched/fair: take into account scheduling domain in select_idle_smt() Vincent Guittot
@ 2024-01-10 17:57 ` Keisuke Nishimura
2024-01-11 8:52 ` Vincent Guittot
0 siblings, 1 reply; 8+ messages in thread
From: Keisuke Nishimura @ 2024-01-10 17:57 UTC (permalink / raw
To: Vincent Guittot
Cc: Ingo Molnar, Peter Zijlstra, linux-kernel, Dietmar Eggemann,
Mel Gorman, Valentin Schneider, Julia Lawall, Xunlei Pang,
Abel Wu
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 533547e3c90a..66457d4b8965 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -7311,13 +7311,19 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu
>> /*
>> * Scan the local SMT mask for idle CPUs.
>> */
>> -static int select_idle_smt(struct task_struct *p, int target)
>> +static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target)
>> {
>> int cpu;
>>
>> for_each_cpu_and(cpu, cpu_smt_mask(target), p->cpus_ptr) {
>> if (cpu == target)
>> continue;
>> + /*
>> + * Check if the CPU is in the LLC scheduling domain of @target.
>> + * Due to isolcpus, there is no guarantee that all the siblings are in the domain.
>> + */
>> + if (!cpumask_test_cpu(cpu, sched_domain_span(sd)))
>
> commit df3cb4ea1fb6 ("sched/fair: Fix wrong cpu selecting from isolated domain")
> also checked if (!cpumask_test_cpu(cpu, p->cpus_ptr) ||
>
> Why didn't you also re-add this test ?
>
>
Thank you for your comment. This is because the iterator "for_each_cpu_and(cpu,
cpu_smt_mask(target), p->cpus_ptr)" ensures that cpu is included in p->cpus_ptr.
The iterator has changed. FYI, here is the change made in commit df3cb4ea1fb6:
for_each_cpu(cpu, cpu_smt_mask(target)) {
- if (!cpumask_test_cpu(cpu, p->cpus_ptr))
+ if (!cpumask_test_cpu(cpu, p->cpus_ptr) ||
+ !cpumask_test_cpu(cpu, sched_domain_span(sd)))
continue;
if (available_idle_cpu(cpu) || sched_idle_cpu(cpu))
best,
Keisuke
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2 RESEND] sched/fair: take into account scheduling domain in select_idle_smt()
2024-01-10 17:57 ` Keisuke Nishimura
@ 2024-01-11 8:52 ` Vincent Guittot
0 siblings, 0 replies; 8+ messages in thread
From: Vincent Guittot @ 2024-01-11 8:52 UTC (permalink / raw
To: Keisuke Nishimura
Cc: Ingo Molnar, Peter Zijlstra, linux-kernel, Dietmar Eggemann,
Mel Gorman, Valentin Schneider, Julia Lawall, Xunlei Pang,
Abel Wu
On Wed, 10 Jan 2024 at 18:57, Keisuke Nishimura
<keisuke.nishimura@inria.fr> wrote:
>
> >>
> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> >> index 533547e3c90a..66457d4b8965 100644
> >> --- a/kernel/sched/fair.c
> >> +++ b/kernel/sched/fair.c
> >> @@ -7311,13 +7311,19 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu
> >> /*
> >> * Scan the local SMT mask for idle CPUs.
> >> */
> >> -static int select_idle_smt(struct task_struct *p, int target)
> >> +static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target)
> >> {
> >> int cpu;
> >>
> >> for_each_cpu_and(cpu, cpu_smt_mask(target), p->cpus_ptr) {
> >> if (cpu == target)
> >> continue;
> >> + /*
> >> + * Check if the CPU is in the LLC scheduling domain of @target.
> >> + * Due to isolcpus, there is no guarantee that all the siblings are in the domain.
> >> + */
> >> + if (!cpumask_test_cpu(cpu, sched_domain_span(sd)))
> >
> > commit df3cb4ea1fb6 ("sched/fair: Fix wrong cpu selecting from isolated domain")
> > also checked if (!cpumask_test_cpu(cpu, p->cpus_ptr) ||
> >
> > Why didn't you also re-add this test ?
> >
> >
>
> Thank you for your comment. This is because the iterator "for_each_cpu_and(cpu,
> cpu_smt_mask(target), p->cpus_ptr)" ensures that cpu is included in p->cpus_ptr.
Ah yes indeed. I didn't notice that it was now included in the for_each_cpu_and
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
>
> The iterator has changed. FYI, here is the change made in commit df3cb4ea1fb6:
>
> for_each_cpu(cpu, cpu_smt_mask(target)) {
> - if (!cpumask_test_cpu(cpu, p->cpus_ptr))
> + if (!cpumask_test_cpu(cpu, p->cpus_ptr) ||
> + !cpumask_test_cpu(cpu, sched_domain_span(sd)))
> continue;
> if (available_idle_cpu(cpu) || sched_idle_cpu(cpu))
>
> best,
> Keisuke
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip: sched/core] sched/fair: Take the scheduling domain into account in select_idle_core()
2024-01-10 13:17 ` [PATCH v2 2/2 RESEND] sched/fair: take into account scheduling domain in select_idle_core() Keisuke Nishimura
2024-01-10 17:16 ` Vincent Guittot
@ 2024-02-28 22:00 ` tip-bot2 for Keisuke Nishimura
1 sibling, 0 replies; 8+ messages in thread
From: tip-bot2 for Keisuke Nishimura @ 2024-02-28 22:00 UTC (permalink / raw
To: linux-tip-commits
Cc: Keisuke Nishimura, Julia Lawall, Ingo Molnar, x86, linux-kernel
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 23d04d8c6b8ec339057264659b7834027f3e6a63
Gitweb: https://git.kernel.org/tip/23d04d8c6b8ec339057264659b7834027f3e6a63
Author: Keisuke Nishimura <keisuke.nishimura@inria.fr>
AuthorDate: Wed, 10 Jan 2024 14:17:07 +01:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 28 Feb 2024 15:15:49 +01:00
sched/fair: Take the scheduling domain into account in select_idle_core()
When picking a CPU on task wakeup, select_idle_core() has to take
into account the scheduling domain where the function looks for the CPU.
This is because the "isolcpus" kernel command line option can remove CPUs
from the domain to isolate them from other SMT siblings.
This change replaces the set of CPUs allowed to run the task from
p->cpus_ptr by the intersection of p->cpus_ptr and sched_domain_span(sd)
which is stored in the 'cpus' argument provided by select_idle_cpu().
Fixes: 9fe1f127b913 ("sched/fair: Merge select_idle_core/cpu()")
Signed-off-by: Keisuke Nishimura <keisuke.nishimura@inria.fr>
Signed-off-by: Julia Lawall <julia.lawall@inria.fr>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20240110131707.437301-2-keisuke.nishimura@inria.fr
---
kernel/sched/fair.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 005f6d3..352222d 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7289,7 +7289,7 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu
if (!available_idle_cpu(cpu)) {
idle = false;
if (*idle_cpu == -1) {
- if (sched_idle_cpu(cpu) && cpumask_test_cpu(cpu, p->cpus_ptr)) {
+ if (sched_idle_cpu(cpu) && cpumask_test_cpu(cpu, cpus)) {
*idle_cpu = cpu;
break;
}
@@ -7297,7 +7297,7 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu
}
break;
}
- if (*idle_cpu == -1 && cpumask_test_cpu(cpu, p->cpus_ptr))
+ if (*idle_cpu == -1 && cpumask_test_cpu(cpu, cpus))
*idle_cpu = cpu;
}
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [tip: sched/core] sched/fair: Take the scheduling domain into account in select_idle_smt()
2024-01-10 13:17 [PATCH v2 1/2 RESEND] sched/fair: take into account scheduling domain in select_idle_smt() Keisuke Nishimura
2024-01-10 13:17 ` [PATCH v2 2/2 RESEND] sched/fair: take into account scheduling domain in select_idle_core() Keisuke Nishimura
2024-01-10 17:21 ` [PATCH v2 1/2 RESEND] sched/fair: take into account scheduling domain in select_idle_smt() Vincent Guittot
@ 2024-02-28 22:00 ` tip-bot2 for Keisuke Nishimura
2 siblings, 0 replies; 8+ messages in thread
From: tip-bot2 for Keisuke Nishimura @ 2024-02-28 22:00 UTC (permalink / raw
To: linux-tip-commits
Cc: Keisuke Nishimura, Julia Lawall, Ingo Molnar, Vincent Guittot,
x86, linux-kernel
The following commit has been merged into the sched/core branch of tip:
Commit-ID: 8aeaffef8c6eceab0e1498486fdd4f3dc3b7066c
Gitweb: https://git.kernel.org/tip/8aeaffef8c6eceab0e1498486fdd4f3dc3b7066c
Author: Keisuke Nishimura <keisuke.nishimura@inria.fr>
AuthorDate: Wed, 10 Jan 2024 14:17:06 +01:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Wed, 28 Feb 2024 15:15:48 +01:00
sched/fair: Take the scheduling domain into account in select_idle_smt()
When picking a CPU on task wakeup, select_idle_smt() has to take
into account the scheduling domain of @target. This is because the
"isolcpus" kernel command line option can remove CPUs from the domain to
isolate them from other SMT siblings.
This fix checks if the candidate CPU is in the target scheduling domain.
Commit:
df3cb4ea1fb6 ("sched/fair: Fix wrong cpu selecting from isolated domain")
... originally introduced this fix by adding the check of the scheduling
domain in the loop.
However, commit:
3e6efe87cd5cc ("sched/fair: Remove redundant check in select_idle_smt()")
... accidentally removed the check. Bring it back.
Fixes: 3e6efe87cd5c ("sched/fair: Remove redundant check in select_idle_smt()")
Signed-off-by: Keisuke Nishimura <keisuke.nishimura@inria.fr>
Signed-off-by: Julia Lawall <julia.lawall@inria.fr>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://lore.kernel.org/r/20240110131707.437301-1-keisuke.nishimura@inria.fr
---
kernel/sched/fair.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index ba36339..005f6d3 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7311,13 +7311,19 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu
/*
* Scan the local SMT mask for idle CPUs.
*/
-static int select_idle_smt(struct task_struct *p, int target)
+static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target)
{
int cpu;
for_each_cpu_and(cpu, cpu_smt_mask(target), p->cpus_ptr) {
if (cpu == target)
continue;
+ /*
+ * Check if the CPU is in the LLC scheduling domain of @target.
+ * Due to isolcpus, there is no guarantee that all the siblings are in the domain.
+ */
+ if (!cpumask_test_cpu(cpu, sched_domain_span(sd)))
+ continue;
if (available_idle_cpu(cpu) || sched_idle_cpu(cpu))
return cpu;
}
@@ -7341,7 +7347,7 @@ static inline int select_idle_core(struct task_struct *p, int core, struct cpuma
return __select_idle_cpu(core, p);
}
-static inline int select_idle_smt(struct task_struct *p, int target)
+static inline int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target)
{
return -1;
}
@@ -7591,7 +7597,7 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
has_idle_core = test_idle_cores(target);
if (!has_idle_core && cpus_share_cache(prev, target)) {
- i = select_idle_smt(p, prev);
+ i = select_idle_smt(p, sd, prev);
if ((unsigned int)i < nr_cpumask_bits)
return i;
}
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-02-28 22:00 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-10 13:17 [PATCH v2 1/2 RESEND] sched/fair: take into account scheduling domain in select_idle_smt() Keisuke Nishimura
2024-01-10 13:17 ` [PATCH v2 2/2 RESEND] sched/fair: take into account scheduling domain in select_idle_core() Keisuke Nishimura
2024-01-10 17:16 ` Vincent Guittot
2024-02-28 22:00 ` [tip: sched/core] sched/fair: Take the scheduling domain into account " tip-bot2 for Keisuke Nishimura
2024-01-10 17:21 ` [PATCH v2 1/2 RESEND] sched/fair: take into account scheduling domain in select_idle_smt() Vincent Guittot
2024-01-10 17:57 ` Keisuke Nishimura
2024-01-11 8:52 ` Vincent Guittot
2024-02-28 22:00 ` [tip: sched/core] sched/fair: Take the scheduling domain into account " tip-bot2 for Keisuke Nishimura
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 read-only IMAP folder(s) and NNTP newsgroup(s).