Linux-man Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH] clone.2: add CLONE_PIDFD entry
       [not found] <20190511064908.21956-1-christian.brauner@ubuntu.com>
@ 2019-09-11  8:58 ` Michael Kerrisk (man-pages)
  2019-09-16  7:40   ` Christian Brauner
  0 siblings, 1 reply; 2+ messages in thread
From: Michael Kerrisk (man-pages) @ 2019-09-11  8:58 UTC (permalink / raw)
  To: Christian Brauner, linux-man; +Cc: mtk.manpages, jannh, oleg

Hello Christian,

On 5/11/19 8:49 AM, Christian Brauner wrote:
> From: Christian Brauner <christian@brauner.io>
> 
> Add an entry for CLONE_PIDFD. This flag is available starting with
> kernel 5.2. If specified, a process file descriptor ("pidfd") referring
> to the child process will be returned in the ptid argument.

I've applied this patch in a local branch, and made some minor edits
and added a piece. And I have some questions. See below.

> Signed-off-by: Christian Brauner <christian@brauner.io>
> ---
>  man2/clone.2 | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/man2/clone.2 b/man2/clone.2
> index 7e880beb8..ee08aeb42 100644
> --- a/man2/clone.2
> +++ b/man2/clone.2
> @@ -539,6 +539,42 @@ The flag disappeared completely from the kernel sources in Linux 2.5.16.
>  Since then, the kernel silently ignores this bit if it is specified in
>  .IR flags .
>  .TP
> +.BR CLONE_PIDFD " (since Linux 5.2)"
> +If
> +.B CLONE_PIDFD
> +is set,
> +.BR clone ()
> +stores a process file descriptor ("pidfd") referring to the child process at
> +the location
> +.I ptid
> +in the parent's memory. 

I added a note that the close-on-exec flag is set on the new FD.

> Note, that the kernel verifies that the value for
> +.I ptid
> +is zero. If it is not an error will be returned. This ensures that
> +.I ptid
> +can potentially be used to specify additional options for
> +.B CLONE_PIDFD
> +in the future.

This piece is no longer true, right? At least I can't see such 
a check in the kernel code, and my testing doesn't yield an error
when ptid != 0 before the call.(No need to send me a patch; if I'm
correct just let me know and I'll edit out this piece.)

> +.IP
> +Since the
> +.I ptid
> +argument is used to return the pidfd,
> +.B CLONE_PIDFD
> +cannot be used with
> +.B CLONE_PARENT_SETTID.
> +.IP
> +It is currently not possible to use this flag together with
> +.B CLONE_THREAD.
> +This means that the process identified by the pidfd will always be a
> +thread-group leader.
> +.IP
> +For a while there was a
> +.B CLONE_DETACHED
> +flag. This flag is usually ignored when passed along with other flags.
> +However, when passed alongside
> +.B CLONE_PIDFD
> +an error will be returned. This ensures that this flag can be reused
> +for further pidfd features in the future.
> +.TP
>  .BR CLONE_PTRACE " (since Linux 2.2)"
>  If
>  .B CLONE_PTRACE
> @@ -1122,6 +1158,21 @@ For example, on aarch64,
>  .I child_stack
>  must be a multiple of 16.
>  .TP
> +.B EINVAL
> +.B CLONE_PIDFD
> +was specified together with
> +.B CLONE_DETACHED.
> +.TP
> +.B EINVAL
> +.B CLONE_PIDFD
> +was specified together with
> +.B CLONE_PARENT_SETTID.
> +.TP
> +.B EINVAL
> +.B CLONE_PIDFD
> +was specified together with
> +.B CLONE_THREAD.
> +.TP
>  .B ENOMEM
>  Cannot allocate sufficient memory to allocate a task structure for the
>  child, or to copy those parts of the caller's context that need to be

One other piece seems to be missing: the returned file descriptor can
be fed to poll()/select()/epoll and the FD will test as readable when
the child terminates. Right? Did that functionality also land in
kernel 5.2? And did it get implemented as a separate commit, or did
the behavior just fall naturally out of the implementation of pidfd's?
Let me know the details, and I will craft a patch.

Also, as far as I can see (from testing) the FD only gives pollable
events on process termination, not on other process transitions such
as stop and continue. Right? (Are there any plans to implement such
functionality for stop/contine transitions?

By the way, when do you expect the pidfd-wait functionality to land 
in the kernel?

Thanks,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] clone.2: add CLONE_PIDFD entry
  2019-09-11  8:58 ` [PATCH] clone.2: add CLONE_PIDFD entry Michael Kerrisk (man-pages)
@ 2019-09-16  7:40   ` Christian Brauner
  0 siblings, 0 replies; 2+ messages in thread
From: Christian Brauner @ 2019-09-16  7:40 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages); +Cc: linux-man, jannh, oleg

