linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] sched/fair: Don't pass sd to select_idle_smt()
@ 2019-02-07 10:46 Viresh Kumar
  2019-02-07 10:46 ` [PATCH 2/2] sched/fair: Improve the for loop in select_idle_core() Viresh Kumar
  2019-02-11 10:54 ` [tip:sched/core] sched/fair: Remove unused 'sd' parameter from select_idle_smt() tip-bot for Viresh Kumar
  0 siblings, 2 replies; 6+ messages in thread
From: Viresh Kumar @ 2019-02-07 10:46 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra; +Cc: Viresh Kumar, Vincent Guittot, linux-kernel

The parameter sd isn't getting used in select_idle_smt(), drop it.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 kernel/sched/fair.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 38d4669aa2ef..8d5c82342a36 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6102,7 +6102,7 @@ static int select_idle_core(struct task_struct *p, struct sched_domain *sd, int
 /*
  * Scan the local SMT mask for idle CPUs.
  */
-static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target)
+static int select_idle_smt(struct task_struct *p, int target)
 {
 	int cpu;
 
@@ -6126,7 +6126,7 @@ static inline int select_idle_core(struct task_struct *p, struct sched_domain *s
 	return -1;
 }
 
-static inline int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target)
+static inline int select_idle_smt(struct task_struct *p, int target)
 {
 	return -1;
 }
@@ -6231,7 +6231,7 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
 	if ((unsigned)i < nr_cpumask_bits)
 		return i;
 
-	i = select_idle_smt(p, sd, target);
+	i = select_idle_smt(p, target);
 	if ((unsigned)i < nr_cpumask_bits)
 		return i;
 
-- 
2.20.1.321.g9e740568ce00


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/2] sched/fair: Improve the for loop in select_idle_core()
  2019-02-07 10:46 [PATCH 1/2] sched/fair: Don't pass sd to select_idle_smt() Viresh Kumar
@ 2019-02-07 10:46 ` Viresh Kumar
  2019-02-11  9:30   ` Peter Zijlstra
  2019-02-11 10:54 ` [tip:sched/core] sched/fair: Remove unused 'sd' parameter from select_idle_smt() tip-bot for Viresh Kumar
  1 sibling, 1 reply; 6+ messages in thread
From: Viresh Kumar @ 2019-02-07 10:46 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra; +Cc: Viresh Kumar, Vincent Guittot, linux-kernel

Once a non-idle thread is found for a core, there is no point in
traversing rest of the threads of that core. We continue traversal
currently to clear those threads from "cpus" mask. Clear all the
threads with a single call to cpumask_andnot(), which will also let us
exit the loop earlier.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 kernel/sched/fair.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 8d5c82342a36..ccd0ae9878a2 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6068,6 +6068,7 @@ void __update_idle_core(struct rq *rq)
 static int select_idle_core(struct task_struct *p, struct sched_domain *sd, int target)
 {
 	struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_idle_mask);
+	const struct cpumask *smt;
 	int core, cpu;
 
 	if (!static_branch_likely(&sched_smt_present))
@@ -6081,10 +6082,14 @@ static int select_idle_core(struct task_struct *p, struct sched_domain *sd, int
 	for_each_cpu_wrap(core, cpus, target) {
 		bool idle = true;
 
-		for_each_cpu(cpu, cpu_smt_mask(core)) {
-			cpumask_clear_cpu(cpu, cpus);
-			if (!available_idle_cpu(cpu))
+		smt = cpu_smt_mask(core);
+		cpumask_andnot(cpus, cpus, smt);
+
+		for_each_cpu(cpu, smt) {
+			if (!available_idle_cpu(cpu)) {
 				idle = false;
+				break;
+			}
 		}
 
 		if (idle)
-- 
2.20.1.321.g9e740568ce00


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] sched/fair: Improve the for loop in select_idle_core()
  2019-02-07 10:46 ` [PATCH 2/2] sched/fair: Improve the for loop in select_idle_core() Viresh Kumar
