[PATCH] balance RT task resched only on runqueue

[PATCH] balance RT task resched only on runqueue

[Steven Rostedt]

In the RT kernel we discovered a place that can reschedule a task
without holding the tasks runqueue lock.  This was caused by the RT
balancing code that pulls RT tasks to the current run queue and will
reschedule the current task.

There's a slight chance that the pulling of the RT tasks will release
the current runqueue's lock and retake it (in the double_lock_balance).
During this time that the runqueue is released, the current task can
migrate to another runqueue.

In the prio_changed_rt code, after the pull, if the current task is of
lesser priority than one of the RT tasks pulled, resched_task is called
on the current task. If the current task had migrated in that small
window, resched_task will be called without holding the runqueue lock
for the runqueue that the task is on.

This race condition also exists in the mainline kernel and this patch
adds a check to make sure the task hasn't migrated before calling
resched_task.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 kernel/sched_rt.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Index: linux-compile.git/kernel/sched_rt.c
===================================================================
--- linux-compile.git.orig/kernel/sched_rt.c	2008-03-05 09:07:28.000000000 -0500
+++ linux-compile.git/kernel/sched_rt.c	2008-03-05 09:18:58.000000000 -0500
@@ -1107,9 +1107,11 @@ static void prio_changed_rt(struct rq *r
 			pull_rt_task(rq);
 		/*
 		 * If there's a higher priority task waiting to run
-		 * then reschedule.
+		 * then reschedule. Note, the above pull_rt_task
+		 * can release the rq lock and p could migrate.
+		 * Only reschedule if p is still on the same runqueue.
 		 */
-		if (p->prio > rq->rt.highest_prio)
+		if (p->prio > rq->rt.highest_prio && task_rq(p) == rq)
 			resched_task(p);
 #else
 		/* For UP simply resched on drop of prio */

[PATCH -v2] balance RT task resched only on runqueue

[Steven Rostedt]

[
  Update: I didn't like the task_rq(p), so I changed the test to be
  rq->curr == p. No need to resched if the p isn't running. And
  I think this is a bit cleaner.
]

In the RT kernel we discovered a place that can reschedule a task
without holding the tasks runqueue lock.  This was caused by the RT
balancing code that pulls RT tasks to the current run queue and will
reschedule the current task.

There's a slight chance that the pulling of the RT tasks will release
the current runqueue's lock and retake it (in the double_lock_balance).
During this time that the runqueue is released, the current task can
migrate to another runqueue.

In the prio_changed_rt code, after the pull, if the current task is of
lesser priority than one of the RT tasks pulled, resched_task is called
on the current task. If the current task had migrated in that small
window, resched_task will be called without holding the runqueue lock
for the runqueue that the task is on.

This race condition also exists in the mainline kernel and this patch
adds a check to make sure the task hasn't migrated before calling
resched_task.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 kernel/sched_rt.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Index: linux-compile.git/kernel/sched_rt.c
===================================================================
--- linux-compile.git.orig/kernel/sched_rt.c	2008-03-05 09:31:22.000000000 -0500
+++ linux-compile.git/kernel/sched_rt.c	2008-03-05 09:53:00.000000000 -0500
@@ -1107,9 +1107,11 @@ static void prio_changed_rt(struct rq *r
 			pull_rt_task(rq);
 		/*
 		 * If there's a higher priority task waiting to run
-		 * then reschedule.
+		 * then reschedule. Note, the above pull_rt_task
+		 * can release the rq lock and p could migrate.
+		 * Only reschedule if p is still on the same runqueue.
 		 */
-		if (p->prio > rq->rt.highest_prio)
+		if (p->prio > rq->rt.highest_prio && rq->curr == p)
 			resched_task(p);
 #else
 		/* For UP simply resched on drop of prio */

Re: [PATCH -v2] balance RT task resched only on runqueue

[Peter Zijlstra]

On Wed, 2008-03-05 at 10:00 -0500, Steven Rostedt wrote:
> [
>   Update: I didn't like the task_rq(p), so I changed the test to be
>   rq->curr == p. No need to resched if the p isn't running. And
>   I think this is a bit cleaner.
> ]
> 
> In the RT kernel we discovered a place that can reschedule a task
> without holding the tasks runqueue lock.  This was caused by the RT
> balancing code that pulls RT tasks to the current run queue and will
> reschedule the current task.
> 
> There's a slight chance that the pulling of the RT tasks will release
> the current runqueue's lock and retake it (in the double_lock_balance).
> During this time that the runqueue is released, the current task can
> migrate to another runqueue.
> 
> In the prio_changed_rt code, after the pull, if the current task is of
> lesser priority than one of the RT tasks pulled, resched_task is called
> on the current task. If the current task had migrated in that small
> window, resched_task will be called without holding the runqueue lock
> for the runqueue that the task is on.
> 
> This race condition also exists in the mainline kernel and this patch
> adds a check to make sure the task hasn't migrated before calling
> resched_task.
> 
> Signed-off-by: Steven Rostedt <srostedt@redhat.com>

Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

> ---
>  kernel/sched_rt.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Index: linux-compile.git/kernel/sched_rt.c
> ===================================================================
> --- linux-compile.git.orig/kernel/sched_rt.c	2008-03-05 09:31:22.000000000 -0500
> +++ linux-compile.git/kernel/sched_rt.c	2008-03-05 09:53:00.000000000 -0500
> @@ -1107,9 +1107,11 @@ static void prio_changed_rt(struct rq *r
>  			pull_rt_task(rq);
>  		/*
>  		 * If there's a higher priority task waiting to run
> -		 * then reschedule.
> +		 * then reschedule. Note, the above pull_rt_task
> +		 * can release the rq lock and p could migrate.
> +		 * Only reschedule if p is still on the same runqueue.
>  		 */
> -		if (p->prio > rq->rt.highest_prio)
> +		if (p->prio > rq->rt.highest_prio && rq->curr == p)
>  			resched_task(p);
>  #else
>  		/* For UP simply resched on drop of prio */
> 
> 

Re: [PATCH] balance RT task resched only on runqueue

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> In the RT kernel we discovered a place that can reschedule a task 
> without holding the tasks runqueue lock.  This was caused by the RT 
> balancing code that pulls RT tasks to the current run queue and will 
> reschedule the current task.
> 
> There's a slight chance that the pulling of the RT tasks will release 
> the current runqueue's lock and retake it (in the 
> double_lock_balance). During this time that the runqueue is released, 
> the current task can migrate to another runqueue.
> 
> In the prio_changed_rt code, after the pull, if the current task is of 
> lesser priority than one of the RT tasks pulled, resched_task is 
> called on the current task. If the current task had migrated in that 
> small window, resched_task will be called without holding the runqueue 
> lock for the runqueue that the task is on.
> 
> This race condition also exists in the mainline kernel and this patch 
> adds a check to make sure the task hasn't migrated before calling 
> resched_task.

thanks Steve, applied.

	Ingo