Make posix-timers do a get_task_struct / put_task_struct if either
SIGEV_SIGNAL or SIGEV_THREAD_ID is set. Currently the get/put is done
only if both are set.
Compiles but is otherwise untested.
Joe
--- base/kernel/posix-timers.c 2004-10-18 17:54:22.000000000 -0400
+++ new/kernel/posix-timers.c 2004-10-22 10:25:51.000000000 -0400
@@ -548,7 +548,7 @@
}
sigqueue_free(tmr->sigq);
if (unlikely(tmr->it_process) &&
- tmr->it_sigev_notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID))
+ (tmr->it_sigev_notify & (SIGEV_SIGNAL | SIGEV_THREAD_ID)))
put_task_struct(tmr->it_process);
kmem_cache_free(posix_timers_cache, tmr);
}
@@ -650,7 +650,7 @@
list_add(&new_timer->list,
&process->signal->posix_timers);
spin_unlock_irqrestore(&process->sighand->siglock, flags);
- if (new_timer->it_sigev_notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID))
+ if (new_timer->it_sigev_notify & (SIGEV_SIGNAL | SIGEV_THREAD_ID))
get_task_struct(process);
} else {
spin_unlock_irqrestore(&process->sighand->siglock, flags);
@@ -1101,7 +1101,7 @@
* they got something (see the lock code above).
*/
if (timer->it_process) {
- if (timer->it_sigev_notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID))
+ if (timer->it_sigev_notify & (SIGEV_SIGNAL | SIGEV_THREAD_ID))
put_task_struct(timer->it_process);
timer->it_process = NULL;
}
@@ -1138,7 +1138,7 @@
* they got something (see the lock code above).
*/
if (timer->it_process) {
- if (timer->it_sigev_notify == (SIGEV_SIGNAL|SIGEV_THREAD_ID))
+ if (timer->it_sigev_notify & (SIGEV_SIGNAL | SIGEV_THREAD_ID))
put_task_struct(timer->it_process);
timer->it_process = NULL;
}
> Make posix-timers do a get_task_struct / put_task_struct if either
> SIGEV_SIGNAL or SIGEV_THREAD_ID is set. Currently the get/put is done
> only if both are set.
What is the purpose of this change? The `good_sigevent' check ensures that
if SIGEV_THREAD_ID is set, then the value is exactly
SIGEV_SIGNAL|SIGEV_THREAD_ID. In fact, this change has no effect at all
because SIGEV_SIGNAL is zero. If it weren't, it would have an undesireable
effect of doing the task_struct refcounting all the time instead of only
for SIGEV_THREAD_ID. That refcounting is never required in the plain
SIGEV_SIGNAL case, because the task_struct pointer stored in the
group_leader, and that is never freed before all the posix-timers data
structures get cleared out anyway (exit_itimers). It's only required for
SIGEV_THREAD_ID, where the target thread might have died before the timer
was next examined.
Thanks,
Roland
On Fri, Oct 22, 2004 at 03:03:20PM -0700, Roland McGrath wrote:
> > Make posix-timers do a get_task_struct / put_task_struct if either
> > SIGEV_SIGNAL or SIGEV_THREAD_ID is set. Currently the get/put is done
> > only if both are set.
>
> What is the purpose of this change? The `good_sigevent' check ensures that
> if SIGEV_THREAD_ID is set, then the value is exactly
> SIGEV_SIGNAL|SIGEV_THREAD_ID. In fact, this change has no effect at all
> because SIGEV_SIGNAL is zero. If it weren't, it would have an undesireable
> effect of doing the task_struct refcounting all the time instead of only
> for SIGEV_THREAD_ID. That refcounting is never required in the plain
> SIGEV_SIGNAL case, because the task_struct pointer stored in the
> group_leader, and that is never freed before all the posix-timers data
> structures get cleared out anyway (exit_itimers). It's only required for
> SIGEV_THREAD_ID, where the target thread might have died before the timer
> was next examined.
Hi Roland,
Thanks for answering my mistaken impressions. I saw after I wrote that
SIGEV_SIGNAL == 0 which makes everything work. And I was laboring under
the misconception that SIGEV_SIGNAL and SIGEV_THREAD were mutually exclusive
which isn't true, one always has SIGEV_SIGNAL if SIGEV_THREAD is set.
Regards,
Joe
> Thanks for answering my mistaken impressions. I saw after I wrote that
> SIGEV_SIGNAL == 0 which makes everything work. And I was laboring under
> the misconception that SIGEV_SIGNAL and SIGEV_THREAD were mutually exclusive
> which isn't true, one always has SIGEV_SIGNAL if SIGEV_THREAD is set.
In the semantic sense they are mutually exclusive. They are not so in the
bitwise sense, because the value is really not a bitmask overall, only the
SIGEV_THREAD_ID bit is used that way.
Thanks,
Roland