@ 2019-02-11  9:30   ` Peter Zijlstra
  2019-02-11 10:26     ` Viresh Kumar
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2019-02-11  9:30 UTC (permalink / raw)
  To: Viresh Kumar; +Cc: Ingo Molnar, Vincent Guittot, linux-kernel

On Thu, Feb 07, 2019 at 04:16:06PM +0530, Viresh Kumar wrote:
> @@ -6081,10 +6082,14 @@ static int select_idle_core(struct task_struct *p, struct sched_domain *sd, int
>  	for_each_cpu_wrap(core, cpus, target) {
>  		bool idle = true;
>  
> -		for_each_cpu(cpu, cpu_smt_mask(core)) {
> -			cpumask_clear_cpu(cpu, cpus);
> -			if (!available_idle_cpu(cpu))
> +		smt = cpu_smt_mask(core);
> +		cpumask_andnot(cpus, cpus, smt);

So where the previous code was like 1-2 stores, you just added 16.

(assuming 64bit and NR_CPUS=1024)

And we still do the iteration anyway:

> +		for_each_cpu(cpu, smt) {
> +			if (!available_idle_cpu(cpu)) {
>  				idle = false;
> +				break;
> +			}
>  		}

An actual improvement would've been:

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 38d4669aa2ef..2d352d6d15c7 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6082,7 +6082,7 @@ static int select_idle_core(struct task_struct *p, struct sched_domain *sd, int
 		bool idle = true;
 
 		for_each_cpu(cpu, cpu_smt_mask(core)) {
-			cpumask_clear_cpu(cpu, cpus);
+			__cpumask_clear_cpu(cpu, cpus);
 			if (!available_idle_cpu(cpu))
 				idle = false;
 		}

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] sched/fair: Improve the for loop in select_idle_core()
  2019-02-11  9:30   ` Peter Zijlstra
@ 2019-02-11 10:26     ` Viresh Kumar
  2019-02-11 10:44       ` Peter Zijlstra
  0 siblings, 1 reply; 6+ messages in thread
From: Viresh Kumar @ 2019-02-11 10:26 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Ingo Molnar, Vincent Guittot, linux-kernel

On 11-02-19, 10:30, Peter Zijlstra wrote:
> On Thu, Feb 07, 2019 at 04:16:06PM +0530, Viresh Kumar wrote:
> > @@ -6081,10 +6082,14 @@ static int select_idle_core(struct task_struct *p, struct sched_domain *sd, int
> >  	for_each_cpu_wrap(core, cpus, target) {
> >  		bool idle = true;
> >  
> > -		for_each_cpu(cpu, cpu_smt_mask(core)) {
> > -			cpumask_clear_cpu(cpu, cpus);
> > -			if (!available_idle_cpu(cpu))
> > +		smt = cpu_smt_mask(core);
> > +		cpumask_andnot(cpus, cpus, smt);
> 
> So where the previous code was like 1-2 stores, you just added 16.

Is the max number of possible threads per core just 2? That's what I
read just now and I wasn't aware of that earlier. This commit doesn't
improve anything then. Sorry for the noise.

-- 
viresh

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] sched/fair: Improve the for loop in select_idle_core()
  2019-02-11 10:26     ` Viresh Kumar
@ 2019-02-11 10:44       ` Peter Zijlstra
  0 siblings, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2019-02-11 10:44 UTC (permalink / raw)
  To: Viresh Kumar; +Cc: Ingo Molnar, Vincent Guittot, linux-kernel

On Mon, Feb 11, 2019 at 03:56:59PM +0530, Viresh Kumar wrote:
> On 11-02-19, 10:30, Peter Zijlstra wrote:
> > On Thu, Feb 07, 2019 at 04:16:06PM +0530, Viresh Kumar wrote:
> > > @@ -6081,10 +6082,14 @@ static int select_idle_core(struct task_struct *p, struct sched_domain *sd, int
> > >  	for_each_cpu_wrap(core, cpus, target) {
> > >  		bool idle = true;
> > >  
> > > -		for_each_cpu(cpu, cpu_smt_mask(core)) {
> > > -			cpumask_clear_cpu(cpu, cpus);
> > > -			if (!available_idle_cpu(cpu))
> > > +		smt = cpu_smt_mask(core);
> > > +		cpumask_andnot(cpus, cpus, smt);
> > 
> > So where the previous code was like 1-2 stores, you just added 16.
> 
> Is the max number of possible threads per core just 2? That's what I
> read just now and I wasn't aware of that earlier. This commit doesn't
> improve anything then. Sorry for the noise.

We've got up to SMT8 in the tree (Sparc64, Power8 and some MIPS IIRC),
but that's still less than having to touch the entire bitmap.

Also, Power9 went back to SMT4 and I think the majory of SMT deployments
is that or less.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [tip:sched/core] sched/fair: Remove unused 'sd' parameter from select_idle_smt()
  2019-02-07 10:46 [PATCH 1/2] sched/fair: Don't pass sd to select_idle_smt() Viresh Kumar
  2019-02-07 10:46 ` [PATCH 2/2] sched/fair: Improve the for loop in select_idle_core() Viresh Kumar
@ 2019-02-11 10:54 ` tip-bot for Viresh Kumar
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Viresh Kumar @ 2019-02-11 10:54 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, mingo, tglx, vincent.guittot, viresh.kumar, hpa,
	torvalds, linux-kernel

Commit-ID:  1b5500d73466c62fe048153f0cea1610d2543c7f
Gitweb:     https://git.kernel.org/tip/1b5500d73466c62fe048153f0cea1610d2543c7f
Author:     Viresh Kumar <viresh.kumar@linaro.org>
AuthorDate: Thu, 7 Feb 2019 16:16:05 +0530
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 11 Feb 2019 08:48:27 +0100

sched/fair: Remove unused 'sd' parameter from select_idle_smt()

The 'sd' parameter isn't getting used in select_idle_smt(), drop it.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Link: http://lkml.kernel.org/r/f91c5e118183e79d4a982e9ac4ce5e47948f6c1b.1549536337.git.viresh.kumar@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/fair.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index f44da9b491ff..8abd1c271499 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6117,7 +6117,7 @@ static int select_idle_core(struct task_struct *p, struct sched_domain *sd, int
 /*
  * Scan the local SMT mask for idle CPUs.
  */
-static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target)
+static int select_idle_smt(struct task_struct *p, int target)
 {
 	int cpu;
 
@@ -6141,7 +6141,7 @@ static inline int select_idle_core(struct task_struct *p, struct sched_domain *s
 	return -1;
 }
 
-static inline int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target)
+static inline int select_idle_smt(struct task_struct *p, int target)
 {
 	return -1;
 }
@@ -6246,7 +6246,7 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
 	if ((unsigned)i < nr_cpumask_bits)
 		return i;
 
-	i = select_idle_smt(p, sd, target);
+	i = select_idle_smt(p, target);
 	if ((unsigned)i < nr_cpumask_bits)
 		return i;
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-11 10:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-07 10:46 [PATCH 1/2] sched/fair: Don't pass sd to select_idle_smt() Viresh Kumar
2019-02-07 10:46 ` [PATCH 2/2] sched/fair: Improve the for loop in select_idle_core() Viresh Kumar
2019-02-11  9:30   ` Peter Zijlstra
2019-02-11 10:26     ` Viresh Kumar
2019-02-11 10:44       ` Peter Zijlstra
2019-02-11 10:54 ` [tip:sched/core] sched/fair: Remove unused 'sd' parameter from select_idle_smt() tip-bot for Viresh Kumar

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 NNTP newsgroup(s).