LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Roland McGrath <roland@redhat.com>
To: Oleg Nesterov <oleg@tv-sign.ru>
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,  6 Mar 2008 02:50:43 -0800 (PST)	[thread overview]
Message-ID: <20080306105043.8FD982700FD@magilla.localdomain> (raw)
In-Reply-To: Oleg Nesterov's message of  Monday, 3 March 2008 16:25:02 +0300 <20080303132502.GA81@tv-sign.ru>

> Actually, it is easy to re-use signal_struct->flags from the very beginning,
> no need to introduce the new flag temporary, see the attached patches. I also
> have a couple of simple cleanups on top of them (use cached ->signal value).

Looks like a good start.

> Perhaps, we can add the new helper which sets SIGNAL_GROUP_EXIT but preserves
> SIGNAL_CLD_MASK, but I don't think this is really needed.

Actually, I think it is.  This made think of another pedantic concern.
(Not that I'm sure we get this right now anyway.)

Say a child was previously stopped, parent had seen CLD_STOPPED and done
WSTOPPED wait.  Now we send SIGCONT followed shortly by SIGTERM (unhandled
fatal signal).  In the interval before the SIGTERM has yet been dequeued
(nor SIGKILLs sent to other threads by __group_complete_signal), the parent
does waitpid(pid, WSTOPPED|WCONTINUED|WEXITED|WNOHANG).  It should see one
of: WIFSTOPPED (still); WIFSIGNALED (dead); WIFCONTINUED.  Instead, waitpid
will return 0 (it's running, not stopped, not continued).  This says
SIGNAL_STOP_CONTINUED ought to function as normal (i.e. WCONTINUED polling)
until we're actually ready for wait to report the process as dead.

We already break this pedantic nit because flags = SIGNAL_GROUP_EXIT clears
SIGNAL_STOP_CONTINUED.  But if we clean up all flags usage, I think we
should make it preserve SIGNAL_STOP_CONTINUED.

I'll admit it's thin since a SIGKILL not yet dequeued or in the middle of
delivery brings the task to running so WNOHANG wait won't report it as
still stopped, won't report it for WCONTINUED at all, and won't report it
as dead yet.  But if we were to make wait key on SIGNAL_STOP_STOPPED, as is
probably necessary to do WSTOPPED waits for processes with a zombie
group_leader, then this too could be made consistent.  (Leave the
SIGNAL_STOP_STOPPED bit set throughout, so WSTOPPED|WNOHANG waits continues
to report it as stopped until it actually dies.)

> @@ -1761,6 +1757,19 @@ int get_signal_to_deliver(siginfo_t *inf
>  
>  relock:
>  	spin_lock_irq(&current->sighand->siglock);
> +
> +	if (unlikely(current->signal->flags & SIGNAL_CLD_MASK)) {
> +		int why = (current->signal->flags & SIGNAL_STOP_CONTINUED)

This should have a comment before the block.  Explain that it is finishing
the report set up by handle_stop_signal while we were in TASK_STOPPED in a
previous iteration here inside get_signal_to_deliver.


Thanks,
Roland

  reply	other threads:[~2008-03-06 10:51 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
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 [this message]
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=20080306105043.8FD982700FD@magilla.localdomain \
    --to=roland@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=davidel@xmailserver.org \
    --cc=jkosina@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@tv-sign.ru \
    --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).