public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Christian Brauner <christian@brauner.io>
Cc: Oleg Nesterov <oleg@redhat.com>,
	linux-kernel@vger.kernel.org, arnd@arndb.de,
	keescook@chromium.org, joel@joelfernandes.org,
	tglx@linutronix.de, tj@kernel.org, dhowells@redhat.com,
	jannh@google.com, luto@kernel.org, akpm@linux-foundation.org,
	cyphar@cyphar.com, torvalds@linux-foundation.org,
	viro@zeniv.linux.org.uk, kernel-team@android.com,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-api@vger.kernel.org
Subject: Re: [PATCH 4/5] pidfd: add CLONE_WAIT_PID
Date: Thu, 25 Jul 2019 11:57:19 -0500	[thread overview]
Message-ID: <878ssmuk5s.fsf@xmission.com> (raw)
In-Reply-To: <20190725122650.4i3arct5rpchqmyt@brauner.io> (Christian Brauner's message of "Thu, 25 Jul 2019 14:26:51 +0200")

Christian Brauner <christian@brauner.io> writes:

> On Thu, Jul 25, 2019 at 01:43:59PM +0200, Oleg Nesterov wrote:
>> Or. We can change wait_consider_task() to not clear ->notask_error if
>> WXXX and the child is PF_WAIT_PID.
>> 
>> This way you can "safely" use wait() without WNOHANG, it won't block if
>> all the children which can report an even are PF_WAIT_PID.
>> 
>> But I do not understand your use-cases, I have no idea if this can help
>
> One usecase (among others listed in the commit message) are shared
> libraries. P_ALL is usually something you can't really use in a shared
> library because you have no idea what else might be fork()ed off. Only
> the main program can use this but none of the auxiliary libraries that
> it uses.
> The other way around you want to be able fork() off something without
> affecting P_ALL in the main program.
> The key is that you want to be able to create child processes in a
> shared library without the main programing having to know about this so
> that it can use P_ALL and never get stuff from the library.
>
> Assume you have a project with a main loop with a million things
> happening in that mainloop like some gui app running an avi video. For
> example, gtk uses gstreamer which forks off all codecs in child
> processes which are sandboxed for security. So gstreamer is using helper
> processes in the background which are my children now. Now I'm creating
> four more additional helper processes as well. Now, in my (glib, qt
> whatever) mainloop on SIGCHLD some part of the app is checking with
> WNHOANG and finds a process has exited. It's cleaning this thing up now
> but it's not a process it wanted to clean up. The other part of the app
> is now doing waitid(P_PID, pid) but will find the process already gone
> it wanted to reap.
>
> I hope I'm expressing this well enough.


I think so.

A) I think Oleg is correct that you should test the flag in
   do_wait_thread rather than elsewhere.

B) We have a deficiency in do_wait that should be addressed.  The
   do_wait function does not have a fast path for waiting on a
   particular process.  For adding this functionality I such a fast path
   goes from a nice to have to a necessity for getting all of the
   fiddly details correct.

C) I believe the semantics should be that while such a file descriptor
   is open, only that file descriptor can be used to reap the process.
   And that it should be allowed to pass the file descriptor between
   processes.  Which means the parent can die and the process be
   reparted to init and we should still be able to reap the process with
   the file descriptor.

D) I think it is a toss up how we should deal with such a process when
   the file descriptor is closed.  Setting the process to autoreap
   or reparent to init and let init deal with it.  My inclination is
   that autoreap is the correct behavior.
   
Eric

  




  parent reply	other threads:[~2019-07-25 16:57 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-24 14:46 [PATCH 0/5] pidfd: waiting on processes through pidfds Christian Brauner
2019-07-24 14:46 ` [RFC][PATCH 1/5] exit: kill struct waitid_info Christian Brauner
2019-07-24 17:37   ` Linus Torvalds
2019-07-24 22:01     ` Christian Brauner
2019-07-25 12:46     ` Eric W. Biederman
2019-07-26  8:01       ` Christian Brauner
2019-07-26 11:59         ` Eric W. Biederman
2019-07-26 12:37           ` Christian Brauner
2019-07-25  9:40   ` Oleg Nesterov
2019-07-25 10:07     ` Christian Brauner
2019-07-24 14:46 ` [PATCH 2/5] pidfd: add pidfd_wait() Christian Brauner
2019-07-24 17:45   ` Linus Torvalds
2019-07-24 17:50     ` Christian Brauner
2019-07-24 17:52       ` Christian Brauner
2019-07-25 14:34     ` Eric W. Biederman
2019-07-25 10:16   ` Oleg Nesterov
2019-07-25 10:21     ` Christian Brauner
2019-07-26  8:19   ` Arnd Bergmann
2019-07-26  8:24     ` Christian Brauner
2019-07-26  9:57       ` Arnd Bergmann
2019-07-24 14:46 ` [PATCH 3/5] arch: wire-up pidfd_wait() Christian Brauner
2019-07-24 14:46 ` [PATCH 4/5] pidfd: add CLONE_WAIT_PID Christian Brauner
2019-07-24 18:14   ` Jann Horn
2019-07-24 18:27     ` Christian Brauner
2019-07-24 19:07       ` Jann Horn
2019-07-24 19:10         ` Christian Brauner
2019-07-25 10:11           ` Christian Brauner
2019-07-25 10:30         ` Oleg Nesterov
2019-07-25 10:36           ` Christian Brauner
2019-07-25 10:35   ` Oleg Nesterov
2019-07-25 10:40     ` Christian Brauner
2019-07-25 11:25       ` Oleg Nesterov
2019-07-25 11:41         ` Christian Brauner
2019-07-25 11:43         ` Oleg Nesterov
2019-07-25 12:26           ` Christian Brauner
2019-07-25 16:13             ` Oleg Nesterov
2019-07-25 16:56               ` Christian Brauner
2019-07-25 16:57             ` Eric W. Biederman [this message]
2019-07-24 14:46 ` [PATCH 5/5] pidfd: add pidfd_wait tests Christian Brauner

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=878ssmuk5s.fsf@xmission.com \
    --to=ebiederm@xmission.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=christian@brauner.io \
    --cc=cyphar@cyphar.com \
    --cc=dhowells@redhat.com \
    --cc=jannh@google.com \
    --cc=joel@joelfernandes.org \
    --cc=keescook@chromium.org \
    --cc=kernel-team@android.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox