public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>,
	Peter Zijlstra <peterz@infradead.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
	David Woodhouse <dwmw2@infradead.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Ingo Molnar <mingo@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Richard Weinberger <richard@nod.at>,
	Steven Rostedt <rostedt@goodmis.org>, Tejun Heo <tj@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH RESEND2 07/11] signals: jffs2: fix the wrong usage of disallow_signal()
Date: Tue, 15 Apr 2014 20:36:59 +0200	[thread overview]
Message-ID: <20140415183659.GA13419@redhat.com> (raw)
In-Reply-To: <20140415183625.GA13371@redhat.com>

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


  parent reply	other threads:[~2014-04-15 18:37 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-15 18:36 [PATCH RESEND2 00/11] cleanup/fix allow_signal/disallow_signal Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 01/11] signals: kill sigfindinword() Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 02/11] signals: s/siginitset/sigemptyset/ in do_sigtimedwait() Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 03/11] signals: kill rm_from_queue(), change prepare_signal() to use for_each_thread() Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 04/11] signals: rename rm_from_queue_full() to flush_sigqueue_mask() Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 05/11] signals: cleanup the usage of t/current in do_sigaction() Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 06/11] signals: mv {dis,}allow_signal() from sched.h/exit.c to signal.[ch] Oleg Nesterov
2014-04-15 18:36 ` Oleg Nesterov [this message]
2014-04-15 18:37 ` [PATCH RESEND2 08/11] signals: kill the obsolete sigdelset() and recalc_sigpending() in allow_signal() Oleg Nesterov
2014-04-15 18:37 ` [PATCH RESEND2 09/11] signals: disallow_signal() should flush the potentially pending signal Oleg Nesterov
2014-04-15 18:37 ` [PATCH RESEND2 10/11] signals: introduce kernel_sigaction() Oleg Nesterov
2014-04-15 18:37 ` [PATCH RESEND2 11/11] signals: change wait_for_helper() to use kernel_sigaction() Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140415183659.GA13419@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=dwmw2@infradead.org \
    --cc=fweisbec@gmail.com \
    --cc=geert@linux-m68k.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=richard@nod.at \
    --cc=rostedt@goodmis.org \
    --cc=tj@kernel.org \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox