* (no subject)
@ 2025-09-22 4:29 NeilBrown
2025-09-22 4:29 ` [PATCH v4 1/6] VFS/ovl: add lookup_one_positive_killable() NeilBrown
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: NeilBrown @ 2025-09-22 4:29 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Amir Goldstein, Jeff Layton
Cc: Jan Kara, linux-fsdevel
This is a re-re-revised selection of cleanups and API renaming which continues
my work to centralise locking of create/remove/rename operations.
Since v3 I've fixed a bug in 5/6 were I was using path-> where
I should have been using parent_path.
Thanks,
NeilBrown
[PATCH v4 1/6] VFS/ovl: add lookup_one_positive_killable()
[PATCH v4 2/6] VFS: discard err2 in filename_create()
[PATCH v4 3/6] VFS: unify old_mnt_idmap and new_mnt_idmap in
[PATCH v4 4/6] VFS/audit: introduce kern_path_parent() for audit
[PATCH v4 5/6] VFS: rename kern_path_locked() and related functions.
[PATCH v4 6/6] debugfs: rename start_creating() to
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v4 1/6] VFS/ovl: add lookup_one_positive_killable() 2025-09-22 4:29 NeilBrown @ 2025-09-22 4:29 ` NeilBrown 2025-09-23 10:39 ` Christian Brauner 2025-09-22 4:29 ` [PATCH v4 2/6] VFS: discard err2 in filename_create() NeilBrown ` (4 subsequent siblings) 5 siblings, 1 reply; 12+ messages in thread From: NeilBrown @ 2025-09-22 4:29 UTC (permalink / raw) To: Alexander Viro, Christian Brauner, Amir Goldstein, Jeff Layton Cc: Jan Kara, linux-fsdevel From: NeilBrown <neil@brown.name> ovl wants a lookup which won't block on a fatal signal. It currently uses down_write_killable() and then repeatedly calls to lookup_one() The lock may not be needed if the name is already in the dcache and it aids proposed future changes if the locking is kept internal to namei.c So this patch adds lookup_one_positive_killable() which is like lookup_one_positive() but will abort in the face of a fatal signal. overlayfs is changed to use this. Note that instead of always getting an exclusive lock, ovl now only gets a shared lock, and only sometimes. The exclusive lock was never needed. However down_read_killable() was only added in v4.15 but overlayfs started using down_write_killable() here in v4.7. Note that the linked list ->first_maybe_whiteout ->next_maybe_white is local to the thread so there is no concurrency in that list which could be threatened by removing the locking. Reviewed-by: Amir Goldstein <amir73il@gmail.com> Signed-off-by: NeilBrown <neil@brown.name> --- fs/namei.c | 55 ++++++++++++++++++++++++++++++++++++++++++ fs/overlayfs/readdir.c | 28 ++++++++++----------- include/linux/namei.h | 3 +++ 3 files changed, 72 insertions(+), 14 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index cd43ff89fbaa..c7c6b255db2c 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -1827,6 +1827,20 @@ static struct dentry *lookup_slow(const struct qstr *name, return res; } +static struct dentry *lookup_slow_killable(const struct qstr *name, + struct dentry *dir, + unsigned int flags) +{ + struct inode *inode = dir->d_inode; + struct dentry *res; + + if (inode_lock_shared_killable(inode)) + return ERR_PTR(-EINTR); + res = __lookup_slow(name, dir, flags); + inode_unlock_shared(inode); + return res; +} + static inline int may_lookup(struct mnt_idmap *idmap, struct nameidata *restrict nd) { @@ -3010,6 +3024,47 @@ struct dentry *lookup_one_unlocked(struct mnt_idmap *idmap, struct qstr *name, } EXPORT_SYMBOL(lookup_one_unlocked); +/** + * lookup_one_positive_killable - lookup single pathname component + * @idmap: idmap of the mount the lookup is performed from + * @name: qstr olding pathname component to lookup + * @base: base directory to lookup from + * + * This helper will yield ERR_PTR(-ENOENT) on negatives. The helper returns + * known positive or ERR_PTR(). This is what most of the users want. + * + * Note that pinned negative with unlocked parent _can_ become positive at any + * time, so callers of lookup_one_unlocked() need to be very careful; pinned + * positives have >d_inode stable, so this one avoids such problems. + * + * This can be used for in-kernel filesystem clients such as file servers. + * + * It should be called without the parent i_rwsem held, and will take + * the i_rwsem itself if necessary. If a fatal signal is pending or + * delivered, it will return %-EINTR if the lock is needed. + */ +struct dentry *lookup_one_positive_killable(struct mnt_idmap *idmap, + struct qstr *name, + struct dentry *base) +{ + int err; + struct dentry *ret; + + err = lookup_one_common(idmap, name, base); + if (err) + return ERR_PTR(err); + + ret = lookup_dcache(name, base, 0); + if (!ret) + ret = lookup_slow_killable(name, base, 0); + if (!IS_ERR(ret) && d_flags_negative(smp_load_acquire(&ret->d_flags))) { + dput(ret); + ret = ERR_PTR(-ENOENT); + } + return ret; +} +EXPORT_SYMBOL(lookup_one_positive_killable); + /** * lookup_one_positive_unlocked - lookup single pathname component * @idmap: idmap of the mount the lookup is performed from diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index b65cdfce31ce..15cb06fa0c9a 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -270,26 +270,26 @@ static bool ovl_fill_merge(struct dir_context *ctx, const char *name, static int ovl_check_whiteouts(const struct path *path, struct ovl_readdir_data *rdd) { - int err; + int err = 0; struct dentry *dentry, *dir = path->dentry; const struct cred *old_cred; old_cred = ovl_override_creds(rdd->dentry->d_sb); - err = down_write_killable(&dir->d_inode->i_rwsem); - if (!err) { - while (rdd->first_maybe_whiteout) { - struct ovl_cache_entry *p = - rdd->first_maybe_whiteout; - rdd->first_maybe_whiteout = p->next_maybe_whiteout; - dentry = lookup_one(mnt_idmap(path->mnt), - &QSTR_LEN(p->name, p->len), dir); - if (!IS_ERR(dentry)) { - p->is_whiteout = ovl_is_whiteout(dentry); - dput(dentry); - } + while (rdd->first_maybe_whiteout) { + struct ovl_cache_entry *p = + rdd->first_maybe_whiteout; + rdd->first_maybe_whiteout = p->next_maybe_whiteout; + dentry = lookup_one_positive_killable(mnt_idmap(path->mnt), + &QSTR_LEN(p->name, p->len), + dir); + if (!IS_ERR(dentry)) { + p->is_whiteout = ovl_is_whiteout(dentry); + dput(dentry); + } else if (PTR_ERR(dentry) == -EINTR) { + err = -EINTR; + break; } - inode_unlock(dir->d_inode); } ovl_revert_creds(old_cred); diff --git a/include/linux/namei.h b/include/linux/namei.h index 5d085428e471..551a1a01e5e7 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -80,6 +80,9 @@ struct dentry *lookup_one_unlocked(struct mnt_idmap *idmap, struct dentry *lookup_one_positive_unlocked(struct mnt_idmap *idmap, struct qstr *name, struct dentry *base); +struct dentry *lookup_one_positive_killable(struct mnt_idmap *idmap, + struct qstr *name, + struct dentry *base); extern int follow_down_one(struct path *); extern int follow_down(struct path *path, unsigned int flags); -- 2.50.0.107.gf914562f5916.dirty ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/6] VFS/ovl: add lookup_one_positive_killable() 2025-09-22 4:29 ` [PATCH v4 1/6] VFS/ovl: add lookup_one_positive_killable() NeilBrown @ 2025-09-23 10:39 ` Christian Brauner 0 siblings, 0 replies; 12+ messages in thread From: Christian Brauner @ 2025-09-23 10:39 UTC (permalink / raw) To: NeilBrown Cc: Christian Brauner, Jan Kara, linux-fsdevel, Alexander Viro, Amir Goldstein, Jeff Layton On Mon, 22 Sep 2025 14:29:48 +1000, NeilBrown wrote: > ovl wants a lookup which won't block on a fatal signal. It currently > uses down_write_killable() and then repeatedly calls to lookup_one() > > The lock may not be needed if the name is already in the dcache and it > aids proposed future changes if the locking is kept internal to namei.c > > So this patch adds lookup_one_positive_killable() which is like > lookup_one_positive() but will abort in the face of a fatal signal. > overlayfs is changed to use this. > > [...] Applied to the vfs-6.18.async branch of the vfs/vfs.git tree. Patches in the vfs-6.18.async branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs-6.18.async [1/6] VFS/ovl: add lookup_one_positive_killable() https://git.kernel.org/vfs/vfs/c/17eb98d6b517 [2/6] VFS: discard err2 in filename_create() https://git.kernel.org/vfs/vfs/c/e66ccd30dcdc [3/6] VFS: unify old_mnt_idmap and new_mnt_idmap in renamedata https://git.kernel.org/vfs/vfs/c/d7fb2c410240 [4/6] VFS/audit: introduce kern_path_parent() for audit https://git.kernel.org/vfs/vfs/c/76a53de6f7ff [5/6] VFS: rename kern_path_locked() and related functions. https://git.kernel.org/vfs/vfs/c/3d18f80ce181 [6/6] debugfs: rename start_creating() to debugfs_start_creating() https://git.kernel.org/vfs/vfs/c/0a2c70594704 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 2/6] VFS: discard err2 in filename_create() 2025-09-22 4:29 NeilBrown 2025-09-22 4:29 ` [PATCH v4 1/6] VFS/ovl: add lookup_one_positive_killable() NeilBrown @ 2025-09-22 4:29 ` NeilBrown 2025-09-22 4:29 ` [PATCH v4 3/6] VFS: unify old_mnt_idmap and new_mnt_idmap in renamedata NeilBrown ` (3 subsequent siblings) 5 siblings, 0 replies; 12+ messages in thread From: NeilBrown @ 2025-09-22 4:29 UTC (permalink / raw) To: Alexander Viro, Christian Brauner, Amir Goldstein, Jeff Layton Cc: Jan Kara, linux-fsdevel From: NeilBrown <neil@brown.name> Since 204a575e91f3 "VFS: add common error checks to lookup_one_qstr_excl()" filename_create() does not need to stash the error value from mnt_want_write() into a separate variable - the logic that used to clobber 'error' after the call of mnt_want_write() has migrated into lookup_one_qstr_excl(). So there is no need for two different err variables. This patch discards "err2" and uses "error' throughout. Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: NeilBrown <neil@brown.name> --- fs/namei.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index c7c6b255db2c..e2c2ab286bc0 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -4169,7 +4169,6 @@ static struct dentry *filename_create(int dfd, struct filename *name, unsigned int reval_flag = lookup_flags & LOOKUP_REVAL; unsigned int create_flags = LOOKUP_CREATE | LOOKUP_EXCL; int type; - int err2; int error; error = filename_parentat(dfd, name, reval_flag, path, &last, &type); @@ -4184,7 +4183,7 @@ static struct dentry *filename_create(int dfd, struct filename *name, goto out; /* don't fail immediately if it's r/o, at least try to report other errors */ - err2 = mnt_want_write(path->mnt); + error = mnt_want_write(path->mnt); /* * Do the final lookup. Suppress 'create' if there is a trailing * '/', and a directory wasn't requested. @@ -4197,17 +4196,16 @@ static struct dentry *filename_create(int dfd, struct filename *name, if (IS_ERR(dentry)) goto unlock; - if (unlikely(err2)) { - error = err2; + if (unlikely(error)) goto fail; - } + return dentry; fail: dput(dentry); dentry = ERR_PTR(error); unlock: inode_unlock(path->dentry->d_inode); - if (!err2) + if (!error) mnt_drop_write(path->mnt); out: path_put(path); -- 2.50.0.107.gf914562f5916.dirty ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 3/6] VFS: unify old_mnt_idmap and new_mnt_idmap in renamedata 2025-09-22 4:29 NeilBrown 2025-09-22 4:29 ` [PATCH v4 1/6] VFS/ovl: add lookup_one_positive_killable() NeilBrown 2025-09-22 4:29 ` [PATCH v4 2/6] VFS: discard err2 in filename_create() NeilBrown @ 2025-09-22 4:29 ` NeilBrown 2025-09-22 4:29 ` [PATCH v4 4/6] VFS/audit: introduce kern_path_parent() for audit NeilBrown ` (2 subsequent siblings) 5 siblings, 0 replies; 12+ messages in thread From: NeilBrown @ 2025-09-22 4:29 UTC (permalink / raw) To: Alexander Viro, Christian Brauner, Amir Goldstein, Jeff Layton Cc: Jan Kara, linux-fsdevel From: NeilBrown <neil@brown.name> A rename operation can only rename within a single mount. Callers of vfs_rename() must and do ensure this is the case. So there is no point in having two mnt_idmaps in renamedata as they are always the same. Only one of them is passed to ->rename in any case. This patch replaces both with a single "mnt_idmap" and changes all callers. Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: NeilBrown <neil@brown.name> --- fs/cachefiles/namei.c | 3 +-- fs/ecryptfs/inode.c | 3 +-- fs/namei.c | 17 ++++++++--------- fs/nfsd/vfs.c | 3 +-- fs/overlayfs/overlayfs.h | 3 +-- fs/smb/server/vfs.c | 3 +-- include/linux/fs.h | 6 ++---- 7 files changed, 15 insertions(+), 23 deletions(-) diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c index 91dfd0231877..d1edb2ac3837 100644 --- a/fs/cachefiles/namei.c +++ b/fs/cachefiles/namei.c @@ -387,10 +387,9 @@ int cachefiles_bury_object(struct cachefiles_cache *cache, cachefiles_io_error(cache, "Rename security error %d", ret); } else { struct renamedata rd = { - .old_mnt_idmap = &nop_mnt_idmap, + .mnt_idmap = &nop_mnt_idmap, .old_parent = dir, .old_dentry = rep, - .new_mnt_idmap = &nop_mnt_idmap, .new_parent = cache->graveyard, .new_dentry = grave, }; diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index 72fbe1316ab8..abd954c6a14e 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -634,10 +634,9 @@ ecryptfs_rename(struct mnt_idmap *idmap, struct inode *old_dir, goto out_lock; } - rd.old_mnt_idmap = &nop_mnt_idmap; + rd.mnt_idmap = &nop_mnt_idmap; rd.old_parent = lower_old_dir_dentry; rd.old_dentry = lower_old_dentry; - rd.new_mnt_idmap = &nop_mnt_idmap; rd.new_parent = lower_new_dir_dentry; rd.new_dentry = lower_new_dentry; rc = vfs_rename(&rd); diff --git a/fs/namei.c b/fs/namei.c index e2c2ab286bc0..180037b96956 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -5077,20 +5077,20 @@ int vfs_rename(struct renamedata *rd) if (source == target) return 0; - error = may_delete(rd->old_mnt_idmap, old_dir, old_dentry, is_dir); + error = may_delete(rd->mnt_idmap, old_dir, old_dentry, is_dir); if (error) return error; if (!target) { - error = may_create(rd->new_mnt_idmap, new_dir, new_dentry); + error = may_create(rd->mnt_idmap, new_dir, new_dentry); } else { new_is_dir = d_is_dir(new_dentry); if (!(flags & RENAME_EXCHANGE)) - error = may_delete(rd->new_mnt_idmap, new_dir, + error = may_delete(rd->mnt_idmap, new_dir, new_dentry, is_dir); else - error = may_delete(rd->new_mnt_idmap, new_dir, + error = may_delete(rd->mnt_idmap, new_dir, new_dentry, new_is_dir); } if (error) @@ -5105,13 +5105,13 @@ int vfs_rename(struct renamedata *rd) */ if (new_dir != old_dir) { if (is_dir) { - error = inode_permission(rd->old_mnt_idmap, source, + error = inode_permission(rd->mnt_idmap, source, MAY_WRITE); if (error) return error; } if ((flags & RENAME_EXCHANGE) && new_is_dir) { - error = inode_permission(rd->new_mnt_idmap, target, + error = inode_permission(rd->mnt_idmap, target, MAY_WRITE); if (error) return error; @@ -5179,7 +5179,7 @@ int vfs_rename(struct renamedata *rd) if (error) goto out; } - error = old_dir->i_op->rename(rd->new_mnt_idmap, old_dir, old_dentry, + error = old_dir->i_op->rename(rd->mnt_idmap, old_dir, old_dentry, new_dir, new_dentry, flags); if (error) goto out; @@ -5322,10 +5322,9 @@ int do_renameat2(int olddfd, struct filename *from, int newdfd, rd.old_parent = old_path.dentry; rd.old_dentry = old_dentry; - rd.old_mnt_idmap = mnt_idmap(old_path.mnt); + rd.mnt_idmap = mnt_idmap(old_path.mnt); rd.new_parent = new_path.dentry; rd.new_dentry = new_dentry; - rd.new_mnt_idmap = mnt_idmap(new_path.mnt); rd.delegated_inode = &delegated_inode; rd.flags = flags; error = vfs_rename(&rd); diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index edf050766e57..aa4a95713a48 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -1951,10 +1951,9 @@ nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen, goto out_dput_old; } else { struct renamedata rd = { - .old_mnt_idmap = &nop_mnt_idmap, + .mnt_idmap = &nop_mnt_idmap, .old_parent = fdentry, .old_dentry = odentry, - .new_mnt_idmap = &nop_mnt_idmap, .new_parent = tdentry, .new_dentry = ndentry, }; diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index bb0d7ded8e76..4f84abaa0d68 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -361,10 +361,9 @@ static inline int ovl_do_rename(struct ovl_fs *ofs, struct dentry *olddir, { int err; struct renamedata rd = { - .old_mnt_idmap = ovl_upper_mnt_idmap(ofs), + .mnt_idmap = ovl_upper_mnt_idmap(ofs), .old_parent = olddir, .old_dentry = olddentry, - .new_mnt_idmap = ovl_upper_mnt_idmap(ofs), .new_parent = newdir, .new_dentry = newdentry, .flags = flags, diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c index 04539037108c..07739055ac9f 100644 --- a/fs/smb/server/vfs.c +++ b/fs/smb/server/vfs.c @@ -770,10 +770,9 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, const struct path *old_path, goto out4; } - rd.old_mnt_idmap = mnt_idmap(old_path->mnt), + rd.mnt_idmap = mnt_idmap(old_path->mnt), rd.old_parent = old_parent, rd.old_dentry = old_child, - rd.new_mnt_idmap = mnt_idmap(new_path.mnt), rd.new_parent = new_path.dentry, rd.new_dentry = new_dentry, rd.flags = flags, diff --git a/include/linux/fs.h b/include/linux/fs.h index d7ab4f96d705..73b39e5bb9e4 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2008,20 +2008,18 @@ int vfs_unlink(struct mnt_idmap *, struct inode *, struct dentry *, /** * struct renamedata - contains all information required for renaming - * @old_mnt_idmap: idmap of the old mount the inode was found from + * @mnt_idmap: idmap of the mount in which the rename is happening. * @old_parent: parent of source * @old_dentry: source - * @new_mnt_idmap: idmap of the new mount the inode was found from * @new_parent: parent of destination * @new_dentry: destination * @delegated_inode: returns an inode needing a delegation break * @flags: rename flags */ struct renamedata { - struct mnt_idmap *old_mnt_idmap; + struct mnt_idmap *mnt_idmap; struct dentry *old_parent; struct dentry *old_dentry; - struct mnt_idmap *new_mnt_idmap; struct dentry *new_parent; struct dentry *new_dentry; struct inode **delegated_inode; -- 2.50.0.107.gf914562f5916.dirty ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 4/6] VFS/audit: introduce kern_path_parent() for audit 2025-09-22 4:29 NeilBrown ` (2 preceding siblings ...) 2025-09-22 4:29 ` [PATCH v4 3/6] VFS: unify old_mnt_idmap and new_mnt_idmap in renamedata NeilBrown @ 2025-09-22 4:29 ` NeilBrown 2025-09-22 4:29 ` [PATCH v4 5/6] VFS: rename kern_path_locked() and related functions NeilBrown 2025-09-22 4:29 ` [PATCH v4 6/6] debugfs: rename start_creating() to debugfs_start_creating() NeilBrown 5 siblings, 0 replies; 12+ messages in thread From: NeilBrown @ 2025-09-22 4:29 UTC (permalink / raw) To: Alexander Viro, Christian Brauner, Amir Goldstein, Jeff Layton Cc: Jan Kara, linux-fsdevel From: NeilBrown <neil@brown.name> audit_alloc_mark() and audit_get_nd() both need to perform a path lookup getting the parent dentry (which must exist) and the final target (following a LAST_NORM name) which sometimes doesn't need to exist. They don't need the parent to be locked, but use kern_path_locked() or kern_path_locked_negative() anyway. This is somewhat misleading to the casual reader. This patch introduces a more targeted function, kern_path_parent(), which returns not holding locks. On success the "path" will be set to the parent, which must be found, and the return value is the dentry of the target, which might be negative. This will clear the way to rename kern_path_locked() which is otherwise only used to prepare for removing something. It also allows us to remove kern_path_locked_negative(), which is transformed into the new kern_path_parent(). Signed-off-by: NeilBrown <neil@brown.name> --- fs/namei.c | 23 +++++++++++++++++------ include/linux/namei.h | 2 +- kernel/audit_fsnotify.c | 11 ++++++----- kernel/audit_watch.c | 3 +-- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index 180037b96956..4017bc8641d3 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2781,7 +2781,20 @@ static struct dentry *__kern_path_locked(int dfd, struct filename *name, struct return d; } -struct dentry *kern_path_locked_negative(const char *name, struct path *path) +/** + * kern_path_parent: lookup path returning parent and target + * @name: path name + * @path: path to store parent in + * + * The path @name should end with a normal component, not "." or ".." or "/". + * A lookup is performed and if successful the parent information + * is store in @parent and the dentry is returned. + * + * The dentry maybe negative, the parent will be positive. + * + * Returns: dentry or error. + */ +struct dentry *kern_path_parent(const char *name, struct path *path) { struct path parent_path __free(path_put) = {}; struct filename *filename __free(putname) = getname_kernel(name); @@ -2794,12 +2807,10 @@ struct dentry *kern_path_locked_negative(const char *name, struct path *path) return ERR_PTR(error); if (unlikely(type != LAST_NORM)) return ERR_PTR(-EINVAL); - inode_lock_nested(parent_path.dentry->d_inode, I_MUTEX_PARENT); - d = lookup_one_qstr_excl(&last, parent_path.dentry, LOOKUP_CREATE); - if (IS_ERR(d)) { - inode_unlock(parent_path.dentry->d_inode); + + d = lookup_noperm_unlocked(&last, parent_path.dentry); + if (IS_ERR(d)) return d; - } path->dentry = no_free_ptr(parent_path.dentry); path->mnt = no_free_ptr(parent_path.mnt); return d; diff --git a/include/linux/namei.h b/include/linux/namei.h index 551a1a01e5e7..1d5038c21c20 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -57,12 +57,12 @@ struct dentry *lookup_one_qstr_excl(const struct qstr *name, struct dentry *base, unsigned int flags); extern int kern_path(const char *, unsigned, struct path *); +struct dentry *kern_path_parent(const char *name, struct path *parent); extern struct dentry *kern_path_create(int, const char *, struct path *, unsigned int); extern struct dentry *user_path_create(int, const char __user *, struct path *, unsigned int); extern void done_path_create(struct path *, struct dentry *); extern struct dentry *kern_path_locked(const char *, struct path *); -extern struct dentry *kern_path_locked_negative(const char *, struct path *); extern struct dentry *user_path_locked_at(int , const char __user *, struct path *); int vfs_path_parent_lookup(struct filename *filename, unsigned int flags, struct path *parent, struct qstr *last, int *type, diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c index c565fbf66ac8..b92805b317a2 100644 --- a/kernel/audit_fsnotify.c +++ b/kernel/audit_fsnotify.c @@ -76,17 +76,18 @@ struct audit_fsnotify_mark *audit_alloc_mark(struct audit_krule *krule, char *pa struct audit_fsnotify_mark *audit_mark; struct path path; struct dentry *dentry; - struct inode *inode; int ret; if (pathname[0] != '/' || pathname[len-1] == '/') return ERR_PTR(-EINVAL); - dentry = kern_path_locked(pathname, &path); + dentry = kern_path_parent(pathname, &path); if (IS_ERR(dentry)) return ERR_CAST(dentry); /* returning an error */ - inode = path.dentry->d_inode; - inode_unlock(inode); + if (d_really_is_negative(dentry)) { + audit_mark = ERR_PTR(-ENOENT); + goto out; + } audit_mark = kzalloc(sizeof(*audit_mark), GFP_KERNEL); if (unlikely(!audit_mark)) { @@ -100,7 +101,7 @@ struct audit_fsnotify_mark *audit_alloc_mark(struct audit_krule *krule, char *pa audit_update_mark(audit_mark, dentry->d_inode); audit_mark->rule = krule; - ret = fsnotify_add_inode_mark(&audit_mark->mark, inode, 0); + ret = fsnotify_add_inode_mark(&audit_mark->mark, path.dentry->d_inode, 0); if (ret < 0) { audit_mark->path = NULL; fsnotify_put_mark(&audit_mark->mark); diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c index 0ebbbe37a60f..a700e3c8925f 100644 --- a/kernel/audit_watch.c +++ b/kernel/audit_watch.c @@ -349,7 +349,7 @@ static int audit_get_nd(struct audit_watch *watch, struct path *parent) { struct dentry *d; - d = kern_path_locked_negative(watch->path, parent); + d = kern_path_parent(watch->path, parent); if (IS_ERR(d)) return PTR_ERR(d); @@ -359,7 +359,6 @@ static int audit_get_nd(struct audit_watch *watch, struct path *parent) watch->ino = d_backing_inode(d)->i_ino; } - inode_unlock(d_backing_inode(parent->dentry)); dput(d); return 0; } -- 2.50.0.107.gf914562f5916.dirty ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v4 5/6] VFS: rename kern_path_locked() and related functions. 2025-09-22 4:29 NeilBrown ` (3 preceding siblings ...) 2025-09-22 4:29 ` [PATCH v4 4/6] VFS/audit: introduce kern_path_parent() for audit NeilBrown @ 2025-09-22 4:29 ` NeilBrown 2025-09-22 5:21 ` Al Viro 2025-09-22 4:29 ` [PATCH v4 6/6] debugfs: rename start_creating() to debugfs_start_creating() NeilBrown 5 siblings, 1 reply; 12+ messages in thread From: NeilBrown @ 2025-09-22 4:29 UTC (permalink / raw) To: Alexander Viro, Christian Brauner, Amir Goldstein, Jeff Layton Cc: Jan Kara, linux-fsdevel From: NeilBrown <neil@brown.name> kern_path_locked() is now only used to prepare for removing an object from the filesystem (and that is the only credible reason for wanting a positive locked dentry). Thus it corresponds to kern_path_create() and so should have a corresponding name. Unfortunately the name "kern_path_create" is somewhat misleading as it doesn't actually create anything. The recently added simple_start_creating() provides a better pattern I believe. The "start" can be matched with "end" to bracket the creating or removing. So this patch changes names: kern_path_locked -> start_removing_path kern_path_create -> start_creating_path user_path_create -> start_creating_user_path user_path_locked_at -> start_removing_user_path_at done_path_create -> end_creating_path and also introduces end_removing_path() which is identical to end_creating_path(). __start_removing_path (which was __kern_path_locked) is enhanced to call mnt_want_write() for consistency with the start_creating_path(). Reviewed-by: Amir Goldstein <amir73il@gmail.com> Signed-off-by: NeilBrown <neil@brown.name> --- Documentation/filesystems/porting.rst | 12 ++++ arch/powerpc/platforms/cell/spufs/syscalls.c | 4 +- drivers/base/devtmpfs.c | 22 +++----- fs/bcachefs/fs-ioctl.c | 10 ++-- fs/init.c | 17 +++--- fs/namei.c | 59 ++++++++++++-------- fs/ocfs2/refcounttree.c | 4 +- fs/smb/server/vfs.c | 8 +-- include/linux/namei.h | 14 +++-- kernel/bpf/inode.c | 4 +- net/unix/af_unix.c | 6 +- 11 files changed, 93 insertions(+), 67 deletions(-) diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst index 85f590254f07..e0494860be6b 100644 --- a/Documentation/filesystems/porting.rst +++ b/Documentation/filesystems/porting.rst @@ -1285,3 +1285,15 @@ rather than a VMA, as the VMA at this stage is not yet valid. The vm_area_desc provides the minimum required information for a filesystem to initialise state upon memory mapping of a file-backed region, and output parameters for the file system to set this state. + +--- + +**mandatory** + +Several functions are renamed: + +- kern_path_locked -> start_removing_path +- kern_path_create -> start_creating_path +- user_path_create -> start_creating_user_path +- user_path_locked_at -> start_removing_user_path_at +- done_path_create -> end_creating_path diff --git a/arch/powerpc/platforms/cell/spufs/syscalls.c b/arch/powerpc/platforms/cell/spufs/syscalls.c index 157e046e6e93..ea4ba1b6ce6a 100644 --- a/arch/powerpc/platforms/cell/spufs/syscalls.c +++ b/arch/powerpc/platforms/cell/spufs/syscalls.c @@ -67,11 +67,11 @@ static long do_spu_create(const char __user *pathname, unsigned int flags, struct dentry *dentry; int ret; - dentry = user_path_create(AT_FDCWD, pathname, &path, LOOKUP_DIRECTORY); + dentry = start_creating_user_path(AT_FDCWD, pathname, &path, LOOKUP_DIRECTORY); ret = PTR_ERR(dentry); if (!IS_ERR(dentry)) { ret = spufs_create(&path, dentry, flags, mode, neighbor); - done_path_create(&path, dentry); + end_creating_path(&path, dentry); } return ret; diff --git a/drivers/base/devtmpfs.c b/drivers/base/devtmpfs.c index 31bfb3194b4c..9d4e46ad8352 100644 --- a/drivers/base/devtmpfs.c +++ b/drivers/base/devtmpfs.c @@ -176,7 +176,7 @@ static int dev_mkdir(const char *name, umode_t mode) struct dentry *dentry; struct path path; - dentry = kern_path_create(AT_FDCWD, name, &path, LOOKUP_DIRECTORY); + dentry = start_creating_path(AT_FDCWD, name, &path, LOOKUP_DIRECTORY); if (IS_ERR(dentry)) return PTR_ERR(dentry); @@ -184,7 +184,7 @@ static int dev_mkdir(const char *name, umode_t mode) if (!IS_ERR(dentry)) /* mark as kernel-created inode */ d_inode(dentry)->i_private = &thread; - done_path_create(&path, dentry); + end_creating_path(&path, dentry); return PTR_ERR_OR_ZERO(dentry); } @@ -222,10 +222,10 @@ static int handle_create(const char *nodename, umode_t mode, kuid_t uid, struct path path; int err; - dentry = kern_path_create(AT_FDCWD, nodename, &path, 0); + dentry = start_creating_path(AT_FDCWD, nodename, &path, 0); if (dentry == ERR_PTR(-ENOENT)) { create_path(nodename); - dentry = kern_path_create(AT_FDCWD, nodename, &path, 0); + dentry = start_creating_path(AT_FDCWD, nodename, &path, 0); } if (IS_ERR(dentry)) return PTR_ERR(dentry); @@ -246,7 +246,7 @@ static int handle_create(const char *nodename, umode_t mode, kuid_t uid, /* mark as kernel-created inode */ d_inode(dentry)->i_private = &thread; } - done_path_create(&path, dentry); + end_creating_path(&path, dentry); return err; } @@ -256,7 +256,7 @@ static int dev_rmdir(const char *name) struct dentry *dentry; int err; - dentry = kern_path_locked(name, &parent); + dentry = start_removing_path(name, &parent); if (IS_ERR(dentry)) return PTR_ERR(dentry); if (d_inode(dentry)->i_private == &thread) @@ -265,9 +265,7 @@ static int dev_rmdir(const char *name) else err = -EPERM; - dput(dentry); - inode_unlock(d_inode(parent.dentry)); - path_put(&parent); + end_removing_path(&parent, dentry); return err; } @@ -325,7 +323,7 @@ static int handle_remove(const char *nodename, struct device *dev) int deleted = 0; int err = 0; - dentry = kern_path_locked(nodename, &parent); + dentry = start_removing_path(nodename, &parent); if (IS_ERR(dentry)) return PTR_ERR(dentry); @@ -349,10 +347,8 @@ static int handle_remove(const char *nodename, struct device *dev) if (!err || err == -ENOENT) deleted = 1; } - dput(dentry); - inode_unlock(d_inode(parent.dentry)); + end_removing_path(&parent, dentry); - path_put(&parent); if (deleted && strchr(nodename, '/')) delete_path(nodename); return err; diff --git a/fs/bcachefs/fs-ioctl.c b/fs/bcachefs/fs-ioctl.c index 4e72e654da96..43510da5e734 100644 --- a/fs/bcachefs/fs-ioctl.c +++ b/fs/bcachefs/fs-ioctl.c @@ -255,7 +255,7 @@ static long bch2_ioctl_subvolume_create(struct bch_fs *c, struct file *filp, snapshot_src = inode_inum(to_bch_ei(src_path.dentry->d_inode)); } - dst_dentry = user_path_create(arg.dirfd, + dst_dentry = start_creating_user_path(arg.dirfd, (const char __user *)(unsigned long)arg.dst_ptr, &dst_path, lookup_flags); error = PTR_ERR_OR_ZERO(dst_dentry); @@ -314,7 +314,7 @@ static long bch2_ioctl_subvolume_create(struct bch_fs *c, struct file *filp, d_instantiate(dst_dentry, &inode->v); fsnotify_mkdir(dir, dst_dentry); err3: - done_path_create(&dst_path, dst_dentry); + end_creating_path(&dst_path, dst_dentry); err2: if (arg.src_ptr) path_put(&src_path); @@ -334,7 +334,7 @@ static long bch2_ioctl_subvolume_destroy(struct bch_fs *c, struct file *filp, if (arg.flags) return -EINVAL; - victim = user_path_locked_at(arg.dirfd, name, &path); + victim = start_removing_user_path_at(arg.dirfd, name, &path); if (IS_ERR(victim)) return PTR_ERR(victim); @@ -351,9 +351,7 @@ static long bch2_ioctl_subvolume_destroy(struct bch_fs *c, struct file *filp, d_invalidate(victim); } err: - inode_unlock(dir); - dput(victim); - path_put(&path); + end_removing_path(&path, victim); return ret; } diff --git a/fs/init.c b/fs/init.c index eef5124885e3..07f592ccdba8 100644 --- a/fs/init.c +++ b/fs/init.c @@ -149,7 +149,7 @@ int __init init_mknod(const char *filename, umode_t mode, unsigned int dev) else if (!(S_ISBLK(mode) || S_ISCHR(mode))) return -EINVAL; - dentry = kern_path_create(AT_FDCWD, filename, &path, 0); + dentry = start_creating_path(AT_FDCWD, filename, &path, 0); if (IS_ERR(dentry)) return PTR_ERR(dentry); @@ -158,7 +158,7 @@ int __init init_mknod(const char *filename, umode_t mode, unsigned int dev) if (!error) error = vfs_mknod(mnt_idmap(path.mnt), path.dentry->d_inode, dentry, mode, new_decode_dev(dev)); - done_path_create(&path, dentry); + end_creating_path(&path, dentry); return error; } @@ -173,7 +173,7 @@ int __init init_link(const char *oldname, const char *newname) if (error) return error; - new_dentry = kern_path_create(AT_FDCWD, newname, &new_path, 0); + new_dentry = start_creating_path(AT_FDCWD, newname, &new_path, 0); error = PTR_ERR(new_dentry); if (IS_ERR(new_dentry)) goto out; @@ -191,7 +191,7 @@ int __init init_link(const char *oldname, const char *newname) error = vfs_link(old_path.dentry, idmap, new_path.dentry->d_inode, new_dentry, NULL); out_dput: - done_path_create(&new_path, new_dentry); + end_creating_path(&new_path, new_dentry); out: path_put(&old_path); return error; @@ -203,14 +203,14 @@ int __init init_symlink(const char *oldname, const char *newname) struct path path; int error; - dentry = kern_path_create(AT_FDCWD, newname, &path, 0); + dentry = start_creating_path(AT_FDCWD, newname, &path, 0); if (IS_ERR(dentry)) return PTR_ERR(dentry); error = security_path_symlink(&path, dentry, oldname); if (!error) error = vfs_symlink(mnt_idmap(path.mnt), path.dentry->d_inode, dentry, oldname); - done_path_create(&path, dentry); + end_creating_path(&path, dentry); return error; } @@ -225,7 +225,8 @@ int __init init_mkdir(const char *pathname, umode_t mode) struct path path; int error; - dentry = kern_path_create(AT_FDCWD, pathname, &path, LOOKUP_DIRECTORY); + dentry = start_creating_path(AT_FDCWD, pathname, &path, + LOOKUP_DIRECTORY); if (IS_ERR(dentry)) return PTR_ERR(dentry); mode = mode_strip_umask(d_inode(path.dentry), mode); @@ -236,7 +237,7 @@ int __init init_mkdir(const char *pathname, umode_t mode) if (IS_ERR(dentry)) error = PTR_ERR(dentry); } - done_path_create(&path, dentry); + end_creating_path(&path, dentry); return error; } diff --git a/fs/namei.c b/fs/namei.c index 4017bc8641d3..92973a7a8091 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2758,7 +2758,8 @@ static int filename_parentat(int dfd, struct filename *name, } /* does lookup, returns the object with parent locked */ -static struct dentry *__kern_path_locked(int dfd, struct filename *name, struct path *path) +static struct dentry *__start_removing_path(int dfd, struct filename *name, + struct path *path) { struct path parent_path __free(path_put) = {}; struct dentry *d; @@ -2770,15 +2771,26 @@ static struct dentry *__kern_path_locked(int dfd, struct filename *name, struct return ERR_PTR(error); if (unlikely(type != LAST_NORM)) return ERR_PTR(-EINVAL); + /* don't fail immediately if it's r/o, at least try to report other errors */ + error = mnt_want_write(parent_path.mnt); inode_lock_nested(parent_path.dentry->d_inode, I_MUTEX_PARENT); d = lookup_one_qstr_excl(&last, parent_path.dentry, 0); - if (IS_ERR(d)) { - inode_unlock(parent_path.dentry->d_inode); - return d; - } + if (IS_ERR(d)) + goto unlock; + if (error) + goto fail; path->dentry = no_free_ptr(parent_path.dentry); path->mnt = no_free_ptr(parent_path.mnt); return d; + +fail: + dput(d); + d = ERR_PTR(error); +unlock: + inode_unlock(parent_path.dentry->d_inode); + if (!error) + mnt_drop_write(parent_path.mnt); + return d; } /** @@ -2816,24 +2828,26 @@ struct dentry *kern_path_parent(const char *name, struct path *path) return d; } -struct dentry *kern_path_locked(const char *name, struct path *path) +struct dentry *start_removing_path(const char *name, struct path *path) { struct filename *filename = getname_kernel(name); - struct dentry *res = __kern_path_locked(AT_FDCWD, filename, path); + struct dentry *res = __start_removing_path(AT_FDCWD, filename, path); putname(filename); return res; } -struct dentry *user_path_locked_at(int dfd, const char __user *name, struct path *path) +struct dentry *start_removing_user_path_at(int dfd, + const char __user *name, + struct path *path) { struct filename *filename = getname(name); - struct dentry *res = __kern_path_locked(dfd, filename, path); + struct dentry *res = __start_removing_path(dfd, filename, path); putname(filename); return res; } -EXPORT_SYMBOL(user_path_locked_at); +EXPORT_SYMBOL(start_removing_user_path_at); int kern_path(const char *name, unsigned int flags, struct path *path) { @@ -4223,8 +4237,8 @@ static struct dentry *filename_create(int dfd, struct filename *name, return dentry; } -struct dentry *kern_path_create(int dfd, const char *pathname, - struct path *path, unsigned int lookup_flags) +struct dentry *start_creating_path(int dfd, const char *pathname, + struct path *path, unsigned int lookup_flags) { struct filename *filename = getname_kernel(pathname); struct dentry *res = filename_create(dfd, filename, path, lookup_flags); @@ -4232,9 +4246,9 @@ struct dentry *kern_path_create(int dfd, const char *pathname, putname(filename); return res; } -EXPORT_SYMBOL(kern_path_create); +EXPORT_SYMBOL(start_creating_path); -void done_path_create(struct path *path, struct dentry *dentry) +void end_creating_path(struct path *path, struct dentry *dentry) { if (!IS_ERR(dentry)) dput(dentry); @@ -4242,10 +4256,11 @@ void done_path_create(struct path *path, struct dentry *dentry) mnt_drop_write(path->mnt); path_put(path); } -EXPORT_SYMBOL(done_path_create); +EXPORT_SYMBOL(end_creating_path); -inline struct dentry *user_path_create(int dfd, const char __user *pathname, - struct path *path, unsigned int lookup_flags) +inline struct dentry *start_creating_user_path( + int dfd, const char __user *pathname, + struct path *path, unsigned int lookup_flags) { struct filename *filename = getname(pathname); struct dentry *res = filename_create(dfd, filename, path, lookup_flags); @@ -4253,7 +4268,7 @@ inline struct dentry *user_path_create(int dfd, const char __user *pathname, putname(filename); return res; } -EXPORT_SYMBOL(user_path_create); +EXPORT_SYMBOL(start_creating_user_path); /** * vfs_mknod - create device node or file @@ -4361,7 +4376,7 @@ static int do_mknodat(int dfd, struct filename *name, umode_t mode, break; } out2: - done_path_create(&path, dentry); + end_creating_path(&path, dentry); if (retry_estale(error, lookup_flags)) { lookup_flags |= LOOKUP_REVAL; goto retry; @@ -4465,7 +4480,7 @@ int do_mkdirat(int dfd, struct filename *name, umode_t mode) if (IS_ERR(dentry)) error = PTR_ERR(dentry); } - done_path_create(&path, dentry); + end_creating_path(&path, dentry); if (retry_estale(error, lookup_flags)) { lookup_flags |= LOOKUP_REVAL; goto retry; @@ -4819,7 +4834,7 @@ int do_symlinkat(struct filename *from, int newdfd, struct filename *to) if (!error) error = vfs_symlink(mnt_idmap(path.mnt), path.dentry->d_inode, dentry, from->name); - done_path_create(&path, dentry); + end_creating_path(&path, dentry); if (retry_estale(error, lookup_flags)) { lookup_flags |= LOOKUP_REVAL; goto retry; @@ -4988,7 +5003,7 @@ int do_linkat(int olddfd, struct filename *old, int newdfd, error = vfs_link(old_path.dentry, idmap, new_path.dentry->d_inode, new_dentry, &delegated_inode); out_dput: - done_path_create(&new_path, new_dentry); + end_creating_path(&new_path, new_dentry); if (delegated_inode) { error = break_deleg_wait(&delegated_inode); if (!error) { diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c index 8f732742b26e..267b50e8e42e 100644 --- a/fs/ocfs2/refcounttree.c +++ b/fs/ocfs2/refcounttree.c @@ -4418,7 +4418,7 @@ int ocfs2_reflink_ioctl(struct inode *inode, return error; } - new_dentry = user_path_create(AT_FDCWD, newname, &new_path, 0); + new_dentry = start_creating_user_path(AT_FDCWD, newname, &new_path, 0); error = PTR_ERR(new_dentry); if (IS_ERR(new_dentry)) { mlog_errno(error); @@ -4435,7 +4435,7 @@ int ocfs2_reflink_ioctl(struct inode *inode, d_inode(new_path.dentry), new_dentry, preserve); out_dput: - done_path_create(&new_path, new_dentry); + end_creating_path(&new_path, new_dentry); out: path_put(&old_path); diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c index 07739055ac9f..1cfa688904b2 100644 --- a/fs/smb/server/vfs.c +++ b/fs/smb/server/vfs.c @@ -196,7 +196,7 @@ int ksmbd_vfs_create(struct ksmbd_work *work, const char *name, umode_t mode) pr_err("File(%s): creation failed (err:%d)\n", name, err); } - done_path_create(&path, dentry); + end_creating_path(&path, dentry); return err; } @@ -237,7 +237,7 @@ int ksmbd_vfs_mkdir(struct ksmbd_work *work, const char *name, umode_t mode) if (!err && dentry != d) ksmbd_vfs_inherit_owner(work, d_inode(path.dentry), d_inode(dentry)); - done_path_create(&path, dentry); + end_creating_path(&path, dentry); if (err) pr_err("mkdir(%s): creation failed (err:%d)\n", name, err); return err; @@ -669,7 +669,7 @@ int ksmbd_vfs_link(struct ksmbd_work *work, const char *oldname, ksmbd_debug(VFS, "vfs_link failed err %d\n", err); out3: - done_path_create(&newpath, dentry); + end_creating_path(&newpath, dentry); out2: path_put(&oldpath); out1: @@ -1325,7 +1325,7 @@ struct dentry *ksmbd_vfs_kern_path_create(struct ksmbd_work *work, if (!abs_name) return ERR_PTR(-ENOMEM); - dent = kern_path_create(AT_FDCWD, abs_name, path, flags); + dent = start_creating_path(AT_FDCWD, abs_name, path, flags); kfree(abs_name); return dent; } diff --git a/include/linux/namei.h b/include/linux/namei.h index 1d5038c21c20..a7800ef04e76 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -59,11 +59,15 @@ struct dentry *lookup_one_qstr_excl(const struct qstr *name, extern int kern_path(const char *, unsigned, struct path *); struct dentry *kern_path_parent(const char *name, struct path *parent); -extern struct dentry *kern_path_create(int, const char *, struct path *, unsigned int); -extern struct dentry *user_path_create(int, const char __user *, struct path *, unsigned int); -extern void done_path_create(struct path *, struct dentry *); -extern struct dentry *kern_path_locked(const char *, struct path *); -extern struct dentry *user_path_locked_at(int , const char __user *, struct path *); +extern struct dentry *start_creating_path(int, const char *, struct path *, unsigned int); +extern struct dentry *start_creating_user_path(int, const char __user *, struct path *, unsigned int); +extern void end_creating_path(struct path *, struct dentry *); +extern struct dentry *start_removing_path(const char *, struct path *); +extern struct dentry *start_removing_user_path_at(int , const char __user *, struct path *); +static inline void end_removing_path(struct path *path , struct dentry *dentry) +{ + end_creating_path(path, dentry); +} int vfs_path_parent_lookup(struct filename *filename, unsigned int flags, struct path *parent, struct qstr *last, int *type, const struct path *root); diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c index 5c2e96b19392..fadf3817a9c5 100644 --- a/kernel/bpf/inode.c +++ b/kernel/bpf/inode.c @@ -442,7 +442,7 @@ static int bpf_obj_do_pin(int path_fd, const char __user *pathname, void *raw, umode_t mode; int ret; - dentry = user_path_create(path_fd, pathname, &path, 0); + dentry = start_creating_user_path(path_fd, pathname, &path, 0); if (IS_ERR(dentry)) return PTR_ERR(dentry); @@ -471,7 +471,7 @@ static int bpf_obj_do_pin(int path_fd, const char __user *pathname, void *raw, ret = -EPERM; } out: - done_path_create(&path, dentry); + end_creating_path(&path, dentry); return ret; } diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index 6d7c110814ff..768098dec231 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -1387,7 +1387,7 @@ static int unix_bind_bsd(struct sock *sk, struct sockaddr_un *sunaddr, * Get the parent directory, calculate the hash for last * component. */ - dentry = kern_path_create(AT_FDCWD, addr->name->sun_path, &parent, 0); + dentry = start_creating_path(AT_FDCWD, addr->name->sun_path, &parent, 0); if (IS_ERR(dentry)) { err = PTR_ERR(dentry); goto out; @@ -1417,7 +1417,7 @@ static int unix_bind_bsd(struct sock *sk, struct sockaddr_un *sunaddr, unix_table_double_unlock(net, old_hash, new_hash); unix_insert_bsd_socket(sk); mutex_unlock(&u->bindlock); - done_path_create(&parent, dentry); + end_creating_path(&parent, dentry); return 0; out_unlock: @@ -1427,7 +1427,7 @@ static int unix_bind_bsd(struct sock *sk, struct sockaddr_un *sunaddr, /* failed after successful mknod? unlink what we'd created... */ vfs_unlink(idmap, d_inode(parent.dentry), dentry, NULL); out_path: - done_path_create(&parent, dentry); + end_creating_path(&parent, dentry); out: unix_release_addr(addr); return err == -EEXIST ? -EADDRINUSE : err; -- 2.50.0.107.gf914562f5916.dirty ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 5/6] VFS: rename kern_path_locked() and related functions. 2025-09-22 4:29 ` [PATCH v4 5/6] VFS: rename kern_path_locked() and related functions NeilBrown @ 2025-09-22 5:21 ` Al Viro 2025-09-22 7:30 ` NeilBrown 0 siblings, 1 reply; 12+ messages in thread From: Al Viro @ 2025-09-22 5:21 UTC (permalink / raw) To: NeilBrown Cc: Christian Brauner, Amir Goldstein, Jeff Layton, Jan Kara, linux-fsdevel On Mon, Sep 22, 2025 at 02:29:52PM +1000, NeilBrown wrote: > From: NeilBrown <neil@brown.name> > > kern_path_locked() is now only used to prepare for removing an object > from the filesystem (and that is the only credible reason for wanting a > positive locked dentry). Thus it corresponds to kern_path_create() and > so should have a corresponding name. > > Unfortunately the name "kern_path_create" is somewhat misleading as it > doesn't actually create anything. The recently added > simple_start_creating() provides a better pattern I believe. The > "start" can be matched with "end" to bracket the creating or removing. > > So this patch changes names: > > kern_path_locked -> start_removing_path > kern_path_create -> start_creating_path > user_path_create -> start_creating_user_path > user_path_locked_at -> start_removing_user_path_at > done_path_create -> end_creating_path > > and also introduces end_removing_path() which is identical to > end_creating_path(). > > __start_removing_path (which was __kern_path_locked) is enhanced to > call mnt_want_write() for consistency with the start_creating_path(). Documentation/filesystems/porting.rst, please. Either in this commit, or as a followup. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 5/6] VFS: rename kern_path_locked() and related functions. 2025-09-22 5:21 ` Al Viro @ 2025-09-22 7:30 ` NeilBrown 2025-09-22 13:32 ` Al Viro 0 siblings, 1 reply; 12+ messages in thread From: NeilBrown @ 2025-09-22 7:30 UTC (permalink / raw) To: Al Viro Cc: Christian Brauner, Amir Goldstein, Jeff Layton, Jan Kara, linux-fsdevel On Mon, 22 Sep 2025, Al Viro wrote: > On Mon, Sep 22, 2025 at 02:29:52PM +1000, NeilBrown wrote: > > From: NeilBrown <neil@brown.name> > > > > kern_path_locked() is now only used to prepare for removing an object > > from the filesystem (and that is the only credible reason for wanting a > > positive locked dentry). Thus it corresponds to kern_path_create() and > > so should have a corresponding name. > > > > Unfortunately the name "kern_path_create" is somewhat misleading as it > > doesn't actually create anything. The recently added > > simple_start_creating() provides a better pattern I believe. The > > "start" can be matched with "end" to bracket the creating or removing. > > > > So this patch changes names: > > > > kern_path_locked -> start_removing_path > > kern_path_create -> start_creating_path > > user_path_create -> start_creating_user_path > > user_path_locked_at -> start_removing_user_path_at > > done_path_create -> end_creating_path > > > > and also introduces end_removing_path() which is identical to > > end_creating_path(). > > > > __start_removing_path (which was __kern_path_locked) is enhanced to > > call mnt_want_write() for consistency with the start_creating_path(). > > Documentation/filesystems/porting.rst, please. Either in this commit, > or as a followup. > Patch already contains: diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst index 85f590254f07..e0494860be6b 100644 --- a/Documentation/filesystems/porting.rst +++ b/Documentation/filesystems/porting.rst @@ -1285,3 +1285,15 @@ rather than a VMA, as the VMA at this stage is not yet valid. The vm_area_desc provides the minimum required information for a filesystem to initialise state upon memory mapping of a file-backed region, and output parameters for the file system to set this state. + +--- + +**mandatory** + +Several functions are renamed: + +- kern_path_locked -> start_removing_path +- kern_path_create -> start_creating_path +- user_path_create -> start_creating_user_path +- user_path_locked_at -> start_removing_user_path_at +- done_path_create -> end_creating_path Are you saying that it also needs to mention that start_removing_path() now calls mnt_want_write()? Or that end_removing_path() should be called to clean up? Or both? I agree that the latter is sensible, I'm not certain that the former is needed, though I guess it doesn't hurt. Thanks, NeilBrown ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v4 5/6] VFS: rename kern_path_locked() and related functions. 2025-09-22 7:30 ` NeilBrown @ 2025-09-22 13:32 ` Al Viro 2025-09-23 10:40 ` Christian Brauner 0 siblings, 1 reply; 12+ messages in thread From: Al Viro @ 2025-09-22 13:32 UTC (permalink / raw) To: NeilBrown Cc: Christian Brauner, Amir Goldstein, Jeff Layton, Jan Kara, linux-fsdevel On Mon, Sep 22, 2025 at 05:30:04PM +1000, NeilBrown wrote: > +- kern_path_locked -> start_removing_path > +- kern_path_create -> start_creating_path > +- user_path_create -> start_creating_user_path > +- user_path_locked_at -> start_removing_user_path_at > +- done_path_create -> end_creating_path > > > Are you saying that it also needs to mention that start_removing_path() > now calls mnt_want_write()? Or that end_removing_path() should be > called to clean up? Or both? > I agree that the latter is sensible, I'm not certain that the former is > needed, though I guess it doesn't hurt. My apologies - I'd managed to miss the relevant part of patch, actually ;-/ Al, seriously embarrassed. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 5/6] VFS: rename kern_path_locked() and related functions. 2025-09-22 13:32 ` Al Viro @ 2025-09-23 10:40 ` Christian Brauner 0 siblings, 0 replies; 12+ messages in thread From: Christian Brauner @ 2025-09-23 10:40 UTC (permalink / raw) To: Al Viro; +Cc: NeilBrown, Amir Goldstein, Jeff Layton, Jan Kara, linux-fsdevel On Mon, Sep 22, 2025 at 02:32:19PM +0100, Al Viro wrote: > On Mon, Sep 22, 2025 at 05:30:04PM +1000, NeilBrown wrote: > > +- kern_path_locked -> start_removing_path > > +- kern_path_create -> start_creating_path > > +- user_path_create -> start_creating_user_path > > +- user_path_locked_at -> start_removing_user_path_at > > +- done_path_create -> end_creating_path > > > > > > Are you saying that it also needs to mention that start_removing_path() > > now calls mnt_want_write()? Or that end_removing_path() should be > > called to clean up? Or both? > > I agree that the latter is sensible, I'm not certain that the former is > > needed, though I guess it doesn't hurt. > > My apologies - I'd managed to miss the relevant part of patch, actually ;-/ > > Al, seriously embarrassed. I think reviewing with the expectation to never miss anything is a losing strategy. ;) ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 6/6] debugfs: rename start_creating() to debugfs_start_creating() 2025-09-22 4:29 NeilBrown ` (4 preceding siblings ...) 2025-09-22 4:29 ` [PATCH v4 5/6] VFS: rename kern_path_locked() and related functions NeilBrown @ 2025-09-22 4:29 ` NeilBrown 5 siblings, 0 replies; 12+ messages in thread From: NeilBrown @ 2025-09-22 4:29 UTC (permalink / raw) To: Alexander Viro, Christian Brauner, Amir Goldstein, Jeff Layton Cc: Jan Kara, linux-fsdevel From: NeilBrown <neil@brown.name> start_creating() is a generic name which I would like to use for a function similar to simple_start_creating(), only not quite so simple. debugfs is using this name which, though static, will cause complaints if then name is given a different signature in a header file. So rename it to debugfs_start_creating(). Signed-off-by: NeilBrown <neil@brown.name> --- fs/debugfs/inode.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index c12d649df6a5..661a99a7dfbe 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -362,7 +362,8 @@ struct dentry *debugfs_lookup(const char *name, struct dentry *parent) } EXPORT_SYMBOL_GPL(debugfs_lookup); -static struct dentry *start_creating(const char *name, struct dentry *parent) +static struct dentry *debugfs_start_creating(const char *name, + struct dentry *parent) { struct dentry *dentry; int error; @@ -428,7 +429,7 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode, if (!(mode & S_IFMT)) mode |= S_IFREG; BUG_ON(!S_ISREG(mode)); - dentry = start_creating(name, parent); + dentry = debugfs_start_creating(name, parent); if (IS_ERR(dentry)) return dentry; @@ -577,7 +578,7 @@ EXPORT_SYMBOL_GPL(debugfs_create_file_size); */ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent) { - struct dentry *dentry = start_creating(name, parent); + struct dentry *dentry = debugfs_start_creating(name, parent); struct inode *inode; if (IS_ERR(dentry)) @@ -624,7 +625,7 @@ struct dentry *debugfs_create_automount(const char *name, debugfs_automount_t f, void *data) { - struct dentry *dentry = start_creating(name, parent); + struct dentry *dentry = debugfs_start_creating(name, parent); struct inode *inode; if (IS_ERR(dentry)) @@ -687,7 +688,7 @@ struct dentry *debugfs_create_symlink(const char *name, struct dentry *parent, if (!link) return ERR_PTR(-ENOMEM); - dentry = start_creating(name, parent); + dentry = debugfs_start_creating(name, parent); if (IS_ERR(dentry)) { kfree(link); return dentry; -- 2.50.0.107.gf914562f5916.dirty ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2025-09-23 10:40 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-22 4:29 NeilBrown 2025-09-22 4:29 ` [PATCH v4 1/6] VFS/ovl: add lookup_one_positive_killable() NeilBrown 2025-09-23 10:39 ` Christian Brauner 2025-09-22 4:29 ` [PATCH v4 2/6] VFS: discard err2 in filename_create() NeilBrown 2025-09-22 4:29 ` [PATCH v4 3/6] VFS: unify old_mnt_idmap and new_mnt_idmap in renamedata NeilBrown 2025-09-22 4:29 ` [PATCH v4 4/6] VFS/audit: introduce kern_path_parent() for audit NeilBrown 2025-09-22 4:29 ` [PATCH v4 5/6] VFS: rename kern_path_locked() and related functions NeilBrown 2025-09-22 5:21 ` Al Viro 2025-09-22 7:30 ` NeilBrown 2025-09-22 13:32 ` Al Viro 2025-09-23 10:40 ` Christian Brauner 2025-09-22 4:29 ` [PATCH v4 6/6] debugfs: rename start_creating() to debugfs_start_creating() NeilBrown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).