All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND 04/11] signals: rename rm_from_queue_full() to flush_sigqueue_mask()
       [not found] <20140414151929.GA21470@redhat.com>
@ 2014-04-14 15:19 ` Oleg Nesterov
  2014-04-14 22:02   ` Andrew Morton
  2014-04-14 15:20 ` [PATCH RESEND 05/11] signals: cleanup the usage of t/current in do_sigaction() Oleg Nesterov
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Oleg Nesterov @ 2014-04-14 15:19 UTC (permalink / raw)
  To: Andrew Morton, Peter Zijlstra
  Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
	Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
	Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel

"rm_from_queue_full" looks ugly and misleading, especially now that
rm_from_queue() has gone away. Rename it to flush_sigqueue_mask(),
this matches flush_sigqueue() we already have.

Also remove the obsolete comment which explains the difference with
rm_from_queue() we already killed.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/signal.c |   19 ++++++++-----------
 1 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 6072efa..c0b7d0c 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -705,11 +705,8 @@ void signal_wake_up_state(struct task_struct *t, unsigned int state)
  * Returns 1 if any signals were found.
  *
  * All callers must be holding the siglock.
- *
- * This version takes a sigset mask and looks at all signals,
- * not just those in the first mask word.
  */
-static int rm_from_queue_full(sigset_t *mask, struct sigpending *s)
+static int flush_sigqueue_mask(sigset_t *mask, struct sigpending *s)
 {
 	struct sigqueue *q, *n;
 	sigset_t m;
@@ -851,18 +848,18 @@ static bool prepare_signal(int sig, struct task_struct *p, bool force)
 		 * This is a stop signal.  Remove SIGCONT from all queues.
 		 */
 		siginitset(&flush, sigmask(SIGCONT));
-		rm_from_queue_full(&flush, &signal->shared_pending);
+		flush_sigqueue_mask(&flush, &signal->shared_pending);
 		for_each_thread(p, t)
-			rm_from_queue_full(&flush, &t->pending);
+			flush_sigqueue_mask(&flush, &t->pending);
 	} else if (sig == SIGCONT) {
 		unsigned int why;
 		/*
 		 * Remove all stop signals from all queues, wake all threads.
 		 */
 		siginitset(&flush, SIG_KERNEL_STOP_MASK);
-		rm_from_queue_full(&flush, &signal->shared_pending);
+		flush_sigqueue_mask(&flush, &signal->shared_pending);
 		for_each_thread(p, t) {
-			rm_from_queue_full(&flush, &t->pending);
+			flush_sigqueue_mask(&flush, &t->pending);
 			task_clear_jobctl_pending(t, JOBCTL_STOP_PENDING);
 			if (likely(!(t->ptrace & PT_SEIZED)))
 				wake_up_state(t, __TASK_STOPPED);
@@ -3101,9 +3098,9 @@ int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
 		if (sig_handler_ignored(sig_handler(t, sig), sig)) {
 			sigemptyset(&mask);
 			sigaddset(&mask, sig);
-			rm_from_queue_full(&mask, &t->signal->shared_pending);
+			flush_sigqueue_mask(&mask, &t->signal->shared_pending);
 			do {
-				rm_from_queue_full(&mask, &t->pending);
+				flush_sigqueue_mask(&mask, &t->pending);
 			} while_each_thread(current, t);
 		}
 	}
@@ -3112,7 +3109,7 @@ int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
 	return 0;
 }
 
-static int 
+static int
 do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long sp)
 {
 	stack_t oss;
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RESEND 05/11] signals: cleanup the usage of t/current in do_sigaction()
       [not found] <20140414151929.GA21470@redhat.com>
  2014-04-14 15:19 ` [PATCH RESEND 04/11] signals: rename rm_from_queue_full() to flush_sigqueue_mask() Oleg Nesterov
