lkml.org 
[lkml]   [2006]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][PATCH] Avoid race w/ posix-cpu-timer and exiting tasks
john stultz wrote:
>
> Hey Ingo,
> We've occasionally come across OOPSes in posix-cpu-timer thread (as
> well as tripping over the BUG_ON(tsk->exit_state there) where it appears
> the task we're processing exits out on us while we're using it.
>
> Thus this fix tries to avoid running the posix-cpu-timers on a task that
> is exiting.
>
> --- 2.6-rt/kernel/posix-cpu-timers.c 2006-06-11 15:38:58.000000000 -0500
> +++ devrt/kernel/posix-cpu-timers.c 2006-06-12 10:52:20.000000000 -0500
> @@ -1290,12 +1290,15 @@
>
> #undef UNEXPIRED
>
> - BUG_ON(tsk->exit_state);
> -
> /*
> * Double-check with locks held.
> */
> read_lock(&tasklist_lock);
> + /* Make sure the task doesn't exit under us. */
> + if(unlikely(tsk->exit_state)) {
> + read_unlock(&tasklist_lock);
> + return;
> + }
> spin_lock(&tsk->sighand->siglock);

I strongly believe this BUG_ON() is indeed wrong, and I did a similar patch
a long ago:

[PATCH] posix-timers: remove false BUG_ON() from run_posix_cpu_timers()
Commit 3de463c7d9d58f8cf3395268230cb20a4c15bffa

However it was reverted due to some unclear problems (I think those problems
were not related to this patch).

Instead this patch was added:

[PATCH] Yet more posix-cpu-timer fixes
Commit 3de463c7d9d58f8cf3395268230cb20a4c15bffa

and I still think this patch is not correct.

Quoting myself:
>
> Roland McGrath wrote:
> >
> > @@ -566,6 +566,9 @@ static void arm_timer(struct k_itimer *t
> > struct cpu_timer_list *next;
> > unsigned long i;
> >
> > + if (CPUCLOCK_PERTHREAD(timer->it_clock) && (p->flags & PF_EXITING))
> > + return;
> > +
>
> Why CPUCLOCK_PERTHREAD() ?.
>
> Also, this is racy, no? Why should arm_timer() see PF_EXITING which is
> set on another cpu without any barriers/locking? After all, arm_timer()
> can test PF_EXITING before do_exit() sets this flag, but set ->it_xxx_expires
> after do_exit() resets it.

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: 2006-06-14 00:51    [W:0.034 / U:1.584 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site