Skip to contentSkip to conversation

sched/rt: Fix picking RT and DL tasks from empty queue

PATCH In review Kirill Tkhai 3 participants 3 messages Mar 4, 2014 – Mar 11, 2014 Scheduler

lore
Share & tools
lore

Kirill Tkhai

author
#0 lore raw

PATCH 1/1 sched/rt: Fix picking RT and DL tasks from empty queue

The problems:

1)We check for rt_nr_running before call of put_prev_task(). If previous task is RT, its rt_rq may become throttled and dequeued after this call.

In case of p is from rt->rq this just causes picking a task from throttled queue, but in case of its rt_rq is child we are guaranteed catch BUG_ON.

2)The same with deadline class. The only difference we operate on only dl_rq.

This patch fix all above and it adds a small skip in DL update like we've already done for RT class:

	if (unlikely((s64)delta_exec <= 0))
		return;

This will optimize sequential update_curr_dl() calls a little.

Signed-off-by Kirill Tkhai <[email protected]>
CC Juri Lelli <[email protected]>
CC Peter Zijlstra <[email protected]>
CC Ingo Molnar <[email protected]>
---
 kernel/sched/deadline.c | 10 ++++++++--
 kernel/sched/rt.c       |  7 +++++++
 2 files changed, 15 insertions(+), 2 deletions(-)
kernel/sched/deadline.c +8 -2
index e4f3ac3..27ef409 100644
@@ -609,8 +609,8 @@ static void update_curr_dl(struct rq *rq)
 	 * approach need further study.
 	 */
 	delta_exec = rq_clock_task(rq) - curr->se.exec_start;
-	if (unlikely((s64)delta_exec < 0))
-		delta_exec = 0;
+	if (unlikely((s64)delta_exec <= 0))
+		return;
 
 	schedstat_set(curr->se.statistics.exec_max,
 		      max(curr->se.statistics.exec_max, delta_exec));

@@ -1023,6 +1023,12 @@ struct task_struct *pick_next_task_dl(struct rq *rq, struct task_struct *prev)
 
 	if (need_pull_dl_task(rq, prev))
 		pull_dl_task(rq);
+	/*
+	 * When prev is DL, we may throttle it in put_prev_task().
+	 * So, we update time before we check for dl_nr_running.
+	 */
+	if (prev->sched_class == &dl_sched_class)
+		update_curr_dl(rq);
 
 	if (unlikely(!dl_rq->dl_nr_running))
 		return NULL;
kernel/sched/rt.c +7 -0
index facc824..f3cee0a 100644
@@ -1379,6 +1379,13 @@ pick_next_task_rt(struct rq *rq, struct task_struct *prev)
 			return RETRY_TASK;
 	}
 
+	/*
+	 * We may dequeue prev's rt_rq in put_prev_task().
+	 * So, we update time before rt_nr_running check.
+	 */
+	if (prev->sched_class == &rt_sched_class)
+		update_curr_rt(rq);
+
 	if (!rt_rq->rt_nr_running)
 		return NULL;
maintainer
#1 lore raw
Quoted 13 lines · Kirill Tkhai

The problems:

1)We check for rt_nr_running before call of put_prev_task(). If previous task is RT, its rt_rq may become throttled and dequeued after this call.

In case of p is from rt->rq this just causes picking a task from throttled queue, but in case of its rt_rq is child we are guaranteed catch BUG_ON.

2)The same with deadline class. The only difference we operate on only dl_rq.

Quoted 8 lines

This patch fix all above and it adds a small skip in DL update like we've already done for RT class:

	if (unlikely((s64)delta_exec <= 0))
		return;

This will optimize sequential update_curr_dl() calls a little.

Ah indeed, good catch that. Thanks!

merge bot
#2 lore raw
In reply to Kirill Tkhai
sched/rt: Fix picking RT and DL tasks from empty queue
Commit-ID:  734ff2a71f9e6aa6fedfa5a9a34818b8586516d5
Gitweb:     http://git.kernel.org/tip/734ff2a71f9e6aa6fedfa5a9a34818b8586516d5
Author:     Kirill Tkhai <[email protected]>
AuthorDate: Tue, 4 Mar 2014 19:25:46 +0400
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 11 Mar 2014 12:05:35 +0100

sched/rt: Fix picking RT and DL tasks from empty queue

The problems:

1) We check for rt_nr_running before call of put_prev_task(). If previous task is RT, its rt_rq may become throttled and dequeued after this call.

In case of p is from rt->rq this just causes picking a task from throttled queue, but in case of its rt_rq is child we are guaranteed catch BUG_ON.

2) The same with deadline class. The only difference we operate on only dl_rq.

This patch fixes all the above problems and it adds a small skip in the DL update like we've already done for RT class:

	if (unlikely((s64)delta_exec <= 0))
		return;

This will optimize sequential update_curr_dl() calls a little.

Signed-off-by Kirill Tkhai <[email protected]>
Signed-off-by Peter Zijlstra <[email protected]>
Cc Juri Lelli <[email protected]>
Link http://lkml.kernel.org/r/1393946746.3643.3.camel@tkhai open here
Signed-off-by Ingo Molnar <[email protected]>
---
 kernel/sched/deadline.c | 10 ++++++++--
 kernel/sched/rt.c       |  7 +++++++
 2 files changed, 15 insertions(+), 2 deletions(-)
kernel/sched/deadline.c +8 -2
index e4f3ac3..27ef409 100644
@@ -609,8 +609,8 @@ static void update_curr_dl(struct rq *rq)
 	 * approach need further study.
 	 */
 	delta_exec = rq_clock_task(rq) - curr->se.exec_start;
-	if (unlikely((s64)delta_exec < 0))
-		delta_exec = 0;
+	if (unlikely((s64)delta_exec <= 0))
+		return;
 
 	schedstat_set(curr->se.statistics.exec_max,
 		      max(curr->se.statistics.exec_max, delta_exec));

@@ -1023,6 +1023,12 @@ struct task_struct *pick_next_task_dl(struct rq *rq, struct task_struct *prev)
 
 	if (need_pull_dl_task(rq, prev))
 		pull_dl_task(rq);
+	/*
+	 * When prev is DL, we may throttle it in put_prev_task().
+	 * So, we update time before we check for dl_nr_running.
+	 */
+	if (prev->sched_class == &dl_sched_class)
+		update_curr_dl(rq);
 
 	if (unlikely(!dl_rq->dl_nr_running))
 		return NULL;
kernel/sched/rt.c +7 -0
index facc824..f3cee0a 100644
@@ -1379,6 +1379,13 @@ pick_next_task_rt(struct rq *rq, struct task_struct *prev)
 			return RETRY_TASK;
 	}
 
+	/*
+	 * We may dequeue prev's rt_rq in put_prev_task().
+	 * So, we update time before rt_nr_running check.
+	 */
+	if (prev->sched_class == &rt_sched_class)
+		update_curr_rt(rq);
+
 	if (!rt_rq->rt_nr_running)
 		return NULL;