LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@tv-sign.ru>
To: Roland McGrath <roland@redhat.com>
Cc: Jiri Kosina <jkosina@suse.cz>,
	Davide Libenzi <davidel@xmailserver.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] [RFC] fix missed SIGCONT cases
Date: Thu, 28 Feb 2008 18:30:43 +0300	[thread overview]
Message-ID: <20080228153043.GA11484@tv-sign.ru> (raw)
In-Reply-To: <20080228102649.071572700FD@magilla.localdomain>

On 02/28, Roland McGrath wrote:
>
> -static void handle_stop_signal(int sig, struct task_struct *p)
> +static int prepare_signal(int sig, struct task_struct *p)
>  {
>  	struct task_struct *t;
> +	int cont = 0;
>  
>  	if (p->signal->flags & SIGNAL_GROUP_EXIT)
>  		/*
>  		 * The process is in the middle of dying already.
>  		 */
> -		return;
> +		return cont;
>  
>  	if (sig_kernel_stop(sig)) {
>  		/*
> @@ -635,11 +639,9 @@ static void handle_stop_signal(int sig, struct task_struct *p)
>  			 * We were in fact stopped, and are now continued.
>  			 * Notify the parent with CLD_CONTINUED.
>  			 */
> +			cont = 1;
>  			p->signal->flags = SIGNAL_STOP_CONTINUED;
>  			p->signal->group_exit_code = 0;
> -			spin_unlock(&p->sighand->siglock);
> -			do_notify_parent_cldstop(p, CLD_CONTINUED);
> -			spin_lock(&p->sighand->siglock);
>  		} else {

Oh, I'd like to suggest something simpler, but I can't :(

BTW, I think we have the same problem when handle_stop_signal() does
do_notify_parent_cldstop(p, CLD_STOPPED) above. The multithreaded app
in the middle of the group stop can resume without actually seeing
SIGCONT, no?

Currently I have the very vagues idea, please see the "patch" below
(it is not right of course, just for illustration).

These unlock(->siglock)'s on the signal sending path are really nasty,
it would be wonderfull to avoid them. Note also sig_needs_tasklist(),
we take tasklist_lock() exactly because we will probably drop siglock.

What do you think about the approach at least?

Oleg.

--- include/linux/sched.h	2008-02-15 16:59:17.000000000 +0300
+++ include/linux/sched.h	2008-02-28 18:12:20.833069408 +0300
@@ -455,6 +455,10 @@ struct signal_struct {
 	int			group_stop_count;
 	unsigned int		flags; /* see SIGNAL_* flags below */
 
+	#define __CLD_STOPPED		1
+	#define	__CLD_CONTINUED		2
+	unsigned int		notify_parent;	/* can live in fact in ->flags */
+
 	/* POSIX.1b Interval Timers */
 	struct list_head posix_timers;
 
--- kernel/signal.c	2008-02-15 16:59:17.000000000 +0300
+++ kernel/signal.c	2008-02-28 18:19:01.542067371 +0300
@@ -596,9 +596,7 @@ static void handle_stop_signal(int sig, 
 			 */
 			p->signal->group_stop_count = 0;
 			p->signal->flags = SIGNAL_STOP_CONTINUED;
-			spin_unlock(&p->sighand->siglock);
-			do_notify_parent_cldstop(p, CLD_STOPPED);
-			spin_lock(&p->sighand->siglock);
+			p->signal->notify_parent |= __CLD_STOPPED;
 		}
 		rm_from_queue(SIG_KERNEL_STOP_MASK, &p->signal->shared_pending);
 		t = p;
@@ -637,9 +635,7 @@ static void handle_stop_signal(int sig, 
 			 */
 			p->signal->flags = SIGNAL_STOP_CONTINUED;
 			p->signal->group_exit_code = 0;
-			spin_unlock(&p->sighand->siglock);
-			do_notify_parent_cldstop(p, CLD_CONTINUED);
-			spin_lock(&p->sighand->siglock);
+			p->signal->notify_parent |= __CLD_CONTINUED;
 		} else {
 			/*
 			 * We are not stopped, but there could be a stop
@@ -1755,6 +1751,7 @@ int get_signal_to_deliver(siginfo_t *inf
 			  struct pt_regs *regs, void *cookie)
 {
 	sigset_t *mask = &current->blocked;
+	int notify_parent;
 	int signr = 0;
 
 	try_to_freeze();
@@ -1890,7 +1887,15 @@ relock:
 		do_group_exit(signr);
 		/* NOTREACHED */
 	}
+	notify_parent = current->signal->notify_parent;
+	current->signal->notify_parent = 0;
 	spin_unlock_irq(&current->sighand->siglock);
+
+	if (notify_parent & __CLD_STOPPED)
+		do_notify_parent_cldstop(p, CLD_STOPPED);
+	if (notify_parent & __CLD_CONTINUED)
+		do_notify_parent_cldstop(p, CLD_CONTINUED);
+
 	return signr;
 }
 


  parent reply	other threads:[~2008-02-28 15:27 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-27 15:22 Jiri Kosina
2008-02-27 21:00 ` Roland McGrath
2008-02-27 22:04   ` Jiri Kosina
2008-02-28 10:26     ` Roland McGrath
2008-02-28 11:42       ` Jiri Kosina
2008-02-28 15:32         ` Oleg Nesterov
2008-02-28 15:32           ` Jiri Kosina
2008-02-28 15:30       ` Oleg Nesterov [this message]
2008-02-28 15:40         ` Jiri Kosina
2008-02-28 16:08           ` Oleg Nesterov
2008-02-28 16:13             ` Jiri Kosina
2008-02-28 16:45               ` [PATCH] handle_stop_signal: don't wake up the stopped task until it sees SIGCONT Oleg Nesterov
2008-02-28 16:56                 ` Jiri Kosina
2008-02-29  2:39         ` [PATCH] [RFC] fix missed SIGCONT cases Roland McGrath
2008-02-29 12:01           ` Oleg Nesterov
2008-02-29 13:20             ` Oleg Nesterov
2008-03-01  1:59             ` Roland McGrath
2008-03-01 16:41               ` Oleg Nesterov
2008-03-03 13:25                 ` Oleg Nesterov
2008-03-06 10:50                   ` Roland McGrath
2008-03-06  9:05                 ` Roland McGrath

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=20080228153043.GA11484@tv-sign.ru \
    --to=oleg@tv-sign.ru \
    --cc=akpm@linux-foundation.org \
    --cc=davidel@xmailserver.org \
    --cc=jkosina@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=roland@redhat.com \
    --subject='Re: [PATCH] [RFC] fix missed SIGCONT cases' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).