[PATCH 3/3] arm_timer: remove a racy and obsolete PF_EXITING check

[PATCH 3/3] arm_timer: remove a racy and obsolete PF_EXITING check

[Oleg Nesterov]

arm_timer() checks PF_EXITING to prevent BUG_ON(->exit_state)
in run_posix_cpu_timers().

However, for some reason it does so only for CPUCLOCK_PERTHREAD
case (which is imho wrong).

Also, this check is not reliable, PF_EXITING could be set on
another cpu without any locks/barriers just after the check,
so it can't prevent from attaching the timer to the exiting
task.

The previous patch makes this check unneeded.

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

--- 2.6.17-rc6/kernel/posix-cpu-timers.c~3_ARM	2006-06-15 18:46:00.000000000 +0400
+++ 2.6.17-rc6/kernel/posix-cpu-timers.c	2006-06-15 19:04:54.000000000 +0400
@@ -555,9 +555,6 @@ 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;
-
 	head = (CPUCLOCK_PERTHREAD(timer->it_clock) ?
 		p->cpu_timers : p->signal->cpu_timers);
 	head += CPUCLOCK_WHICH(timer->it_clock);

Re: [PATCH 3/3] arm_timer: remove a racy and obsolete PF_EXITING check

[Roland McGrath]

I think this and your other two patches are fine fixes.  Thanks, Oleg.

> However, for some reason it does so only for CPUCLOCK_PERTHREAD
> case (which is imho wrong).

For a process CPU clock timer, ->it.cpu.task is the thread group leader.
The group leader can exit and will be a lingering zombie for as long as
other threads in the group live.  The process timers need to keep getting
armed and working both during and after the group leader's exit processing.

> Also, this check is not reliable, PF_EXITING could be set on
> another cpu without any locks/barriers just after the check,
> so it can't prevent from attaching the timer to the exiting
> task.
> 
> The previous patch makes this check unneeded.

Thanks for cleaning that up.  The original rationale behind the checking
for exiting threads was an attempt to avoid process_timer_rebalance
counting a dead thread as live and setting all the live threads' expiry
timers too short, as well as avoiding setting it_*_expires after do_exit
cleared them.  The latter didn't suffice anyway and you've fixed it another
way.  The former doesn't really do any harm, since it just means a
possibility of early wakeup and recheck in the remaining live threads.


Thanks,
Roland

Re: [PATCH 3/3] arm_timer: remove a racy and obsolete PF_EXITING check

[Oleg Nesterov]

On 06/19, Roland McGrath wrote:
> 
> > However, for some reason it does so only for CPUCLOCK_PERTHREAD
> > case (which is imho wrong).
> 
> For a process CPU clock timer, ->it.cpu.task is the thread group leader.
> The group leader can exit and will be a lingering zombie for as long as
> other threads in the group live.  The process timers need to keep getting
> armed and working both during and after the group leader's exit processing.

Ah, yes. Also, I missed the fact that process_timer_rebalance() checks
PF_EXITING anyway (For a process CPU clock timer), so I was wrong twice.

Thanks!

Oleg.