On Wed, Sep 11, 2019 at 10:58:57AM +0200, Michael Kerrisk (man-pages) wrote:
> Hello Christian,
> 
> On 5/11/19 8:49 AM, Christian Brauner wrote:
> > From: Christian Brauner <christian@brauner.io>
> > 
> > Add an entry for CLONE_PIDFD. This flag is available starting with
> > kernel 5.2. If specified, a process file descriptor ("pidfd") referring
> > to the child process will be returned in the ptid argument.
> 
> I've applied this patch in a local branch, and made some minor edits

Thank you! :)

> and added a piece. And I have some questions. See below.
> 
> > Signed-off-by: Christian Brauner <christian@brauner.io>
> > ---
> >  man2/clone.2 | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> > 
> > diff --git a/man2/clone.2 b/man2/clone.2
> > index 7e880beb8..ee08aeb42 100644
> > --- a/man2/clone.2
> > +++ b/man2/clone.2
> > @@ -539,6 +539,42 @@ The flag disappeared completely from the kernel sources in Linux 2.5.16.
> >  Since then, the kernel silently ignores this bit if it is specified in
> >  .IR flags .
> >  .TP
> > +.BR CLONE_PIDFD " (since Linux 5.2)"
> > +If
> > +.B CLONE_PIDFD
> > +is set,
> > +.BR clone ()
> > +stores a process file descriptor ("pidfd") referring to the child process at
> > +the location
> > +.I ptid
> > +in the parent's memory. 
> 
> I added a note that the close-on-exec flag is set on the new FD.

Ack.

> 
> > Note, that the kernel verifies that the value for
> > +.I ptid
> > +is zero. If it is not an error will be returned. This ensures that
> > +.I ptid
> > +can potentially be used to specify additional options for
> > +.B CLONE_PIDFD
> > +in the future.
> 
> This piece is no longer true, right? At least I can't see such 

Correct.

> a check in the kernel code, and my testing doesn't yield an error
> when ptid != 0 before the call.(No need to send me a patch; if I'm
> correct just let me know and I'll edit out this piece.)
> 
> > +.IP
> > +Since the
> > +.I ptid
> > +argument is used to return the pidfd,
> > +.B CLONE_PIDFD
> > +cannot be used with
> > +.B CLONE_PARENT_SETTID.
> > +.IP
> > +It is currently not possible to use this flag together with
> > +.B CLONE_THREAD.
> > +This means that the process identified by the pidfd will always be a
> > +thread-group leader.
> > +.IP
> > +For a while there was a
> > +.B CLONE_DETACHED
> > +flag. This flag is usually ignored when passed along with other flags.
> > +However, when passed alongside
> > +.B CLONE_PIDFD
> > +an error will be returned. This ensures that this flag can be reused
> > +for further pidfd features in the future.
> > +.TP
> >  .BR CLONE_PTRACE " (since Linux 2.2)"
> >  If
> >  .B CLONE_PTRACE
> > @@ -1122,6 +1158,21 @@ For example, on aarch64,
> >  .I child_stack
> >  must be a multiple of 16.
> >  .TP
> > +.B EINVAL
> > +.B CLONE_PIDFD
> > +was specified together with
> > +.B CLONE_DETACHED.
> > +.TP
> > +.B EINVAL
> > +.B CLONE_PIDFD
> > +was specified together with
> > +.B CLONE_PARENT_SETTID.
> > +.TP
> > +.B EINVAL
> > +.B CLONE_PIDFD
> > +was specified together with
> > +.B CLONE_THREAD.
> > +.TP
> >  .B ENOMEM
> >  Cannot allocate sufficient memory to allocate a task structure for the
> >  child, or to copy those parts of the caller's context that need to be
> 
> One other piece seems to be missing: the returned file descriptor can
> be fed to poll()/select()/epoll and the FD will test as readable when
> the child terminates. Right? Did that functionality also land in
> kernel 5.2? And did it get implemented as a separate commit, or did
> the behavior just fall naturally out of the implementation of pidfd's?
> Let me know the details, and I will craft a patch.

It landed in 5.3. The relevant commit is:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b53b0b9d9a613c418057f6cb921c2f40a6f78c24
and belongs to the following merge:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5450e8a316a64cddcbc15f90733ebc78aa736545

> 
> Also, as far as I can see (from testing) the FD only gives pollable
> events on process termination, not on other process transitions such
> as stop and continue. Right? (Are there any plans to implement such

Correct.

> functionality for stop/contine transitions?

Yes, at some point we will likely want this.

> 
> By the way, when do you expect the pidfd-wait functionality to land 
> in the kernel?

I've sent a PR for 5.4:
https://lkml.org/lkml/2019/9/10/682
which contains the P_PIDFD extension to waitid().

Thanks for the work, Michael!
Christian

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20190511064908.21956-1-christian.brauner@ubuntu.com>
2019-09-11  8:58 ` [PATCH] clone.2: add CLONE_PIDFD entry Michael Kerrisk (man-pages)
2019-09-16  7:40   ` Christian Brauner

Linux-man Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-man/0 linux-man/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-man linux-man/ https://lore.kernel.org/linux-man \
		linux-man@vger.kernel.org linux-man@archiver.kernel.org
	public-inbox-index linux-man


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-man


AGPL code for this site: git clone https://public-inbox.org/ public-inbox