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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
