Re: [RFC PATCH v2 11/17] sched: Basic tracking of matching tasks

[Vineeth Remanan Pillai]

> > I do not have a strong opinion on both. Probably a better approach
> > would be to replace both cpu_prio_less/core_prio_less with prio_less
> > which takes the third arguement 'bool on_same_rq'?
> >
>
> Fwiw, I find the two names easier to read than a boolean flag. Could still
> be wrapped to a single implementation I suppose.
>
> An enum to control cpu or core would be more readable, but probably overkill...
>
I think we can infact remove the boolean altogether and still have a single
function to compare the priority. If tasks are on the same cpu, use the task's
vruntime, else do the normalization.

Thanks,
Vineeth

---
-static inline bool __prio_less(struct task_struct *a, struct task_struct *b, bool core_cmp)
+static inline bool prio_less(struct task_struct *a, struct task_struct *b)
 {
-	u64 vruntime;
 
 	int pa = __task_prio(a), pb = __task_prio(b);
 
@@ -119,25 +105,21 @@ static inline bool __prio_less(struct task_struct *a, struct task_struct *b, boo
 	if (pa == -1) /* dl_prio() doesn't work because of stop_class above */
 		return !dl_time_before(a->dl.deadline, b->dl.deadline);
 
-	vruntime = b->se.vruntime;
-	if (core_cmp) {
-		vruntime -= task_cfs_rq(b)->min_vruntime;
-		vruntime += task_cfs_rq(a)->min_vruntime;
-	}
-	if (pa == MAX_RT_PRIO + MAX_NICE) /* fair */
-		return !((s64)(a->se.vruntime - vruntime) <= 0);
+	if (pa == MAX_RT_PRIO + MAX_NICE)  { /* fair */
+		u64 vruntime = b->se.vruntime;
 
-	return false;
-}
+		/*
+		 * Normalize the vruntime if tasks are in different cpus.
+		 */
+		if (task_cpu(a) != task_cpu(b)) {
+			vruntime -= task_cfs_rq(b)->min_vruntime;
+			vruntime += task_cfs_rq(a)->min_vruntime;
+		}
 
-static inline bool cpu_prio_less(struct task_struct *a, struct task_struct *b)
-{
-	return __prio_less(a, b, false);
-}
+		return !((s64)(a->se.vruntime - vruntime) <= 0);
+	}
 
-static inline bool core_prio_less(struct task_struct *a, struct task_struct *b)
-{
-	return __prio_less(a, b, true);
+	return false;
 }
 
 static inline bool __sched_core_less(struct task_struct *a, struct task_struct *b)
@@ -149,7 +131,7 @@ static inline bool __sched_core_less(struct task_struct *a, struct task_struct *
 		return false;
 
 	/* flip prio, so high prio is leftmost */
-	if (cpu_prio_less(b, a))
+	if (prio_less(b, a))
 		return true;
 
 	return false;
@@ -3621,7 +3603,7 @@ pick_task(struct rq *rq, const struct sched_class *class, struct task_struct *ma
 		 * higher priority than max.
 		 */
 		if (max && class_pick->core_cookie &&
-		    core_prio_less(class_pick, max))
+		    prio_less(class_pick, max))
 			return idle_sched_class.pick_task(rq);
 
 		return class_pick;
@@ -3640,8 +3622,8 @@ pick_task(struct rq *rq, const struct sched_class *class, struct task_struct *ma
 	 * the core (so far) and it must be selected, otherwise we must go with
 	 * the cookie pick in order to satisfy the constraint.
 	 */
-	if (cpu_prio_less(cookie_pick, class_pick) &&
-	    (!max || core_prio_less(max, class_pick)))
+	if (prio_less(cookie_pick, class_pick) &&
+	    (!max || prio_less(max, class_pick)))
 		return class_pick;
 
 	return cookie_pick;