@ 2014-04-14 15:20 ` Oleg Nesterov
  2014-04-14 15:20 ` [PATCH RESEND 06/11] signals: mv {dis,}allow_signal() from sched.h/exit.c to signal.[ch] Oleg Nesterov
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Oleg Nesterov @ 2014-04-14 15:20 UTC (permalink / raw)
  To: Andrew Morton, Peter Zijlstra
  Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
	Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
	Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel

The usage of "task_struct *t" and "current" in do_sigaction() looks
really annoying and chaotic. Initially "t" is used as a cached value
of current but not consistently, then it is reused as a loop variable
and we have to use "current" again.

Cleanup this mess and also convert the code to use for_each_thread().

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/signal.c |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index c0b7d0c..a58b0db 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3067,16 +3067,16 @@ COMPAT_SYSCALL_DEFINE4(rt_tgsigqueueinfo,
 
 int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
 {
-	struct task_struct *t = current;
+	struct task_struct *p = current, *t;
 	struct k_sigaction *k;
 	sigset_t mask;
 
 	if (!valid_signal(sig) || sig < 1 || (act && sig_kernel_only(sig)))
 		return -EINVAL;
 
-	k = &t->sighand->action[sig-1];
+	k = &p->sighand->action[sig-1];
 
-	spin_lock_irq(&current->sighand->siglock);
+	spin_lock_irq(&p->sighand->siglock);
 	if (oact)
 		*oact = *k;
 
@@ -3095,17 +3095,16 @@ int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
 		 *   (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_handler_ignored(sig_handler(p, sig), sig)) {
 			sigemptyset(&mask);
 			sigaddset(&mask, sig);
-			flush_sigqueue_mask(&mask, &t->signal->shared_pending);
-			do {
+			flush_sigqueue_mask(&mask, &p->signal->shared_pending);
+			for_each_thread(p, t)
 				flush_sigqueue_mask(&mask, &t->pending);
-			} while_each_thread(current, t);
 		}
 	}
 
-	spin_unlock_irq(&current->sighand->siglock);
+	spin_unlock_irq(&p->sighand->siglock);
 	return 0;
 }
 
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RESEND 06/11] signals: mv {dis,}allow_signal() from sched.h/exit.c to signal.[ch]
       [not found] <20140414151929.GA21470@redhat.com>
  2014-04-14 15:19 ` [PATCH RESEND 04/11] signals: rename rm_from_queue_full() to flush_sigqueue_mask() Oleg Nesterov
  2014-04-14 15:20 ` [PATCH RESEND 05/11] signals: cleanup the usage of t/current in do_sigaction() Oleg Nesterov
@ 2014-04-14 15:20 ` Oleg Nesterov
  2014-04-14 15:20 ` [PATCH RESEND 07/11] signals: jffs2: fix the wrong usage of disallow_signal() Oleg Nesterov
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Oleg Nesterov @ 2014-04-14 15:20 UTC (permalink / raw)
  To: Andrew Morton, Peter Zijlstra
  Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
	Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
	Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel

Move the declaration/definition of allow_signal/disallow_signal to
signal.h/signal.c. The new place is more logical and allows to use
the static helpers in signal.c (see the next changes).

While at it, make them return void and remove the valid_signal()
check. Nobody checks the returned value, and in-kernel users must
not pass the wrong signal number.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 include/linux/sched.h  |    3 ---
 include/linux/signal.h |    2 ++
 kernel/exit.c          |   39 ---------------------------------------
 kernel/signal.c        |   29 +++++++++++++++++++++++++++++
 4 files changed, 31 insertions(+), 42 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index a04214f..82fe3da 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2360,9 +2360,6 @@ extern void flush_itimer_signals(void);
 
 extern void do_group_exit(int);
 
-extern int allow_signal(int);
-extern int disallow_signal(int);
-
 extern int do_execve(struct filename *,
 		     const char __user * const __user *,
 		     const char __user * const __user *);
diff --git a/include/linux/signal.h b/include/linux/signal.h
index ae744c3..ac83c59 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -284,6 +284,8 @@ extern int get_signal_to_deliver(siginfo_t *info, struct k_sigaction *return_ka,
 extern void signal_setup_done(int failed, struct ksignal *ksig, int stepping);
 extern void signal_delivered(int sig, siginfo_t *info, struct k_sigaction *ka, struct pt_regs *regs, int stepping);
 extern void exit_signals(struct task_struct *tsk);
+extern void allow_signal(int);
+extern void disallow_signal(int);
 
 /*
  * Eventually that'll replace get_signal_to_deliver(); macro for now,
diff --git a/kernel/exit.c b/kernel/exit.c
index 6ed6a1d..ad7183a 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -313,45 +313,6 @@ kill_orphaned_pgrp(struct task_struct *tsk, struct task_struct *parent)
 	}
 }
 
-/*
- * Let kernel threads use this to say that they allow a certain signal.
- * Must not be used if kthread was cloned with CLONE_SIGHAND.
- */
-int allow_signal(int sig)
-{
-	if (!valid_signal(sig) || sig < 1)
-		return -EINVAL;
-
-	spin_lock_irq(&current->sighand->siglock);
-	/* This is only needed for daemonize()'ed kthreads */
-	sigdelset(&current->blocked, sig);
-	/*
-	 * Kernel threads handle their own signals. Let the signal code
-	 * know it'll be handled, so that they don't get converted to
-	 * SIGKILL or just silently dropped.
-	 */
-	current->sighand->action[(sig)-1].sa.sa_handler = (void __user *)2;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
-	return 0;
-}
-
-EXPORT_SYMBOL(allow_signal);
-
-int disallow_signal(int sig)
-{
-	if (!valid_signal(sig) || sig < 1)
-		return -EINVAL;
-
-	spin_lock_irq(&current->sighand->siglock);
-	current->sighand->action[(sig)-1].sa.sa_handler = SIG_IGN;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
-	return 0;
-}
-
-EXPORT_SYMBOL(disallow_signal);
-
 #ifdef CONFIG_MM_OWNER
 /*
  * A task is exiting.   If it owned this mm, find a new owner for the mm.
diff --git a/kernel/signal.c b/kernel/signal.c
index a58b0db..8a7ee44 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3065,6 +3065,35 @@ COMPAT_SYSCALL_DEFINE4(rt_tgsigqueueinfo,
 }
 #endif
 
+/*
+ * Let kernel threads use this to say that they allow a certain signal.
+ * Must not be used if kthread was cloned with CLONE_SIGHAND.
+ */
+void allow_signal(int sig)
+{
+	spin_lock_irq(&current->sighand->siglock);
+	/* This is only needed for daemonize()'ed kthreads */
+	sigdelset(&current->blocked, sig);
+	/*
+	 * Kernel threads handle their own signals. Let the signal code
+	 * know it'll be handled, so that they don't get converted to
+	 * SIGKILL or just silently dropped.
+	 */
+	current->sighand->action[(sig)-1].sa.sa_handler = (void __user *)2;
+	recalc_sigpending();
+	spin_unlock_irq(&current->sighand->siglock);
+}
+EXPORT_SYMBOL(allow_signal);
+
+void disallow_signal(int sig)
+{
+	spin_lock_irq(&current->sighand->siglock);
+	current->sighand->action[(sig)-1].sa.sa_handler = SIG_IGN;
+	recalc_sigpending();
+	spin_unlock_irq(&current->sighand->siglock);
+}
+EXPORT_SYMBOL(disallow_signal);
+
 int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
 {
 	struct task_struct *p = current, *t;
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RESEND 07/11] signals: jffs2: fix the wrong usage of disallow_signal()
       [not found] <20140414151929.GA21470@redhat.com>
                   ` (2 preceding siblings ...)
  2014-04-14 15:20 ` [PATCH RESEND 06/11] signals: mv {dis,}allow_signal() from sched.h/exit.c to signal.[ch] Oleg Nesterov
@ 2014-04-14 15:20 ` Oleg Nesterov
  2014-04-14 22:09   ` Andrew Morton
  2014-04-14 15:20 ` [PATCH RESEND 08/11] signals: kill the obsolete sigdelset() and recalc_sigpending() in allow_signal() Oleg Nesterov
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Oleg Nesterov @ 2014-04-14 15:20 UTC (permalink / raw)
  To: Andrew Morton, Peter Zijlstra
  Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
	Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
	Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel

jffs2_garbage_collect_thread() does disallow_signal(SIGHUP) around
jffs2_garbage_collect_pass() and the comment says "We don't want
SIGHUP to interrupt us".

But disallow_signal() can't ensure that jffs2_garbage_collect_pass()
won't be interrupted by SIGHUP, the problem is that SIGHUP can be
already pending when disallow_signal() is called, and in this case
any interruptible sleep won't block.

Note: this is in fact because disallow_signal() is buggy and should
be fixed, see the next changes.

But there is another reason why disallow_signal() is wrong: SIG_IGN
set by disallow_signal() silently discards any SIGHUP which can be
sent before the next allow_signal(SIGHUP).

Change this code to use sigprocmask(SIG_UNBLOCK/SIG_BLOCK, SIGHUP).
This even matches the old (and wrong) semantics allow/disallow had
when this logic was written.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 fs/jffs2/background.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/jffs2/background.c b/fs/jffs2/background.c
index 2b60ce1..bb9cebc 100644
--- a/fs/jffs2/background.c
+++ b/fs/jffs2/background.c
@@ -75,10 +75,13 @@ void jffs2_stop_garbage_collect_thread(struct jffs2_sb_info *c)
 static int jffs2_garbage_collect_thread(void *_c)
 {
 	struct jffs2_sb_info *c = _c;
+	sigset_t hupmask;
 
+	siginitset(&hupmask, sigmask(SIGHUP));
 	allow_signal(SIGKILL);
 	allow_signal(SIGSTOP);
 	allow_signal(SIGCONT);
+	allow_signal(SIGHUP);
 
 	c->gc_task = current;
 	complete(&c->gc_thread_start);
@@ -87,7 +90,7 @@ static int jffs2_garbage_collect_thread(void *_c)
 
 	set_freezable();
 	for (;;) {
-		allow_signal(SIGHUP);
+		sigprocmask(SIG_UNBLOCK, &hupmask, NULL);
 	again:
 		spin_lock(&c->erase_completion_lock);
 		if (!jffs2_thread_should_wake(c)) {
@@ -95,10 +98,9 @@ static int jffs2_garbage_collect_thread(void *_c)
 			spin_unlock(&c->erase_completion_lock);
 			jffs2_dbg(1, "%s(): sleeping...\n", __func__);
 			schedule();
-		} else
+		} else {
 			spin_unlock(&c->erase_completion_lock);
-			
-
+		}
 		/* Problem - immediately after bootup, the GCD spends a lot
 		 * of time in places like jffs2_kill_fragtree(); so much so
 		 * that userspace processes (like gdm and X) are starved
@@ -150,7 +152,7 @@ static int jffs2_garbage_collect_thread(void *_c)
 			}
 		}
 		/* We don't want SIGHUP to interrupt us. STOP and KILL are OK though. */
-		disallow_signal(SIGHUP);
+		sigprocmask(SIG_BLOCK, &hupmask, NULL);
 
 		jffs2_dbg(1, "%s(): pass\n", __func__);
 		if (jffs2_garbage_collect_pass(c) == -ENOSPC) {
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RESEND 08/11] signals: kill the obsolete sigdelset() and recalc_sigpending() in allow_signal()
       [not found] <20140414151929.GA21470@redhat.com>
                   ` (3 preceding siblings ...)
  2014-04-14 15:20 ` [PATCH RESEND 07/11] signals: jffs2: fix the wrong usage of disallow_signal() Oleg Nesterov
@ 2014-04-14 15:20 ` Oleg Nesterov
  2014-04-14 15:20 ` [PATCH RESEND 09/11] signals: disallow_signal() should flush the potentially pending signal Oleg Nesterov
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Oleg Nesterov @ 2014-04-14 15:20 UTC (permalink / raw)
  To: Andrew Morton, Peter Zijlstra
  Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
	Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
	Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel

allow_signal() does sigdelset(current->blocked) due to historic
reason, previously it could be called by a daemonize()'ed kthread,
and daemonize() played with current->blocked.

Now that daemonize() has gone away we can remove sigdelset() and
recalc_sigpending(). If a user really wants to unblock a signal,
it must use sigprocmask() or set_current_block() explicitely.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/signal.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 8a7ee44..3eec27b 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3071,16 +3071,13 @@ COMPAT_SYSCALL_DEFINE4(rt_tgsigqueueinfo,
  */
 void allow_signal(int sig)
 {
-	spin_lock_irq(&current->sighand->siglock);
-	/* This is only needed for daemonize()'ed kthreads */
-	sigdelset(&current->blocked, sig);
 	/*
 	 * Kernel threads handle their own signals. Let the signal code
 	 * know it'll be handled, so that they don't get converted to
 	 * SIGKILL or just silently dropped.
 	 */
+	spin_lock_irq(&current->sighand->siglock);
 	current->sighand->action[(sig)-1].sa.sa_handler = (void __user *)2;
-	recalc_sigpending();
 	spin_unlock_irq(&current->sighand->siglock);
 }
 EXPORT_SYMBOL(allow_signal);
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RESEND 09/11] signals: disallow_signal() should flush the potentially pending signal
       [not found] <20140414151929.GA21470@redhat.com>
                   ` (4 preceding siblings ...)
  2014-04-14 15:20 ` [PATCH RESEND 08/11] signals: kill the obsolete sigdelset() and recalc_sigpending() in allow_signal() Oleg Nesterov
@ 2014-04-14 15:20 ` Oleg Nesterov
  2014-04-14 15:20 ` [PATCH RESEND 10/11] signals: introduce kernel_sigaction() Oleg Nesterov
  2014-04-14 15:20 ` [PATCH RESEND 11/11] signals: change wait_for_helper() to use kernel_sigaction() Oleg Nesterov
  7 siblings, 0 replies; 13+ messages in thread
From: Oleg Nesterov @ 2014-04-14 15:20 UTC (permalink / raw)
  To: Andrew Morton, Peter Zijlstra
  Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
	Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
	Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel

disallow_signal() simply sets SIG_IGN, this is not enough and
recalc_sigpending() is simply pointless because in can never
change the state of TIF_SIGPENDING.

If we ignore a signal, we also need to do flush_sigqueue_mask() for
the case when this signal is pending, this way recalc_sigpending()
can actually clear TIF_SIGPENDING and we do not "leak" the allocated
siginfo's.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/signal.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 3eec27b..4bab1b7 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3084,8 +3084,15 @@ EXPORT_SYMBOL(allow_signal);
 
 void disallow_signal(int sig)
 {
+	sigset_t mask;
+
+	sigemptyset(&mask);
+	sigaddset(&mask, sig);
+
 	spin_lock_irq(&current->sighand->siglock);
 	current->sighand->action[(sig)-1].sa.sa_handler = SIG_IGN;
+	flush_sigqueue_mask(&mask, &current->signal->shared_pending);
+	flush_sigqueue_mask(&mask, &current->pending);
 	recalc_sigpending();
 	spin_unlock_irq(&current->sighand->siglock);
 }
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RESEND 10/11] signals: introduce kernel_sigaction()
       [not found] <20140414151929.GA21470@redhat.com>
                   ` (5 preceding siblings ...)
  2014-04-14 15:20 ` [PATCH RESEND 09/11] signals: disallow_signal() should flush the potentially pending signal Oleg Nesterov
