diff options
author | Oleg Nesterov <oleg@redhat.com> | 2014-06-06 17:36:55 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2014-06-06 19:08:11 -0400 |
commit | c240837fa7a7dd8cb3bef017ffb8af2c32cb0caf (patch) | |
tree | b788451a3d45ad25aa5560af9c7e631f8892b671 /fs | |
parent | 0341729b4b832e753c5e745c6ba0e797f6198be0 (diff) |
signals: jffs2: fix the wrong usage of disallow_signal()
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>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'fs')
-rw-r--r-- | 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 2b60ce1996aa..bb9cebc9ca8a 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) | |||
75 | static int jffs2_garbage_collect_thread(void *_c) | 75 | static int jffs2_garbage_collect_thread(void *_c) |
76 | { | 76 | { |
77 | struct jffs2_sb_info *c = _c; | 77 | struct jffs2_sb_info *c = _c; |
78 | sigset_t hupmask; | ||
78 | 79 | ||
80 | siginitset(&hupmask, sigmask(SIGHUP)); | ||
79 | allow_signal(SIGKILL); | 81 | allow_signal(SIGKILL); |
80 | allow_signal(SIGSTOP); | 82 | allow_signal(SIGSTOP); |
81 | allow_signal(SIGCONT); | 83 | allow_signal(SIGCONT); |
84 | allow_signal(SIGHUP); | ||
82 | 85 | ||
83 | c->gc_task = current; | 86 | c->gc_task = current; |
84 | complete(&c->gc_thread_start); | 87 | complete(&c->gc_thread_start); |
@@ -87,7 +90,7 @@ static int jffs2_garbage_collect_thread(void *_c) | |||
87 | 90 | ||
88 | set_freezable(); | 91 | set_freezable(); |
89 | for (;;) { | 92 | for (;;) { |
90 | allow_signal(SIGHUP); | 93 | sigprocmask(SIG_UNBLOCK, &hupmask, NULL); |
91 | again: | 94 | again: |
92 | spin_lock(&c->erase_completion_lock); | 95 | spin_lock(&c->erase_completion_lock); |
93 | if (!jffs2_thread_should_wake(c)) { | 96 | if (!jffs2_thread_should_wake(c)) { |
@@ -95,10 +98,9 @@ static int jffs2_garbage_collect_thread(void *_c) | |||
95 | spin_unlock(&c->erase_completion_lock); | 98 | spin_unlock(&c->erase_completion_lock); |
96 | jffs2_dbg(1, "%s(): sleeping...\n", __func__); | 99 | jffs2_dbg(1, "%s(): sleeping...\n", __func__); |
97 | schedule(); | 100 | schedule(); |
98 | } else | 101 | } else { |
99 | spin_unlock(&c->erase_completion_lock); | 102 | spin_unlock(&c->erase_completion_lock); |
100 | 103 | } | |
101 | |||
102 | /* Problem - immediately after bootup, the GCD spends a lot | 104 | /* Problem - immediately after bootup, the GCD spends a lot |
103 | * of time in places like jffs2_kill_fragtree(); so much so | 105 | * of time in places like jffs2_kill_fragtree(); so much so |
104 | * that userspace processes (like gdm and X) are starved | 106 | * that userspace processes (like gdm and X) are starved |
@@ -150,7 +152,7 @@ static int jffs2_garbage_collect_thread(void *_c) | |||
150 | } | 152 | } |
151 | } | 153 | } |
152 | /* We don't want SIGHUP to interrupt us. STOP and KILL are OK though. */ | 154 | /* We don't want SIGHUP to interrupt us. STOP and KILL are OK though. */ |
153 | disallow_signal(SIGHUP); | 155 | sigprocmask(SIG_BLOCK, &hupmask, NULL); |
154 | 156 | ||
155 | jffs2_dbg(1, "%s(): pass\n", __func__); | 157 | jffs2_dbg(1, "%s(): pass\n", __func__); |
156 | if (jffs2_garbage_collect_pass(c) == -ENOSPC) { | 158 | if (jffs2_garbage_collect_pass(c) == -ENOSPC) { |