Re: [PATCH v2] pidfs: ensure that PIDFS_INFO_EXIT is available

[Christian Brauner <brauner@kernel.org>]

On Tue, Mar 18, 2025 at 03:46:54PM +0100, Oleg Nesterov wrote:
> I'll try to actually read this patch (and pidfs: improve multi-threaded
> exec and premature thread-group leader exit polling) tomorrow, but I am
> a bit confused after the quick glance...
> 
> 
> On 03/18, Christian Brauner wrote:
> >
> > +static inline bool pidfs_pid_valid(struct pid *pid, const struct path *path,
> > +				   unsigned int flags)
> > +{
> > +	enum pid_type type;
> > +
> > +	if (flags & CLONE_PIDFD)
> > +		return true;
> 
> OK, this is clear.
> 
> > +	if (flags & PIDFD_THREAD)
> > +		type = PIDTYPE_PID;
> > +	else
> > +		type = PIDTYPE_TGID;
> > +
> > +	/*
> > +	 * Since pidfs_exit() is called before struct pid's task linkage
> > +	 * is removed the case where the task got reaped but a dentry
> > +	 * was already attached to struct pid and exit information was
> > +	 * recorded and published can be handled correctly.
> > +	 */
> > +	if (unlikely(!pid_has_task(pid, type))) {
> > +		struct inode *inode = d_inode(path->dentry);
> > +		return !!READ_ONCE(pidfs_i(inode)->exit_info);
> > +	}
> 
> Why pidfs_pid_valid() can't simply return false if !pid_has_task(pid,type) ?
> 
> pidfd_open() paths check pid_has_task() too and fail if it returns NULL.
> If this task is already reaped when pidfs_pid_valid() is called, we can
> pretend it was reaped before sys_pidfd_open() was called?

We could for sure but why would we. If we know that exit information is
available then returning a pidfd can still be valuable for userspace as
they can retrieve exit information via PIDFD_INFO_EXIT and it really
doesn't hurt to do this.