Re: [PATCH] sched: fix exec_start/task_hot on migrated tasks

2014-05-19 Thread Preeti U Murthy
On 05/16/2014 07:47 PM, Peter Zijlstra wrote:
> On Fri, May 16, 2014 at 07:27:32PM +0530, Preeti Murthy wrote:
>>> 0 isn't strictly the right thing to do here, since the clock can wrap,
>>> but being wrong every ~585 years isn't too big an issue for this.
>>
>> I don't understand this. Will setting it to 0 not indicate beginning
>> of ticking? 
> 
> In modular spaces there is no beginnings nor endings.
> 
>> So when you find out for how long the task has run, the
>> difference would be larger than what would have been had you
>> let exec_start be at its previous value of the old cpu's clock_task right?
> 
> Nope, see the modular space thing, once every ~585 years we'll wrap the
> clock and 0 is within range of recently ran.
> 
>> Will not setting exec_start to the clock_task of the destination rq
>> during migration be better? This would be the closest we could
>> come to estimating the amount of time the task has run on this new
>> cpu while deciding task_hot or not no?
> 
> Setting it to the exact clock_task of the destination rq would make it
> hot on that rq, even though it hasn't yet ran there, so you'd have to do
> something like: rq_clock_task(dst_rq) - sysctl_sched_migration_cost.
> 
> But seeing as how that is far more work, and all this is heuristics
> anyhow and an extra fail term of 1/585 years is far below the current
> fail rate, all is well.
> 

Ok now I understand this.Thanks!

Reviewed-by: Preeti U Murthy 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] sched: fix exec_start/task_hot on migrated tasks

2014-05-19 Thread Preeti U Murthy
On 05/16/2014 07:47 PM, Peter Zijlstra wrote:
 On Fri, May 16, 2014 at 07:27:32PM +0530, Preeti Murthy wrote:
 0 isn't strictly the right thing to do here, since the clock can wrap,
 but being wrong every ~585 years isn't too big an issue for this.

 I don't understand this. Will setting it to 0 not indicate beginning
 of ticking? 
 
 In modular spaces there is no beginnings nor endings.
 
 So when you find out for how long the task has run, the
 difference would be larger than what would have been had you
 let exec_start be at its previous value of the old cpu's clock_task right?
 
 Nope, see the modular space thing, once every ~585 years we'll wrap the
 clock and 0 is within range of recently ran.
 
 Will not setting exec_start to the clock_task of the destination rq
 during migration be better? This would be the closest we could
 come to estimating the amount of time the task has run on this new
 cpu while deciding task_hot or not no?
 
 Setting it to the exact clock_task of the destination rq would make it
 hot on that rq, even though it hasn't yet ran there, so you'd have to do
 something like: rq_clock_task(dst_rq) - sysctl_sched_migration_cost.
 
 But seeing as how that is far more work, and all this is heuristics
 anyhow and an extra fail term of 1/585 years is far below the current
 fail rate, all is well.
 

Ok now I understand this.Thanks!

Reviewed-by: Preeti U Murthy pre...@linux.vnet.ibm.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] sched: fix exec_start/task_hot on migrated tasks

2014-05-16 Thread bsegall
Peter Zijlstra  writes:

> On Thu, May 15, 2014 at 03:59:20PM -0700, Ben Segall wrote:
>> task_hot checks exec_start on any runnable task, but if it has been
>> migrated since the it last ran, then exec_start is a clock_task from
>> another cpu. If the old cpu's clock_task was sufficiently far ahead of
>> this cpu's then the task will not be considered for another migration
>> until it has run. Instead reset exec_start whenever a task is migrated,
>> since it is presumably no longer hot anyway.
>> 
>> Signed-off-by: Ben Segall 
>> ---
>>  kernel/sched/fair.c |3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 28ccf50..9f8dfeb 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -4544,6 +4544,9 @@ migrate_task_rq_fair(struct task_struct *p, int 
>> next_cpu)
>>  atomic_long_add(se->avg.load_avg_contrib,
>>  _rq->removed_load);
>>  }
>> +
>> +/* We have migrated, no longer consider this task hot */
>> +    se.exec_start = 0;
>
> se->exec_start compiles loads better
Welp. Indeed it does.


From: Ben Segall 
Date: Thu, 15 May 2014 15:38:10 -0700
Subject: [PATCH] sched: fix exec_start/task_hot on migrated tasks

task_hot checks exec_start on any runnable task, but if it has been
migrated since the it last ran, then exec_start is a clock_task from
another cpu. If the old cpu's clock_task was sufficiently far ahead of
this cpu's then the task will not be considered for another migration
until it has run. Instead reset exec_start whenever a task is migrated,
since it is presumably no longer hot anyway.

Signed-off-by: Ben Segall 
---
 kernel/sched/fair.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 28ccf50..dd3fa14 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4544,6 +4544,9 @@ migrate_task_rq_fair(struct task_struct *p, int next_cpu)
atomic_long_add(se->avg.load_avg_contrib,
_rq->removed_load);
}
+
+   /* We have migrated, no longer consider this task hot */
+   se->exec_start = 0;
 }
 #endif /* CONFIG_SMP */
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] sched: fix exec_start/task_hot on migrated tasks

2014-05-16 Thread Peter Zijlstra
On Fri, May 16, 2014 at 07:27:32PM +0530, Preeti Murthy wrote:
> > 0 isn't strictly the right thing to do here, since the clock can wrap,
> > but being wrong every ~585 years isn't too big an issue for this.
> 
> I don't understand this. Will setting it to 0 not indicate beginning
> of ticking? 

In modular spaces there is no beginnings nor endings.

> So when you find out for how long the task has run, the
> difference would be larger than what would have been had you
> let exec_start be at its previous value of the old cpu's clock_task right?

Nope, see the modular space thing, once every ~585 years we'll wrap the
clock and 0 is within range of recently ran.

> Will not setting exec_start to the clock_task of the destination rq
> during migration be better? This would be the closest we could
> come to estimating the amount of time the task has run on this new
> cpu while deciding task_hot or not no?

Setting it to the exact clock_task of the destination rq would make it
hot on that rq, even though it hasn't yet ran there, so you'd have to do
something like: rq_clock_task(dst_rq) - sysctl_sched_migration_cost.

But seeing as how that is far more work, and all this is heuristics
anyhow and an extra fail term of 1/585 years is far below the current
fail rate, all is well.


pgpZ96wptZspf.pgp
Description: PGP signature


Re: [PATCH] sched: fix exec_start/task_hot on migrated tasks

2014-05-16 Thread Preeti Murthy
Hi,

On Fri, May 16, 2014 at 1:34 PM, Peter Zijlstra  wrote:
> On Thu, May 15, 2014 at 03:59:20PM -0700, Ben Segall wrote:
>> task_hot checks exec_start on any runnable task, but if it has been
>> migrated since the it last ran, then exec_start is a clock_task from
>> another cpu. If the old cpu's clock_task was sufficiently far ahead of
>> this cpu's then the task will not be considered for another migration
>> until it has run. Instead reset exec_start whenever a task is migrated,
>> since it is presumably no longer hot anyway.
>>
>> Signed-off-by: Ben Segall 
>> ---
>>  kernel/sched/fair.c |3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 28ccf50..9f8dfeb 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -4544,6 +4544,9 @@ migrate_task_rq_fair(struct task_struct *p, int 
>> next_cpu)
>>   atomic_long_add(se->avg.load_avg_contrib,
>>   _rq->removed_load);
>>   }
>> +
>> + /* We have migrated, no longer consider this task hot */
>> + se.exec_start = 0;
>>  }
>
> 0 isn't strictly the right thing to do here, since the clock can wrap,
> but being wrong every ~585 years isn't too big an issue for this.

I don't understand this. Will setting it to 0 not indicate beginning
of ticking? So when you find out for how long the task has run, the
difference would be larger than what would have been had you
let exec_start be at its previous value of the old cpu's clock_task right?

Will not setting exec_start to the clock_task of the destination rq
during migration be better? This would be the closest we could
come to estimating the amount of time the task has run on this new
cpu while deciding task_hot or not no?

Regards
Preeti U Murthy

