All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.