lkml.org 
[lkml]   [2011]   [May]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 09/11] job control: reorganize wait_task_stopped()
    On 05/08, Tejun Heo wrote:
    >
    > wait_task_stopped() tested task_stopped_code() without acquiring
    > siglock and, if stop condition existed, called wait_task_stopped() and
    > directly returned the result.
    >
    > it may race against SIGCONT generation.

    Hmm. This is the plain bug, even if unlikely and minor.

    > It seems that WNOHANG wait correctness has never been guaranteed and
    > everybody has been happy with it for very long time.

    Yes, the window is tiny. May be it was never noticed or never
    reported because this is hard to diagnose/reproduced.

    > As such,
    > although this reorganization improves the situation a bit, I don't
    > consider this to be a bug fix.

    But it is?

    Can't we push this patch ahead of these changes? I can merge it into
    ptrace branch.


    > static int wait_task_stopped(struct wait_opts *wo,
    > int ptrace, struct task_struct *p)
    > @@ -1397,6 +1409,9 @@ static int wait_task_stopped(struct wait_opts *wo,
    > if (!ptrace && !(wo->wo_flags & WUNTRACED))
    > return 0;
    >
    > + if (!task_stopped_code(p, ptrace))
    > + return 0;
    > +
    > exit_code = 0;
    > spin_lock_irq(&p->sighand->siglock);
    >
    > @@ -1607,8 +1622,9 @@ static int wait_consider_task(struct wait_opts *wo, int ptrace,
    > * Wait for stopped. Depending on @ptrace, different stopped state
    > * is used and the two don't interact with each other.
    > */
    > - if (task_stopped_code(p, ptrace))
    > - return wait_task_stopped(wo, ptrace, p);
    > + ret = wait_task_stopped(wo, ptrace, p);
    > + if (ret)
    > + return ret;

    Reviewed-by: Oleg Nesterov <oleg@redhat.com>



    \
     
     \ /
      Last update: 2011-05-11 18:15    [W:0.023 / U:209.676 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site