On Thu, Aug 22, 2024 at 01:41:49AM +0100, Al Viro wrote: > Once we'd opened the file, nd->path and file->f_path have the > same contents. Rather than having both pinned and nd->path > dropped by terminate_walk(), let's have them share the > references from the moment when FMODE_OPENED is set and > clear nd->path just before the terminate_walk() in such case. > > To do that, we > * add a variant of vfs_open() that does *not* do conditional > path_get() (vfs_open_borrow()); use it in do_open(). > * don't grab f->f_path.mnt in finish_open() - only > f->f_path.dentry. Have atomic_open() drop the child dentry > in FMODE_OPENED case and return f->path.dentry without grabbing it. > * adjust vfs_tmpfile() for finish_open() change (it > is called from ->tmpfile() instances). > * make do_o_path() use vfs_open_borrow(), collapse path_put() > there with the conditional path_get() we would've get in vfs_open(). > * in FMODE_OPENED case clear nd->path before calling > terminate_walk(). > > diff --git a/fs/open.c b/fs/open.c > index 0ec2e9a33856..f9988427fb97 100644 > --- a/fs/open.c > +++ b/fs/open.c > @@ -1046,7 +1046,7 @@ int finish_open(struct file *file, struct dentry *dentry, > file->f_path.dentry = dentry; > err = do_dentry_open(file, open); > if (file->f_mode & FMODE_OPENED) > - path_get(&file->f_path); > + dget(&file->f_path.dentry); > return err; > } There are numerous consumers of finish_open(), I don't see how they got adjusted to cope with this (or why they would not need adjustment). For example fuse_create_open(). If this is sorted out I would argue it needs to be explained in the commit message. fwiw I don't think patching up the convention of finish_open() is needed for avoiding the extra ref cycle to work.
