lkml.org 
[lkml]   [2011]   [Dec]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [BUG] TASK_DEAD task is able to be woken up in special condition
    Thank you for your response.


    > > ----
    > > Signed-off-by: Yasunori Goto<y-goto@jp.fujitsu.com>
    > >
    > > ---
    > > kernel/exit.c | 3 +++
    > > 1 file changed, 3 insertions(+)
    > >
    > > Index: linux-3.2-rc4/kernel/exit.c
    > > ===================================================================
    > > --- linux-3.2-rc4.orig/kernel/exit.c
    > > +++ linux-3.2-rc4/kernel/exit.c
    > > @@ -1038,8 +1038,11 @@ NORET_TYPE void do_exit(long code)
    > >
    > > preempt_disable();
    > > exit_rcu();
    > > +
    > > + spin_lock(&tsk->pi_lock, flags);
    > > /* causes final put_task_struct in finish_task_switch(). */
    > > tsk->state = TASK_DEAD;
    > > + spin_unlock(&tsk->pi_lock, flags);
    > > schedule();
    > > BUG();
    > > /* Avoid "noreturn function does return". */
    >
    > I doubt it is not only TASK_DEAD issue, it is rwsem fundamental issue.
    > Because of, a lot of place assume "current->state = newstate" is safe
    > and don't need any synchronization. So, I'm worry about to lost
    > TASK_UNINTERRUPTIBLE can make catastrophe like TASK_DEAD.

    I don't understand why this is catastrophe.
    I suppose it is just waken up from TASK_UNINTERRUPTIBLE
    by try_to_wake_up() in race condition. It seems to be normal situation.....

    But TASK_DEAD status is special. It must not return to TASK_RUNNING state.

    >
    > How about following patch? anyway, rwsem_down_failed_common() is
    > definitely slowpath. so killing micro optimization is not so much
    > problem, I guess.
    >
    >
    >
    > diff --git a/lib/rwsem.c b/lib/rwsem.c
    > index 410aa11..e2a0c9a 100644
    > --- a/lib/rwsem.c
    > +++ b/lib/rwsem.c
    > @@ -208,9 +208,9 @@ rwsem_down_failed_common(struct rw_semaphore *sem,
    >
    > /* wait to be given the lock */
    > for (;;) {
    > + schedule();
    > if (!waiter.task)
    > break;
    > - schedule();
    > set_task_state(tsk, TASK_UNINTERRUPTIBLE);
    > }
    >

    Hmmmmmmm.
    Are you sure there is no route which TASK_DEAD task is waken up like rwsem?


    Thanks.
    --
    Yasunori Goto




    \
     
     \ /
      Last update: 2011-12-22 09:25    [W:0.026 / U:0.176 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site