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: Andrew Morton <akpm@linux-foundation.org>,
	Alan Cox <alan@redhat.com>,
	Davide Libenzi <davidel@xmailserver.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Ingo Molnar <mingo@elte.hu>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/3] orphaned pgrp fixes
Date: Thu,  6 Mar 2008 19:53:08 -0800 (PST)	[thread overview]
Message-ID: <20080307035308.2D8CD2700FD@magilla.localdomain> (raw)
In-Reply-To: Oleg Nesterov's message of  Wednesday, 5 March 2008 20:11:09 +0300 <20080305171109.GB6723@tv-sign.ru>

> > It's excluded from counting as in your session if you consider to to be init.
> 
> Yes. Not that I really understand ;) But Eric also suggested to use
> is_container_init().

That uses a slightly different test, which I specifically did not chose.
is_container_init says "your parent thinks its PID is 1".  The test I
posted says "you think your parent's PID is 1".  (I'm being anal again.)
It's an arcane distinction.  But it seems like the correct mapping of the
original pre-pid_ns logic.

I agree with Eric that the init special case ought to just go away.
I'd suggest we do this alone first by removing the existing 
is_global_init(p) check in a tiny commit before we get into the rest
of the cleanup.  If someone's init ever has a problem, they can bisect
it back to that and we can discuss what they think it should be doing.

> So perhaps it's:
> > 
> > 	do_each_pid_task(pgrp, PIDTYPE_PGID, p) {
> > 		if (task_session(p->real_parent) == task_session(p) &&
> > 		    task_pgrp(p->real_parent) != pgrp &&
> >  		    atomic_read(&p->signal->live) > 0 &&
> > 		    task_tgid_nr_ns(p->real_parent, p->nsproxy->pid_ns) != 1)
> > 			return 0;
> > 	} while_each_pid_task(pgrp, PIDTYPE_PGID, p);
> 
> I am hopeless, I can't understand orphaned pgrps.

Eric explained it pretty well.  The idea is a pgrp is an orphan if it has
no "controller".  That term is used only a couple of times in POSIX.  In
real examples, the "controller" is always your shell.  If the shell is
gone, stopped jobs could linger around forever.  Consider:

	$ foo
	^Z
	[1]+  Stopped                 foo
	$ kill -9 $$

So foo gets the SIGHUP+SIGCONT to make it likely to die when it should.
In practice, you use "logout" not "kill -9 $$", and I suspect nowadays
the shell kills the jobs for you whenever it exits gracefully.  But this
is how the behavior arose.

Even more likely, consider:

	$ stty tostop
	...
	$ trap '' SIGHUP	# signal(SIGHUP, SIG_IGN)
	$ foo &
	$ logout

You wanted foo to keep running when you were gone, and forgot about doing
stty tostop.  Now it does some output, but the tty is someone else's, so
it gets SIGTTOU.  If this made it stop, it would be stopped until morning.
That's why it ignores such stops.  (This is why the nohup command
redirects stdout if it's a tty.)  

Nowadays there are all manner of things that make these scenarios far less
likely.  e.g. revocation of ttys, the fancy nohup command, etc.  But POSIX
specifies only a slight refinement (with the "session" idea) from what was
the original status quo of job control in 4.2/4.3 BSD.

> But still. Let's suppose that pgrp should be considered as orphaned when
> 2 tasks A and B exit. They both exit at the same time and decrement ->live
> down to zero. Now, they both can send SIGHUP to the stopped tasks.
> 
> No?

Yes, I think.  What we want is that the act of orphaning a given pgrp
happens only once.  It's that act, the transition from "controlled"
pgrp to "orphaned pgrp" that should generate the signals.  Currently
what we really check is whether it is an orphaned pgrp right now, just
after removing the currently-exiting task from the pgrp.  That is not
a transition that happens once, it's a state that prevails once the
transition has occurred.

We need to think about this more.  I just spent a while hurting my brain.
I didn't come up with anything clever.

> Stupid question, just to be sure. Suppose that SIGTSTP was sent by a
> "regular" kill_pid_info() (not by tty or kill_pgrp_info). In that case
> get_signal_to_deliver() still must check is_current_pgrp_orphaned(),
> yes?

Yes.  It is specifically said that SIGTSTP, SIGTTIN, SIGTTOU may not
cause a stop in a member of an orphaned pgrp.  It doesn't matter how
it came to be that the signal got delivered.


Thanks,
Roland

      parent reply	other threads:[~2008-03-07  3:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-02 18:44 Oleg Nesterov
2008-03-04 12:26 ` Roland McGrath
2008-03-04 15:51   ` Oleg Nesterov
2008-03-05  1:11     ` Roland McGrath
2008-03-05 16:48       ` Oleg Nesterov
2008-03-05 17:11   ` Oleg Nesterov
2008-03-06  1:14     ` Eric W. Biederman
2008-03-07  1:52       ` Oleg Nesterov
2008-03-07  3:53     ` Roland McGrath [this message]

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=20080307035308.2D8CD2700FD@magilla.localdomain \
    --to=roland@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=alan@redhat.com \
    --cc=davidel@xmailserver.org \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=oleg@tv-sign.ru \
    --cc=torvalds@linux-foundation.org \
    --subject='Re: [PATCH 0/3] orphaned pgrp fixes' \
    /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).