Commit 2548d546 authored by Peter Zijlstra's avatar Peter Zijlstra Committed by Ingo Molnar

nohz/full, sched/rt: Fix missed tick-reenabling bug in sched_can_stop_tick()

Chris Metcalf reported a that sched_can_stop_tick() sometimes fails to
re-enable the tick.

His observed problem is that rq->cfs.nr_running can be 1 even though
there are multiple runnable CFS tasks. This happens in the cgroup
case, in which case cfs.nr_running is the number of runnable entities
for that level.

If there is a single runnable cgroup (which can have an arbitrary
number of runnable child entries itself) rq->cfs.nr_running will be 1.

However, looking at that function I think there's more problems with it.

It seems to assume that if there's FIFO tasks, those will run. This is
incorrect. The FIFO task can have a lower prio than an RR task, in which
case the RR task will run.

So the whole fifo_nr_running test seems misplaced, it should go after
the rr_nr_running tests. That is, only if !rr_nr_running, can we use
fifo_nr_running like this.
Reported-by: default avatarChris Metcalf <cmetcalf@mellanox.com>
Tested-by: default avatarChris Metcalf <cmetcalf@mellanox.com>
Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Luiz Capitulino <lcapitulino@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Wanpeng Li <kernellwp@gmail.com>
Fixes: 76d92ac3 ("sched: Migrate sched to use new tick dependency mask model")
Link: http://lkml.kernel.org/r/20160421160315.GK24771@twins.programming.kicks-ass.netSigned-off-by: default avatarIngo Molnar <mingo@kernel.org>
parent 02da2d72
...@@ -596,17 +596,8 @@ bool sched_can_stop_tick(struct rq *rq) ...@@ -596,17 +596,8 @@ bool sched_can_stop_tick(struct rq *rq)
return false; return false;
/* /*
* FIFO realtime policy runs the highest priority task (after DEADLINE). * If there are more than one RR tasks, we need the tick to effect the
* Other runnable tasks are of a lower priority. The scheduler tick * actual RR behaviour.
* isn't needed.
*/
fifo_nr_running = rq->rt.rt_nr_running - rq->rt.rr_nr_running;
if (fifo_nr_running)
return true;
/*
* Round-robin realtime tasks time slice with other tasks at the same
* realtime priority.
*/ */
if (rq->rt.rr_nr_running) { if (rq->rt.rr_nr_running) {
if (rq->rt.rr_nr_running == 1) if (rq->rt.rr_nr_running == 1)
...@@ -615,8 +606,20 @@ bool sched_can_stop_tick(struct rq *rq) ...@@ -615,8 +606,20 @@ bool sched_can_stop_tick(struct rq *rq)
return false; return false;
} }
/* Normal multitasking need periodic preemption checks */ /*
if (rq->cfs.nr_running > 1) * If there's no RR tasks, but FIFO tasks, we can skip the tick, no
* forced preemption between FIFO tasks.
*/
fifo_nr_running = rq->rt.rt_nr_running - rq->rt.rr_nr_running;
if (fifo_nr_running)
return true;
/*
* If there are no DL,RR/FIFO tasks, there must only be CFS tasks left;
* if there's more than one we need the tick for involuntary
* preemption.
*/
if (rq->nr_running > 1)
return false; return false;
return true; return true;
......
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