signals: rename rm_from_queue_full() to flush_sigqueue_mask()
Subsystems (2)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH RESEND 4/11 signals: rename rm_from_queue_full() to flush_sigqueue_mask()
"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.
--- kernel/signal.c | 19 ++++++++----------- 1 files changed, 8 insertions(+), 11 deletions(-)
kernel/signal.c +8 -11
@@ -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;
Signature · 1 line
1.5.5.1
Quoted 7 lines · On Mon, 14 Apr 2014 17:19:59 +0200 Oleg Nesterov <[email protected]> wrote:
"
rm_from_queue_full" looks ugly and misleading, especially now thatrm_from_queue()has gone away. Rename it toflush_sigqueue_mask(), this matchesflush_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 ;)
Peter Zijlstra
maintainerQuoted 2 lines · Andrew Morton
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.
Quoted 6 lines · On Tue, 15 Apr 2014 09:40:40 +0200 Peter Zijlstra <[email protected]> wrote:
Quoted 2 lines · Andrew Morton
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.
Quoted 7 lines · Andrew Morton
Quoted 4 lines · On Mon, 14 Apr 2014 17:19:59 +0200 Oleg Nesterov <[email protected]> wrote:
"
rm_from_queue_full" looks ugly and misleading, especially now thatrm_from_queue()has gone away. Rename it toflush_sigqueue_mask(), this matchesflush_sigqueue()we already have.
Quoted 7 lines
Quoted 3 lines
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 ;)
Quoted 2 lines
Anyway, emails are cheap and nobody reads lkml so please just resend everything ;)
Yes, will resend the whole series, thanks.
Oleg.
PATCH RESEND 5/11 signals: cleanup the usage of t/current in do_sigaction()
In reply to a message not in the archiveThe 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().
--- kernel/signal.c | 15 +++++++-------- 1 files changed, 7 insertions(+), 8 deletions(-)
kernel/signal.c +7 -8
@@ -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 (¤t->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(¤t->sighand->siglock); +spin_unlock_irq (&p->sighand->siglock);return 0 ; }
Signature · 1 line
1.5.5.1
PATCH RESEND 6/11 signals: mv {dis,}allow_signal() from sched.h/exit.c to signal.[ch]
In reply to a message not in the archiveMove 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.
--- 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(-)
include/linux/sched.h +0 -3
@@ -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 *);
include/linux/signal.h +2 -0
@@ -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,
kernel/exit.c +0 -39
@@ -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 (¤t->sighand->siglock); -/* This is only needed for daemonize()'ed kthreads */ -sigdelset (¤t->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 (¤t->sighand->siglock); -return 0 ; -} - -EXPORT_SYMBOL (allow_signal); - -int disallow_signal (int sig) -{ -if (!valid_signal (sig) || sig <1 ) -return -EINVAL ; - -spin_lock_irq (¤t->sighand->siglock); - current->sighand->action[(sig)-1 ].sa.sa_handler =SIG_IGN ; -recalc_sigpending (); -spin_unlock_irq (¤t->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.
kernel/signal.c +29 -0
@@ -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 (¤t->sighand->siglock); +/* This is only needed for daemonize()'ed kthreads */ +sigdelset (¤t->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 (¤t->sighand->siglock); +} +EXPORT_SYMBOL (allow_signal); + +void disallow_signal (int sig) +{ +spin_lock_irq (¤t->sighand->siglock); + current->sighand->action[(sig)-1 ].sa.sa_handler =SIG_IGN ; +recalc_sigpending (); +spin_unlock_irq (¤t->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;
Signature · 1 line
1.5.5.1
PATCH RESEND 7/11 signals: jffs2: fix the wrong usage of disallow_signal()
In reply to a message not in the archivejffs2_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.
--- fs/jffs2/background.c | 12 +++++++----- 1 files changed, 7 insertions(+), 5 deletions(-)
fs/jffs2/background.c +7 -5
@@ -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 ) {
Signature · 1 line
1.5.5.1
Re: [PATCH RESEND 07/11] signals: jffs2: fix the wrong usage of disallow_signal()
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.
PATCH RESEND 8/11 signals: kill the obsolete sigdelset() and recalc_sigpending() in allow_signal()
In reply to a message not in the archiveallow_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.
--- kernel/signal.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-)
kernel/signal.c +1 -4
@@ -3071,16 +3071,13 @@ COMPAT_SYSCALL_DEFINE4(rt_tgsigqueueinfo,*/ void allow_signal (int sig) { -spin_lock_irq (¤t->sighand->siglock); -/* This is only needed for daemonize()'ed kthreads */ -sigdelset (¤t->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 (¤t->sighand->siglock); current->sighand->action[(sig)-1 ].sa.sa_handler = (void __user *)2 ; -recalc_sigpending ();spin_unlock_irq (¤t->sighand->siglock); }EXPORT_SYMBOL (allow_signal);
Signature · 1 line
1.5.5.1
PATCH RESEND 9/11 signals: disallow_signal() should flush the potentially pending signal
In reply to a message not in the archivedisallow_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.
--- kernel/signal.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
kernel/signal.c +7 -0
@@ -3084,8 +3084,15 @@ EXPORT_SYMBOL(allow_signal);void disallow_signal (int sig) { +sigset_t mask; + +sigemptyset (&mask); +sigaddset (&mask, sig); +spin_lock_irq (¤t->sighand->siglock); current->sighand->action[(sig)-1 ].sa.sa_handler =SIG_IGN ; +flush_sigqueue_mask (&mask, ¤t->signal->shared_pending); +flush_sigqueue_mask (&mask, ¤t->pending);recalc_sigpending ();spin_unlock_irq (¤t->sighand->siglock); }
Signature · 1 line
1.5.5.1
PATCH RESEND 10/11 signals: introduce kernel_sigaction()
In reply to a message not in the archiveNow 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.
--- include/linux/signal.h | 18 ++++++++++++++++-- kernel/signal.c | 36 ++++++++++++------------------------ 2 files changed, 28 insertions(+), 26 deletions(-)
include/linux/signal.h +16 -2
@@ -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,
kernel/signal.c +12 -24
@@ -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 (¤t->sighand->siglock); - current->sighand->action[(sig)-1 ].sa.sa_handler = (void __user *)2 ; -spin_unlock_irq (¤t->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 (¤t->sighand->siglock); - current->sighand->action[(sig)-1 ].sa.sa_handler =SIG_IGN ; -flush_sigqueue_mask (&mask, ¤t->signal->shared_pending); -flush_sigqueue_mask (&mask, ¤t->pending); -recalc_sigpending (); +flush_sigqueue_mask (&mask, ¤t->signal->shared_pending); +flush_sigqueue_mask (&mask, ¤t->pending); +recalc_sigpending (); + }spin_unlock_irq (¤t->sighand->siglock); } -EXPORT_SYMBOL (disallow_signal); +EXPORT_SYMBOL (kernel_sigaction);int do_sigaction (int sig,struct k_sigaction *act,struct k_sigaction *oact) {
Signature · 1 line
1.5.5.1
PATCH RESEND 11/11 signals: change wait_for_helper() to use kernel_sigaction()
In reply to a message not in the archiveNow that we have kernel_sigaction() we can change wait_for_helper() to use it and cleanups the code a bit.
--- kernel/kmod.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-)
kernel/kmod.c +1 -4
@@ -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 (¤t->sighand->siglock); - current->sighand->action[SIGCHLD -1 ].sa.sa_handler =SIG_DFL ; -spin_unlock_irq (¤t->sighand->siglock); - +kernel_sigaction (SIGCHLD ,SIG_DFL ); pid =kernel_thread (____call_usermodehelper, sub_info,SIGCHLD );if (pid <0 ) { sub_info->retval = pid;
Signature · 1 line
1.5.5.1