Re: For review: pidfd_open(2) manual page

Hello Christian,

On 9/23/19 4:38 PM, Christian Brauner wrote:

On Mon, Sep 23, 2019 at 11:11:53AM +0200, Michael Kerrisk (man-pages) wrote:

quoted

Hello Christian and all,

Below, I have the rendered version of the current draft of
the pidfd_open(2) manual page that I have written.
The page source can be found in a Git branch at:
https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/log/?h=draft_pidfd

I would be pleased to receive corrections and notes on any
details that should be added. (For example, are there error
cases that I have missed?)

Would you be able to review please?

Again, thank you Michael for doing this!

quoted

Thanks,

Michael


NAME
       pidfd_open - obtain a file descriptor that refers to a process

SYNOPSIS
       int pidfd_open(pid_t pid, unsigned int flags);

DESCRIPTION
       The  pidfd_open()  system creates a file descriptor that refers to

s/system/system call/

Fixed.

quoted

       the process whose PID is specified in pid.  The file descriptor is
       returned  as the function result; the close-on-exec flag is set on
       the file descriptor.

       The flags argument is reserved for  future  use;  currently,  this
       argument must be specified as 0.

RETURN VALUE
       On  success,  pidfd_open()  returns a nonnegative file descriptor.
       On success, -1 is returned and errno is set to indicate the  cause

s/On success/On error/g

Fixed.

quoted

       of the error.

ERRORS
       EINVAL flags is not 0.

       EINVAL pid is not valid.

       ESRCH  The process specified by pid does not exist.

VERSIONS
       pidfd_open() first appeared in Linux 5.3.

CONFORMING TO
       pidfd_open() is Linux specific.

NOTES
       Currently, there is no glibc wrapper for this system call; call it
       using syscall(2).

       The pidfd_send_signal(2) system call can be used to send a  signal
       to the process referred to by a PID file descriptor.

       A  PID  file descriptor can be monitored using poll(2), select(2),
       and epoll(7).  When the process that it refers to terminates,  the
       file descriptor indicates as readable.  Note, however, that in the

Not a native English speaker but should this be "indicates it is
readable"?

See my reply to Florian.

quoted

       current implementation, nothing can be read from the file descrip‐
       tor.

       The  pidfd_open()  system call is the preferred way of obtaining a
       PID file descriptor.  The alternative is to obtain a file descrip‐
       tor by opening a /proc/[pid] directory.  However, the latter tech‐
       nique is possible only if the proc(5) file system is mounted; fur‐
       thermore,  the  file  descriptor  obtained in this way is not pol‐
       lable.

I mentioned this already in the CLONE_PIDFD manpage, we should probably
not make a big deal out of this and not mention /proc/<pid> here at all.

The thing is, people *will* learn about these two different types
of FDs, whether we document them or not. So, I think it's better to
be up front about what's available, and make a suitably strong
recommendation about the preferred technique.

Reading between the lines, it sounds like just a couple of releases
after it was implemented, you're saying that implementing
open(/proc/PID) was a mistake?

(Crazy idea, but we could also have a config option that allows you to
turn of proc-pid-dirfds as pidfds if we start to feel really strongly
about this or a sysctl whatever...)

quoted

       See also the discussion of the CLONE_PIDFD flag in clone(2).

EXAMPLE
       The program below opens a PID  file  descriptor  for  the  process
       whose PID is specified as its command-line argument.  It then mon‐
       itors the file descriptor for readability (POLLIN) using  poll(2).

Yeah, maybe say "monitors the file descriptor for process exit indicated
by an EPOLLIN event" or something. Readability might be confusing.

I like that suggestion! I reworded to something close to what you suggest.

quoted

       When  the  process  with  the specified by PID terminates, poll(2)
       returns, and indicates that the file descriptor is readable.

See comment above "readable". (I'm on my phone and I think someone
pointed this out already.)

Actually, I think I can just remove that sentence. It doesn't really
add much.

quoted

   Program source

       #define _GNU_SOURCE
       #include <sys/syscall.h>
       #include <unistd.h>
       #include <poll.h>
       #include <stdlib.h>
       #include <stdio.h>

       #ifndef __NR_pidfd_open
       #define __NR_pidfd_open 434
       #endif

Alpha is special... (and not in a good way).
So you would need to special case Alpha since that's the only arch where
we haven't been able to unify syscall numbering. :D
But it's not super important.

Okay.

I like the program example.

Good.

Thanks for reviewing! I've pushed various changes
to the Git branch at 
https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/log/?h=draft_pidfd


Cheers,

Michael

quoted

       static
       int pidfd_open(pid_t pid, unsigned int flags)
       {
           return syscall(__NR_pidfd_open, pid, flags);
       }

       int
       main(int argc, char *argv[])
       {
           struct pollfd pollfd;
           int pidfd, ready;

           if (argc != 2) {
               fprintf(stderr, "Usage: %s <pid>\n", argv[0]);
               exit(EXIT_SUCCESS);
           }

           pidfd = pidfd_open(atoi(argv[1]), 0);
           if (pidfd == -1) {
               perror("pidfd_open");
               exit(EXIT_FAILURE);
           }

           pollfd.fd = pidfd;
           pollfd.events = POLLIN;

           ready = poll(&pollfd, 1, -1);
           if (ready == -1) {
               perror("poll");
               exit(EXIT_FAILURE);
           }

           printf("Events (0x%x): POLLIN is %sset\n", pollfd.revents,
                   (pollfd.revents & POLLIN) ? "" : "not ");

           exit(EXIT_SUCCESS);
       }

SEE ALSO
       clone(2),  kill(2),  pidfd_send_signal(2),   poll(2),   select(2),
       epoll(7)


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

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