>
> Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] sched: fix exec_start/task_hot on migrated tasks

2014-05-16 Thread Peter Zijlstra
On Thu, May 15, 2014 at 03:59:20PM -0700, Ben Segall wrote:
> task_hot checks exec_start on any runnable task, but if it has been
> migrated since the it last ran, then exec_start is a clock_task from
> another cpu. If the old cpu's clock_task was sufficiently far ahead of
> this cpu's then the task will not be considered for another migration
> until it has run. Instead reset exec_start whenever a task is migrated,
> since it is presumably no longer hot anyway.
> 
> Signed-off-by: Ben Segall 
> ---
>  kernel/sched/fair.c |3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 28ccf50..9f8dfeb 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4544,6 +4544,9 @@ migrate_task_rq_fair(struct task_struct *p, int 
> next_cpu)
>   atomic_long_add(se->avg.load_avg_contrib,
>   _rq->removed_load);
>   }
> +
> + /* We have migrated, no longer consider this task hot */
> + se.exec_start = 0;

se->exec_start compiles loads better


pgp4AdA6EKCZi.pgp
Description: PGP signature


Re: [PATCH] sched: fix exec_start/task_hot on migrated tasks

2014-05-16 Thread Peter Zijlstra
On Thu, May 15, 2014 at 03:59:20PM -0700, Ben Segall wrote:
> task_hot checks exec_start on any runnable task, but if it has been
> migrated since the it last ran, then exec_start is a clock_task from
> another cpu. If the old cpu's clock_task was sufficiently far ahead of
> this cpu's then the task will not be considered for another migration
> until it has run. Instead reset exec_start whenever a task is migrated,
> since it is presumably no longer hot anyway.
> 
> Signed-off-by: Ben Segall 
> ---
>  kernel/sched/fair.c |3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 28ccf50..9f8dfeb 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -4544,6 +4544,9 @@ migrate_task_rq_fair(struct task_struct *p, int 
> next_cpu)
>   atomic_long_add(se->avg.load_avg_contrib,
>   _rq->removed_load);
>   }
> +
> + /* We have migrated, no longer consider this task hot */
> + se.exec_start = 0;
>  }

0 isn't strictly the right thing to do here, since the clock can wrap,
but being wrong every ~585 years isn't too big an issue for this.

Thanks!


pgp2FeUZ3o1pG.pgp
Description: PGP signature


Re: [PATCH] sched: fix exec_start/task_hot on migrated tasks

2014-05-16 Thread Peter Zijlstra
On Thu, May 15, 2014 at 03:59:20PM -0700, Ben Segall wrote:
 task_hot checks exec_start on any runnable task, but if it has been
 migrated since the it last ran, then exec_start is a clock_task from
 another cpu. If the old cpu's clock_task was sufficiently far ahead of
 this cpu's then the task will not be considered for another migration
 until it has run. Instead reset exec_start whenever a task is migrated,
 since it is presumably no longer hot anyway.
 
 Signed-off-by: Ben Segall bseg...@google.com
 ---
  kernel/sched/fair.c |3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
 index 28ccf50..9f8dfeb 100644
 --- a/kernel/sched/fair.c
 +++ b/kernel/sched/fair.c
 @@ -4544,6 +4544,9 @@ migrate_task_rq_fair(struct task_struct *p, int 
 next_cpu)
   atomic_long_add(se-avg.load_avg_contrib,
   cfs_rq-removed_load);
   }
 +
 + /* We have migrated, no longer consider this task hot */
 + se.exec_start = 0;
  }

0 isn't strictly the right thing to do here, since the clock can wrap,
but being wrong every ~585 years isn't too big an issue for this.

Thanks!


pgp2FeUZ3o1pG.pgp
Description: PGP signature


Re: [PATCH] sched: fix exec_start/task_hot on migrated tasks

