mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Yuyang Du <yuyang.du@intel.com>, Ingo Molnar <mingo@kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Mike Galbraith <umgwanakikbuti@gmail.com>,
	Benjamin Segall <bsegall@google.com>,
	Paul Turner <pjt@google.com>,
	Morten Rasmussen <morten.rasmussen@arm.com>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Matt Fleming <matt@codeblueprint.co.uk>,
	Vincent Guittot <vincent.guittot@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Subject: [PATCH 2/4] sched/fair: Fix PELT integrity for new groups
Date: Fri, 17 Jun 2016 14:01:38 +0200	[thread overview]
Message-ID: <20160617120454.006731089@infradead.org> (raw)
In-Reply-To: <20160617120136.064100812@infradead.org>

[-- Attachment #1: peterz-sched-fork-4.patch --]
[-- Type: text/plain, Size: 2417 bytes --]

Vincent reported that when a new task is moved into a new cgroup it
gets attached twice to the load tracking.

  sched_move_task()
    task_move_group_fair()
      detach_task_cfs_rq()
      set_task_rq()
      attach_task_cfs_rq()
        attach_entity_load_avg()
          se->avg.last_load_update = cfs_rq->avg.last_load_update // == 0

  enqueue_entity()
    enqueue_entity_load_avg()
      update_cfs_rq_load_avg()
        now = clock()
        __update_load_avg(&cfs_rq->avg)
          cfs_rq->avg.last_load_update = now
          // ages load/util for: now - 0, load/util -> 0
      if (migrated)
        attach_entity_load_avg()
          se->avg.last_load_update = cfs_rq->avg.last_load_update; // now != 0

The problem is that we don't update cfs_rq load_avg before all
entity attach/detach operations. Only enqueue and migrate_task do
this.

By fixing this, the above will not happen, because the
sched_move_task() attach will have updated cfs_rq's last_load_update
time before attach, and in turn the attach will have set the entity's
last_load_update stamp.

Note that there is a further problem with sched_move_task() calling
detach on a task that hasn't yet been attached; this will be taken
care of in a subsequent patch.

Cc: Yuyang Du <yuyang.du@intel.com>
Reported-by: Vincent Guittot <vincent.guittot@linaro.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 kernel/sched/fair.c |    4 ++++
 1 file changed, 4 insertions(+)

--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -8366,6 +8366,7 @@ static void detach_task_cfs_rq(struct ta
 {
 	struct sched_entity *se = &p->se;
 	struct cfs_rq *cfs_rq = cfs_rq_of(se);
+	u64 now = cfs_rq_clock_task(cfs_rq);
 
 	if (!vruntime_normalized(p)) {
 		/*
@@ -8377,6 +8378,7 @@ static void detach_task_cfs_rq(struct ta
 	}
 
 	/* Catch up with the cfs_rq and remove our load when we leave */
+	update_cfs_rq_load_avg(now, cfs_rq, false);
 	detach_entity_load_avg(cfs_rq, se);
 }
 
@@ -8384,6 +8386,7 @@ static void attach_task_cfs_rq(struct ta
 {
 	struct sched_entity *se = &p->se;
 	struct cfs_rq *cfs_rq = cfs_rq_of(se);
+	u64 now = cfs_rq_clock_task(cfs_rq);
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
 	/*
@@ -8394,6 +8397,7 @@ static void attach_task_cfs_rq(struct ta
 #endif
 
 	/* Synchronize task with its cfs_rq */
+	update_cfs_rq_load_avg(now, cfs_rq, false);
 	attach_entity_load_avg(cfs_rq, se);
 
 	if (!vruntime_normalized(p))

  parent reply	other threads:[~2016-06-17 12:06 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-17 12:01 [PATCH 0/4] sched/fair: Fix PELT wobblies Peter Zijlstra
2016-06-17 12:01 ` [PATCH 1/4] sched: Optimize fork() paths Peter Zijlstra
2016-06-17 12:01 ` Peter Zijlstra [this message]
2016-06-17 13:51   ` [PATCH 2/4] sched/fair: Fix PELT integrity for new groups Vincent Guittot
2016-06-17 12:01 ` [PATCH 3/4] sched,cgroup: Fix cpu_cgroup_fork() Peter Zijlstra
2016-06-17 13:58   ` Vincent Guittot
2016-06-17 14:06     ` Peter Zijlstra
2016-06-17 12:01 ` [PATCH 4/4] sched,fair: Fix PELT integrity for new tasks Peter Zijlstra
2016-06-17 14:09   ` Vincent Guittot
2016-06-17 14:28     ` Peter Zijlstra
2016-06-17 16:02       ` Peter Zijlstra
2016-06-17 16:14         ` Vincent Guittot
2016-06-17 16:18         ` Peter Zijlstra
2016-06-19 22:55           ` Yuyang Du
2016-06-20  9:23           ` Vincent Guittot
2016-06-20  9:52             ` Peter Zijlstra
2016-06-20 10:07               ` Vincent Guittot
2016-06-21 11:43             ` Peter Zijlstra
2016-06-21 12:36               ` Vincent Guittot
2016-06-21 12:47                 ` Peter Zijlstra
2016-06-21 12:56                   ` Vincent Guittot
2016-06-20 11:35           ` Dietmar Eggemann
2016-06-20 12:35             ` Vincent Guittot
2016-06-20 14:49               ` Dietmar Eggemann
2016-06-21  8:41                 ` Peter Zijlstra
2016-06-21  4:51                   ` Yuyang Du
2016-06-24 13:03                     ` Peter Zijlstra
2016-08-09 23:19                       ` Yuyang Du
2016-06-21 13:17                   ` Peter Zijlstra
2016-06-21 13:29                     ` Vincent Guittot
2016-06-22 11:46                       ` Peter Zijlstra
2016-06-23 15:35                   ` Dietmar Eggemann
2016-06-23 17:15                     ` Peter Zijlstra
2016-06-20 14:27             ` Peter Zijlstra
2016-06-23 11:19   ` Peter Zijlstra
2016-08-01  7:30   ` Wanpeng Li
2016-08-01  9:31     ` Mike Galbraith
2016-08-01  9:56       ` Wanpeng Li
2016-08-01 11:52         ` Mike Galbraith

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=20160617120454.006731089@infradead.org \
    --to=peterz@infradead.org \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matt@codeblueprint.co.uk \
    --cc=mingo@kernel.org \
    --cc=morten.rasmussen@arm.com \
    --cc=pjt@google.com \
    --cc=umgwanakikbuti@gmail.com \
    --cc=vincent.guittot@linaro.org \
    --cc=yuyang.du@intel.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

all inboxes | Powered by JetHome®