On Sat, Feb 17, 2024 at 02:59:16PM +0100, Oleg Nesterov wrote:
> On 02/16, Christian Brauner wrote:
> >
> > +struct file *pidfdfs_alloc_file(struct pid *pid, unsigned int flags)
> > +{
> > +
> > + struct inode *inode;
> > + struct file *pidfd_file;
> > +
> > + inode = iget_locked(pidfdfs_sb, pid->ino);
> > + if (!inode)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + if (inode->i_state & I_NEW) {
> > + inode->i_ino = pid->ino;
>
> I guess this is unnecessary, iget_locked() should initialize i_ino if I_NEW ?
Yes, it does. I just like to be explicit in such cases.
>
> But I have a really stupid (I know nothing about vfs) question, why do we
> need pidfdfs_ino and pid->ino ? Can you explain why pidfdfs_alloc_file()
> can't simply use, say, iget_locked(pidfdfs_sb, (unsigned long)pid) ?
>
> IIUC, if this pid is freed and then another "struct pid" has the same address
> we can rely on __wait_on_freeing_inode() ?
Yeah, I had thought about something like this but see Linus' reply.
