2020-08-04 00:08:11

by Josh Don

Subject: [PATCH] sched/fair: ignore cache hotness for SMT migration

SMT siblings share caches, so cache hotness should be irrelevant for
cross-sibling migration.

Proposed-by: Venkatesh Pallipadi <[email protected]>
Signed-off-by: Josh Don <[email protected]>
---
kernel/sched/fair.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 2ba8f230feb9..5b203b55bcb2 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7402,6 +7402,9 @@ static int task_hot(struct task_struct *p, struct lb_env *env)
if (unlikely(task_has_idle_policy(p)))
return 0;

+ if ((env->sd->flags & cpu_smt_flags()) == cpu_smt_flags())
+ return 0;
+
/*
* Buddy candidates are cache hot:
*/
--
2.28.0.163.g6104cc2f0b6-goog

2020-08-04 10:59:39

by Peter Zijlstra

Subject: Re: [PATCH] sched/fair: ignore cache hotness for SMT migration

On Mon, Aug 03, 2020 at 05:06:14PM -0700, Josh Don wrote:
> SMT siblings share caches, so cache hotness should be irrelevant for
> cross-sibling migration.
>
> Proposed-by: Venkatesh Pallipadi <[email protected]>
> Signed-off-by: Josh Don <[email protected]>
> ---
> kernel/sched/fair.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 2ba8f230feb9..5b203b55bcb2 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7402,6 +7402,9 @@ static int task_hot(struct task_struct *p, struct lb_env *env)
> if (unlikely(task_has_idle_policy(p)))
> return 0;
>
> + if ((env->sd->flags & cpu_smt_flags()) == cpu_smt_flags())
> + return 0;

I think that wants to be:

if (env->sd->flags & SD_SHARE_CPUCAPACITY)

Also, perhaps stick a comment on top with the rationale for this.

> +
> /*
> * Buddy candidates are cache hot:
> */
> --
> 2.28.0.163.g6104cc2f0b6-goog
>

2020-08-04 19:26:13

by Josh Don

Subject: Re: [PATCH] sched/fair: ignore cache hotness for SMT migration

On Tue, Aug 4, 2020 at 3:56 AM <[email protected]> wrote:
>
> On Mon, Aug 03, 2020 at 05:06:14PM -0700, Josh Don wrote:
> > SMT siblings share caches, so cache hotness should be irrelevant for
> > cross-sibling migration.
> >
> > Proposed-by: Venkatesh Pallipadi <[email protected]>
> > Signed-off-by: Josh Don <[email protected]>
> > ---
> > kernel/sched/fair.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 2ba8f230feb9..5b203b55bcb2 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -7402,6 +7402,9 @@ static int task_hot(struct task_struct *p, struct lb_env *env)
> > if (unlikely(task_has_idle_policy(p)))
> > return 0;
> >
> > + if ((env->sd->flags & cpu_smt_flags()) == cpu_smt_flags())
> > + return 0;
>
> I think that wants to be:
>
> if (env->sd->flags & SD_SHARE_CPUCAPACITY)
>

Agreed

> Also, perhaps stick a comment on top with the rationale for this.
>
> > +
> > /*
> > * Buddy candidates are cache hot:
> > */
> > --
> > 2.28.0.163.g6104cc2f0b6-goog
> >

2020-08-04 19:34:47

by Josh Don

Subject: [PATCH v2] sched/fair: ignore cache hotness for SMT migration

SMT siblings share caches, so cache hotness should be irrelevant for
cross-sibling migration.

Proposed-by: Venkatesh Pallipadi <[email protected]>
Signed-off-by: Josh Don <[email protected]>
---
kernel/sched/fair.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 1a68a0536add..abdb54e2339f 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7402,6 +7402,10 @@ static int task_hot(struct task_struct *p, struct lb_env *env)
if (unlikely(task_has_idle_policy(p)))
return 0;

+ /* SMT siblings share cache */
+ if (env->sd->flags & SD_SHARE_CPUCAPACITY)
+ return 0;
+
/*
* Buddy candidates are cache hot:
*/
--
2.28.0.163.g6104cc2f0b6-goog

