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 3/4] signal: turn SIGNAL_STOP_DEQUEUED into GROUP_STOP_DEQUEUED
Date: Fri, 1 Apr 2011 20:12:38 +0200	[thread overview]
Message-ID: <20110401181238.GD9010@redhat.com> (raw)
In-Reply-To: <20110401181123.GA9010@redhat.com>

This patch moves SIGNAL_STOP_DEQUEUED from signal_struct->flags to
task_struct->group_stop, and thus makes it per-thread.

Like SIGNAL_STOP_DEQUEUED, GROUP_STOP_DEQUEUED can be false-positive
after return from get_signal_to_deliver, this is fine. The only purpose
of this bit is: we can drop ->siglock after __dequeue_signal() returns
the sig_kernel_stop() signal and before we call do_signal_stop(), in
this case we must not miss SIGCONT if it comes in between.

But, unlike SIGNAL_STOP_DEQUEUED, GROUP_STOP_DEQUEUED can not be
false-positive in do_signal_stop() if multiple threads dequeue the
sig_kernel_stop() signal at the same time.

Consider two threads T1 and T2, SIGTTIN has a hanlder.

	- T1 dequeues SIGTSTP and sets SIGNAL_STOP_DEQUEUED, then
	  it drops ->siglock

	- SIGCONT comes and clears SIGNAL_STOP_DEQUEUED, SIGTSTP
	  should be cancelled.

	- T2 dequeues SIGTTIN and sets SIGNAL_STOP_DEQUEUED again.
	  Since we have a handler we should not stop, T2 returns
	  to usermode to run the handler.

	- T1 continues, calls do_signal_stop() and wrongly starts
	  the group stop because SIGNAL_STOP_DEQUEUED was restored
	  in between.

With or without this change:

	- we need to do something with ptrace_signal() which can
	  return SIGSTOP, but this needs another discussion

	- SIGSTOP can be lost if it races with the mt exec, will
	  be fixed later.

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

 include/linux/sched.h |    6 +++---
 kernel/signal.c       |   14 ++++----------
 2 files changed, 7 insertions(+), 13 deletions(-)

--- ptrace/include/linux/sched.h~3_make_stop_dequeued_per_thread	2011-04-01 16:36:29.000000000 +0200
+++ ptrace/include/linux/sched.h	2011-04-01 18:50:38.000000000 +0200
@@ -652,9 +652,8 @@ struct signal_struct {
  * Bits in flags field of signal_struct.
  */
 #define SIGNAL_STOP_STOPPED	0x00000001 /* job control stop in effect */
-#define SIGNAL_STOP_DEQUEUED	0x00000002 /* stop signal dequeued */
-#define SIGNAL_STOP_CONTINUED	0x00000004 /* SIGCONT since WCONTINUED reap */
-#define SIGNAL_GROUP_EXIT	0x00000008 /* group exit in progress */
+#define SIGNAL_STOP_CONTINUED	0x00000002 /* SIGCONT since WCONTINUED reap */
+#define SIGNAL_GROUP_EXIT	0x00000004 /* group exit in progress */
 /*
  * Pending notifications to parent.
  */
@@ -1779,6 +1778,7 @@ extern void thread_group_times(struct ta
 #define GROUP_STOP_PENDING	(1 << 16) /* task should stop for group stop */
 #define GROUP_STOP_CONSUME	(1 << 17) /* consume group stop count */
 #define GROUP_STOP_TRAPPING	(1 << 18) /* switching from STOPPED to TRACED */
+#define GROUP_STOP_DEQUEUED	(1 << 19) /* stop signal dequeued */
 
 extern void task_clear_group_stop_pending(struct task_struct *task);
 
--- ptrace/kernel/signal.c~3_make_stop_dequeued_per_thread	2011-04-01 18:10:18.000000000 +0200
+++ ptrace/kernel/signal.c	2011-04-01 18:58:41.000000000 +0200
@@ -254,7 +254,8 @@ static void task_clear_group_stop_trappi
  */
 void task_clear_group_stop_pending(struct task_struct *task)
 {
-	task->group_stop &= ~(GROUP_STOP_PENDING | GROUP_STOP_CONSUME);
+	task->group_stop &= ~(GROUP_STOP_PENDING | GROUP_STOP_CONSUME |
+				GROUP_STOP_DEQUEUED);
 }
 
 /**
@@ -602,7 +603,7 @@ int dequeue_signal(struct task_struct *t
 		 * is to alert stop-signal processing code when another
 		 * processor has come along and cleared the flag.
 		 */
-		tsk->signal->flags |= SIGNAL_STOP_DEQUEUED;
+		current->group_stop |= GROUP_STOP_DEQUEUED;
 	}
 	if ((info->si_code & __SI_MASK) == __SI_TIMER && info->si_sys_private) {
 		/*
@@ -821,13 +822,6 @@ static int prepare_signal(int sig, struc
 			signal->flags = why | SIGNAL_STOP_CONTINUED;
 			signal->group_stop_count = 0;
 			signal->group_exit_code = 0;
-		} else {
-			/*
-			 * We are not stopped, but there could be a stop
-			 * signal in the middle of being processed after
-			 * being removed from the queue.  Clear that too.
-			 */
-			signal->flags &= ~SIGNAL_STOP_DEQUEUED;
 		}
 	}
 
@@ -1855,7 +1849,7 @@ static int do_signal_stop(int signr)
 		/* signr will be recorded in task->group_stop for retries */
 		WARN_ON_ONCE(signr & ~GROUP_STOP_SIGMASK);
 
-		if (!likely(sig->flags & SIGNAL_STOP_DEQUEUED) ||
+		if (!likely(current->group_stop & GROUP_STOP_DEQUEUED) ||
 		    unlikely(signal_group_exit(sig)))
 			return 0;
 		/*


  parent reply	other threads:[~2011-04-01 18:13 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                 ` [PATCH 1/4] signal: prepare_signal(SIGCONT) shouldn't play with TIF_SIGPENDING Oleg Nesterov
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                 ` Oleg Nesterov [this message]
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=20110401181238.GD9010@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