Re: [PATCH] sched: fix exec_start/task_hot on migrated tasks
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
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
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
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
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
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
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
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
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
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
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
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
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
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/