All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Stas Sergeev <stsp@aknet.ru>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Linux kernel <linux-kernel@vger.kernel.org>
Subject: Re: [path][rfc] add PR_DETACH prctl command [2/2]
Date: Wed, 20 Apr 2011 21:33:07 +0200	[thread overview]
Message-ID: <20110420193307.GA1445@redhat.com> (raw)
In-Reply-To: <4DAF29AD.1080603@aknet.ru>

On 04/20, Stas Sergeev wrote:
>
> 20.04.2011 20:50, Oleg Nesterov wrote:
>>> The attached patch implements the PR_DETACH prctl
>>> command. It detaches the entire process group from
>>> its parent, allowing the parent to still read the detach
>>> code with normal wait().
>>> Can be used to daemonize process with threads.
>> At first glance, this version does not reparent the caller to init,
>> but still PR_DETACH checks ->real_parent != /sbin/init for unknown
>> reason...
> The reason is that the task could have been reparented
> to ini by some other means (parent died)

This is clear,

- in this case no
> detaching.

Why? Now that we do not reparent PR_DETACH could works in this case too.
Nevermind, this is minor and probably makes sense, forget.

I still do not understand the point of PR_DETACH. Why do you think it is
needed without reparenting? OK, I do not really care ;)

>> IIUC, PR_DETACH simply fools ->real_parent so that it think this child
>> exits, while the child in fact runs after that but do_wait() can't see it.
> It can check si_code to see what actually happened.
>
>> Why? I don't understand the point.
> It was the same also when reparenting was done: the
> original parent was only able to read the detach code,
> and nothing more. _Nothing_ was changed, the semantic
> is completely the same!

The same? what about, say, ppid?

>> And. To hide the pr_detached task from do_wait(). you changed
>> do_notify_parent() to returnd DEATH_REAP.
> No, its hidden by the check in wait_consider_task().
> do_notify_parent() was changed only to not allow the
> second notification to the same parent.

Not only. Please look at your own code ;) wait_consider_task() checks
exit_state == EXIT_ZOMBIE before p->pr_detached, and thus do_notify_parent()
haas to return DEATH_REAP so that the caller will set EXIT_DEAD. Otherwise
the old parent could see EXIT_ZOMBIE && pr_detached task again.

> Again, this is
> to completely preserve the old semantic, where's the
> original parent was always notified only once.

Of course, it should be notified only once. but afaics it can be notified
twice.

>> This looks very ugly I must admit. And not 100% correct, we
>> notify the parent twice if ->detaching != 0.
>
> No: the do_dignal_parent() is not called in that case too.

OK, I misread this code. In fact I missed something else, can't recall
what I meant. Probably I was wrong anyway.

But. What if the child stops after PR_DETACH? It will notify the parent
anyway, no? And this can even happen after wait(WEXITED) succeeded.

>> task_pid_vnr(p). Hmm, the change in reparent_leader doesn't look right,
>> at least in case we reparent to sub-thread.
> Why not? Whereever we reparented, I allow reparenting again.

Even if the child reparents to the original parent's sub-thread?

> But, more importantly, I unhide that task in wait() and allow the
> notifications again, so without this change it can't work.

This is clear but see above.

>> And of course, this breaks ptrace _completely_.
> Why? What exactly breaks?

Everything. ptrace relies on do_wait(). wait_consider_task() doesn't
work if pr_detached (except for WEXITED of course).

>> Stas, I am sorry, I am tired ;) You are sending more and more versions
>> with the different semantics and they all are buggy.
> I am not that sure the last ones are very buggy.

Well, I am not sure.

> but at least that semantic
> was a kind of "agreed on", and didn't change at all.

Cough. So far nobody except you agrees with this semantics ;)

Oleg.


  reply	other threads:[~2011-04-20 19:33 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-23 13:50 [path][rfc] add PR_DETACH prctl command Stas Sergeev
2011-02-23 19:14 ` Oleg Nesterov
2011-02-23 20:35   ` Stas Sergeev
2011-02-24 13:29     ` Oleg Nesterov
2011-02-24 15:13       ` Stas Sergeev
2011-02-24 15:32         ` Oleg Nesterov
2011-03-31 16:10           ` Stas Sergeev
2011-03-31 17:02             ` Oleg Nesterov
2011-03-31 17:47               ` Stas Sergeev
2011-03-31 18:18                 ` Oleg Nesterov
2011-03-31 20:58                   ` Stas Sergeev
2011-04-02 13:55                     ` Oleg Nesterov
2011-04-02 18:20                       ` Stas Sergeev
2011-04-02 22:00                       ` Stas Sergeev
2011-04-01 17:02               ` Stas Sergeev
2011-04-02 14:06                 ` Oleg Nesterov
2011-04-04 14:34               ` Stas Sergeev
2011-04-04 16:03                 ` Oleg Nesterov
2011-04-04 20:05                   ` Stas Sergeev
2011-04-05 15:15                     ` Oleg Nesterov
2011-04-05 16:25                       ` Stas Sergeev
2011-04-05 16:45                         ` Oleg Nesterov
2011-04-05 17:51                           ` Stas Sergeev
2011-04-08 10:51                           ` Stas Sergeev
2011-04-08 18:55                             ` Oleg Nesterov
2011-04-08 20:16                               ` Stas Sergeev
2011-04-11 11:15                           ` Stas Sergeev
2011-04-19 14:44                           ` [path][rfc] add PR_DETACH prctl command [1/3] Stas Sergeev
2011-04-19 14:50                           ` [path][rfc] add PR_DETACH prctl command [2/3] Stas Sergeev
2011-04-19 14:54                           ` [path][rfc] add PR_DETACH prctl command [3/3] Stas Sergeev
2011-04-19 14:58                             ` Alan Cox
2011-04-19 15:08                               ` Stas Sergeev
2011-04-19 15:54                                 ` Alan Cox
2011-04-19 16:13                                   ` Oleg Nesterov
2011-04-19 16:29                                     ` Oleg Nesterov
2011-04-19 16:54                                       ` Stas Sergeev
2011-04-19 17:20                                         ` Oleg Nesterov
2011-04-19 17:41                                           ` Stas Sergeev
2011-04-19 18:17                                             ` Oleg Nesterov
2011-04-19 16:19                                   ` Stas Sergeev
2011-04-20 13:12                                   ` [path][rfc] add PR_DETACH prctl command [1/2] Stas Sergeev
2011-04-20 13:14                                   ` [path][rfc] add PR_DETACH prctl command [2/2] Stas Sergeev
2011-04-20 16:50                                     ` Oleg Nesterov
2011-04-20 18:45                                       ` Stas Sergeev
2011-04-20 19:33                                         ` Oleg Nesterov [this message]
2011-04-20 20:35                                           ` Stas Sergeev
2011-04-21 20:00                                             ` Oleg Nesterov
2011-04-21 20:11                                               ` Stas Sergeev
2011-04-21 10:02                                       ` Stas Sergeev
2011-04-21 20:15                                         ` Oleg Nesterov
2011-04-21 20:32                                           ` Stas Sergeev
2011-04-08 18:13 ` [path][rfc] add PR_DETACH prctl command Bryan Donlan
2011-04-08 20:26   ` Stas Sergeev
2011-04-08 20:52     ` Bryan Donlan
2011-04-08 21:14       ` Stas Sergeev
2011-04-08 21:25         ` Bryan Donlan
2011-04-08 21:38           ` Stas Sergeev

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=20110420193307.GA1445@redhat.com \
    --to=oleg@redhat.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stsp@aknet.ru \
    /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.