[PATCH] posix timers using == instead of & for bitmask tests

[PATCH] posix timers using == instead of & for bitmask tests

[Joe Korty]

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;
 	}

Re: [PATCH] posix timers using == instead of & for bitmask tests

[Roland McGrath]

> 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

Re: [PATCH] posix timers using == instead of & for bitmask tests

[Joe Korty]

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

Re: [PATCH] posix timers using == instead of & for bitmask tests

[Roland McGrath]

>  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