Skip to contentSkip to conversation

simplify sig_ignored() pathes

PATCH In review Oleg Nesterov 2 participants 2 messages Nov 18, 2008 – Nov 19, 2008 Tracing

lore
Share & tools
lore
#0 lore raw

PATCH 2/2 simplify sig_ignored() pathes

The code which checks for ignored signals is a bit overcomplicated because tracehook_consider_ignored_signal() wants the "handler" arg. Can't we kill it? It is not used even with utrace, and even if it will be used later it is OK to read ->sighand again in the slow path.

include/linux/tracehook.h renamed from include/linux/tracehook.h~2_SIG_IGNORED +1 -4
@@ -388,17 +388,14 @@ static inline void tracehook_signal_hand
  * tracehook_consider_ignored_signal - suppress short-circuit of ignored signal
  * @task:		task receiving the signal
  * @sig:		signal number being sent
- * @handler:		%SIG_IGN or %SIG_DFL
  *
  * Return zero iff tracing doesn't care to examine this ignored signal,
  * so it can short-circuit normal delivery and never even get queued.
- * Either @handler is %SIG_DFL and @sig's default is ignore, or it's %SIG_IGN.
  *
  * Called with @task->sighand->siglock held.
  */
 static inline int tracehook_consider_ignored_signal(struct task_struct *task,
-						    int sig,
-						    void __user *handler)
+						    int sig)
 {
 	return (task_ptrace(task) & PT_PTRACED) != 0;
 }
kernel/signal.c renamed from kernel/signal.c~2_SIG_IGNORED +12 -23
@@ -41,44 +41,33 @@
 
 static struct kmem_cache *sigqueue_cachep;
 
-static void __user *sig_handler(struct task_struct *t, int sig)
+static int sig_task_ignored(struct task_struct *t, int sig)
 {
 	void __user *h = t->sighand->action[sig - 1].sa.sa_handler;
 
-	/* drop SIGKILL early to not confuse wait_xxx_killable/etc */
-	if (unlikely(t->signal->flags & SIGNAL_UNKILLABLE) && h == SIG_DFL)
-		h = SIG_IGN;
-
-	return h;
-}
-
-static int sig_handler_ignored(void __user *handler, int sig)
-{
-	/* Is it explicitly or implicitly ignored? */
-	return handler == SIG_IGN ||
-		(handler == SIG_DFL && sig_kernel_ignore(sig));
+	if (h == SIG_DFL) {
+		/* init drops SIGKILL early to not confuse xxx_killable/etc */
+		return sig_kernel_ignore(sig) ||
+			unlikely(t->signal->flags & SIGNAL_UNKILLABLE);
+	}
+	return h == SIG_IGN;
 }
 
 static int sig_ignored(struct task_struct *t, int sig)
 {
-	void __user *handler;
-
 	/*
 	 * Blocked signals are never ignored, since the
 	 * signal handler may change by the time it is
 	 * unblocked.
 	 */
-	if (sigismember(&t->blocked, sig) || sigismember(&t->real_blocked, sig))
-		return 0;
-
-	handler = sig_handler(t, sig);
-	if (!sig_handler_ignored(handler, sig))
+	if (sigismember(&t->blocked, sig) ||
+	    sigismember(&t->real_blocked, sig) ||
+	    !sig_task_ignored(t, sig))
 		return 0;
-
 	/*
 	 * Tracers may want to know about even ignored signals.
 	 */
-	return !tracehook_consider_ignored_signal(t, sig, handler);
+	return !tracehook_consider_ignored_signal(t, sig);
 }
 
 /*
@@ -2338,7 +2327,7 @@ int do_sigaction(int sig, struct k_sigac
 		 *   (for example, SIGCHLD), shall cause the pending signal to
 		 *   be discarded, whether or not it is blocked"
 		 */
-		if (sig_handler_ignored(sig_handler(t, sig), sig)) {
+		if (sig_task_ignored(t, sig)) {
 			sigemptyset(&mask);
 			sigaddset(&mask, sig);
 			rm_from_queue_full(&mask, &t->signal->shared_pending);
#1 lore raw
Quoted 4 lines

The code which checks for ignored signals is a bit overcomplicated because tracehook_consider_ignored_signal() wants the "handler" arg. Can't we kill it? It is not used even with utrace, and even if it will be used later it is OK to read ->sighand again in the slow path.

Yeah, it's fine to kill the arg. For consistency, change tracehook_consider_fatal_signal to match. They are specified as called with the siglock held, so it will indeed be easy and safe to check ->sighand if tracing code wants to distinguish the cases in the future.

Thanks,
Roland