2014-05-16 Thread Peter Zijlstra
On Thu, May 15, 2014 at 03:59:20PM -0700, Ben Segall wrote:
 task_hot checks exec_start on any runnable task, but if it has been
 migrated since the it last ran, then exec_start is a clock_task from
 another cpu. If the old cpu's clock_task was sufficiently far ahead of
 this cpu's then the task will not be considered for another migration
 until it has run. Instead reset exec_start whenever a task is migrated,
 since it is presumably no longer hot anyway.
 
 Signed-off-by: Ben Segall bseg...@google.com
 ---
  kernel/sched/fair.c |3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
 index 28ccf50..9f8dfeb 100644
 --- a/kernel/sched/fair.c
 +++ b/kernel/sched/fair.c
 @@ -4544,6 +4544,9 @@ migrate_task_rq_fair(struct task_struct *p, int 
 next_cpu)
   atomic_long_add(se-avg.load_avg_contrib,
   cfs_rq-removed_load);
   }
 +
 + /* We have migrated, no longer consider this task hot */
 + se.exec_start = 0;

se-exec_start compiles loads better


pgp4AdA6EKCZi.pgp
Description: PGP signature


Re: [PATCH] sched: fix exec_start/task_hot on migrated tasks

2014-05-16 Thread Preeti Murthy
Hi,

On Fri, May 16, 2014 at 1:34 PM, Peter Zijlstra pet...@infradead.org wrote:
 On Thu, May 15, 2014 at 03:59:20PM -0700, Ben Segall wrote:
 task_hot checks exec_start on any runnable task, but if it has been
 migrated since the it last ran, then exec_start is a clock_task from
 another cpu. If the old cpu's clock_task was sufficiently far ahead of
 this cpu's then the task will not be considered for another migration
 until it has run. Instead reset exec_start whenever a task is migrated,
 since it is presumably no longer hot anyway.

 Signed-off-by: Ben Segall bseg...@google.com
 ---
  kernel/sched/fair.c |3 +++
  1 file changed, 3 insertions(+)

 diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
 index 28ccf50..9f8dfeb 100644
 --- a/kernel/sched/fair.c
 +++ b/kernel/sched/fair.c
 @@ -4544,6 +4544,9 @@ migrate_task_rq_fair(struct task_struct *p, int 
 next_cpu)
   atomic_long_add(se-avg.load_avg_contrib,
   cfs_rq-removed_load);
   }
 +
 + /* We have migrated, no longer consider this task hot */
 + se.exec_start = 0;
  }

 0 isn't strictly the right thing to do here, since the clock can wrap,
 but being wrong every ~585 years isn't too big an issue for this.

I don't understand this. Will setting it to 0 not indicate beginning
of ticking? So when you find out for how long the task has run, the
difference would be larger than what would have been had you
let exec_start be at its previous value of the old cpu's clock_task right?

Will not setting exec_start to the clock_task of the destination rq
during migration be better? This would be the closest we could
come to estimating the amount of time the task has run on this new
cpu while deciding task_hot or not no?

Regards
Preeti U Murthy


 Thanks!
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] sched: fix exec_start/task_hot on migrated tasks

2014-05-16 Thread Peter Zijlstra
On Fri, May 16, 2014 at 07:27:32PM +0530, Preeti Murthy wrote:
  0 isn't strictly the right thing to do here, since the clock can wrap,
  but being wrong every ~585 years isn't too big an issue for this.
 
 I don't understand this. Will setting it to 0 not indicate beginning
 of ticking? 

In modular spaces there is no beginnings nor endings.

 So when you find out for how long the task has run, the
 difference would be larger than what would have been had you
 let exec_start be at its previous value of the old cpu's clock_task right?

Nope, see the modular space thing, once every ~585 years we'll wrap the
clock and 0 is within range of recently ran.

 Will not setting exec_start to the clock_task of the destination rq
 during migration be better? This would be the closest we could
 come to estimating the amount of time the task has run on this new
 cpu while deciding task_hot or not no?

Setting it to the exact clock_task of the destination rq would make it
hot on that rq, even though it hasn't yet ran there, so you'd have to do
something like: rq_clock_task(dst_rq) - sysctl_sched_migration_cost.

But seeing as how that is far more work, and all this is heuristics
anyhow and an extra fail term of 1/585 years is far below the current
fail rate, all is well.


pgpZ96wptZspf.pgp
Description: PGP signature