@ 2014-04-14 15:20 ` Oleg Nesterov
  2014-04-14 15:20 ` [PATCH RESEND 11/11] signals: change wait_for_helper() to use kernel_sigaction() Oleg Nesterov
  7 siblings, 0 replies; 13+ messages in thread
From: Oleg Nesterov @ 2014-04-14 15:20 UTC (permalink / raw)
  To: Andrew Morton, Peter Zijlstra
  Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
	Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
	Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel

Now that allow_signal() is really trivial we can unify it with
disallow_signal(). Add the new helper, kernel_sigaction(), and
reimplement allow_signal/disallow_signal as a trivial wrappers.

This saves one EXPORT_SYMBOL() and the new helper can have more
users.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 include/linux/signal.h |   18 ++++++++++++++++--
 kernel/signal.c        |   36 ++++++++++++------------------------
 2 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/include/linux/signal.h b/include/linux/signal.h
index ac83c59..c9e6536 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -284,8 +284,22 @@ extern int get_signal_to_deliver(siginfo_t *info, struct k_sigaction *return_ka,
 extern void signal_setup_done(int failed, struct ksignal *ksig, int stepping);
 extern void signal_delivered(int sig, siginfo_t *info, struct k_sigaction *ka, struct pt_regs *regs, int stepping);
 extern void exit_signals(struct task_struct *tsk);
-extern void allow_signal(int);
-extern void disallow_signal(int);
+extern void kernel_sigaction(int, __sighandler_t);
+
+static inline void allow_signal(int sig)
+{
+	/*
+	 * Kernel threads handle their own signals. Let the signal code
+	 * know it'll be handled, so that they don't get converted to
+	 * SIGKILL or just silently dropped.
+	 */
+	kernel_sigaction(sig, (__force __sighandler_t)2);
+}
+
+static inline void disallow_signal(int sig)
+{
+	kernel_sigaction(sig, SIG_IGN);
+}
 
 /*
  * Eventually that'll replace get_signal_to_deliver(); macro for now,
diff --git a/kernel/signal.c b/kernel/signal.c
index 4bab1b7..45dba61 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3066,37 +3066,25 @@ COMPAT_SYSCALL_DEFINE4(rt_tgsigqueueinfo,
 #endif
 
 /*
- * Let kernel threads use this to say that they allow a certain signal.
- * Must not be used if kthread was cloned with CLONE_SIGHAND.
+ * For kthreads only, must not be used if cloned with CLONE_SIGHAND
  */
-void allow_signal(int sig)
+void kernel_sigaction(int sig, __sighandler_t action)
 {
-	/*
-	 * Kernel threads handle their own signals. Let the signal code
-	 * know it'll be handled, so that they don't get converted to
-	 * SIGKILL or just silently dropped.
-	 */
 	spin_lock_irq(&current->sighand->siglock);
-	current->sighand->action[(sig)-1].sa.sa_handler = (void __user *)2;
-	spin_unlock_irq(&current->sighand->siglock);
-}
-EXPORT_SYMBOL(allow_signal);
+	current->sighand->action[sig - 1].sa.sa_handler = action;
+	if (action == SIG_IGN) {
+		sigset_t mask;
 
-void disallow_signal(int sig)
-{
-	sigset_t mask;
+		sigemptyset(&mask);
+		sigaddset(&mask, sig);
 
-	sigemptyset(&mask);
-	sigaddset(&mask, sig);
-
-	spin_lock_irq(&current->sighand->siglock);
-	current->sighand->action[(sig)-1].sa.sa_handler = SIG_IGN;
-	flush_sigqueue_mask(&mask, &current->signal->shared_pending);
-	flush_sigqueue_mask(&mask, &current->pending);
-	recalc_sigpending();
+		flush_sigqueue_mask(&mask, &current->signal->shared_pending);
+		flush_sigqueue_mask(&mask, &current->pending);
+		recalc_sigpending();
+	}
 	spin_unlock_irq(&current->sighand->siglock);
 }
-EXPORT_SYMBOL(disallow_signal);
+EXPORT_SYMBOL(kernel_sigaction);
 
 int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
 {
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH RESEND 11/11] signals: change wait_for_helper() to use kernel_sigaction()
       [not found] <20140414151929.GA21470@redhat.com>
                   ` (6 preceding siblings ...)
  2014-04-14 15:20 ` [PATCH RESEND 10/11] signals: introduce kernel_sigaction() Oleg Nesterov
@ 2014-04-14 15:20 ` Oleg Nesterov
  7 siblings, 0 replies; 13+ messages in thread
From: Oleg Nesterov @ 2014-04-14 15:20 UTC (permalink / raw)
  To: Andrew Morton, Peter Zijlstra
  Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
	Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
	Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel

Now that we have kernel_sigaction() we can change wait_for_helper()
to use it and cleanups the code a bit.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/kmod.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/kernel/kmod.c b/kernel/kmod.c
index 6b375af..c18c891 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -285,10 +285,7 @@ static int wait_for_helper(void *data)
 	pid_t pid;
 
 	/* If SIGCLD is ignored sys_wait4 won't populate the status. */
-	spin_lock_irq(&current->sighand->siglock);
-	current->sighand->action[SIGCHLD-1].sa.sa_handler = SIG_DFL;
-	spin_unlock_irq(&current->sighand->siglock);
-
+	kernel_sigaction(SIGCHLD, SIG_DFL);
 	pid = kernel_thread(____call_usermodehelper, sub_info, SIGCHLD);
 	if (pid < 0) {
 		sub_info->retval = pid;
-- 
1.5.5.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH RESEND 04/11] signals: rename rm_from_queue_full() to flush_sigqueue_mask()
  2014-04-14 15:19 ` [PATCH RESEND 04/11] signals: rename rm_from_queue_full() to flush_sigqueue_mask() Oleg Nesterov
@ 2014-04-14 22:02   ` Andrew Morton
  2014-04-15  7:40     ` Peter Zijlstra
  2014-04-15 18:28     ` Oleg Nesterov
  0 siblings, 2 replies; 13+ messages in thread
From: Andrew Morton @ 2014-04-14 22:02 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Peter Zijlstra, Al Viro, David Woodhouse, Frederic Weisbecker,
	Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
	Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel

On Mon, 14 Apr 2014 17:19:59 +0200 Oleg Nesterov <oleg@redhat.com> wrote:

> "rm_from_queue_full" looks ugly and misleading, especially now that
> rm_from_queue() has gone away. Rename it to flush_sigqueue_mask(),
> this matches flush_sigqueue() we already have.
> 
> Also remove the obsolete comment which explains the difference with
> rm_from_queue() we already killed.
> 

Confused.  You sent 0..11 on March 24 and today you resent just 4, 5,
10 and 11.  If they're supposed to be replacement versions then please
explain this!

Anyway, emails are cheap and nobody reads lkml so please just resend
everything ;)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH RESEND 07/11] signals: jffs2: fix the wrong usage of disallow_signal()
  2014-04-14 15:20 ` [PATCH RESEND 07/11] signals: jffs2: fix the wrong usage of disallow_signal() Oleg Nesterov
@ 2014-04-14 22:09   ` Andrew Morton
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Morton @ 2014-04-14 22:09 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Peter Zijlstra, Al Viro, David Woodhouse, Frederic Weisbecker,
	Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
	Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel


hm, 6, 7, 8 and 9 hit my inbox via the you->me route but I didn't get
the second copy of those 4 via the vger->me route.  And several in
the series remain lost

https://lkml.org/lkml/2014/4/14/ received 4,5,6,7,8,9,10,11 but seems
to be missing 0,1,2,3.

Something is screwed up somewhere.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH RESEND 04/11] signals: rename rm_from_queue_full() to flush_sigqueue_mask()
  2014-04-14 22:02   ` Andrew Morton
@ 2014-04-15  7:40     ` Peter Zijlstra
  2014-04-15  7:45       ` Andrew Morton
  2014-04-15 18:28     ` Oleg Nesterov
  1 sibling, 1 reply; 13+ messages in thread
From: Peter Zijlstra @ 2014-04-15  7:40 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Oleg Nesterov, Al Viro, David Woodhouse, Frederic Weisbecker,
	Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
	Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel

On Mon, Apr 14, 2014 at 03:02:21PM -0700, Andrew Morton wrote:
> Anyway, emails are cheap and nobody reads lkml so please just resend
> everything ;)

You're nobody right? ;-) At least the rumour went you actually read
lkml.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH RESEND 04/11] signals: rename rm_from_queue_full() to flush_sigqueue_mask()
  2014-04-15  7:40     ` Peter Zijlstra
@ 2014-04-15  7:45       ` Andrew Morton
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Morton @ 2014-04-15  7:45 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Oleg Nesterov, Al Viro, David Woodhouse, Frederic Weisbecker,
	Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
	Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel

On Tue, 15 Apr 2014 09:40:40 +0200 Peter Zijlstra <peterz@infradead.org> wrote:

> On Mon, Apr 14, 2014 at 03:02:21PM -0700, Andrew Morton wrote:
> > Anyway, emails are cheap and nobody reads lkml so please just resend
> > everything ;)
> 
> You're nobody right? ;-) At least the rumour went you actually read
> lkml.

I scan them all to pick out neglected patches and interesting bug
reports, but my days of reading all the message bodies ended long
ago.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH RESEND 04/11] signals: rename rm_from_queue_full() to flush_sigqueue_mask()
  2014-04-14 22:02   ` Andrew Morton
  2014-04-15  7:40     ` Peter Zijlstra
@ 2014-04-15 18:28     ` Oleg Nesterov
  1 sibling, 0 replies; 13+ messages in thread
From: Oleg Nesterov @ 2014-04-15 18:28 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Peter Zijlstra, Al Viro, David Woodhouse, Frederic Weisbecker,
	Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
	Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel,
	Marek Mahut

On 04/14, Andrew Morton wrote:
>
> On Mon, 14 Apr 2014 17:19:59 +0200 Oleg Nesterov <oleg@redhat.com> wrote:
>
> > "rm_from_queue_full" looks ugly and misleading, especially now that
> > rm_from_queue() has gone away. Rename it to flush_sigqueue_mask(),
> > this matches flush_sigqueue() we already have.
> >
> > Also remove the obsolete comment which explains the difference with
> > rm_from_queue() we already killed.
> >
>
> Confused.  You sent 0..11 on March 24 and today you resent just 4, 5,
> 10 and 11.  If they're supposed to be replacement versions then please
> explain this!

No, no, this is another case when my emails go to nowhere :[

Marek, perhaps you can take a look ;)

> Anyway, emails are cheap and nobody reads lkml so please just resend
> everything ;)

Yes, will resend the whole series, thanks.

Oleg.


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-04-15 18:29 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20140414151929.GA21470@redhat.com>
2014-04-14 15:19 ` [PATCH RESEND 04/11] signals: rename rm_from_queue_full() to flush_sigqueue_mask() Oleg Nesterov
2014-04-14 22:02   ` Andrew Morton
2014-04-15  7:40     ` Peter Zijlstra
2014-04-15  7:45       ` Andrew Morton
2014-04-15 18:28     ` Oleg Nesterov
2014-04-14 15:20 ` [PATCH RESEND 05/11] signals: cleanup the usage of t/current in do_sigaction() Oleg Nesterov
2014-04-14 15:20 ` [PATCH RESEND 06/11] signals: mv {dis,}allow_signal() from sched.h/exit.c to signal.[ch] Oleg Nesterov
2014-04-14 15:20 ` [PATCH RESEND 07/11] signals: jffs2: fix the wrong usage of disallow_signal() Oleg Nesterov
2014-04-14 22:09   ` Andrew Morton
2014-04-14 15:20 ` [PATCH RESEND 08/11] signals: kill the obsolete sigdelset() and recalc_sigpending() in allow_signal() Oleg Nesterov
2014-04-14 15:20 ` [PATCH RESEND 09/11] signals: disallow_signal() should flush the potentially pending signal Oleg Nesterov
2014-04-14 15:20 ` [PATCH RESEND 10/11] signals: introduce kernel_sigaction() Oleg Nesterov
2014-04-14 15:20 ` [PATCH RESEND 11/11] signals: change wait_for_helper() to use kernel_sigaction() Oleg Nesterov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.