lkml.org 
[lkml]   [2005]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH] fix exit_itimers() vs posix_timer_event() AB-BA deadlock
    CPU_0                                   CPU_1

    release_task: posix_timer_fn:
    write_lock(tasklist); spin_lock(timer->it_lock);

    exit_timers: send_sigqueue:
    spin_lock(timer->it_lock) read_lock(tasklist);

    Actually, it is a bit worse. If posix timer starts between tasklist
    locking and del_timer_sync() call this deadlock will happen because
    of TIMER_RETRY logic.

    With this patch posix_timer_event() detects the exiting thread group
    and aborts the sending of signal (taking tasklist_lock). To simplify
    the code tasklist locking moved from send_{,group_}sigqueue to the
    posix_timer_event().

    Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>

    --- 2.6.14-rc2/kernel/posix-timers.c~1_DLOCK 2005-09-17 18:57:30.000000000 +0400
    +++ 2.6.14-rc2/kernel/posix-timers.c 2005-09-24 18:03:05.000000000 +0400
    @@ -411,6 +411,10 @@ exit:

    int posix_timer_event(struct k_itimer *timr,int si_private)
    {
    + struct task_struct *leader, *zombie;
    + int (*send_actor)(int, struct sigqueue *, struct task_struct *);
    + int ret;
    +
    memset(&timr->sigq->info, 0, sizeof(siginfo_t));
    timr->sigq->info.si_sys_private = si_private;
    /*
    @@ -428,22 +432,40 @@ int posix_timer_event(struct k_itimer *t
    timr->sigq->info.si_tid = timr->it_id;
    timr->sigq->info.si_value = timr->it_sigev_value;

    - if (timr->it_sigev_notify & SIGEV_THREAD_ID) {
    - struct task_struct *leader;
    - int ret = send_sigqueue(timr->it_sigev_signo, timr->sigq,
    - timr->it_process);
    -
    - if (likely(ret >= 0))
    - return ret;
    -
    - timr->it_sigev_notify = SIGEV_SIGNAL;
    - leader = timr->it_process->group_leader;
    - put_task_struct(timr->it_process);
    - timr->it_process = leader;
    - }
    -
    - return send_group_sigqueue(timr->it_sigev_signo, timr->sigq,
    - timr->it_process);
    + /*
    + * We are locking ->it_lock + tasklist_lock backwards
    + * from release_task()->exit_itimers(), beware deadlock.
    + */
    + leader = timr->it_process->group_leader;
    + while (unlikely(!read_trylock(&tasklist_lock))) {
    + if (leader->flags & PF_EXITING) {
    + smp_rmb();
    + if (thread_group_empty(leader))
    + return 0;
    + }
    + cpu_relax();
    + }
    +
    + zombie = NULL;
    + send_actor = send_group_sigqueue;
    + if (timr->it_sigev_notify & SIGEV_THREAD_ID) {
    + if (unlikely(timr->it_process->flags & PF_EXITING)) {
    + zombie = timr->it_process;
    + timr->it_process = leader;
    + timr->it_sigev_notify = SIGEV_SIGNAL;
    + } else
    + send_actor = send_sigqueue;
    + }
    +
    + ret = send_actor(timr->it_sigev_signo, timr->sigq,
    + timr->it_process);
    +
    + read_unlock(&tasklist_lock);
    +
    + if (unlikely(zombie != NULL))
    + put_task_struct(zombie);
    +
    + return ret;
    }
    EXPORT_SYMBOL_GPL(posix_timer_event);

    --- 2.6.14-rc2/kernel/signal.c~1_DLOCK 2005-09-17 18:57:30.000000000 +0400
    +++ 2.6.14-rc2/kernel/signal.c 2005-09-24 18:01:11.000000000 +0400
    @@ -1367,22 +1367,14 @@ send_sigqueue(int sig, struct sigqueue *
    int ret = 0;

    BUG_ON(!(q->flags & SIGQUEUE_PREALLOC));
    - read_lock(&tasklist_lock);
    -
    - if (unlikely(p->flags & PF_EXITING)) {
    - ret = -1;
    - goto out_err;
    - }
    -
    spin_lock_irqsave(&p->sighand->siglock, flags);

    if (unlikely(!list_empty(&q->list))) {
    + BUG_ON(q->info.si_code != SI_TIMER);
    /*
    * If an SI_TIMER entry is already queue just increment
    * the overrun count.
    */
    - if (q->info.si_code != SI_TIMER)
    - BUG();
    q->info.si_overrun++;
    goto out;
    }
    @@ -1400,9 +1392,6 @@ send_sigqueue(int sig, struct sigqueue *

    out:
    spin_unlock_irqrestore(&p->sighand->siglock, flags);
    -out_err:
    - read_unlock(&tasklist_lock);
    -
    return ret;
    }

    @@ -1413,7 +1402,6 @@ send_group_sigqueue(int sig, struct sigq
    int ret = 0;

    BUG_ON(!(q->flags & SIGQUEUE_PREALLOC));
    - read_lock(&tasklist_lock);
    spin_lock_irqsave(&p->sighand->siglock, flags);
    handle_stop_signal(sig, p);

    @@ -1424,13 +1412,12 @@ send_group_sigqueue(int sig, struct sigq
    }

    if (unlikely(!list_empty(&q->list))) {
    + BUG_ON(q->info.si_code != SI_TIMER);
    /*
    * If an SI_TIMER entry is already queue just increment
    * the overrun count. Other uses should not try to
    * send the signal multiple times.
    */
    - if (q->info.si_code != SI_TIMER)
    - BUG();
    q->info.si_overrun++;
    goto out;
    }
    @@ -1447,8 +1434,7 @@ send_group_sigqueue(int sig, struct sigq
    __group_complete_signal(sig, p);
    out:
    spin_unlock_irqrestore(&p->sighand->siglock, flags);
    - read_unlock(&tasklist_lock);
    - return(ret);
    + return ret;
    }

    /*
    -
    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-09-24 15:34    [W:2.815 / U:0.188 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site