lkml.org 
[lkml]   [2005]   [Aug]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] fix send_sigqueue() vs thread exit race
    Thomas Gleixner wrote:
    >
    > On Mon, 2005-08-22 at 20:45 +0400, Oleg Nesterov wrote:
    > >
    > > kernel/posix-timers.c:common_timer_del() calls del_timer_sync(), after
    > > that nobody can access this timer, so we don't need to lock timer->it_lock
    > > at all in this case. No lock - no deadlock.
    >
    > It still deadlocks:
    >
    > CPU 0 CPU 1
    > write_lock(&tasklist_lock);
    > __exit_signal()
    > timer expires
    > base->running_timer = timer
    > send_group_sigqueue()
    > read_lock(&tasklist_lock();
    > exit_itimers()
    > del_timer_sync(timer)
    > waits for ever because waits for ever on tasklist_lock
    > base->running_timer == timer

    Silly me.

    > I still think the last patch I sent is still necessary.

    Thomas, you know that I like this change in __exit_{signal,sighand},
    but i think this change is dangerous, should go in a separate patch,
    and needs a lot of testing. But the decision is up to Ingo and Roland.

    I am looking at your previous patch:

    > - read_lock(&tasklist_lock);
    > +retry:
    > + if (unlikely(p->flags & PF_EXITING))
    > + return -1;
    > +
    > + if (unlikely(!read_trylock(&tasklist_lock))) {
    > + cpu_relax();
    > + goto retry;
    > + }
    > + if (unlikely(p->flags & PF_EXITING)) {
    > + ret = -1;
    > + goto out_err;

    What do you think about this:

    int try_to_lock_this_beep_tasklist_lock(struct task_struct *group_leader)
    {
    while (unlikely(!read_trylock(&tasklist_lock))) {
    if (group_leader->flags & PF_EXITING) {
    smp_rmb();
    if (thread_group_empty(group_leader))
    return 0;
    }
    cpu_relax();
    }

    return 1;
    }

    No need to re-check after we got tasklist, the signal will be flushed.
    I think it's better to move the locking into the posix_timer_event, btw.
    In that case we can drop my patch.

    What is your opinion, can it work?

    P.S.
    Thomas, thanks for explanation about posix-cpu-timers.

    Oleg.
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-08-23 18:09    [W:4.129 / U:0.132 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site