Re: [PATCH 6/6] clone4: Introduce new CLONE_FD flag to get task exit notification via fd

5 messages, 4 authors, 2015-03-14 · open the first message on its own page

Re: [PATCH 6/6] clone4: Introduce new CLONE_FD flag to get task exit notification via fd

From: Oleg Nesterov <hidden>
Date: 2015-03-14 14:32:35

And let me add another note before I forget...

On 03/14, Oleg Nesterov wrote:
On 03/13, josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org wrote:
quoted

A process launching a new process with CLONE_FD is explicitly requesting
that the process be automatically reaped without any other process
having to wait on it.  The task needs to not become a zombie, because
otherwise, it'll show up in waitpid(-1, ...)
This is clear.

But please note that this task can be traced/debugged by unrelated process,
not its real_parent/creator. Say, the system admin does "strace -p". This
simply breaks the current API.

Again, again, I didn't read this series yet. But the proper solution (afaics)
should move this "autoreap" check in release_task/__ptrace_detach(). If the
task is traced. Debugger should check ->autoreap and skip another
do_notify_parent().

Speaking of autoreap... If ->exit_signal is zero, then the exiting child
doesn't send the notification to its parent, still it doesn't autoreap
itself. To me this looks strange, and in fact it seems to me that this
is only by mistake. I am wondering if we can treat ->exit_signal == 0
as "autoreap" too. As usual, most probably the answer is "no, because it
is too late to change the historical behaviour". But this is off-topic.

It is not clear to me what do_wait() should do with ->autoreap child, even
ignoring ptrace.

Just suppose that real_parent has a single "autoreap" child. Should wait(NULL)
hanf then?

If yes, who will wake the parent up?

If no, I do not see the necessary changes in wait_cosnider_task().

Oleg.

Re: [PATCH 6/6] clone4: Introduce new CLONE_FD flag to get task exit notification via fd

From: Thiago Macieira <hidden>
Date: 2015-03-14 18:38:29

On Saturday 14 March 2015 15:32:35 Oleg Nesterov wrote:
It is not clear to me what do_wait() should do with ->autoreap child, even
ignoring ptrace.

Just suppose that real_parent has a single "autoreap" child. Should
wait(NULL) hanf then?
It should ignore the child that is set to autoreap. wait(NULL) should return -
ECHILD, indicating there are no children waiting to be reaped.

But now I realise that this might have implications for session management and 
job control.
If yes, who will wake the parent up?

If no, I do not see the necessary changes in wait_cosnider_task().
-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center

Re: [PATCH 6/6] clone4: Introduce new CLONE_FD flag to get task exit notification via fd

From: Oleg Nesterov <oleg@redhat.com>
Date: 2015-03-14 18:56:42

On 03/14, Thiago Macieira wrote:
On Saturday 14 March 2015 15:32:35 Oleg Nesterov wrote:
quoted
It is not clear to me what do_wait() should do with ->autoreap child, even
ignoring ptrace.

Just suppose that real_parent has a single "autoreap" child. Should
wait(NULL) hanf then?
It should ignore the child that is set to autoreap. wait(NULL) should return -
ECHILD, indicating there are no children waiting to be reaped.
I disagree. I won't really argue now, because I think that this needs
a separate discussion. And imo "autoreap" should come as a separate feature.

I think that wait(NULL) should hang like it hangs even if the parent ignores
SIGCHLD. But in this case the parent should be woken up when the "autoreap"
child exits.

If nothing else. Suppose that the parent does waitid(WEXITED|WSTOPPED).
Should WSTOPPED work? I think it should.

At the same time, if we add autoreap then probably it also makes sense to add
WEXITIED_UNLESS_AUTOREAP.

In short: this all certainly needs more discussion, but (afaics) this patch
is wrong in any case.



In fact I have some concerns about file descriptor from clone, it doesn't look
like a "right" interface to me. But I will not comment this part until at least
I read 0/4 ;)

Oleg.

Re: [PATCH 6/6] clone4: Introduce new CLONE_FD flag to get task exit notification via fd

From: Josh Triplett <josh@joshtriplett.org>
Date: 2015-03-14 19:01:32

On Sat, Mar 14, 2015 at 11:38:29AM -0700, Thiago Macieira wrote:
On Saturday 14 March 2015 15:32:35 Oleg Nesterov wrote:
quoted
It is not clear to me what do_wait() should do with ->autoreap child, even
ignoring ptrace.

Just suppose that real_parent has a single "autoreap" child. Should
wait(NULL) hanf then?
It should ignore the child that is set to autoreap. wait(NULL) should return -
ECHILD, indicating there are no children waiting to be reaped.
Right.  And I don't think the current code does this.  I think we need
to change wait_consider_task to early-return for ->autoreap just as it
does for task_state == EXIT_DEAD.  I'll do that in v2.

- Josh Triplett

Re: [PATCH 6/6] clone4: Introduce new CLONE_FD flag to get task exit notification via fd

From: Oleg Nesterov <hidden>
Date: 2015-03-14 19:18:36

On 03/14, Josh Triplett wrote:
On Sat, Mar 14, 2015 at 11:38:29AM -0700, Thiago Macieira wrote:
quoted
On Saturday 14 March 2015 15:32:35 Oleg Nesterov wrote:
quoted
It is not clear to me what do_wait() should do with ->autoreap child, even
ignoring ptrace.

Just suppose that real_parent has a single "autoreap" child. Should
wait(NULL) hanf then?
It should ignore the child that is set to autoreap. wait(NULL) should return -
ECHILD, indicating there are no children waiting to be reaped.
Right.  And I don't think the current code does this.  I think we need
to change wait_consider_task to early-return for ->autoreap just as it
does for task_state == EXIT_DEAD.
No. This EXIT_DEAD is absolutely different. And this is another indication
that you might use it wrongly ;)

What we actually want is BUG_ON(task_state == EXIT_DEAD) here. We do not
want the EXIT_DEAD tasks in ->children/ptraced lists. These EXIT_DEAD tasks
complicate the exit/wait/reparent paths.

However, currently this is TODO. The main problem is the locking in
wait_task_zombie(), we can set EXIT_DEAD and remove the task from list
under read_lock().

And please see another email from me. So far  I disagree that wait(NULL)
should return ECHILD unconditionally. At least unless this is discussed
separately.

Oleg.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help