lkml.org 
[lkml]   [2005]   [Aug]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2.6.13-rc6-rt9] PI aware dynamic priority adjustment
Thomas Gleixner wrote:
>
> On Sat, 2005-08-20 at 18:10 +0400, Oleg Nesterov wrote:
>
> > posix_timer_event() first checks that the thread (SIGEV_THREAD_ID
> > case) does not have PF_EXITING flag, then it calls send_sigqueue()
> > which locks task list. But if the thread exits in between the kernel
> > will oops.
>
> > posix_timer_event() runs under k_itimer.it_lock, but this does not
> > help if that thread was not the only one in thread group, in this
> > case we don't call exit_itimers().
>
> I added exit_notify_itimers() for that case. It is synchronized vs.
> posix_timer_event() via the timer lock and therefor protects against the
> race described above.
>
> It solves the problem for the only user of send_sigqueue for now, but I
> think we should have a more general interface to such functionality to
> allow simple usage.
>
> tglx
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>
> diff -uprN --exclude-from=/usr/local/bin/diffit.exclude linux-2.6.13-rc6/include/linux/sched.h linux-2.6.13-rc6.work/include/linux/sched.h
> --- linux-2.6.13-rc6/include/linux/sched.h 2005-08-13 12:25:56.000000000 +0200
> +++ linux-2.6.13-rc6.work/include/linux/sched.h 2005-08-20 17:43:36.000000000 +0200
> @@ -1051,6 +1051,7 @@ extern void __exit_signal(struct task_st
> extern void exit_sighand(struct task_struct *);
> extern void __exit_sighand(struct task_struct *);
> extern void exit_itimers(struct signal_struct *);
> +extern void exit_notify_itimers(struct signal_struct *);
>
> extern NORET_TYPE void do_group_exit(int);
>
> diff -uprN --exclude-from=/usr/local/bin/diffit.exclude linux-2.6.13-rc6/kernel/posix-timers.c linux-2.6.13-rc6.work/kernel/posix-timers.c
> --- linux-2.6.13-rc6/kernel/posix-timers.c 2005-08-13 12:25:58.000000000 +0200
> +++ linux-2.6.13-rc6.work/kernel/posix-timers.c 2005-08-20 17:43:36.000000000 +0200
> @@ -1169,6 +1169,33 @@ void exit_itimers(struct signal_struct *
> }
>
> /*
> + * This is called by __exit_signal, when there are still references to
> + * the shared signal_struct
> + */
> +void exit_notify_itimers(struct signal_struct *sig)
> +{
> + struct k_itimer *timer;
> + struct list_head *tmp;
> + unsigned long flags;
> +
> + list_for_each(tmp, &sig->posix_timers) {
> +
> + timer = list_entry(tmp, struct k_itimer, list);
> +
> + /* Protect against posix_timer_fn */
> + spin_lock_irqsave(&timer->it_lock, flags);

I think this is deadlockable. We already (release_task) hold
tasklist_lock here. What if this timer has no SIGEV_THREAD_ID ?

posix_timer_fn locks timer->it_lock first, then locks tasklist_lock
in send_group_sigqueue().

Also, sys_timer_delete() locks ->it_lock, then ->sighand.lock.

I think the better fix would be re-check ->sighand in send_sigqueue,
like Paul's patches do.

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-20 19:41    [from the cache]
©2003-2011 Jasper Spaans