Re: [PATCH 2/4] clone: add CLONE_PIDFD

[Oleg Nesterov]

On 04/15, Christian Brauner wrote:
>
> > CLONE_PARENT_SETTID doesn't look very usefule, so what if we add
> >
> > 	if ((clone_flags & (CLONE_PIDFD|CLONE_PARENT_SETTID)) ==
> > 	                   (CLONE_PIDFD|CLONE_PARENT_SETTID))
> > 		return ERR_PTR(-EINVAL);
> >
> > at the start of copy_process() ?
> >
> > Then it can do
> >
> > 	if (clone_flags & CLONE_PIDFD) {
> > 		retval = pidfd_create(pid, &pidfdf);
> > 		if (retval < 0)
> > 			goto bad_fork_free_pid;
> > 		retval = put_user(retval, parent_tidptr)
> > 		if (retval < 0)
> > 			goto bad_fork_free_pid;
> > 	}
>
> Uhhh Oleg, that is nifty. I have to say I like that a lot. This would
> let us return the pid and the pidfd in one go and we can also start
> pidfd numbering at 0.

Christian, sorry if it was already discussed, but I can't force myself to
read all the previous discussions ;)

If we forget about CONFIG_PROC_FS, why do we really want to create a file?


Suppose we add a global u64 counter incremented by copy_process and reported
in /proc/$pid/status. Suppose that clone(CLONE_PIDFD) writes this counter to
*parent_tidptr. Let's denote this counter as UNIQ_PID.

Now, if you want to (say) safely kill a task and you have its UNIQ_PID, you
can do

	kill_by_pid_uniq(int pid, u64 uniq_pid)
	{
		pidfd = open("/proc/$pid", O_DIRECTORY);

		status = openat(pidfd, "status");
		u64 this_uniq_pid = ... read UNIQ_PID from status ...;

		if (uniq_pid != this_uniq_pid)
			return;

		pidfd_send_signal(pidfd);
	}

Why else do we want pidfd?

Oleg.