Re: [PATCH v3 1/3] pidfd: allow pidfd_open() on non-thread-group leaders
From: Tycho Andersen
Date: Thu Jan 25 2024 - 13:03:59 EST
On Thu, Jan 25, 2024 at 06:51:14PM +0100, Oleg Nesterov wrote:
> > > What if we add the new PIDFD_THREAD flag? With this flag
> > >
> > > - sys_pidfd_open() doesn't require the must be a group leader
> >
> > Yes.
> >
> > >
> > > - pidfd_poll() succeeds when the task passes exit_notify() and
> > > becomes a zombie, even if it is a leader and has other threads.
> >
> > Iiuc, if an existing user creates a pidfd for a thread-group leader and
> > then polls that pidfd they would currently only get notified if the
> > thread-group is empty and the leader has exited.
> >
> > If we now start notifying when the thread-group leader exits but the
> > thread-group isn't empty then this would be a fairly big api change
>
> Hmm... again, this patch doesn't (shouldn't) change the current behavior.
>
> Please note "with this flag" above. If sys_pidfd_open() was called
> without PIDFD_THREAD, then sys_pidfd_open() still requires that the
> target task must be a group leader, and pidfd_poll() won't succeed
> until the leader exits and thread_group_empty() is true.
Thanks for sending your patch, I'll take a look at it (probably
tomorrow at this rate).
One of the things I don't like about PIDFD_THREAD is that it's hard to
tell whether an arbitrary thread is a leader or not. Right now we do
it by parsing /proc/pid/status, which shows all the stuff from
do_task_stat() that we don't care about but which is quite expensive
to compute. (Maybe there's a better way?)
With PIDFD_THREAD we could could do it twice, once with the flag, get
EINVAL, and then do it again. But ideally we wouldn't have to.
Still, if that's the only way that makes sense, that's fine.
Tycho