From: Al Viro <[email protected]> On Behalf Of Al Viro Sent: Friday, August 28, 2020 6:56 PM > On Fri, Aug 28, 2020 at 07:39:32AM -0700, Konstantin Komarov wrote: > > > +static int ntfs_atomic_open(struct inode *dir, struct dentry *dentry, > > + struct file *file, u32 flags, umode_t mode) > > +{ [] > > + if (d_really_is_positive(dentry)) { > > + if (file->f_mode & FMODE_OPENED) { > > How do we get FMODE_OPENED here? > > > + dput(d); > > + err = 0; > > + } else > > + err = finish_no_open(file, d); > > + goto out1; > > + } > > + WARN_ON(d); > > + } > > + > > + if (!(flags & O_CREAT)) { > > + err = -ENOENT; > > + goto out1; > > + } > > Just return finish_no_open() in that case. And let the caller handle > that. > > > + err = ntfs_create_inode(dir, dentry, file, mode, 0, NULL, 0, excl, fnd, > > + &inode); > > + > > +out1: > > + fnd_put(fnd); > > +out: > > + ni_unlock(ni); > > + > > + return err; > > +} > > BTW, what's the point of that ni_lock() here? d_in_lookup() is stable > regardless of that and any attempts to create something in the parent > are serialized by ->i_rwsem. If you want it around the actual file > creation, why not take it just there, and replace the open-coded > ntfs_lookup() with the call of the real thing? As in > if (d_in_lookup(dentry)) { > d = ntfs_lookup(....); > if (IS_ERR(d)) > return d; > if (d) > dentry = d; > } > if (!(flags & O_CREAT) || d_really_is_positive(dentry)) > return finish_no_open(file, d); > /* deal with creation of file */ > ni_lock(...); > ....
Thanks for the feedback! We refactored the atomic_open() based on your concerns. Will be published in v4 patch today.
