From: NeilBrown The mnt_want_write() call and the parent inode locking in open_last_lookups() are only needed for lookup_open(). So we can move them and all the got_write handling into lookup_open(). Note that we need to also check create_error when determining whether to unlock shared or not, as O_CREAT can be cleared, but create_error is only set of O_CREAT was set. The fsnotify calls come too as they must be in the locked region. Also use the existing dir_inode uniformly for dir->d_inode. This is a step towards exporting an better "open/create" interface to nfsd. Reviewed-by: Jan Kara Reviewed-by: Jori Koolstra Signed-off-by: NeilBrown --- fs/namei.c | 77 +++++++++++++++++++++++++++++------------------------- 1 file changed, 41 insertions(+), 36 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index 7ead3db55be2..55404fe0e269 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -4398,7 +4398,7 @@ static struct dentry *atomic_open(const struct path *path, struct dentry *dentry */ static struct dentry *lookup_open(struct nameidata *nd, struct file *file, const struct open_flags *op, - bool got_write, struct delegated_inode *delegated_inode) + struct delegated_inode *delegated_inode) { struct mnt_idmap *idmap; struct dentry *dir = nd->path.dentry; @@ -4407,9 +4407,25 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file, struct dentry *dentry; int error, create_error = 0; umode_t mode = op->mode; + bool got_write = false; - if (unlikely(IS_DEADDIR(dir_inode))) - return ERR_PTR(-ENOENT); + if (open_flag & (O_CREAT | O_TRUNC | O_WRONLY | O_RDWR)) { + got_write = !mnt_want_write(nd->path.mnt); + /* + * do _not_ fail yet - we might not need that or fail with + * a different error; let lookup_open() decide; we'll be + * dropping this one anyway. + */ + } + if (open_flag & O_CREAT) + inode_lock(dir_inode); + else + inode_lock_shared(dir_inode); + + if (unlikely(IS_DEADDIR(dir_inode))) { + dentry = ERR_PTR(-ENOENT); + goto out; + } file->f_mode &= ~FMODE_CREATED; dentry = d_lookup(dir, &nd->last); @@ -4417,7 +4433,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file, if (!dentry) { dentry = d_alloc_parallel(dir, &nd->last); if (IS_ERR(dentry)) - return dentry; + goto out; } if (d_in_lookup(dentry)) break; @@ -4433,7 +4449,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file, } if (dentry->d_inode) { /* Cached positive dentry: will open in f_op->open */ - return dentry; + goto out; } if (open_flag & O_CREAT) @@ -4454,7 +4470,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file, if (open_flag & O_CREAT) { if (open_flag & O_EXCL) open_flag &= ~O_TRUNC; - mode = vfs_prepare_mode(idmap, dir->d_inode, mode, mode, mode); + mode = vfs_prepare_mode(idmap, dir_inode, mode, mode, mode); if (likely(got_write)) create_error = may_o_create(idmap, &nd->path, dentry, mode); @@ -4469,7 +4485,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file, dentry = atomic_open(&nd->path, dentry, file, open_flag, mode); if (unlikely(create_error) && dentry == ERR_PTR(-ENOENT)) dentry = ERR_PTR(create_error); - return dentry; + goto out; } if (d_in_lookup(dentry)) { @@ -4508,11 +4524,27 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file, error = create_error; goto out_dput; } +out: + if (!IS_ERR(dentry)) { + if (file->f_mode & FMODE_CREATED) + fsnotify_create(dir_inode, dentry); + if (file->f_mode & FMODE_OPENED) + fsnotify_open(file); + } + if ((open_flag & O_CREAT) || create_error) + inode_unlock(dir_inode); + else + inode_unlock_shared(dir_inode); + + if (got_write) + mnt_drop_write(nd->path.mnt); + return dentry; out_dput: dput(dentry); - return ERR_PTR(error); + dentry = ERR_PTR(error); + goto out; } static inline bool trailing_slashes(struct nameidata *nd) @@ -4555,9 +4587,7 @@ static const char *open_last_lookups(struct nameidata *nd, struct file *file, const struct open_flags *op) { struct delegated_inode delegated_inode = { }; - struct dentry *dir = nd->path.dentry; int open_flag = op->open_flag; - bool got_write = false; struct dentry *dentry; const char *res; @@ -4587,32 +4617,7 @@ static const char *open_last_lookups(struct nameidata *nd, } } retry: - if (open_flag & (O_CREAT | O_TRUNC | O_WRONLY | O_RDWR)) { - got_write = !mnt_want_write(nd->path.mnt); - /* - * do _not_ fail yet - we might not need that or fail with - * a different error; let lookup_open() decide; we'll be - * dropping this one anyway. - */ - } - if (open_flag & O_CREAT) - inode_lock(dir->d_inode); - else - inode_lock_shared(dir->d_inode); - dentry = lookup_open(nd, file, op, got_write, &delegated_inode); - if (!IS_ERR(dentry)) { - if (file->f_mode & FMODE_CREATED) - fsnotify_create(dir->d_inode, dentry); - if (file->f_mode & FMODE_OPENED) - fsnotify_open(file); - } - if (open_flag & O_CREAT) - inode_unlock(dir->d_inode); - else - inode_unlock_shared(dir->d_inode); - - if (got_write) - mnt_drop_write(nd->path.mnt); + dentry = lookup_open(nd, file, op, &delegated_inode); if (IS_ERR(dentry)) { if (is_delegated(&delegated_inode)) { -- 2.50.0.107.gf914562f5916.dirty