Re: [PATCH] sched: fix exec_start/task_hot on migrated tasks

2014-05-16 Thread bsegall
Peter Zijlstra pet...@infradead.org writes:

 On Thu, May 15, 2014 at 03:59:20PM -0700, Ben Segall wrote:
 task_hot checks exec_start on any runnable task, but if it has been
 migrated since the it last ran, then exec_start is a clock_task from
 another cpu. If the old cpu's clock_task was sufficiently far ahead of
 this cpu's then the task will not be considered for another migration
 until it has run. Instead reset exec_start whenever a task is migrated,
 since it is presumably no longer hot anyway.
 
 Signed-off-by: Ben Segall bseg...@google.com
 ---
  kernel/sched/fair.c |3 +++
  1 file changed, 3 insertions(+)
 
 diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
 index 28ccf50..9f8dfeb 100644
 --- a/kernel/sched/fair.c
 +++ b/kernel/sched/fair.c
 @@ -4544,6 +4544,9 @@ migrate_task_rq_fair(struct task_struct *p, int 
 next_cpu)
  atomic_long_add(se-avg.load_avg_contrib,
  cfs_rq-removed_load);
  }
 +
 +/* We have migrated, no longer consider this task hot */
 +se.exec_start = 0;

 se-exec_start compiles loads better
Welp. Indeed it does.


From: Ben Segall bseg...@google.com
Date: Thu, 15 May 2014 15:38:10 -0700
Subject: [PATCH] sched: fix exec_start/task_hot on migrated tasks

task_hot checks exec_start on any runnable task, but if it has been
migrated since the it last ran, then exec_start is a clock_task from
another cpu. If the old cpu's clock_task was sufficiently far ahead of
this cpu's then the task will not be considered for another migration
until it has run. Instead reset exec_start whenever a task is migrated,
since it is presumably no longer hot anyway.

Signed-off-by: Ben Segall bseg...@google.com
---
 kernel/sched/fair.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 28ccf50..dd3fa14 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4544,6 +4544,9 @@ migrate_task_rq_fair(struct task_struct *p, int next_cpu)
atomic_long_add(se-avg.load_avg_contrib,
cfs_rq-removed_load);
}
+
+   /* We have migrated, no longer consider this task hot */
+   se-exec_start = 0;
 }
 #endif /* CONFIG_SMP */
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] sched: fix exec_start/task_hot on migrated tasks

2014-05-15 Thread Ben Segall
task_hot checks exec_start on any runnable task, but if it has been
migrated since the it last ran, then exec_start is a clock_task from
another cpu. If the old cpu's clock_task was sufficiently far ahead of
this cpu's then the task will not be considered for another migration
until it has run. Instead reset exec_start whenever a task is migrated,
since it is presumably no longer hot anyway.

Signed-off-by: Ben Segall 
---
 kernel/sched/fair.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 28ccf50..9f8dfeb 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4544,6 +4544,9 @@ migrate_task_rq_fair(struct task_struct *p, int next_cpu)
atomic_long_add(se->avg.load_avg_contrib,
_rq->removed_load);
}
+
+   /* We have migrated, no longer consider this task hot */
+   se.exec_start = 0;
 }
 #endif /* CONFIG_SMP */
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] sched: fix exec_start/task_hot on migrated tasks

2014-05-15 Thread Ben Segall
task_hot checks exec_start on any runnable task, but if it has been
migrated since the it last ran, then exec_start is a clock_task from
another cpu. If the old cpu's clock_task was sufficiently far ahead of
this cpu's then the task will not be considered for another migration
until it has run. Instead reset exec_start whenever a task is migrated,
since it is presumably no longer hot anyway.

Signed-off-by: Ben Segall bseg...@google.com
---
 kernel/sched/fair.c |3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 28ccf50..9f8dfeb 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4544,6 +4544,9 @@ migrate_task_rq_fair(struct task_struct *p, int next_cpu)
atomic_long_add(se-avg.load_avg_contrib,
cfs_rq-removed_load);
}
+
+   /* We have migrated, no longer consider this task hot */
+   se.exec_start = 0;
 }
 #endif /* CONFIG_SMP */
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/