Messages in this thread | | | Date | Wed, 27 Jul 2022 18:55:31 +0800 | Subject | Re: [PATCH v2 09/10] sched/fair: stop load tracking when task switched_from_fair() | From | Chengming Zhou <> |
| |
On 2022/7/19 21:20, Vincent Guittot wrote: > On Wed, 13 Jul 2022 at 06:05, Chengming Zhou > <zhouchengming@bytedance.com> wrote: >> >> The same reason as the previous commit, if we don't reset the >> sched_avg last_update_time to 0, after a while in switched_to_fair(): >> >> switched_to_fair >> attach_task_cfs_rq >> attach_entity_cfs_rq >> update_load_avg >> __update_load_avg_se(now, cfs_rq, se) >> >> The delta (now - sa->last_update_time) will wrongly contribute/decay >> sched_avg depends on the task running/runnable status at that time. > > I think that we always decay the time spent in !fair class and never contribute > > This one is a bit more complex than the new task one. Generally > speaking, I would say that we should decay the load while running in > !fair case because the value becomes less and less relevant over time. > What's the meaning of pelt values when the task has run seconds as a > fifo ? The only case that would need a particular attention, is the pi > boost case but there is no way to now how long is runs and sleep when > !fair
After thought for a while, I agree with you on that the PELT values become meaningless after running as a FIFO task. So yes it's best for now to always decay them when switched_to_fair().
So in the next version: 1. don't set for !fair in post_init_entity_util_avg() 2. keep last_update_time when switched_from_fair() 3. should we still keep SKIP_AGE_LOAD ? I don't understand what's its usage in switched_to_fair(). Or should we just delete it?
Thanks!
> >> >> This patch reset it's sched_avg last_update_time to 0, stop load >> tracking for !fair task, later in switched_to_fair() -> >> update_load_avg(), we can use its saved sched_avg. >> >> Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com> >> --- >> kernel/sched/fair.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >> index 50f65a2ede32..576028f5a09e 100644 >> --- a/kernel/sched/fair.c >> +++ b/kernel/sched/fair.c >> @@ -11552,6 +11552,11 @@ static void attach_task_cfs_rq(struct task_struct *p) >> static void switched_from_fair(struct rq *rq, struct task_struct *p) >> { >> detach_task_cfs_rq(p); >> + >> +#ifdef CONFIG_SMP >> + /* Stop load tracking for !fair task */ >> + p->se.avg.last_update_time = 0; >> +#endif >> } >> >> static void switched_to_fair(struct rq *rq, struct task_struct *p) >> -- >> 2.36.1 >>
| |