Messages in this thread | | | Date | Tue, 23 Aug 2005 20:17:57 +0400 | From | Oleg Nesterov <> | Subject | Re: [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/
| |