Messages in this thread Patch in this message | | | Date | Mon, 20 Mar 2023 13:29:05 +0100 | Subject | Re: [PATCH v2] sched/fair: sanitize vruntime of entity being migrated | From | Dietmar Eggemann <> |
| |
On 18/03/2023 08:45, Zhang Qiao wrote: > > > 在 2023/3/18 0:08, Vincent Guittot 写道: >> Commit 829c1651e9c4 ("sched/fair: sanitize vruntime of entity being placed") >> fixes an overflowing bug, but ignore a case that se->exec_start is reset >> after a migration. >> >> For fixing this case, we delay the reset of se->exec_start after >> placing the entity which se->exec_start to detect long sleeping task. >> >> In order to take into account a possible divergence between the clock_task >> of 2 rqs, we increase the threshold to around 104 days. >> >> >> Fixes: 829c1651e9c4 ("sched/fair: sanitize vruntime of entity being placed") >> Signed-off-by: Zhang Qiao <zhangqiao22@huawei.com> >> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> >> --- >> >> My last proposal was not yet correct as the exec_start was not always >> reset after migrating a task. I finally found this solution which keeps >> the long sleep detection to one place as well as the reset of se->exec_start. >> > > Tested-by: Zhang Qiao <zhangqiao22@huawei.com> > > I have retested it with this version, and the result is fine. > > ------- > > Performance counter stats for 'hackbench -g 44 -f 20 --process --pipe -l 60000 -s 100' (10 runs): > > 80.10 +- 1.22 seconds time elapsed ( +- 1.53% )
[...]
>> @@ -8701,7 +8734,7 @@ static void attach_task(struct rq *rq, struct task_struct *p) >> lockdep_assert_rq_held(rq); >> >> WARN_ON_ONCE(task_rq(p) != rq); >> - activate_task(rq, p, ENQUEUE_NOCLOCK); >> + activate_task(rq, p, ENQUEUE_NOCLOCK | ENQUEUE_MIGRATED); >> check_preempt_curr(rq, p, 0);
Why not:
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index b9bc1ab67aaa..96dd3a62e683 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -7674,7 +7674,10 @@ static void migrate_task_rq_fair(struct task_struct *p, int new_cpu) se->vruntime -= u64_u32_load(cfs_rq->min_vruntime); } - if (!task_on_rq_migrating(p)) { + if (task_on_rq_migrating(p)) { + /* We have migrated, no longer consider this task hot */ + se->exec_start = 0; + } else { remove_entity_load_avg(se); /* @@ -8726,7 +8729,7 @@ static void attach_task(struct rq *rq, struct task_struct *p) lockdep_assert_rq_held(rq); WARN_ON_ONCE(task_rq(p) != rq); - activate_task(rq, p, ENQUEUE_NOCLOCK | ENQUEUE_MIGRATED); + activate_task(rq, p, ENQUEUE_NOCLOCK); check_preempt_curr(rq, p, 0); }
entity_is_long_sleeper() will bail early for these rq-migrating tasks for which a long-sleep test would make little sense anyway.
Plus move_queued_task() (e.g. from sched_exex()) would be covered as well.
| |