Commit 7afbba11 authored by Joel Fernandes (Google)'s avatar Joel Fernandes (Google) Committed by Peter Zijlstra

sched: Fix priority inversion of cookied task with sibling

The rationale is as follows. In the core-wide pick logic, even if
need_sync == false, we need to go look at other CPUs (non-local CPUs)
to see if they could be running RT.

Say the RQs in a particular core look like this:

Let CFS1 and CFS2 be 2 tagged CFS tags.
Let RT1 be an untagged RT task.

	rq0		rq1
	CFS1 (tagged)	RT1 (no tag)
	CFS2 (tagged)

Say schedule() runs on rq0. Now, it will enter the above loop and
pick_task(RT) will return NULL for 'p'. It will enter the above if()
block and see that need_sync == false and will skip RT entirely.

The end result of the selection will be (say prio(CFS1) > prio(CFS2)):

	rq0             rq1
	CFS1            IDLE

When it should have selected:

	rq0             rq1
	IDLE            RT
Suggested-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: default avatarJoel Fernandes (Google) <joel@joelfernandes.org>
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Tested-by: default avatarDon Hiatt <dhiatt@digitalocean.com>
Tested-by: default avatarHongyu Ning <hongyu.ning@linux.intel.com>
Tested-by: default avatarVincent Guittot <vincent.guittot@linaro.org>
Link: https://lkml.kernel.org/r/20210422123308.678425748@infradead.org
parent 8039e96f
...@@ -5443,6 +5443,15 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf) ...@@ -5443,6 +5443,15 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
put_prev_task_balance(rq, prev, rf); put_prev_task_balance(rq, prev, rf);
smt_mask = cpu_smt_mask(cpu); smt_mask = cpu_smt_mask(cpu);
need_sync = !!rq->core->core_cookie;
/* reset state */
rq->core->core_cookie = 0UL;
if (rq->core->core_forceidle) {
need_sync = true;
fi_before = true;
rq->core->core_forceidle = false;
}
/* /*
* core->core_task_seq, core->core_pick_seq, rq->core_sched_seq * core->core_task_seq, core->core_pick_seq, rq->core_sched_seq
...@@ -5455,14 +5464,25 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf) ...@@ -5455,14 +5464,25 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
* 'Fix' this by also increasing @task_seq for every pick. * 'Fix' this by also increasing @task_seq for every pick.
*/ */
rq->core->core_task_seq++; rq->core->core_task_seq++;
need_sync = !!rq->core->core_cookie;
/* reset state */ /*
rq->core->core_cookie = 0UL; * Optimize for common case where this CPU has no cookies
if (rq->core->core_forceidle) { * and there are no cookied tasks running on siblings.
*/
if (!need_sync) {
for_each_class(class) {
next = class->pick_task(rq);
if (next)
break;
}
if (!next->core_cookie) {
rq->core_pick = NULL;
goto done;
}
need_sync = true; need_sync = true;
rq->core->core_forceidle = false;
} }
for_each_cpu(i, smt_mask) { for_each_cpu(i, smt_mask) {
struct rq *rq_i = cpu_rq(i); struct rq *rq_i = cpu_rq(i);
...@@ -5492,31 +5512,8 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf) ...@@ -5492,31 +5512,8 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
* core. * core.
*/ */
p = pick_task(rq_i, class, max); p = pick_task(rq_i, class, max);
if (!p) { if (!p)
/*
* If there weren't no cookies; we don't need to
* bother with the other siblings.
* If the rest of the core is not running a tagged
* task, i.e. need_sync == 0, and the current CPU
* which called into the schedule() loop does not
* have any tasks for this class, skip selecting for
* other siblings since there's no point. We don't skip
* for RT/DL because that could make CFS force-idle RT.
*/
if (i == cpu && !need_sync && class == &fair_sched_class)
goto next_class;
continue; continue;
}
/*
* Optimize the 'normal' case where there aren't any
* cookies and we don't need to sync up.
*/
if (i == cpu && !need_sync && !p->core_cookie) {
next = p;
goto done;
}
rq_i->core_pick = p; rq_i->core_pick = p;
...@@ -5544,19 +5541,9 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf) ...@@ -5544,19 +5541,9 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
cpu_rq(j)->core_pick = NULL; cpu_rq(j)->core_pick = NULL;
} }
goto again; goto again;
} else {
/*
* Once we select a task for a cpu, we
* should not be doing an unconstrained
* pick because it might starve a task
* on a forced idle cpu.
*/
need_sync = true;
} }
} }
} }
next_class:;
} }
rq->core->core_pick_seq = rq->core->core_task_seq; rq->core->core_pick_seq = rq->core->core_task_seq;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment