From: Chengming Zhou <zhouchengming@bytedance.com>
To: mingo@redhat.com, peterz@infradead.org,
vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
rostedt@goodmis.org, bsegall@google.com, vschneid@redhat.com
Cc: linux-kernel@vger.kernel.org,
Chengming Zhou <zhouchengming@bytedance.com>
Subject: [PATCH v2 07/10] sched/fair: use update_load_avg() to attach/detach entity load_avg
Date: Wed, 13 Jul 2022 12:04:27 +0800 [thread overview]
Message-ID: <20220713040430.25778-8-zhouchengming@bytedance.com> (raw)
In-Reply-To: <20220713040430.25778-1-zhouchengming@bytedance.com>
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.
1. switch to fair class (twice attach problem)
p->sched_class = fair_class; --> p.se->avg.last_update_time = 0
if (queued)
enqueue_task(p);
...
enqueue_entity()
update_load_avg(UPDATE_TG | DO_ATTACH)
if (!se->avg.last_update_time && (flags & DO_ATTACH)) --> true
attach_entity_load_avg() --> attached, will set last_update_time
check_class_changed()
switched_from() (!fair)
switched_to() (fair)
switched_to_fair()
attach_entity_load_avg() --> unconditional attach again!
2. change cgroup of NEW task (detach unattached task problem)
sched_move_group(p)
if (queued)
dequeue_task()
task_move_group_fair()
detach_task_cfs_rq()
detach_entity_load_avg() --> detach unattached NEW task
set_task_rq()
attach_task_cfs_rq()
attach_entity_load_avg()
if (queued)
enqueue_task()
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.
Second, it attach entity load_avg in post_init_entity_util_avg(),
in which we only set sched_avg last_update_time for !fair tasks,
will cause PELT integrity problem when switched_to_fair().
This patch make update_load_avg() the only location of attach/detach,
and can handle these corner cases like change cgroup of NEW tasks,
by checking last_update_time before attach/detach.
Signed-off-by: Chengming Zhou <zhouchengming@bytedance.com>
---
kernel/sched/fair.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 29811869c1fe..51fc20c161a3 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4307,11 +4307,6 @@ static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s
static inline void remove_entity_load_avg(struct sched_entity *se) {}
-static inline void
-attach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) {}
-static inline void
-detach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) {}
-
static inline int newidle_balance(struct rq *rq, struct rq_flags *rf)
{
return 0;
@@ -11527,9 +11522,7 @@ static void detach_entity_cfs_rq(struct sched_entity *se)
struct cfs_rq *cfs_rq = cfs_rq_of(se);
/* Catch up with the cfs_rq and remove our load when we leave */
- update_load_avg(cfs_rq, se, 0);
- detach_entity_load_avg(cfs_rq, se);
- update_tg_load_avg(cfs_rq);
+ update_load_avg(cfs_rq, se, UPDATE_TG | DO_DETACH);
propagate_entity_cfs_rq(se);
}
@@ -11537,10 +11530,8 @@ static void attach_entity_cfs_rq(struct sched_entity *se)
{
struct cfs_rq *cfs_rq = cfs_rq_of(se);
- /* Synchronize entity with its cfs_rq */
- update_load_avg(cfs_rq, se, 0);
- attach_entity_load_avg(cfs_rq, se);
- update_tg_load_avg(cfs_rq);
+ /* Synchronize entity with its cfs_rq and attach our load */
+ update_load_avg(cfs_rq, se, UPDATE_TG | DO_ATTACH);
propagate_entity_cfs_rq(se);
}
--
2.36.1
next prev parent reply other threads:[~2022-07-13 4:05 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-13 4:04 [PATCH v2 00/10] sched: task load tracking optimization and cleanup Chengming Zhou
2022-07-13 4:04 ` [PATCH v2 01/10] sched/fair: combine detach into dequeue when migrating task Chengming Zhou
2022-07-13 4:04 ` [PATCH v2 02/10] sched/fair: update comments in enqueue/dequeue_entity() Chengming Zhou
2022-07-13 4:04 ` [PATCH v2 03/10] sched/fair: maintain task se depth in set_task_rq() Chengming Zhou
2022-07-14 12:30 ` Dietmar Eggemann
2022-07-14 13:03 ` [External] " Chengming Zhou
2022-07-18 7:16 ` Vincent Guittot
2022-07-13 4:04 ` [PATCH v2 04/10] sched/fair: remove redundant cpu_cgrp_subsys->fork() Chengming Zhou
2022-07-14 12:31 ` Dietmar Eggemann
2022-07-14 13:06 ` [External] " Chengming Zhou
2022-07-13 4:04 ` [PATCH v2 05/10] sched/fair: reset sched_avg last_update_time before set_task_rq() Chengming Zhou
2022-07-14 12:31 ` Dietmar Eggemann
2022-07-19 8:49 ` Vincent Guittot
2022-07-13 4:04 ` [PATCH v2 06/10] sched/fair: delete superfluous SKIP_AGE_LOAD Chengming Zhou
2022-07-14 12:33 ` Dietmar Eggemann
2022-07-14 13:24 ` [External] " Chengming Zhou
2022-07-13 4:04 ` Chengming Zhou [this message]
2022-07-15 11:18 ` [PATCH v2 07/10] sched/fair: use update_load_avg() to attach/detach entity load_avg Dietmar Eggemann
2022-07-15 16:21 ` [External] " Chengming Zhou
2022-07-19 10:29 ` Vincent Guittot
2022-07-20 13:40 ` Chengming Zhou
2022-07-20 15:34 ` Vincent Guittot
2022-07-21 13:56 ` Chengming Zhou
2022-07-21 14:13 ` Vincent Guittot
2022-07-19 15:02 ` Dietmar Eggemann
2022-07-20 13:43 ` Chengming Zhou
2022-07-13 4:04 ` [PATCH v2 08/10] sched/fair: fix load tracking for new forked !fair task Chengming Zhou
2022-07-19 12:35 ` Vincent Guittot
2022-07-20 13:48 ` [External] " Chengming Zhou
2022-07-13 4:04 ` [PATCH v2 09/10] sched/fair: stop load tracking when task switched_from_fair() Chengming Zhou
2022-07-14 12:33 ` Dietmar Eggemann
2022-07-14 13:43 ` [External] " Chengming Zhou
2022-07-15 11:15 ` Dietmar Eggemann
2022-07-15 16:35 ` Chengming Zhou
2022-07-19 13:20 ` Vincent Guittot
2022-07-27 10:55 ` Chengming Zhou
2022-07-13 4:04 ` [PATCH v2 10/10] sched/fair: delete superfluous set_task_rq_fair() Chengming Zhou
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220713040430.25778-8-zhouchengming@bytedance.com \
--to=zhouchengming@bytedance.com \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=vincent.guittot@linaro.org \
--cc=vschneid@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox