Messages in this thread | | | Date | Tue, 19 Jul 2022 17:02:22 +0200 | Subject | Re: [External] Re: [PATCH v2 07/10] sched/fair: use update_load_avg() to attach/detach entity load_avg | From | Dietmar Eggemann <> |
| |
On 15/07/2022 18:21, Chengming Zhou wrote: > On 2022/7/15 19:18, Dietmar Eggemann wrote: >> On 13/07/2022 06:04, Chengming Zhou wrote: >>> Since update_load_avg() support DO_ATTACH and DO_DETACH now, we can >>> use update_load_avg() to implement attach/detach entity load_avg. >>> >>> Another advantage of using update_load_avg() is that it will check >>> last_update_time before attach or detach, instead of unconditional >>> attach/detach in the current code. >>> >>> This way can avoid some corner problematic cases of load tracking, >>> like twice attach problem, detach unattached NEW task problem. >> >> This explanation is somewhat hard to follow for me. Since both issues >> have been fixed already (you mention this further below) you're saying >> that with you change you don't reintroduce them? > > Sorry for this not very clear explanation. > > Yes, both issues have been fixed already, what I want to say is that bugfix > brings its own problem and limitation mentioned below. > > So I want to use another way to solve these problems better.
[...]
>>> These problems have been fixed in commit 7dc603c9028e >>> ("sched/fair: Fix PELT integrity for new tasks"), which also >>> bring its own problems. >>> >>> First, it add a new task state TASK_NEW and an unnessary limitation >>> that we would fail when change the cgroup of TASK_NEW tasks. > > This is the limitation that bugfix has brought. > > We can't change cgroup or switch to fair for task with last_update_time=0 > if we don't have conditional detach/attach. > > So we have to: > > 1. !fair task also need to set last_update_time. > 2. cpu_cgroup_can_attach() have to wait for TASK_NEW to fully attached.
I see.
`cgroup_migrate_execute() -> cpu_cgroup_[can|]_attach()` has to wait for `wake_up_new_task() -> WRITE_ONCE(p->__state, TASK_RUNNING)`.
Just to understand this change better: IMHO, this is still the case for fair tasks, right?
`wake_up_new_task() -> post_init_entity_util_avg() -> attach_entity_cfs_rq()` has to happen before the fair task can move between taskgroups in `cgroup_migrate_execute() -> cpu_cgroup_attach() -> sched_move_task() -> sched_change_group() -> task_change_group_fair()`.
[...]
| |