Messages in this thread | | | Date | Fri, 06 Jul 2012 07:50:27 +0800 | From | Michael Wang <> | Subject | Re: [RFC BUG] There is a potential bug in "yield_to" |
| |
On 07/05/2012 04:35 PM, Peter Zijlstra wrote: > On Thu, 2012-07-05 at 13:31 +0800, Michael Wang wrote: >> Hi, All >> >> I found there may be a potential bug in "yield_to": >> >> local_irq_save(flags); >> rq = this_rq(); >> >> again: >> >> //task's rq may already changed in "sched_move_task" >> >> p_rq = task_rq(p); >> double_rq_lock(rq, p_rq); >> while (task_rq(p) != p_rq) { >> double_rq_unlock(rq, p_rq); >> goto again; >> } >> >> I think it may happen in this scene: >> >> cpu 0 cpu 1(task a) >> >> yield_to { >> disable_irq >> sched_move_task { rq = this_rq(); >> task_rq_lock(task a) double_rq_lock >> >> hold lock of rq 1 >> set_task_rq //task rq changed >> release lock of rq 1 >> >> hold lock of rq 1 >> but task b no longer >> there >> >> set rq 1's current to >> skip which is not task a >> >> which means we hold a rq's lock but it's current is not the one should >> do yield. >> >> Only "sched_move_task" will cause this issue as it will move the task >> which is still running. >> >> The bug will make the task who want to do yield failed to set skip buddy >> to himself, but to a innocent task instead, not very harmful and almost >> impossible to occur in normal, but should we fix it with another check >> "rq == this_rq()"? > > Uhm, what?! > > We've got interrupts disabled, this_rq() cannot ever possibly change, so > rq is always correct. > I know I should have missed some thing, the schedule won't happen until enable the irq later, so even that scene happen, nothing will change on rq.
Thanks for your explain :)
Regards, Michael Wang
> Only p_rq can change, and we have an again loop on that, so what's the > problem again? >
| |