RE: [rfc patch] Re: sched: incorrect argument used in task_hot()

["Chen, Kenneth W" <kenneth.w.chen@intel.com>]

Mike Galbraith wrote on Friday, November 17, 2006 8:57 AM
> On Tue, 2006-11-14 at 15:00 -0800, Chen, Kenneth W wrote:
> > The argument used for task_hot in can_migrate_task() looks wrong:
> > 
> > int can_migrate_task()
> > { ...
> >        if (task_hot(p, rq->timestamp_last_tick, sd))
> >                 return 0;
> >         return 1;
> > }
> > 
> > [....]
> > 
> > So back to the first observation on not enough l-b at HT domain because
> > of inaccurate time calculation, what would be the best solution to fix
> > this?
> 
> One way to improve granularity, and eliminate the possibility of
> p->last_run being > rq->timestamp_tast_tick, and thereby short
> circuiting the evaluation of cache_hot_time, is to cache the last return
> of sched_clock() at both tick and sched times, and use that value as our
> reference instead of the absolute time of the tick.  It won't totally
> eliminate skew, but it moves the reference point closer to the current
> time on the remote cpu.
> 
> Comments?


I like it and think it might do it.  Just for the record, we are tinkering
with the following patch.  The thinking is that logical CPUs in HT and
multi-core domain are usually on the same CPU package, and it is likely that
the tsc are synchronized, so we can take current time stamp and use it
directly to compare with p->last_ran.  I'm planning on running a couple of
experiments with both patches.


--- ./kernel/sched.c.orig	2006-11-07 18:24:20.000000000 -0800
+++ ./kernel/sched.c	2006-11-15 16:01:39.000000000 -0800
@@ -2068,6 +2068,8 @@ int can_migrate_task(struct task_struct 
 		     struct sched_domain *sd, enum idle_type idle,
 		     int *all_pinned)
 {
+	unsigned long long now;
+
 	/*
 	 * We do not migrate tasks that are:
 	 * 1) running (obviously), or
@@ -2090,7 +2092,12 @@ int can_migrate_task(struct task_struct 
 	if (sd->nr_balance_failed > sd->cache_nice_tries)
 		return 1;
 
-	if (task_hot(p, rq->timestamp_last_tick, sd))
+	if (sd->flags & (SD_SHARE_CPUPOWER | SD_SHARE_PKG_RESOURCES))
+		now = sched_clock();
+	else
+		now = rq->timestamp_last_tick;
+
+	if (task_hot(p, now, sd))
 		return 0;
 	return 1;
 }