All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Galbraith <efault@gmx.de>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>, Ingo Molnar <mingo@elte.hu>
Cc: LKML <linux-kernel@vger.kernel.org>
Subject: [patch] sched: fix set_task_cpu() and provide an unlocked runqueue variant
Date: Sun, 22 Nov 2009 13:09:41 +0100	[thread overview]
Message-ID: <1258891781.14325.34.camel@marge.simson.net> (raw)


sched: fix set_task_cpu() and provide an unlocked runqueue variant.

set_task_cpu() falsifies migration stats by unconditionally generating migration
stats whether a task's cpu actually changed or not.  As used in copy_process(),
the runqueue is unlocked, so we need to provide an unlocked variant which does
the locking to provide a write barrier.

Signed-off-by: Mike Galbraith <efault@gmx.de>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <new-submission>

---
 include/linux/sched.h |    5 +++++
 kernel/fork.c         |    2 +-
 kernel/sched.c        |   38 ++++++++++++++++++++++++--------------
 3 files changed, 30 insertions(+), 15 deletions(-)

Index: linux-2.6/kernel/sched.c
===================================================================
--- linux-2.6.orig/kernel/sched.c
+++ linux-2.6/kernel/sched.c
@@ -2056,7 +2056,6 @@ task_hot(struct task_struct *p, u64 now,
 	return delta < (s64)sysctl_sched_migration_cost;
 }
 
-
 void set_task_cpu(struct task_struct *p, unsigned int new_cpu)
 {
 	int old_cpu = task_cpu(p);
@@ -2065,10 +2064,10 @@ void set_task_cpu(struct task_struct *p,
 		      *new_cfsrq = cpu_cfs_rq(old_cfsrq, new_cpu);
 	u64 clock_offset;
 
-	clock_offset = old_rq->clock - new_rq->clock;
-
-	trace_sched_migrate_task(p, new_cpu);
+	if (unlikely(old_cpu == new_cpu))
+		goto out;
 
+	clock_offset = old_rq->clock - new_rq->clock;
 #ifdef CONFIG_SCHEDSTATS
 	if (p->se.wait_start)
 		p->se.wait_start -= clock_offset;
@@ -2076,22 +2075,33 @@ void set_task_cpu(struct task_struct *p,
 		p->se.sleep_start -= clock_offset;
 	if (p->se.block_start)
 		p->se.block_start -= clock_offset;
+	if (task_hot(p, old_rq->clock, NULL))
+		schedstat_inc(p, se.nr_forced2_migrations);
 #endif
-	if (old_cpu != new_cpu) {
-		p->se.nr_migrations++;
-#ifdef CONFIG_SCHEDSTATS
-		if (task_hot(p, old_rq->clock, NULL))
-			schedstat_inc(p, se.nr_forced2_migrations);
-#endif
-		perf_sw_event(PERF_COUNT_SW_CPU_MIGRATIONS,
-				     1, 1, NULL, 0);
-	}
 	p->se.vruntime -= old_cfsrq->min_vruntime -
-					 new_cfsrq->min_vruntime;
+				 new_cfsrq->min_vruntime;
+	p->se.nr_migrations++;
+	trace_sched_migrate_task(p, new_cpu);
+	perf_sw_event(PERF_COUNT_SW_CPU_MIGRATIONS, 1, 1, NULL, 0);
 
+out:
 	__set_task_cpu(p, new_cpu);
 }
 
+void set_task_cpu_unlocked(struct task_struct *p, unsigned int new_cpu)
+{
+	unsigned long flags;
+	struct rq *rq, *new_rq = cpu_rq(new_cpu);
+
+	smp_wmb();
+	rq = task_rq_lock(p, &flags);
+	update_rq_clock(rq);
+	if (rq != new_rq)
+		update_rq_clock(new_rq);
+	set_task_cpu(p, new_cpu);
+	task_rq_unlock(rq, &flags);
+}
+
 struct migration_req {
 	struct list_head list;
 
Index: linux-2.6/include/linux/sched.h
===================================================================
--- linux-2.6.orig/include/linux/sched.h
+++ linux-2.6/include/linux/sched.h
@@ -2457,6 +2457,7 @@ static inline unsigned int task_cpu(cons
 }
 
 extern void set_task_cpu(struct task_struct *p, unsigned int cpu);
+extern void set_task_cpu_unlocked(struct task_struct *p, unsigned int cpu);
 
 #else
 
@@ -2469,6 +2470,10 @@ static inline void set_task_cpu(struct t
 {
 }
 
+static inline void set_task_cpu_unlocked(struct task_struct *p, unsigned int cpu)
+{
+}
+
 #endif /* CONFIG_SMP */
 
 extern void arch_pick_mmap_layout(struct mm_struct *mm);
Index: linux-2.6/kernel/fork.c
===================================================================
--- linux-2.6.orig/kernel/fork.c
+++ linux-2.6/kernel/fork.c
@@ -1242,7 +1242,7 @@ static struct task_struct *copy_process(
 	p->rt.nr_cpus_allowed = current->rt.nr_cpus_allowed;
 	if (unlikely(!cpu_isset(task_cpu(p), p->cpus_allowed) ||
 			!cpu_online(task_cpu(p))))
-		set_task_cpu(p, smp_processor_id());
+		set_task_cpu_unlocked(p, smp_processor_id());
 
 	/* CLONE_PARENT re-uses the old parent */
 	if (clone_flags & (CLONE_PARENT|CLONE_THREAD)) {



             reply	other threads:[~2009-11-22 12:09 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-22 12:09 Mike Galbraith [this message]
2009-11-25 18:27 ` [patch] sched: fix set_task_cpu() and provide an unlocked runqueue variant Peter Zijlstra
2009-11-26  1:01   ` Mike Galbraith
2009-11-26  1:31     ` Mike Galbraith
2009-11-26  9:35       ` Peter Zijlstra
2009-11-26 10:16         ` Mike Galbraith
2009-11-26 14:09           ` Peter Zijlstra
2009-11-26 14:21             ` Peter Zijlstra
2009-11-26 15:32               ` Mike Galbraith
2009-11-26 14:58             ` 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=1258891781.14325.34.camel@marge.simson.net \
    --to=efault@gmx.de \
    --cc=a.p.zijlstra@chello.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.