sched: fix rt_nr_migratory corruption raised in push_rt_task()
PATCH 1/1 sched: fix rt_nr_migratory corruption raised in push_rt_task()
When pushing, if a pushable task could not be pushed, it is dequeued with no updating the rt_nr_migratory element of RT run-queue, then rt_nr_migratory is corrupted, which is fixed by removing the dequeue operation.
--- kernel/sched_rt.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
kernel/sched_rt.c +3 -2
@@ -1378,6 +1378,7 @@ static int push_rt_task(struct rq *rq) {struct task_struct *next_task;struct rq *lowest_rq; +int ret =1 ;if (!rq->rt.overloaded)return 0 ;
@@ -1425,7 +1426,7 @@ retry:* since the other cpus will pull from us when they * are ready. */ -dequeue_pushable_task (rq, next_task); + ret =0 ; goto out; }
@@ -1452,7 +1453,7 @@ retry: out:put_task_struct (next_task); -return 1 ; +return ret; }static void push_rt_tasks (struct rq *rq)
Quoted 3 lines · Hillf Danton
When pushing, if a pushable task could not be pushed, it is dequeued with no updating the
rt_nr_migratoryelement of RT run-queue, thenrt_nr_migratoryis corrupted, which is fixed by removing the dequeue operation.
Hm. I think you're right that this dequeue_pushable_task() call can be removed, at least I didn't see it's reason for existing. I'd word it a bit differently though, and not return 1 unless we really did push.
From: Hillf Danton <[email protected]>
sched, rt: fix rq->rt.pushable_tasks bug in push_rt_task()
Do not call dequeue_pushable_task() when failing to push an eligible task, as it remains pushable, merely not at this particular moment.
--- kernel/sched_rt.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
kernel/sched_rt.c +8 -7
@@ -1378,6 +1378,7 @@ static int push_rt_task(struct rq *rq) {struct task_struct *next_task;struct rq *lowest_rq; +int ret =0 ;if (!rq->rt.overloaded)return 0 ;
@@ -1410,7 +1411,7 @@ retry:if (!lowest_rq) {struct task_struct *task;/* -* find lock_lowest_rq releases rq->lock +* find_lock_lowest_rq releases rq->lock * so it is possible that next_task has migrated. * * We need to make sure that the task is still on the same
@@ -1420,12 +1421,11 @@ retry: task =pick_next_pushable_task (rq);if (task_cpu (next_task) == rq->cpu && task == next_task) {/* -* If we get here, the task hasn't moved at all, but -* it has failed to push. We will not try again, -* since the other cpus will pull from us when they -* are ready. +* The task hasn't migrated, and is still the next +* eligible task, but we failed to find a run-queue +* to push it to. Do not retry in this case, since +* other cpus will pull from us when ready. */ -dequeue_pushable_task (rq, next_task); goto out; }
@@ -1444,6 +1444,7 @@ retry:deactivate_task (rq, next_task,0 );set_task_cpu (next_task, lowest_rq->cpu);activate_task (lowest_rq, next_task,0 ); + ret =1 ;resched_task (lowest_rq->curr);
@@ -1452,7 +1453,7 @@ retry: out:put_task_struct (next_task); -return 1 ; +return ret; }static void push_rt_tasks (struct rq *rq)
Quoted 9 lines · Mike Galbraith
Quoted 3 lines · Hillf Danton
When pushing, if a pushable task could not be pushed, it is dequeued with no updating the
rt_nr_migratoryelement of RT run-queue, thenrt_nr_migratoryis corrupted, which is fixed by removing the dequeue operation.Hm. I think you're right that this
dequeue_pushable_task()call can be removed, at least I didn't see it's reason for existing. I'd word it a bit differently though, and not return 1 unless we really did push.
Hi Mike
Your message shows what is called professional work, but something lost,
Quoted 8 lines
From: Hillf Danton <[email protected]>
sched, rt: fix
rq->rt.pushable_tasksbug inpush_rt_task()Do not call
dequeue_pushable_task()when failing to push an eligible task, as it remains pushable, merely not at this particular moment.Signed-off-by Hillf Danton <[email protected]>
right?
Quoted 6 lines
---
kernel/sched_rt.c | 15 ++++++++-------1 file changed, 8 insertions(+), 7 deletions(-)Index:
linux-2.6.40.git/kernel/sched_rt.c
===================================================================
And how to drive git to print the above two lines?
thanks
Hillf
Quoted 17 lines · Hillf Danton
Quoted 9 lines · Mike Galbraith
Quoted 3 lines · Hillf Danton
When pushing, if a pushable task could not be pushed, it is dequeued with no updating the
rt_nr_migratoryelement of RT run-queue, thenrt_nr_migratoryis corrupted, which is fixed by removing the dequeue operation.Hm. I think you're right that this
dequeue_pushable_task()call can be removed, at least I didn't see it's reason for existing. I'd word it a bit differently though, and not return 1 unless we really did push.Hi Mike
Your message shows what is called professional work, but something lost,
Quoted 2 lines
From: Hillf Danton <[email protected]>
Quoted 9 lines
Quoted 6 lines
sched, rt: fix
rq->rt.pushable_tasksbug inpush_rt_task()Do not call
dequeue_pushable_task()when failing to push an eligible task, as it remains pushable, merely not at this particular moment.Signed-off-by Hillf Danton <[email protected]>Signed-off-by Mike Galbraith <[email protected]>
Yeah, Mike, can you remail this adding your SOB too. You can keep Hillf's SOB as well, since both of you made changes.
Thanks!
-- Steve
PATCH 1/1 sched, rt: fix rq->rt.pushable_tasks bug in push_rt_task()
Quoted 23 lines · Steven Rostedt
Quoted 22 lines · Hillf Danton
Quoted 9 lines · Mike Galbraith
Quoted 3 lines · Hillf Danton
When pushing, if a pushable task could not be pushed, it is dequeued with no updating the
rt_nr_migratoryelement of RT run-queue, thenrt_nr_migratoryis corrupted, which is fixed by removing the dequeue operation.Hm. I think you're right that this
dequeue_pushable_task()call can be removed, at least I didn't see it's reason for existing. I'd word it a bit differently though, and not return 1 unless we really did push.Hi Mike
Your message shows what is called professional work, but something lost,
Quoted 7 lines
From: Hillf Danton <[email protected]>
sched, rt: fix
rq->rt.pushable_tasksbug inpush_rt_task()Do not call
dequeue_pushable_task()when failing to push an eligible task, as it remains pushable, merely not at this particular moment.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Hillf Danton <[email protected]>Signed-off-by Mike Galbraith <[email protected]>Yeah, Mike, can you remail this adding your SOB too. You can keep Hillf's SOB as well, since both of you made changes.
From: Hillf Danton <[email protected]>
AuthorDate: Mon, 30 May 2011 21:39:50 +0800
sched, rt: fix rq->rt.pushable_tasks bug in push_rt_task()
Do not call dequeue_pushable_task() when failing to push an eligible task, as it remains pushable until either it's cpumask says otherwise, or is selected. It is merely not pushable at this particular moment.
--- kernel/sched_rt.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
kernel/sched_rt.c +8 -7
@@ -1378,6 +1378,7 @@ static int push_rt_task(struct rq *rq) {struct task_struct *next_task;struct rq *lowest_rq; +int ret =0 ;if (!rq->rt.overloaded)return 0 ;
@@ -1410,7 +1411,7 @@ retry:if (!lowest_rq) {struct task_struct *task;/* -* find lock_lowest_rq releases rq->lock +* find_lock_lowest_rq releases rq->lock * so it is possible that next_task has migrated. * * We need to make sure that the task is still on the same
@@ -1420,12 +1421,11 @@ retry: task =pick_next_pushable_task (rq);if (task_cpu (next_task) == rq->cpu && task == next_task) {/* -* If we get here, the task hasn't moved at all, but -* it has failed to push. We will not try again, -* since the other cpus will pull from us when they -* are ready. +* The task hasn't migrated, and is still the next +* eligible task, but we failed to find a run-queue +* to push it to. Do not retry in this case, since +* other cpus will pull from us when ready. */ -dequeue_pushable_task (rq, next_task); goto out; }
@@ -1444,6 +1444,7 @@ retry:deactivate_task (rq, next_task,0 );set_task_cpu (next_task, lowest_rq->cpu);activate_task (lowest_rq, next_task,0 ); + ret =1 ;resched_task (lowest_rq->curr);
@@ -1452,7 +1453,7 @@ retry: out:put_task_struct (next_task); -return 1 ; +return ret; }static void push_rt_tasks (struct rq *rq)
Re: [PATCH] sched, rt: fix rq->rt.pushable_tasks bug in push_rt_task()
Stupid mouse (driver) grabbed wrong text blob.
From: Hillf Danton <[email protected]>
sched, rt: fix rq->rt.pushable_tasks bug in push_rt_task()
Do not call dequeue_pushable_task() when failing to push an eligible task, as it remains pushable, merely not at this particular moment.
--- kernel/sched_rt.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
kernel/sched_rt.c +8 -7
@@ -1378,6 +1378,7 @@ static int push_rt_task(struct rq *rq) {struct task_struct *next_task;struct rq *lowest_rq; +int ret =0 ;if (!rq->rt.overloaded)return 0 ;
@@ -1410,7 +1411,7 @@ retry:if (!lowest_rq) {struct task_struct *task;/* -* find lock_lowest_rq releases rq->lock +* find_lock_lowest_rq releases rq->lock * so it is possible that next_task has migrated. * * We need to make sure that the task is still on the same
@@ -1420,12 +1421,11 @@ retry: task =pick_next_pushable_task (rq);if (task_cpu (next_task) == rq->cpu && task == next_task) {/* -* If we get here, the task hasn't moved at all, but -* it has failed to push. We will not try again, -* since the other cpus will pull from us when they -* are ready. +* The task hasn't migrated, and is still the next +* eligible task, but we failed to find a run-queue +* to push it to. Do not retry in this case, since +* other cpus will pull from us when ready. */ -dequeue_pushable_task (rq, next_task); goto out; }
@@ -1444,6 +1444,7 @@ retry:deactivate_task (rq, next_task,0 );set_task_cpu (next_task, lowest_rq->cpu);activate_task (lowest_rq, next_task,0 ); + ret =1 ;resched_task (lowest_rq->curr);
@@ -1452,7 +1453,7 @@ retry: out:put_task_struct (next_task); -return 1 ; +return ret; }static void push_rt_tasks (struct rq *rq)
[tip:sched/core] sched, rt: Fix rq->rt.pushable_tasks bug in push_rt_task()
Commit-ID: 311e800e16f63d909136a64ed17ca353a160be59 Gitweb: http://git.kernel.org/tip/311e800e16f63d909136a64ed17ca353a160be59 Author: Hillf Danton <[email protected]> AuthorDate: Thu, 16 Jun 2011 21:55:20 -0400 Committer: Ingo Molnar <[email protected]> CommitDate: Sun, 14 Aug 2011 12:00:48 +0200
sched, rt: Fix rq->rt.pushable_tasks bug in push_rt_task()
Do not call dequeue_pushable_task() when failing to push an eligible task, as it remains pushable, merely not at this particular moment.
--- kernel/sched_rt.c | 15 ++++++++------- 1 files changed, 8 insertions(+), 7 deletions(-)
kernel/sched_rt.c +8 -7
@@ -1394,6 +1394,7 @@ static int push_rt_task(struct rq *rq) {struct task_struct *next_task;struct rq *lowest_rq; +int ret =0 ;if (!rq->rt.overloaded)return 0 ;
@@ -1426,7 +1427,7 @@ retry:if (!lowest_rq) {struct task_struct *task;/* -* find lock_lowest_rq releases rq->lock +* find_lock_lowest_rq releases rq->lock * so it is possible that next_task has migrated. * * We need to make sure that the task is still on the same
@@ -1436,12 +1437,11 @@ retry: task =pick_next_pushable_task (rq);if (task_cpu (next_task) == rq->cpu && task == next_task) {/* -* If we get here, the task hasn't moved at all, but -* it has failed to push. We will not try again, -* since the other cpus will pull from us when they -* are ready. +* The task hasn't migrated, and is still the next +* eligible task, but we failed to find a run-queue +* to push it to. Do not retry in this case, since +* other cpus will pull from us when ready. */ -dequeue_pushable_task (rq, next_task); goto out; }
@@ -1460,6 +1460,7 @@ retry:deactivate_task (rq, next_task,0 );set_task_cpu (next_task, lowest_rq->cpu);activate_task (lowest_rq, next_task,0 ); + ret =1 ;resched_task (lowest_rq->curr);
@@ -1468,7 +1469,7 @@ retry: out:put_task_struct (next_task); -return 1 ; +return ret; }static void push_rt_tasks (struct rq *rq)
Quoted 19 lines · Hillf Danton
Quoted 9 lines · Mike Galbraith
Quoted 3 lines · Hillf Danton
When pushing, if a pushable task could not be pushed, it is dequeued with no updating the
rt_nr_migratoryelement of RT run-queue, thenrt_nr_migratoryis corrupted, which is fixed by removing the dequeue operation.Hm. I think you're right that this
dequeue_pushable_task()call can be removed, at least I didn't see it's reason for existing. I'd word it a bit differently though, and not return 1 unless we really did push.Hi Mike
Your message shows what is called professional work, but something lost,
Quoted 4 lines
From: Hillf Danton <[email protected]>
sched, rt: fix
rq->rt.pushable_tasksbug inpush_rt_task()
Quoted 8 lines
Quoted 4 lines
Do not call
dequeue_pushable_task()when failing to push an eligible task, as it remains pushable, merely not at this particular moment.Signed-off-by Hillf Danton <[email protected]>Signed-off-by Mike Galbraith <[email protected]>right?
Well, you did the legwork, but I resent it as requested.
Quoted 8 lines
Quoted 6 lines
--- kernel/sched_rt.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)Index:
linux-2.6.40.git/kernel/sched_rt.c
===================================================================And how to drive git to print the above two lines?
That's easy, I used quilt :) I'm no master of git-fu (wimpy whitebelt), use git for hunting, but quilt for twiddling.
-Mike