2020-08-10 06:16:28

by Srikar Dronamraju

Subject: Re: [PATCH v2] sched/fair: ignore cache hotness for SMT migration

* Josh Don <[email protected]> [2020-08-04 12:34:13]:

> SMT siblings share caches, so cache hotness should be irrelevant for
> cross-sibling migration.
>
> Proposed-by: Venkatesh Pallipadi <[email protected]>
> Signed-off-by: Josh Don <[email protected]>
> ---
> kernel/sched/fair.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 1a68a0536add..abdb54e2339f 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7402,6 +7402,10 @@ static int task_hot(struct task_struct *p, struct lb_env *env)
> if (unlikely(task_has_idle_policy(p)))
> return 0;
>
> + /* SMT siblings share cache */
> + if (env->sd->flags & SD_SHARE_CPUCAPACITY)
> + return 0;
> +

If this for retaining cache hotness, should we look at
SD_SHARE_PKG_RESOURCES instead of SD_SHARE_CPUCAPACITY?

> /*
> * Buddy candidates are cache hot:
> */
> --
> 2.28.0.163.g6104cc2f0b6-goog
>

--
Thanks and Regards
Srikar Dronamraju

2020-08-10 08:59:09

by Valentin Schneider

Subject: Re: [PATCH v2] sched/fair: ignore cache hotness for SMT migration

On 10/08/20 07:14, Srikar Dronamraju wrote:
> * Josh Don <[email protected]> [2020-08-04 12:34:13]:
>
>> SMT siblings share caches, so cache hotness should be irrelevant for
>> cross-sibling migration.
>>
>> Proposed-by: Venkatesh Pallipadi <[email protected]>
>> Signed-off-by: Josh Don <[email protected]>
>> ---
>> kernel/sched/fair.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 1a68a0536add..abdb54e2339f 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -7402,6 +7402,10 @@ static int task_hot(struct task_struct *p, struct lb_env *env)
>> if (unlikely(task_has_idle_policy(p)))
>> return 0;
>>
>> + /* SMT siblings share cache */
>> + if (env->sd->flags & SD_SHARE_CPUCAPACITY)
>> + return 0;
>> +
>
> If this for retaining cache hotness, should we look at
> SD_SHARE_PKG_RESOURCES instead of SD_SHARE_CPUCAPACITY?
>

Josh's patch only targets migrating tasks between threads of the same
core - as he points out, cache hotness shouldn't matter here.

Using SD_SHARE_PKG_RESOURCES here would mean freely migrating tasks between
any CPU of an LLC domain, which is quite likely something you do *not* want
to do.

>> /*
>> * Buddy candidates are cache hot:
>> */
>> --
>> 2.28.0.163.g6104cc2f0b6-goog
>>

2020-08-27 07:59:46

by tip-bot2 for Alexey Makhalov

Subject: [tip: sched/core] sched/fair: Ignore cache hotness for SMT migration

The following commit has been merged into the sched/core branch of tip:

Commit-ID: ec73240b1627cddfd7cef018c7fa1c32e64a721e
Gitweb: https://git.kernel.org/tip/ec73240b1627cddfd7cef018c7fa1c32e64a721e
Author: Josh Don <[email protected]>
AuthorDate: Tue, 04 Aug 2020 12:34:13 -07:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Wed, 26 Aug 2020 12:41:57 +02:00

sched/fair: Ignore cache hotness for SMT migration

SMT siblings share caches, so cache hotness should be irrelevant for
cross-sibling migration.

Signed-off-by: Josh Don <[email protected]>
Proposed-by: Venkatesh Pallipadi <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
kernel/sched/fair.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 1a68a05..abdb54e 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7402,6 +7402,10 @@ static int task_hot(struct task_struct *p, struct lb_env *env)
if (unlikely(task_has_idle_policy(p)))
return 0;

+ /* SMT siblings share cache */
+ if (env->sd->flags & SD_SHARE_CPUCAPACITY)
+ return 0;
+
/*
* Buddy candidates are cache hot:
*/