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

[Chen, Kenneth W]

Mike Galbraith wrote on Friday, November 17, 2006 2:19 PM
> > And a changelog, then we're all set!
> > 
> > Oh.  And a patch, too.
> 
> Co-opt rq->timestamp_last_tick to maintain a cache_hot_time evaluation
> reference timestamp at both tick and sched times to prevent said
> reference, formerly rq->timestamp_last_tick, from being behind
> task->last_ran at evaluation time, and to move said reference closer to
> current time on the remote processor, intent being to improve cache hot
> evaluation and timestamp adjustment accuracy for task migration.
> 
> Fix minor sched_time double accounting error which occurs when a task
> passing through schedule() does not schedule off, and takes the next
> timer tick.
> 
> [....]
> 
> @@ -2206,7 +2207,7 @@ skip_queue:
>  	}
>  
>  #ifdef CONFIG_SCHEDSTATS
> -	if (task_hot(tmp, busiest->timestamp_last_tick, sd))
> +	if (task_hot(tmp, busiest->most_recent_timestamp, sd))
>  		schedstat_inc(sd, lb_hot_gained[idle]);
>  #endif


While we at it, let's clean up this hunk.  task_hot is evaluated twice in
the more common case of nr_balance_failed <= cache_nice_tries. We should
only test/increment relevant stats for forced migration.
Patch on top of yours.


Signed-off-by: Ken Chen <kenneth.w.chen@xxxxxxxxx>


--- ./kernel/sched.c.orig	2006-11-17 13:37:46.000000000 -0800
+++ ./kernel/sched.c	2006-11-17 14:20:34.000000000 -0800
@@ -2088,8 +2088,13 @@ int can_migrate_task(struct task_struct 
 	 * 2) too many balance attempts have failed.
 	 */
 
-	if (sd->nr_balance_failed > sd->cache_nice_tries)
+	if (sd->nr_balance_failed > sd->cache_nice_tries) {
+		#ifdef CONFIG_SCHEDSTATS
+		if (task_hot(p, rq->most_recent_timestamp, sd))
+			schedstat_inc(sd, lb_hot_gained[idle]);
+		#endif
 		return 1;
+	}
 
 	if (task_hot(p, rq->most_recent_timestamp, sd))
 		return 0;
@@ -2189,11 +2194,6 @@ skip_queue:
 		goto skip_bitmap;
 	}
 
-#ifdef CONFIG_SCHEDSTATS
-	if (task_hot(tmp, busiest->most_recent_timestamp, sd))
-		schedstat_inc(sd, lb_hot_gained[idle]);
-#endif
-
 	pull_task(busiest, array, tmp, this_rq, dst_array, this_cpu);
 	pulled++;
 	rem_load_move -= tmp->load_weight;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/