[PATCH] consolidate task preempts

[PATCH] consolidate task preempts

[Con Kolivas]

[-- Attachment #1.1: Type: text/plain, Size: 28 bytes --]

consolidate task preempts



[-- Attachment #1.2: sched-consolidate_task_preempts.diff --]
[-- Type: text/x-patch, Size: 2581 bytes --]

TASK_PREEMPTS_CURR is only used when followed by resched_task. Consolidate
the two into a single function.

Allow tasks of equal dynamic priority to preempt tasks of lower static
priority.

Signed-off-by: Con Kolivas <kernel@kolivas.org>

Index: linux-2.6.10-rc1-mm2/kernel/sched.c
===================================================================
--- linux-2.6.10-rc1-mm2.orig/kernel/sched.c	2004-11-02 14:19:32.973509317 +1100
+++ linux-2.6.10-rc1-mm2/kernel/sched.c	2004-11-02 14:26:23.444588405 +1100
@@ -157,9 +157,6 @@
 	(JIFFIES_TO_NS(MAX_SLEEP_AVG * \
 		(MAX_BONUS / 2 + DELTA((p)) + 1) / MAX_BONUS - 1))
 
-#define TASK_PREEMPTS_CURR(p, rq) \
-	((p)->prio < (rq)->curr->prio)
-
 /*
  * task_timeslice() scales user-nice values [ -20 ... 0 ... 19 ]
  * to time slice values: [800ms ... 100ms ... 5ms]
@@ -810,6 +807,13 @@ inline int task_curr(const task_t *p)
 	return cpu_curr(task_cpu(p)) == p;
 }
 
+static void preempt(task_t *p, runqueue_t *rq)
+{
+	if (p->prio < rq->curr->prio || (p->prio == rq->curr->prio &&
+		p->static_prio < rq->curr->static_prio))
+			resched_task(rq->curr);
+}
+
 #ifdef CONFIG_SMP
 enum request_type {
 	REQ_MOVE_TASK,
@@ -1106,10 +1110,8 @@ out_activate:
 	 * to be considered on this CPU.)
 	 */
 	activate_task(p, rq, cpu == this_cpu);
-	if (!sync || cpu != this_cpu) {
-		if (TASK_PREEMPTS_CURR(p, rq))
-			resched_task(rq->curr);
-	}
+	if (!sync || cpu != this_cpu)
+		preempt(p, rq);
 	success = 1;
 
 out_running:
@@ -1263,8 +1265,7 @@ void fastcall wake_up_new_task(task_t * 
 		p->timestamp = (p->timestamp - this_rq->timestamp_last_tick)
 					+ rq->timestamp_last_tick;
 		__activate_task(p, rq);
-		if (TASK_PREEMPTS_CURR(p, rq))
-			resched_task(rq->curr);
+		preempt(p, rq);
 
 		schedstat_inc(rq, wunt_moved);
 		/*
@@ -1621,8 +1622,7 @@ void pull_task(runqueue_t *src_rq, prio_
 	 * Note that idle threads have a prio of MAX_PRIO, for this test
 	 * to be always true for them.
 	 */
-	if (TASK_PREEMPTS_CURR(p, this_rq))
-		resched_task(this_rq->curr);
+	preempt(p, this_rq);
 }
 
 /*
@@ -3306,8 +3306,8 @@ recheck:
 		if (task_running(rq, p)) {
 			if (p->prio > oldprio)
 				resched_task(rq->curr);
-		} else if (TASK_PREEMPTS_CURR(p, rq))
-			resched_task(rq->curr);
+		} else
+			preempt(p, rq);
 	}
 	task_rq_unlock(rq, &flags);
 out_unlock:
@@ -4008,8 +4008,7 @@ static void __migrate_task(struct task_s
 				+ rq_dest->timestamp_last_tick;
 		deactivate_task(p, rq_src);
 		activate_task(p, rq_dest, 0);
-		if (TASK_PREEMPTS_CURR(p, rq_dest))
-			resched_task(rq_dest->curr);
+		preempt(p, rq_dest);
 	}
 
 out:


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 256 bytes --]

Re: [PATCH] consolidate task preempts

[Nick Piggin]

Con Kolivas wrote:
> consolidate task preempts
> 
> 
> 
> ------------------------------------------------------------------------
> 
> TASK_PREEMPTS_CURR is only used when followed by resched_task. Consolidate
> the two into a single function.
> 

I don't like the name.

I actually don't mind the code as it is now; it looks like it gets harder
to read, especially with that name.

Also, I think you might be better off to leave it inline, as it is just
a single comparison.

> Allow tasks of equal dynamic priority to preempt tasks of lower static
> priority.
> 

Although this change makes the condition more complex. Is it really worth
doing?

> Signed-off-by: Con Kolivas <kernel@kolivas.org>
> 
> Index: linux-2.6.10-rc1-mm2/kernel/sched.c
> ===================================================================
> --- linux-2.6.10-rc1-mm2.orig/kernel/sched.c	2004-11-02 14:19:32.973509317 +1100
> +++ linux-2.6.10-rc1-mm2/kernel/sched.c	2004-11-02 14:26:23.444588405 +1100
> @@ -157,9 +157,6 @@
>  	(JIFFIES_TO_NS(MAX_SLEEP_AVG * \
>  		(MAX_BONUS / 2 + DELTA((p)) + 1) / MAX_BONUS - 1))
>  
> -#define TASK_PREEMPTS_CURR(p, rq) \
> -	((p)->prio < (rq)->curr->prio)
> -
>  /*
>   * task_timeslice() scales user-nice values [ -20 ... 0 ... 19 ]
>   * to time slice values: [800ms ... 100ms ... 5ms]
> @@ -810,6 +807,13 @@ inline int task_curr(const task_t *p)
>  	return cpu_curr(task_cpu(p)) == p;
>  }
>  
> +static void preempt(task_t *p, runqueue_t *rq)
> +{
> +	if (p->prio < rq->curr->prio || (p->prio == rq->curr->prio &&
> +		p->static_prio < rq->curr->static_prio))
> +			resched_task(rq->curr);
> +}
> +
>  #ifdef CONFIG_SMP
>  enum request_type {
>  	REQ_MOVE_TASK,
> @@ -1106,10 +1110,8 @@ out_activate:
>  	 * to be considered on this CPU.)
>  	 */
>  	activate_task(p, rq, cpu == this_cpu);
> -	if (!sync || cpu != this_cpu) {
> -		if (TASK_PREEMPTS_CURR(p, rq))
> -			resched_task(rq->curr);
> -	}
> +	if (!sync || cpu != this_cpu)
> +		preempt(p, rq);
>  	success = 1;
>  
>  out_running:
> @@ -1263,8 +1265,7 @@ void fastcall wake_up_new_task(task_t * 
>  		p->timestamp = (p->timestamp - this_rq->timestamp_last_tick)
>  					+ rq->timestamp_last_tick;
>  		__activate_task(p, rq);
> -		if (TASK_PREEMPTS_CURR(p, rq))
> -			resched_task(rq->curr);
> +		preempt(p, rq);
>  
>  		schedstat_inc(rq, wunt_moved);
>  		/*
> @@ -1621,8 +1622,7 @@ void pull_task(runqueue_t *src_rq, prio_
>  	 * Note that idle threads have a prio of MAX_PRIO, for this test
>  	 * to be always true for them.
>  	 */
> -	if (TASK_PREEMPTS_CURR(p, this_rq))
> -		resched_task(this_rq->curr);
> +	preempt(p, this_rq);
>  }
>  
>  /*
> @@ -3306,8 +3306,8 @@ recheck:
>  		if (task_running(rq, p)) {
>  			if (p->prio > oldprio)
>  				resched_task(rq->curr);
> -		} else if (TASK_PREEMPTS_CURR(p, rq))
> -			resched_task(rq->curr);
> +		} else
> +			preempt(p, rq);
>  	}
>  	task_rq_unlock(rq, &flags);
>  out_unlock:
> @@ -4008,8 +4008,7 @@ static void __migrate_task(struct task_s
>  				+ rq_dest->timestamp_last_tick;
>  		deactivate_task(p, rq_src);
>  		activate_task(p, rq_dest, 0);
> -		if (TASK_PREEMPTS_CURR(p, rq_dest))
> -			resched_task(rq_dest->curr);
> +		preempt(p, rq_dest);
>  	}
>  
>  out:
> 

Re: [PATCH] consolidate task preempts

[Ingo Molnar]

* Con Kolivas <kernel@kolivas.org> wrote:

> consolidate task preempts

nack. This change:

-               if (TASK_PREEMPTS_CURR(p, rq))
-                       resched_task(rq->curr);
+               preempt(p, rq);

hides a real decision made. It might be more acceptable if it was called
'maybe_preempt_curr(p, rq)', but i'm not so sure.

	Ingo