public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: jan.kratochvil@redhat.com, vda.linux@googlemail.com,
	linux-kernel@vger.kernel.org, torvalds@linux-foundation.org,
	akpm@linux-foundation.org, indan@nul.nu, roland@hack.frob.com
Subject: [PATCH 1/4] signal: prepare_signal(SIGCONT) shouldn't play with TIF_SIGPENDING
Date: Fri, 1 Apr 2011 20:11:50 +0200	[thread overview]
Message-ID: <20110401181150.GB9010@redhat.com> (raw)
In-Reply-To: <20110401181123.GA9010@redhat.com>

prepare_signal(SIGCONT) should never set TIF_SIGPENDING or wake up
the TASK_INTERRUPTIBLE threads. We are going to call complete_signal()
which should pick the right thread correctly. All we need is to wake
up the TASK_STOPPED threads.

If the task was stopped, it can't return to usermode without taking
->siglock. Otherwise we don't care, and the spurious TIF_SIGPENDING
can't be useful.

The comment says:

	* If there is a handler for SIGCONT, we must make
	* sure that no thread returns to user mode before
	* we post the signal

It is not clear what this means. Probably, "when there's only a single
thread" and this continues to be true. Otherwise, even if this SIGCONT
is not private, with or without this change only one thread can dequeue
SIGCONT, other threads can happily return to user mode before before
that thread handles this signal.

Note also that wake_up_state(t, __TASK_STOPPED) can't race with the task
which changes its state, TASK_STOPPED state is protected by ->siglock as
well.

In short: when it comes to signal delivery, SIGCONT is the normal signal
and does not need any special support.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---

 kernel/signal.c |   27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

--- ptrace/kernel/signal.c~1_cont_remove_sigwake	2011-04-01 16:36:29.000000000 +0200
+++ ptrace/kernel/signal.c	2011-04-01 17:12:07.000000000 +0200
@@ -788,37 +788,14 @@ static int prepare_signal(int sig, struc
 	} else if (sig == SIGCONT) {
 		unsigned int why;
 		/*
-		 * Remove all stop signals from all queues,
-		 * and wake all threads.
+		 * Remove all stop signals from all queues, wake all threads.
 		 */
 		rm_from_queue(SIG_KERNEL_STOP_MASK, &signal->shared_pending);
 		t = p;
 		do {
-			unsigned int state;
-
 			task_clear_group_stop_pending(t);
-
 			rm_from_queue(SIG_KERNEL_STOP_MASK, &t->pending);
-			/*
-			 * If there is a handler for SIGCONT, we must make
-			 * sure that no thread returns to user mode before
-			 * we post the signal, in case it was the only
-			 * thread eligible to run the signal handler--then
-			 * it must not do anything between resuming and
-			 * running the handler.  With the TIF_SIGPENDING
-			 * flag set, the thread will pause and acquire the
-			 * siglock that we hold now and until we've queued
-			 * the pending signal.
-			 *
-			 * Wake up the stopped thread _after_ setting
-			 * TIF_SIGPENDING
-			 */
-			state = __TASK_STOPPED;
-			if (sig_user_defined(t, SIGCONT) && !sigismember(&t->blocked, SIGCONT)) {
-				set_tsk_thread_flag(t, TIF_SIGPENDING);
-				state |= TASK_INTERRUPTIBLE;
-			}
-			wake_up_state(t, state);
+			wake_up_state(t, __TASK_STOPPED);
 		} while_each_thread(p, t);
 
 		/*


  reply	other threads:[~2011-04-01 18:12 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-29 14:46 [PATCH 1/3] signal: Make signal_wake_up() take @sig_type instead of @resume Tejun Heo
2011-03-29 14:46 ` [PATCH 2/3] signal, ptrace: Add SIGTRAP signal_wake_up() Tejun Heo
2011-03-29 14:47   ` [PATCH 3/3] signal, ptrace: Fix delayed CONTINUED notification when ptraced Tejun Heo
2011-03-30 19:29     ` Oleg Nesterov
2011-03-31  7:29       ` Tejun Heo
2011-03-31 15:15         ` Oleg Nesterov
2011-03-31 16:34           ` Tejun Heo
2011-03-31 16:35             ` Tejun Heo
2011-03-31 17:29             ` Oleg Nesterov
2011-04-01 18:11               ` [PATCH 0/4] Was: " Oleg Nesterov
2011-04-01 18:11                 ` Oleg Nesterov [this message]
2011-04-01 18:12                 ` [PATCH 2/4] signal: do_signal_stop: remove the unneeded task_clear_group_stop_pending() Oleg Nesterov
2011-04-01 18:12                 ` [PATCH 3/4] signal: turn SIGNAL_STOP_DEQUEUED into GROUP_STOP_DEQUEUED Oleg Nesterov
2011-04-01 18:13                 ` [PATCH 4/4] ptrace: ptrace_check_attach() should not do s/STOPPED/TRACED/ Oleg Nesterov
2011-04-04  0:11                 ` [PATCH 0/4] Was: signal, ptrace: Fix delayed CONTINUED notification when ptraced Tejun Heo
2011-03-29 18:27 ` [PATCH 1/3] signal: Make signal_wake_up() take @sig_type instead of @resume 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=20110401181150.GB9010@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=indan@nul.nu \
    --cc=jan.kratochvil@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=roland@hack.frob.com \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=vda.linux@googlemail.com \
    /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