From 9d23967b18c64b058cc0a03a8932413bcb37ebb9 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:12 +1000 Subject: ovl: simplify an error path in ovl_copy_up_workdir() If ovl_copy_up_data() fails the error is not immediately handled but the code continues on to call ovl_start_write() and lock_rename(), presumably because both of these locks are needed for the cleanup. Only then (if the lock was successful) is the error checked. This makes the code a little hard to follow and could be fragile. This patch changes to handle the error after the ovl_start_write() (which cannot fail, so there aren't multiple errors to deail with). A new ovl_cleanup_unlocked() is created which takes the required directory lock. This will be used extensively in later patches. In general we need to check the parent is still correct after taking the lock (as ovl_copy_up_workdir() does after a successful lock_rename()) so that is included in ovl_cleanup_unlocked() using new ovl_parent_lock() and ovl_parent_unlock() calls (it is planned to move this API into VFS code eventually, though in a slightly different form). Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-2-neil@brown.name Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/copy_up.c | 20 +++++++++++--------- fs/overlayfs/dir.c | 15 +++++++++++++++ fs/overlayfs/overlayfs.h | 6 ++++++ fs/overlayfs/util.c | 10 ++++++++++ 4 files changed, 42 insertions(+), 9 deletions(-) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 8a3c0d18ec2e..79f41ef6ffa7 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -794,23 +794,24 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c) */ path.dentry = temp; err = ovl_copy_up_data(c, &path); + ovl_start_write(c->dentry); + if (err) + goto cleanup_unlocked; + /* * We cannot hold lock_rename() throughout this helper, because of * lock ordering with sb_writers, which shouldn't be held when calling * ovl_copy_up_data(), so lock workdir and destdir and make sure that * temp wasn't moved before copy up completion or cleanup. */ - ovl_start_write(c->dentry); trap = lock_rename(c->workdir, c->destdir); if (trap || temp->d_parent != c->workdir) { /* temp or workdir moved underneath us? abort without cleanup */ dput(temp); err = -EIO; - if (IS_ERR(trap)) - goto out; - goto unlock; - } else if (err) { - goto cleanup; + if (!IS_ERR(trap)) + unlock_rename(c->workdir, c->destdir); + goto out; } err = ovl_copy_up_metadata(c, temp); @@ -846,7 +847,6 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c) ovl_inode_update(inode, temp); if (S_ISDIR(inode->i_mode)) ovl_set_flag(OVL_WHITEOUTS, inode); -unlock: unlock_rename(c->workdir, c->destdir); out: ovl_end_write(c->dentry); @@ -854,9 +854,11 @@ out: return err; cleanup: - ovl_cleanup(ofs, wdir, temp); + unlock_rename(c->workdir, c->destdir); +cleanup_unlocked: + ovl_cleanup_unlocked(ofs, c->workdir, temp); dput(temp); - goto unlock; + goto out; } /* Copyup using O_TMPFILE which does not require cross dir locking */ diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 4fc221ea6480..67cad3dba8ad 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -43,6 +43,21 @@ int ovl_cleanup(struct ovl_fs *ofs, struct inode *wdir, struct dentry *wdentry) return err; } +int ovl_cleanup_unlocked(struct ovl_fs *ofs, struct dentry *workdir, + struct dentry *wdentry) +{ + int err; + + err = ovl_parent_lock(workdir, wdentry); + if (err) + return err; + + ovl_cleanup(ofs, workdir->d_inode, wdentry); + ovl_parent_unlock(workdir); + + return 0; +} + struct dentry *ovl_lookup_temp(struct ovl_fs *ofs, struct dentry *workdir) { struct dentry *temp; diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 3c52ecddfc9c..a8f18a06a19e 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -414,6 +414,11 @@ static inline bool ovl_open_flags_need_copy_up(int flags) } /* util.c */ +int ovl_parent_lock(struct dentry *parent, struct dentry *child); +static inline void ovl_parent_unlock(struct dentry *parent) +{ + inode_unlock(parent->d_inode); +} int ovl_get_write_access(struct dentry *dentry); void ovl_put_write_access(struct dentry *dentry); void ovl_start_write(struct dentry *dentry); @@ -847,6 +852,7 @@ struct dentry *ovl_create_real(struct ovl_fs *ofs, struct inode *dir, struct dentry *newdentry, struct ovl_cattr *attr); int ovl_cleanup(struct ovl_fs *ofs, struct inode *dir, struct dentry *dentry); +int ovl_cleanup_unlocked(struct ovl_fs *ofs, struct dentry *workdir, struct dentry *dentry); struct dentry *ovl_lookup_temp(struct ovl_fs *ofs, struct dentry *workdir); struct dentry *ovl_create_temp(struct ovl_fs *ofs, struct dentry *workdir, struct ovl_cattr *attr); diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index cc793c8f001f..130aba055ffe 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -1551,3 +1551,13 @@ void ovl_copyattr(struct inode *inode) i_size_write(inode, i_size_read(realinode)); spin_unlock(&inode->i_lock); } + +int ovl_parent_lock(struct dentry *parent, struct dentry *child) +{ + inode_lock_nested(parent->d_inode, I_MUTEX_PARENT); + if (!child || child->d_parent == parent) + return 0; + + inode_unlock(parent->d_inode); + return -EINVAL; +} -- cgit v1.2.3 From c4f8f862b31ccd3d633475fe3982490c744b6565 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:13 +1000 Subject: ovl: change ovl_create_index() to take dir locks ovl_copy_up_workdir() currently take a rename lock on two directories, then use the lock to both create a file in one directory, perform a rename, and possibly unlink the file for cleanup. This is incompatible with proposed changes which will lock just the dentry of objects being acted on. This patch moves the call to ovl_create_index() earlier in ovl_copy_up_workdir() to before the lock is taken. ovl_create_index() then takes the required lock only when needed. Reviewed-by: Amir Goldstein Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-3-neil@brown.name Signed-off-by: Christian Brauner --- fs/overlayfs/copy_up.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 79f41ef6ffa7..d485bd7edd8f 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -517,8 +517,6 @@ static int ovl_set_upper_fh(struct ovl_fs *ofs, struct dentry *upper, /* * Create and install index entry. - * - * Caller must hold i_mutex on indexdir. */ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh, struct dentry *upper) @@ -550,7 +548,9 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh, if (err) return err; + inode_lock(dir); temp = ovl_create_temp(ofs, indexdir, OVL_CATTR(S_IFDIR | 0)); + inode_unlock(dir); err = PTR_ERR(temp); if (IS_ERR(temp)) goto free_name; @@ -559,6 +559,9 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh, if (err) goto out; + err = ovl_parent_lock(indexdir, temp); + if (err) + goto out; index = ovl_lookup_upper(ofs, name.name, indexdir, name.len); if (IS_ERR(index)) { err = PTR_ERR(index); @@ -566,9 +569,10 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh, err = ovl_do_rename(ofs, indexdir, temp, indexdir, index, 0); dput(index); } + ovl_parent_unlock(indexdir); out: if (err) - ovl_cleanup(ofs, dir, temp); + ovl_cleanup_unlocked(ofs, indexdir, temp); dput(temp); free_name: kfree(name.name); @@ -798,6 +802,12 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c) if (err) goto cleanup_unlocked; + if (S_ISDIR(c->stat.mode) && c->indexed) { + err = ovl_create_index(c->dentry, c->origin_fh, temp); + if (err) + goto cleanup_unlocked; + } + /* * We cannot hold lock_rename() throughout this helper, because of * lock ordering with sb_writers, which shouldn't be held when calling @@ -818,12 +828,6 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c) if (err) goto cleanup; - if (S_ISDIR(c->stat.mode) && c->indexed) { - err = ovl_create_index(c->dentry, c->origin_fh, temp); - if (err) - goto cleanup; - } - upper = ovl_lookup_upper(ofs, c->destname.name, c->destdir, c->destname.len); err = PTR_ERR(upper); -- cgit v1.2.3 From d2c995581c7c5d0ff623b2700e76bf22499c66df Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:14 +1000 Subject: ovl: Call ovl_create_temp() without lock held. ovl currently locks a directory or two and then performs multiple actions in one or both directories. This is incompatible with proposed changes which will lock just the dentry of objects being acted on. This patch moves calls to ovl_create_temp() out of the locked regions and has it take and release the relevant lock itself. The lock that was taken before this function was called is now taken after. This means that any code between where the lock was taken and ovl_create_temp() is now unlocked. This necessitates the use of ovl_cleanup_unlocked() and the creation of ovl_lookup_upper_unlocked(). These will be used more widely in future patches. Now that the file is created before the lock is taken for rename, we need to ensure the parent wasn't changed before the lock was gained. ovl_lock_rename_workdir() is changed to optionally receive the dentries that will be involved in the rename. If either is present but has the wrong parent, an error is returned. Reviewed-by: Amir Goldstein Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-4-neil@brown.name Signed-off-by: Christian Brauner --- fs/overlayfs/copy_up.c | 6 ------ fs/overlayfs/dir.c | 54 +++++++++++++++++++++++++----------------------- fs/overlayfs/overlayfs.h | 12 ++++++++++- fs/overlayfs/super.c | 11 ++++++---- fs/overlayfs/util.c | 7 ++++++- 5 files changed, 52 insertions(+), 38 deletions(-) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index d485bd7edd8f..fef873d18b2d 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -523,7 +523,6 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh, { struct ovl_fs *ofs = OVL_FS(dentry->d_sb); struct dentry *indexdir = ovl_indexdir(dentry->d_sb); - struct inode *dir = d_inode(indexdir); struct dentry *index = NULL; struct dentry *temp = NULL; struct qstr name = { }; @@ -548,9 +547,7 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh, if (err) return err; - inode_lock(dir); temp = ovl_create_temp(ofs, indexdir, OVL_CATTR(S_IFDIR | 0)); - inode_unlock(dir); err = PTR_ERR(temp); if (IS_ERR(temp)) goto free_name; @@ -766,7 +763,6 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c) { struct ovl_fs *ofs = OVL_FS(c->dentry->d_sb); struct inode *inode; - struct inode *wdir = d_inode(c->workdir); struct path path = { .mnt = ovl_upper_mnt(ofs) }; struct dentry *temp, *upper, *trap; struct ovl_cu_creds cc; @@ -783,9 +779,7 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c) return err; ovl_start_write(c->dentry); - inode_lock(wdir); temp = ovl_create_temp(ofs, c->workdir, &cattr); - inode_unlock(wdir); ovl_end_write(c->dentry); ovl_revert_cu_creds(&cc); diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 67cad3dba8ad..373335e420fd 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -214,8 +214,12 @@ out: struct dentry *ovl_create_temp(struct ovl_fs *ofs, struct dentry *workdir, struct ovl_cattr *attr) { - return ovl_create_real(ofs, d_inode(workdir), - ovl_lookup_temp(ofs, workdir), attr); + struct dentry *ret; + inode_lock(workdir->d_inode); + ret = ovl_create_real(ofs, d_inode(workdir), + ovl_lookup_temp(ofs, workdir), attr); + inode_unlock(workdir->d_inode); + return ret; } static int ovl_set_opaque_xerr(struct dentry *dentry, struct dentry *upper, @@ -353,7 +357,6 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry, struct dentry *workdir = ovl_workdir(dentry); struct inode *wdir = workdir->d_inode; struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent); - struct inode *udir = upperdir->d_inode; struct path upperpath; struct dentry *upper; struct dentry *opaquedir; @@ -363,27 +366,25 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry, if (WARN_ON(!workdir)) return ERR_PTR(-EROFS); - err = ovl_lock_rename_workdir(workdir, upperdir); - if (err) - goto out; - ovl_path_upper(dentry, &upperpath); err = vfs_getattr(&upperpath, &stat, STATX_BASIC_STATS, AT_STATX_SYNC_AS_STAT); if (err) - goto out_unlock; + goto out; err = -ESTALE; if (!S_ISDIR(stat.mode)) - goto out_unlock; + goto out; upper = upperpath.dentry; - if (upper->d_parent->d_inode != udir) - goto out_unlock; opaquedir = ovl_create_temp(ofs, workdir, OVL_CATTR(stat.mode)); err = PTR_ERR(opaquedir); if (IS_ERR(opaquedir)) - goto out_unlock; + goto out; + + err = ovl_lock_rename_workdir(workdir, opaquedir, upperdir, upper); + if (err) + goto out_cleanup_unlocked; err = ovl_copy_xattr(dentry->d_sb, &upperpath, opaquedir); if (err) @@ -413,10 +414,10 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry, return opaquedir; out_cleanup: - ovl_cleanup(ofs, wdir, opaquedir); - dput(opaquedir); -out_unlock: unlock_rename(workdir, upperdir); +out_cleanup_unlocked: + ovl_cleanup_unlocked(ofs, workdir, opaquedir); + dput(opaquedir); out: return ERR_PTR(err); } @@ -454,15 +455,11 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, return err; } - err = ovl_lock_rename_workdir(workdir, upperdir); - if (err) - goto out; - - upper = ovl_lookup_upper(ofs, dentry->d_name.name, upperdir, - dentry->d_name.len); + upper = ovl_lookup_upper_unlocked(ofs, dentry->d_name.name, upperdir, + dentry->d_name.len); err = PTR_ERR(upper); if (IS_ERR(upper)) - goto out_unlock; + goto out; err = -ESTALE; if (d_is_negative(upper) || !ovl_upper_is_whiteout(ofs, upper)) @@ -473,6 +470,10 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, if (IS_ERR(newdentry)) goto out_dput; + err = ovl_lock_rename_workdir(workdir, newdentry, upperdir, upper); + if (err) + goto out_cleanup_unlocked; + /* * mode could have been mutilated due to umask (e.g. sgid directory) */ @@ -523,10 +524,9 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, ovl_cleanup(ofs, udir, newdentry); dput(newdentry); } + unlock_rename(workdir, upperdir); out_dput: dput(upper); -out_unlock: - unlock_rename(workdir, upperdir); out: if (!hardlink) { posix_acl_release(acl); @@ -535,7 +535,9 @@ out: return err; out_cleanup: - ovl_cleanup(ofs, wdir, newdentry); + unlock_rename(workdir, upperdir); +out_cleanup_unlocked: + ovl_cleanup_unlocked(ofs, workdir, newdentry); dput(newdentry); goto out_dput; } @@ -772,7 +774,7 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, goto out; } - err = ovl_lock_rename_workdir(workdir, upperdir); + err = ovl_lock_rename_workdir(workdir, NULL, upperdir, NULL); if (err) goto out_dput; diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index a8f18a06a19e..002019b40af3 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -405,6 +405,15 @@ static inline struct dentry *ovl_lookup_upper(struct ovl_fs *ofs, return lookup_one(ovl_upper_mnt_idmap(ofs), &QSTR_LEN(name, len), base); } +static inline struct dentry *ovl_lookup_upper_unlocked(struct ovl_fs *ofs, + const char *name, + struct dentry *base, + int len) +{ + return lookup_one_unlocked(ovl_upper_mnt_idmap(ofs), + &QSTR_LEN(name, len), base); +} + static inline bool ovl_open_flags_need_copy_up(int flags) { if (!flags) @@ -544,7 +553,8 @@ bool ovl_is_inuse(struct dentry *dentry); bool ovl_need_index(struct dentry *dentry); int ovl_nlink_start(struct dentry *dentry); void ovl_nlink_end(struct dentry *dentry); -int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir); +int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *work, + struct dentry *upperdir, struct dentry *upper); int ovl_check_metacopy_xattr(struct ovl_fs *ofs, const struct path *path, struct ovl_metacopy *data); int ovl_set_metacopy_xattr(struct ovl_fs *ofs, struct dentry *d, diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index cf99b276fdfb..2e6b25bde83f 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -564,13 +564,16 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs) struct name_snapshot name; int err; - inode_lock_nested(dir, I_MUTEX_PARENT); - temp = ovl_create_temp(ofs, workdir, OVL_CATTR(S_IFREG | 0)); err = PTR_ERR(temp); if (IS_ERR(temp)) - goto out_unlock; + return err; + err = ovl_parent_lock(workdir, temp); + if (err) { + dput(temp); + return err; + } dest = ovl_lookup_temp(ofs, workdir); err = PTR_ERR(dest); if (IS_ERR(dest)) { @@ -606,7 +609,7 @@ cleanup_temp: dput(dest); out_unlock: - inode_unlock(dir); + ovl_parent_unlock(workdir); return err; } diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 130aba055ffe..c25c86e0f4da 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -1227,7 +1227,8 @@ void ovl_nlink_end(struct dentry *dentry) ovl_inode_unlock(inode); } -int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir) +int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *work, + struct dentry *upperdir, struct dentry *upper) { struct dentry *trap; @@ -1241,6 +1242,10 @@ int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *upperdir) goto err; if (trap) goto err_unlock; + if (work && work->d_parent != workdir) + goto err_unlock; + if (upper && upper->d_parent != upperdir) + goto err_unlock; return 0; -- cgit v1.2.3 From a735bdf0b78528970f169870ced234dd3a33ea7b Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:15 +1000 Subject: ovl: narrow the locked region in ovl_copy_up_workdir() In ovl_copy_up_workdir() unlock immediately after the rename. There is nothing else in the function that needs the lock. Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-5-neil@brown.name Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/copy_up.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index fef873d18b2d..8f8dbe8a1d54 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -829,9 +829,10 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c) goto cleanup; err = ovl_do_rename(ofs, c->workdir, temp, c->destdir, upper, 0); + unlock_rename(c->workdir, c->destdir); dput(upper); if (err) - goto cleanup; + goto cleanup_unlocked; inode = d_inode(c->dentry); if (c->metacopy_digest) @@ -845,7 +846,6 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c) ovl_inode_update(inode, temp); if (S_ISDIR(inode->i_mode)) ovl_set_flag(OVL_WHITEOUTS, inode); - unlock_rename(c->workdir, c->destdir); out: ovl_end_write(c->dentry); -- cgit v1.2.3 From a07052e07b67add56328b547ce4459878618334d Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:16 +1000 Subject: ovl: narrow locking in ovl_create_upper() Drop the directory lock immediately after the ovl_create_real() call and take a separate lock later for cleanup in ovl_cleanup_unlocked() - if needed. This makes way for future changes where locks are taken on individual dentries rather than the whole directory. Reviewed-by: Amir Goldstein Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-6-neil@brown.name Signed-off-by: Christian Brauner --- fs/overlayfs/dir.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 373335e420fd..1a146a71993a 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -326,9 +326,9 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode, ovl_lookup_upper(ofs, dentry->d_name.name, upperdir, dentry->d_name.len), attr); - err = PTR_ERR(newdentry); + inode_unlock(udir); if (IS_ERR(newdentry)) - goto out_unlock; + return PTR_ERR(newdentry); if (ovl_type_merge(dentry->d_parent) && d_is_dir(newdentry) && !ovl_allow_offline_changes(ofs)) { @@ -340,14 +340,12 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode, err = ovl_instantiate(dentry, inode, newdentry, !!attr->hardlink, NULL); if (err) goto out_cleanup; -out_unlock: - inode_unlock(udir); - return err; + return 0; out_cleanup: - ovl_cleanup(ofs, udir, newdentry); + ovl_cleanup_unlocked(ofs, upperdir, newdentry); dput(newdentry); - goto out_unlock; + return err; } static struct dentry *ovl_clear_empty(struct dentry *dentry, -- cgit v1.2.3 From 4f622bd9f3e5dd1e882ee8f4194ea1d95dcf752f Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:17 +1000 Subject: ovl: narrow locking in ovl_clear_empty() Drop the locks immediately after rename, and use a separate lock for cleanup. This makes way for future changes where locks are taken on individual dentries rather than the whole directory. Note that ovl_cleanup_whiteouts() operates on "upper", a child of "upperdir" and does not require upperdir or workdir to be locked. Reviewed-by: Amir Goldstein Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-7-neil@brown.name Signed-off-by: Christian Brauner --- fs/overlayfs/dir.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 1a146a71993a..540b67f5cdf5 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -353,7 +353,6 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry, { struct ovl_fs *ofs = OVL_FS(dentry->d_sb); struct dentry *workdir = ovl_workdir(dentry); - struct inode *wdir = workdir->d_inode; struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent); struct path upperpath; struct dentry *upper; @@ -399,12 +398,12 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry, goto out_cleanup; err = ovl_do_rename(ofs, workdir, opaquedir, upperdir, upper, RENAME_EXCHANGE); + unlock_rename(workdir, upperdir); if (err) - goto out_cleanup; + goto out_cleanup_unlocked; ovl_cleanup_whiteouts(ofs, upper, list); - ovl_cleanup(ofs, wdir, upper); - unlock_rename(workdir, upperdir); + ovl_cleanup_unlocked(ofs, workdir, upper); /* dentry's upper doesn't match now, get rid of it */ d_drop(dentry); -- cgit v1.2.3 From e460bc4d012ce144376264609309b14b84b693ff Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:18 +1000 Subject: ovl: narrow locking in ovl_create_over_whiteout() Unlock the parents immediately after the rename, and use ovl_cleanup_unlocked() for cleanup, which takes a separate lock. This makes way for future changes where locks are taken on individual dentries rather than the whole directory. Reviewed-by: Amir Goldstein Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-8-neil@brown.name Signed-off-by: Christian Brauner --- fs/overlayfs/dir.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 540b67f5cdf5..7c92ffb6e312 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -433,9 +433,7 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, { struct ovl_fs *ofs = OVL_FS(dentry->d_sb); struct dentry *workdir = ovl_workdir(dentry); - struct inode *wdir = workdir->d_inode; struct dentry *upperdir = ovl_dentry_upper(dentry->d_parent); - struct inode *udir = upperdir->d_inode; struct dentry *upper; struct dentry *newdentry; int err; @@ -506,22 +504,23 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, err = ovl_do_rename(ofs, workdir, newdentry, upperdir, upper, RENAME_EXCHANGE); + unlock_rename(workdir, upperdir); if (err) - goto out_cleanup; + goto out_cleanup_unlocked; - ovl_cleanup(ofs, wdir, upper); + ovl_cleanup_unlocked(ofs, workdir, upper); } else { err = ovl_do_rename(ofs, workdir, newdentry, upperdir, upper, 0); + unlock_rename(workdir, upperdir); if (err) - goto out_cleanup; + goto out_cleanup_unlocked; } ovl_dir_modified(dentry->d_parent, false); err = ovl_instantiate(dentry, inode, newdentry, hardlink, NULL); if (err) { - ovl_cleanup(ofs, udir, newdentry); + ovl_cleanup_unlocked(ofs, upperdir, newdentry); dput(newdentry); } - unlock_rename(workdir, upperdir); out_dput: dput(upper); out: -- cgit v1.2.3 From 76342c9eb8e28f2e421ec7fb72bb2b9aa4d7dd77 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:19 +1000 Subject: ovl: simplify gotos in ovl_rename() Rather than having three separate goto label: out_unlock, out_dput_old, and out_dput, make use of that fact that dput() happily accepts a NULL pointer to reduce this to just one goto label: out_unlock. olddentry and newdentry are initialised to NULL and only set once a value dentry is found. They are then dput() late in the function. Suggested-by: Amir Goldstein Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-9-neil@brown.name Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/dir.c | 54 +++++++++++++++++++++++++++--------------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 7c92ffb6e312..138dd85d2242 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -1082,9 +1082,9 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir, int err; struct dentry *old_upperdir; struct dentry *new_upperdir; - struct dentry *olddentry; - struct dentry *newdentry; - struct dentry *trap; + struct dentry *olddentry = NULL; + struct dentry *newdentry = NULL; + struct dentry *trap, *de; bool old_opaque; bool new_opaque; bool cleanup_whiteout = false; @@ -1197,21 +1197,23 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir, goto out_revert_creds; } - olddentry = ovl_lookup_upper(ofs, old->d_name.name, old_upperdir, - old->d_name.len); - err = PTR_ERR(olddentry); - if (IS_ERR(olddentry)) + de = ovl_lookup_upper(ofs, old->d_name.name, old_upperdir, + old->d_name.len); + err = PTR_ERR(de); + if (IS_ERR(de)) goto out_unlock; + olddentry = de; err = -ESTALE; if (!ovl_matches_upper(old, olddentry)) - goto out_dput_old; + goto out_unlock; - newdentry = ovl_lookup_upper(ofs, new->d_name.name, new_upperdir, - new->d_name.len); - err = PTR_ERR(newdentry); - if (IS_ERR(newdentry)) - goto out_dput_old; + de = ovl_lookup_upper(ofs, new->d_name.name, new_upperdir, + new->d_name.len); + err = PTR_ERR(de); + if (IS_ERR(de)) + goto out_unlock; + newdentry = de; old_opaque = ovl_dentry_is_opaque(old); new_opaque = ovl_dentry_is_opaque(new); @@ -1220,28 +1222,28 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir, if (d_inode(new) && ovl_dentry_upper(new)) { if (opaquedir) { if (newdentry != opaquedir) - goto out_dput; + goto out_unlock; } else { if (!ovl_matches_upper(new, newdentry)) - goto out_dput; + goto out_unlock; } } else { if (!d_is_negative(newdentry)) { if (!new_opaque || !ovl_upper_is_whiteout(ofs, newdentry)) - goto out_dput; + goto out_unlock; } else { if (flags & RENAME_EXCHANGE) - goto out_dput; + goto out_unlock; } } if (olddentry == trap) - goto out_dput; + goto out_unlock; if (newdentry == trap) - goto out_dput; + goto out_unlock; if (olddentry->d_inode == newdentry->d_inode) - goto out_dput; + goto out_unlock; err = 0; if (ovl_type_merge_or_lower(old)) @@ -1249,7 +1251,7 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir, else if (is_dir && !old_opaque && ovl_type_merge(new->d_parent)) err = ovl_set_opaque_xerr(old, olddentry, -EXDEV); if (err) - goto out_dput; + goto out_unlock; if (!overwrite && ovl_type_merge_or_lower(new)) err = ovl_set_redirect(new, samedir); @@ -1257,12 +1259,12 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir, ovl_type_merge(old->d_parent)) err = ovl_set_opaque_xerr(new, newdentry, -EXDEV); if (err) - goto out_dput; + goto out_unlock; err = ovl_do_rename(ofs, old_upperdir, olddentry, new_upperdir, newdentry, flags); if (err) - goto out_dput; + goto out_unlock; if (cleanup_whiteout) ovl_cleanup(ofs, old_upperdir->d_inode, newdentry); @@ -1284,10 +1286,6 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir, if (d_inode(new) && ovl_dentry_upper(new)) ovl_copyattr(d_inode(new)); -out_dput: - dput(newdentry); -out_dput_old: - dput(olddentry); out_unlock: unlock_rename(new_upperdir, old_upperdir); out_revert_creds: @@ -1297,6 +1295,8 @@ out_revert_creds: else ovl_drop_write(old); out: + dput(newdentry); + dput(olddentry); dput(opaquedir); ovl_cache_free(&list); return err; -- cgit v1.2.3 From 05468498cd2fc5b65e18eb80cdce8ae6ee6c9a77 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:20 +1000 Subject: ovl: narrow locking in ovl_rename() Drop the rename lock immediately after the rename, and use ovl_cleanup_unlocked() for cleanup. This makes way for future changes where locks are taken on individual dentries rather than the whole directory. Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-10-neil@brown.name Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/dir.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 138dd85d2242..e81be60f1125 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -1263,11 +1263,12 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir, err = ovl_do_rename(ofs, old_upperdir, olddentry, new_upperdir, newdentry, flags); + unlock_rename(new_upperdir, old_upperdir); if (err) - goto out_unlock; + goto out_revert_creds; if (cleanup_whiteout) - ovl_cleanup(ofs, old_upperdir->d_inode, newdentry); + ovl_cleanup_unlocked(ofs, old_upperdir, newdentry); if (overwrite && d_inode(new)) { if (new_is_dir) @@ -1286,8 +1287,6 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir, if (d_inode(new) && ovl_dentry_upper(new)) ovl_copyattr(d_inode(new)); -out_unlock: - unlock_rename(new_upperdir, old_upperdir); out_revert_creds: ovl_revert_creds(old_cred); if (update_nlink) @@ -1300,6 +1299,10 @@ out: dput(opaquedir); ovl_cache_free(&list); return err; + +out_unlock: + unlock_rename(new_upperdir, old_upperdir); + goto out_revert_creds; } static int ovl_create_tmpfile(struct file *file, struct dentry *dentry, -- cgit v1.2.3 From 7dfb0722ad0740f6fbf9bd184e3ae4ba4bc19f31 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:21 +1000 Subject: ovl: narrow locking in ovl_cleanup_whiteouts() Rather than lock the directory for the whole operation, use ovl_lookup_upper_unlocked() and ovl_cleanup_unlocked() to take the lock only when needed. This makes way for future changes where locks are taken on individual dentries rather than the whole directory. Reviewed-by: Amir Goldstein Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-11-neil@brown.name Signed-off-by: Christian Brauner --- fs/overlayfs/readdir.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index 68cca52ae2ac..2a222b8185a3 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -1034,14 +1034,13 @@ void ovl_cleanup_whiteouts(struct ovl_fs *ofs, struct dentry *upper, { struct ovl_cache_entry *p; - inode_lock_nested(upper->d_inode, I_MUTEX_CHILD); list_for_each_entry(p, list, l_node) { struct dentry *dentry; if (WARN_ON(!p->is_whiteout || !p->is_upper)) continue; - dentry = ovl_lookup_upper(ofs, p->name, upper, p->len); + dentry = ovl_lookup_upper_unlocked(ofs, p->name, upper, p->len); if (IS_ERR(dentry)) { pr_err("lookup '%s/%.*s' failed (%i)\n", upper->d_name.name, p->len, p->name, @@ -1049,10 +1048,9 @@ void ovl_cleanup_whiteouts(struct ovl_fs *ofs, struct dentry *upper, continue; } if (dentry->d_inode) - ovl_cleanup(ofs, upper->d_inode, dentry); + ovl_cleanup_unlocked(ofs, upper, dentry); dput(dentry); } - inode_unlock(upper->d_inode); } static bool ovl_check_d_type(struct dir_context *ctx, const char *name, -- cgit v1.2.3 From 8290fb412d2f3dced1b744330d22f2d639ccdf36 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:22 +1000 Subject: ovl: narrow locking in ovl_cleanup_index() ovl_cleanup_index() takes a lock on the directory and then does a lookup and possibly one of two different cleanups. This patch narrows the locking to use the _unlocked() versions of the lookup and one cleanup, and just takes the lock for the other cleanup. A subsequent patch will take the lock into the cleanup. Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-12-neil@brown.name Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/util.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index c25c86e0f4da..7a1ca9380601 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -1078,7 +1078,6 @@ static void ovl_cleanup_index(struct dentry *dentry) { struct ovl_fs *ofs = OVL_FS(dentry->d_sb); struct dentry *indexdir = ovl_indexdir(dentry->d_sb); - struct inode *dir = indexdir->d_inode; struct dentry *lowerdentry = ovl_dentry_lower(dentry); struct dentry *upperdentry = ovl_dentry_upper(dentry); struct dentry *index = NULL; @@ -1114,21 +1113,22 @@ static void ovl_cleanup_index(struct dentry *dentry) goto out; } - inode_lock_nested(dir, I_MUTEX_PARENT); - index = ovl_lookup_upper(ofs, name.name, indexdir, name.len); + index = ovl_lookup_upper_unlocked(ofs, name.name, indexdir, name.len); err = PTR_ERR(index); if (IS_ERR(index)) { index = NULL; } else if (ovl_index_all(dentry->d_sb)) { /* Whiteout orphan index to block future open by handle */ - err = ovl_cleanup_and_whiteout(OVL_FS(dentry->d_sb), - indexdir, index); + err = ovl_parent_lock(indexdir, index); + if (!err) { + err = ovl_cleanup_and_whiteout(OVL_FS(dentry->d_sb), + indexdir, index); + ovl_parent_unlock(indexdir); + } } else { /* Cleanup orphan index entries */ - err = ovl_cleanup(ofs, dir, index); + err = ovl_cleanup_unlocked(ofs, indexdir, index); } - - inode_unlock(dir); if (err) goto fail; -- cgit v1.2.3 From 61eb7fec9e79d429939fab16a4558caf7fa83160 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:23 +1000 Subject: ovl: narrow locking in ovl_workdir_create() In ovl_workdir_create() don't hold the dir lock for the whole time, but only take it when needed. It now gets taken separately for ovl_workdir_cleanup(). A subsequent patch will move the locking into that function. Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-13-neil@brown.name Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/super.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 2e6b25bde83f..cb2551a155d8 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -299,8 +299,8 @@ static struct dentry *ovl_workdir_create(struct ovl_fs *ofs, int err; bool retried = false; - inode_lock_nested(dir, I_MUTEX_PARENT); retry: + inode_lock_nested(dir, I_MUTEX_PARENT); work = ovl_lookup_upper(ofs, name, ofs->workbasedir, strlen(name)); if (!IS_ERR(work)) { @@ -311,23 +311,28 @@ retry: if (work->d_inode) { err = -EEXIST; + inode_unlock(dir); if (retried) goto out_dput; if (persist) - goto out_unlock; + return work; retried = true; - err = ovl_workdir_cleanup(ofs, dir, mnt, work, 0); - dput(work); - if (err == -EINVAL) { - work = ERR_PTR(err); - goto out_unlock; + err = ovl_parent_lock(ofs->workbasedir, work); + if (!err) { + err = ovl_workdir_cleanup(ofs, dir, mnt, work, 0); + ovl_parent_unlock(ofs->workbasedir); } + dput(work); + if (err == -EINVAL) + return ERR_PTR(err); + goto retry; } work = ovl_do_mkdir(ofs, dir, work, attr.ia_mode); + inode_unlock(dir); err = PTR_ERR(work); if (IS_ERR(work)) goto out_err; @@ -365,11 +370,10 @@ retry: if (err) goto out_dput; } else { + inode_unlock(dir); err = PTR_ERR(work); goto out_err; } -out_unlock: - inode_unlock(dir); return work; out_dput: @@ -377,8 +381,7 @@ out_dput: out_err: pr_warn("failed to create directory %s/%s (errno: %i); mounting read-only\n", ofs->config.workdir, name, -err); - work = NULL; - goto out_unlock; + return NULL; } static int ovl_check_namelen(const struct path *path, struct ovl_fs *ofs, -- cgit v1.2.3 From d56c6feb69cb8f036855b4d12c84b46b10421278 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:24 +1000 Subject: ovl: narrow locking in ovl_indexdir_cleanup() Instead of taking the directory lock for the whole cleanup, only take it when needed. Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-14-neil@brown.name Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/readdir.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index 2a222b8185a3..95d5284daf8d 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -1194,7 +1194,6 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) if (err) goto out; - inode_lock_nested(dir, I_MUTEX_PARENT); list_for_each_entry(p, &list, l_node) { if (p->name[0] == '.') { if (p->len == 1) @@ -1202,7 +1201,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) if (p->len == 2 && p->name[1] == '.') continue; } - index = ovl_lookup_upper(ofs, p->name, indexdir, p->len); + index = ovl_lookup_upper_unlocked(ofs, p->name, indexdir, p->len); if (IS_ERR(index)) { err = PTR_ERR(index); index = NULL; @@ -1210,7 +1209,11 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) } /* Cleanup leftover from index create/cleanup attempt */ if (index->d_name.name[0] == '#') { - err = ovl_workdir_cleanup(ofs, dir, path.mnt, index, 1); + err = ovl_parent_lock(indexdir, index); + if (!err) { + err = ovl_workdir_cleanup(ofs, dir, path.mnt, index, 1); + ovl_parent_unlock(indexdir); + } if (err) break; goto next; @@ -1220,7 +1223,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) goto next; } else if (err == -ESTALE) { /* Cleanup stale index entries */ - err = ovl_cleanup(ofs, dir, index); + err = ovl_cleanup_unlocked(ofs, indexdir, index); } else if (err != -ENOENT) { /* * Abort mount to avoid corrupting the index if @@ -1233,10 +1236,14 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) * Whiteout orphan index to block future open by * handle after overlay nlink dropped to zero. */ - err = ovl_cleanup_and_whiteout(ofs, indexdir, index); + err = ovl_parent_lock(indexdir, index); + if (!err) { + err = ovl_cleanup_and_whiteout(ofs, indexdir, index); + ovl_parent_unlock(indexdir); + } } else { /* Cleanup orphan index entries */ - err = ovl_cleanup(ofs, dir, index); + err = ovl_cleanup_unlocked(ofs, indexdir, index); } if (err) @@ -1247,7 +1254,6 @@ next: index = NULL; } dput(index); - inode_unlock(dir); out: ovl_cache_free(&list); if (err) -- cgit v1.2.3 From a45ee87ded78876f8106a255f726b89aa69cd7a4 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:25 +1000 Subject: ovl: narrow locking in ovl_workdir_cleanup_recurse() Only take the dir lock when needed, rather than for the whole loop. Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-15-neil@brown.name Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/readdir.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index 95d5284daf8d..b0f9e5a00c1a 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -1122,7 +1122,6 @@ static int ovl_workdir_cleanup_recurse(struct ovl_fs *ofs, const struct path *pa if (err) goto out; - inode_lock_nested(dir, I_MUTEX_PARENT); list_for_each_entry(p, &list, l_node) { struct dentry *dentry; @@ -1137,16 +1136,21 @@ static int ovl_workdir_cleanup_recurse(struct ovl_fs *ofs, const struct path *pa err = -EINVAL; break; } - dentry = ovl_lookup_upper(ofs, p->name, path->dentry, p->len); + dentry = ovl_lookup_upper_unlocked(ofs, p->name, path->dentry, p->len); if (IS_ERR(dentry)) continue; - if (dentry->d_inode) - err = ovl_workdir_cleanup(ofs, dir, path->mnt, dentry, level); + if (dentry->d_inode) { + err = ovl_parent_lock(path->dentry, dentry); + if (!err) { + err = ovl_workdir_cleanup(ofs, dir, path->mnt, + dentry, level); + ovl_parent_unlock(path->dentry); + } + } dput(dentry); if (err) break; } - inode_unlock(dir); out: ovl_cache_free(&list); return err; -- cgit v1.2.3 From 241062ae5d8784158d0af5d7b9212c3b53d91ca5 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:26 +1000 Subject: ovl: change ovl_workdir_cleanup() to take dir lock as needed. Rather than calling ovl_workdir_cleanup() with the dir already locked, change it to take the dir lock only when needed. Also change ovl_workdir_cleanup() to take a dentry for the parent rather than an inode. Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-16-neil@brown.name Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/overlayfs.h | 2 +- fs/overlayfs/readdir.c | 36 +++++++++++++----------------------- fs/overlayfs/super.c | 6 +----- 3 files changed, 15 insertions(+), 29 deletions(-) diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 002019b40af3..df674a12f498 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -742,7 +742,7 @@ void ovl_cleanup_whiteouts(struct ovl_fs *ofs, struct dentry *upper, void ovl_cache_free(struct list_head *list); void ovl_dir_cache_free(struct inode *inode); int ovl_check_d_type_supported(const struct path *realpath); -int ovl_workdir_cleanup(struct ovl_fs *ofs, struct inode *dir, +int ovl_workdir_cleanup(struct ovl_fs *ofs, struct dentry *parent, struct vfsmount *mnt, struct dentry *dentry, int level); int ovl_indexdir_cleanup(struct ovl_fs *ofs); diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index b0f9e5a00c1a..e2d0c314df6c 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -1096,7 +1096,6 @@ static int ovl_workdir_cleanup_recurse(struct ovl_fs *ofs, const struct path *pa int level) { int err; - struct inode *dir = path->dentry->d_inode; LIST_HEAD(list); struct ovl_cache_entry *p; struct ovl_readdir_data rdd = { @@ -1139,14 +1138,9 @@ static int ovl_workdir_cleanup_recurse(struct ovl_fs *ofs, const struct path *pa dentry = ovl_lookup_upper_unlocked(ofs, p->name, path->dentry, p->len); if (IS_ERR(dentry)) continue; - if (dentry->d_inode) { - err = ovl_parent_lock(path->dentry, dentry); - if (!err) { - err = ovl_workdir_cleanup(ofs, dir, path->mnt, - dentry, level); - ovl_parent_unlock(path->dentry); - } - } + if (dentry->d_inode) + err = ovl_workdir_cleanup(ofs, path->dentry, path->mnt, + dentry, level); dput(dentry); if (err) break; @@ -1156,24 +1150,25 @@ out: return err; } -int ovl_workdir_cleanup(struct ovl_fs *ofs, struct inode *dir, +int ovl_workdir_cleanup(struct ovl_fs *ofs, struct dentry *parent, struct vfsmount *mnt, struct dentry *dentry, int level) { int err; - if (!d_is_dir(dentry) || level > 1) { - return ovl_cleanup(ofs, dir, dentry); - } + if (!d_is_dir(dentry) || level > 1) + return ovl_cleanup_unlocked(ofs, parent, dentry); - err = ovl_do_rmdir(ofs, dir, dentry); + err = ovl_parent_lock(parent, dentry); + if (err) + return err; + err = ovl_do_rmdir(ofs, parent->d_inode, dentry); + ovl_parent_unlock(parent); if (err) { struct path path = { .mnt = mnt, .dentry = dentry }; - inode_unlock(dir); err = ovl_workdir_cleanup_recurse(ofs, &path, level + 1); - inode_lock_nested(dir, I_MUTEX_PARENT); if (!err) - err = ovl_cleanup(ofs, dir, dentry); + err = ovl_cleanup_unlocked(ofs, parent, dentry); } return err; @@ -1184,7 +1179,6 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) int err; struct dentry *indexdir = ofs->workdir; struct dentry *index = NULL; - struct inode *dir = indexdir->d_inode; struct path path = { .mnt = ovl_upper_mnt(ofs), .dentry = indexdir }; LIST_HEAD(list); struct ovl_cache_entry *p; @@ -1213,11 +1207,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) } /* Cleanup leftover from index create/cleanup attempt */ if (index->d_name.name[0] == '#') { - err = ovl_parent_lock(indexdir, index); - if (!err) { - err = ovl_workdir_cleanup(ofs, dir, path.mnt, index, 1); - ovl_parent_unlock(indexdir); - } + err = ovl_workdir_cleanup(ofs, indexdir, path.mnt, index, 1); if (err) break; goto next; diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index cb2551a155d8..4c3736bf2db4 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -319,11 +319,7 @@ retry: return work; retried = true; - err = ovl_parent_lock(ofs->workbasedir, work); - if (!err) { - err = ovl_workdir_cleanup(ofs, dir, mnt, work, 0); - ovl_parent_unlock(ofs->workbasedir); - } + err = ovl_workdir_cleanup(ofs, ofs->workbasedir, mnt, work, 0); dput(work); if (err == -EINVAL) return ERR_PTR(err); -- cgit v1.2.3 From c69566b1d11d781d6b586113126ce1a803bbf8fc Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:27 +1000 Subject: ovl: narrow locking on ovl_remove_and_whiteout() This code: performs a lookup_upper creates a whiteout object renames the whiteout over the result of the lookup The create and the rename must be locked separately for proposed directory locking changes. This patch takes a first step of moving the lookup out of the locked region. A subsequent patch will separate the create from the rename. Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-17-neil@brown.name Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/dir.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index e81be60f1125..340f8679b6e7 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -770,15 +770,11 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, goto out; } - err = ovl_lock_rename_workdir(workdir, NULL, upperdir, NULL); - if (err) - goto out_dput; - - upper = ovl_lookup_upper(ofs, dentry->d_name.name, upperdir, - dentry->d_name.len); + upper = ovl_lookup_upper_unlocked(ofs, dentry->d_name.name, upperdir, + dentry->d_name.len); err = PTR_ERR(upper); if (IS_ERR(upper)) - goto out_unlock; + goto out_dput; err = -ESTALE; if ((opaquedir && upper != opaquedir) || @@ -787,17 +783,18 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, goto out_dput_upper; } - err = ovl_cleanup_and_whiteout(ofs, upperdir, upper); + err = ovl_lock_rename_workdir(workdir, NULL, upperdir, upper); if (err) - goto out_d_drop; + goto out_dput_upper; + + err = ovl_cleanup_and_whiteout(ofs, upperdir, upper); + if (!err) + ovl_dir_modified(dentry->d_parent, true); - ovl_dir_modified(dentry->d_parent, true); -out_d_drop: d_drop(dentry); + unlock_rename(workdir, upperdir); out_dput_upper: dput(upper); -out_unlock: - unlock_rename(workdir, upperdir); out_dput: dput(opaquedir); out: -- cgit v1.2.3 From 2fa14cf2dca1913054e0225377d0a9999483d34d Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:28 +1000 Subject: ovl: change ovl_cleanup_and_whiteout() to take rename lock as needed Rather than locking the directory(s) before calling ovl_cleanup_and_whiteout(), change it (and ovl_whiteout()) to do the locking, so the locking can be fine grained as will be needed for proposed locking changes. Sometimes this is called to whiteout something in the index dir, in which case only that dir must be locked. In one case it is called on something in an upperdir, so two directories must be locked. We use ovl_lock_rename_workdir() for this and remove the restriction that upperdir cannot be indexdir - because now sometimes it is. Reviewed-by: Amir Goldstein Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-18-neil@brown.name Signed-off-by: Christian Brauner --- fs/overlayfs/dir.c | 20 +++++++++----------- fs/overlayfs/readdir.c | 6 +----- fs/overlayfs/util.c | 12 ++---------- 3 files changed, 12 insertions(+), 26 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 340f8679b6e7..6a70faeee6fa 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -77,7 +77,6 @@ struct dentry *ovl_lookup_temp(struct ovl_fs *ofs, struct dentry *workdir) return temp; } -/* caller holds i_mutex on workdir */ static struct dentry *ovl_whiteout(struct ovl_fs *ofs) { int err; @@ -85,6 +84,7 @@ static struct dentry *ovl_whiteout(struct ovl_fs *ofs) struct dentry *workdir = ofs->workdir; struct inode *wdir = workdir->d_inode; + inode_lock_nested(wdir, I_MUTEX_PARENT); if (!ofs->whiteout) { whiteout = ovl_lookup_temp(ofs, workdir); if (IS_ERR(whiteout)) @@ -118,14 +118,13 @@ static struct dentry *ovl_whiteout(struct ovl_fs *ofs) whiteout = ofs->whiteout; ofs->whiteout = NULL; out: + inode_unlock(wdir); return whiteout; } -/* Caller must hold i_mutex on both workdir and dir */ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir, struct dentry *dentry) { - struct inode *wdir = ofs->workdir->d_inode; struct dentry *whiteout; int err; int flags = 0; @@ -138,18 +137,22 @@ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir, if (d_is_dir(dentry)) flags = RENAME_EXCHANGE; - err = ovl_do_rename(ofs, ofs->workdir, whiteout, dir, dentry, flags); + err = ovl_lock_rename_workdir(ofs->workdir, whiteout, dir, dentry); + if (!err) { + err = ovl_do_rename(ofs, ofs->workdir, whiteout, dir, dentry, flags); + unlock_rename(ofs->workdir, dir); + } if (err) goto kill_whiteout; if (flags) - ovl_cleanup(ofs, wdir, dentry); + ovl_cleanup_unlocked(ofs, ofs->workdir, dentry); out: dput(whiteout); return err; kill_whiteout: - ovl_cleanup(ofs, wdir, whiteout); + ovl_cleanup_unlocked(ofs, ofs->workdir, whiteout); goto out; } @@ -783,16 +786,11 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, goto out_dput_upper; } - err = ovl_lock_rename_workdir(workdir, NULL, upperdir, upper); - if (err) - goto out_dput_upper; - err = ovl_cleanup_and_whiteout(ofs, upperdir, upper); if (!err) ovl_dir_modified(dentry->d_parent, true); d_drop(dentry); - unlock_rename(workdir, upperdir); out_dput_upper: dput(upper); out_dput: diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index e2d0c314df6c..ecbf39a49d57 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -1230,11 +1230,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) * Whiteout orphan index to block future open by * handle after overlay nlink dropped to zero. */ - err = ovl_parent_lock(indexdir, index); - if (!err) { - err = ovl_cleanup_and_whiteout(ofs, indexdir, index); - ovl_parent_unlock(indexdir); - } + err = ovl_cleanup_and_whiteout(ofs, indexdir, index); } else { /* Cleanup orphan index entries */ err = ovl_cleanup_unlocked(ofs, indexdir, index); diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 7a1ca9380601..a63fda9ef6fd 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -1119,12 +1119,8 @@ static void ovl_cleanup_index(struct dentry *dentry) index = NULL; } else if (ovl_index_all(dentry->d_sb)) { /* Whiteout orphan index to block future open by handle */ - err = ovl_parent_lock(indexdir, index); - if (!err) { - err = ovl_cleanup_and_whiteout(OVL_FS(dentry->d_sb), - indexdir, index); - ovl_parent_unlock(indexdir); - } + err = ovl_cleanup_and_whiteout(OVL_FS(dentry->d_sb), + indexdir, index); } else { /* Cleanup orphan index entries */ err = ovl_cleanup_unlocked(ofs, indexdir, index); @@ -1232,10 +1228,6 @@ int ovl_lock_rename_workdir(struct dentry *workdir, struct dentry *work, { struct dentry *trap; - /* Workdir should not be the same as upperdir */ - if (workdir == upperdir) - goto err; - /* Workdir should not be subdir of upperdir and vice versa */ trap = lock_rename(workdir, upperdir); if (IS_ERR(trap)) -- cgit v1.2.3 From 8afa0a736713898f04d52abad69c07caa2c2f227 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:29 +1000 Subject: ovl: narrow locking in ovl_whiteout() ovl_whiteout() relies on the workdir i_rwsem to provide exclusive access to ofs->whiteout which it manipulates. Rather than depending on this, add a new mutex, "whiteout_lock" to explicitly provide the required locking. Use guard(mutex) for this so that we can return without needing to explicitly unlock. Then take the lock on workdir only when needed - to lookup the temp name and to do the whiteout or link. Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-19-neil@brown.name Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/dir.c | 44 ++++++++++++++++++++++++-------------------- fs/overlayfs/ovl_entry.h | 1 + fs/overlayfs/params.c | 2 ++ 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 6a70faeee6fa..7eb806a4e5f8 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -84,41 +84,45 @@ static struct dentry *ovl_whiteout(struct ovl_fs *ofs) struct dentry *workdir = ofs->workdir; struct inode *wdir = workdir->d_inode; - inode_lock_nested(wdir, I_MUTEX_PARENT); + guard(mutex)(&ofs->whiteout_lock); + if (!ofs->whiteout) { + inode_lock_nested(wdir, I_MUTEX_PARENT); whiteout = ovl_lookup_temp(ofs, workdir); - if (IS_ERR(whiteout)) - goto out; - - err = ovl_do_whiteout(ofs, wdir, whiteout); - if (err) { - dput(whiteout); - whiteout = ERR_PTR(err); - goto out; + if (!IS_ERR(whiteout)) { + err = ovl_do_whiteout(ofs, wdir, whiteout); + if (err) { + dput(whiteout); + whiteout = ERR_PTR(err); + } } + inode_unlock(wdir); + if (IS_ERR(whiteout)) + return whiteout; ofs->whiteout = whiteout; } if (!ofs->no_shared_whiteout) { + inode_lock_nested(wdir, I_MUTEX_PARENT); whiteout = ovl_lookup_temp(ofs, workdir); - if (IS_ERR(whiteout)) - goto out; - - err = ovl_do_link(ofs, ofs->whiteout, wdir, whiteout); - if (!err) - goto out; - - if (err != -EMLINK) { + if (!IS_ERR(whiteout)) { + err = ovl_do_link(ofs, ofs->whiteout, wdir, whiteout); + if (err) { + dput(whiteout); + whiteout = ERR_PTR(err); + } + } + inode_unlock(wdir); + if (!IS_ERR(whiteout)) + return whiteout; + if (PTR_ERR(whiteout) != -EMLINK) { pr_warn("Failed to link whiteout - disabling whiteout inode sharing(nlink=%u, err=%i)\n", ofs->whiteout->d_inode->i_nlink, err); ofs->no_shared_whiteout = true; } - dput(whiteout); } whiteout = ofs->whiteout; ofs->whiteout = NULL; -out: - inode_unlock(wdir); return whiteout; } diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h index afb7762f873f..4c1bae935ced 100644 --- a/fs/overlayfs/ovl_entry.h +++ b/fs/overlayfs/ovl_entry.h @@ -88,6 +88,7 @@ struct ovl_fs { /* Shared whiteout cache */ struct dentry *whiteout; bool no_shared_whiteout; + struct mutex whiteout_lock; /* r/o snapshot of upperdir sb's only taken on volatile mounts */ errseq_t errseq; }; diff --git a/fs/overlayfs/params.c b/fs/overlayfs/params.c index 2b9b31524c38..f4e7fff909ac 100644 --- a/fs/overlayfs/params.c +++ b/fs/overlayfs/params.c @@ -795,6 +795,8 @@ int ovl_init_fs_context(struct fs_context *fc) fc->s_fs_info = ofs; fc->fs_private = ctx; fc->ops = &ovl_context_ops; + + mutex_init(&ofs->whiteout_lock); return 0; out_err: -- cgit v1.2.3 From 09d56cc88c247036da5756fd552f2cb2af8d8c5e Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:30 +1000 Subject: ovl: narrow locking in ovl_check_rename_whiteout() ovl_check_rename_whiteout() now only holds the directory lock when needed, and takes it again if necessary. This makes way for future changes where locks are taken on individual dentries rather than the whole directory. Reviewed-by: Amir Goldstein Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-20-neil@brown.name Signed-off-by: Christian Brauner --- fs/overlayfs/super.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 4c3736bf2db4..0d765aa66bd2 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -556,7 +556,6 @@ out: static int ovl_check_rename_whiteout(struct ovl_fs *ofs) { struct dentry *workdir = ofs->workdir; - struct inode *dir = d_inode(workdir); struct dentry *temp; struct dentry *dest; struct dentry *whiteout; @@ -577,19 +576,22 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs) err = PTR_ERR(dest); if (IS_ERR(dest)) { dput(temp); - goto out_unlock; + ovl_parent_unlock(workdir); + return err; } /* Name is inline and stable - using snapshot as a copy helper */ take_dentry_name_snapshot(&name, temp); err = ovl_do_rename(ofs, workdir, temp, workdir, dest, RENAME_WHITEOUT); + ovl_parent_unlock(workdir); if (err) { if (err == -EINVAL) err = 0; goto cleanup_temp; } - whiteout = ovl_lookup_upper(ofs, name.name.name, workdir, name.name.len); + whiteout = ovl_lookup_upper_unlocked(ofs, name.name.name, + workdir, name.name.len); err = PTR_ERR(whiteout); if (IS_ERR(whiteout)) goto cleanup_temp; @@ -598,18 +600,15 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs) /* Best effort cleanup of whiteout and temp file */ if (err) - ovl_cleanup(ofs, dir, whiteout); + ovl_cleanup_unlocked(ofs, workdir, whiteout); dput(whiteout); cleanup_temp: - ovl_cleanup(ofs, dir, temp); + ovl_cleanup_unlocked(ofs, workdir, temp); release_dentry_name_snapshot(&name); dput(temp); dput(dest); -out_unlock: - ovl_parent_unlock(workdir); - return err; } -- cgit v1.2.3 From ee37c3cfc5df9013dadf42919ca65510abc15632 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:31 +1000 Subject: ovl: change ovl_create_real() to receive dentry parent Instead of passing an inode *dir, pass a dentry *parent. This makes the calling slightly cleaner. Reviewed-by: Amir Goldstein Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-21-neil@brown.name Signed-off-by: Christian Brauner --- fs/overlayfs/dir.c | 7 ++++--- fs/overlayfs/overlayfs.h | 2 +- fs/overlayfs/super.c | 3 +-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index 7eb806a4e5f8..dedf89546e5f 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -160,9 +160,10 @@ kill_whiteout: goto out; } -struct dentry *ovl_create_real(struct ovl_fs *ofs, struct inode *dir, +struct dentry *ovl_create_real(struct ovl_fs *ofs, struct dentry *parent, struct dentry *newdentry, struct ovl_cattr *attr) { + struct inode *dir = parent->d_inode; int err; if (IS_ERR(newdentry)) @@ -223,7 +224,7 @@ struct dentry *ovl_create_temp(struct ovl_fs *ofs, struct dentry *workdir, { struct dentry *ret; inode_lock(workdir->d_inode); - ret = ovl_create_real(ofs, d_inode(workdir), + ret = ovl_create_real(ofs, workdir, ovl_lookup_temp(ofs, workdir), attr); inode_unlock(workdir->d_inode); return ret; @@ -329,7 +330,7 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode, int err; inode_lock_nested(udir, I_MUTEX_PARENT); - newdentry = ovl_create_real(ofs, udir, + newdentry = ovl_create_real(ofs, upperdir, ovl_lookup_upper(ofs, dentry->d_name.name, upperdir, dentry->d_name.len), attr); diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index df674a12f498..4aec0d7aa2dd 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -859,7 +859,7 @@ struct ovl_cattr { #define OVL_CATTR(m) (&(struct ovl_cattr) { .mode = (m) }) struct dentry *ovl_create_real(struct ovl_fs *ofs, - struct inode *dir, struct dentry *newdentry, + struct dentry *parent, struct dentry *newdentry, struct ovl_cattr *attr); int ovl_cleanup(struct ovl_fs *ofs, struct inode *dir, struct dentry *dentry); int ovl_cleanup_unlocked(struct ovl_fs *ofs, struct dentry *workdir, struct dentry *dentry); diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 0d765aa66bd2..9fd0b3acd1a4 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -622,8 +622,7 @@ static struct dentry *ovl_lookup_or_create(struct ovl_fs *ofs, inode_lock_nested(parent->d_inode, I_MUTEX_PARENT); child = ovl_lookup_upper(ofs, name, parent, len); if (!IS_ERR(child) && !child->d_inode) - child = ovl_create_real(ofs, parent->d_inode, child, - OVL_CATTR(mode)); + child = ovl_create_real(ofs, parent, child, OVL_CATTR(mode)); inode_unlock(parent->d_inode); dput(parent); -- cgit v1.2.3 From fe4d3360f9cbb513be6d74bdeb154728cad5c437 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 16 Jul 2025 10:44:32 +1000 Subject: ovl: rename ovl_cleanup_unlocked() to ovl_cleanup() The only remaining user of ovl_cleanup() is ovl_cleanup_locked(), so we no longer need both. This patch renames ovl_cleanup() to ovl_cleanup_locked() and makes it static. ovl_cleanup_unlocked() is renamed to ovl_cleanup(). Signed-off-by: NeilBrown Link: https://lore.kernel.org/20250716004725.1206467-22-neil@brown.name Reviewed-by: Amir Goldstein Signed-off-by: Christian Brauner --- fs/overlayfs/copy_up.c | 4 ++-- fs/overlayfs/dir.c | 27 ++++++++++++++------------- fs/overlayfs/overlayfs.h | 3 +-- fs/overlayfs/readdir.c | 10 +++++----- fs/overlayfs/super.c | 4 ++-- fs/overlayfs/util.c | 2 +- 6 files changed, 25 insertions(+), 25 deletions(-) diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c index 8f8dbe8a1d54..c4d7c281d473 100644 --- a/fs/overlayfs/copy_up.c +++ b/fs/overlayfs/copy_up.c @@ -569,7 +569,7 @@ static int ovl_create_index(struct dentry *dentry, const struct ovl_fh *fh, ovl_parent_unlock(indexdir); out: if (err) - ovl_cleanup_unlocked(ofs, indexdir, temp); + ovl_cleanup(ofs, indexdir, temp); dput(temp); free_name: kfree(name.name); @@ -854,7 +854,7 @@ out: cleanup: unlock_rename(c->workdir, c->destdir); cleanup_unlocked: - ovl_cleanup_unlocked(ofs, c->workdir, temp); + ovl_cleanup(ofs, c->workdir, temp); dput(temp); goto out; } diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index dedf89546e5f..30619777f0f6 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -24,7 +24,8 @@ MODULE_PARM_DESC(redirect_max, static int ovl_set_redirect(struct dentry *dentry, bool samedir); -int ovl_cleanup(struct ovl_fs *ofs, struct inode *wdir, struct dentry *wdentry) +static int ovl_cleanup_locked(struct ovl_fs *ofs, struct inode *wdir, + struct dentry *wdentry) { int err; @@ -43,8 +44,8 @@ int ovl_cleanup(struct ovl_fs *ofs, struct inode *wdir, struct dentry *wdentry) return err; } -int ovl_cleanup_unlocked(struct ovl_fs *ofs, struct dentry *workdir, - struct dentry *wdentry) +int ovl_cleanup(struct ovl_fs *ofs, struct dentry *workdir, + struct dentry *wdentry) { int err; @@ -52,7 +53,7 @@ int ovl_cleanup_unlocked(struct ovl_fs *ofs, struct dentry *workdir, if (err) return err; - ovl_cleanup(ofs, workdir->d_inode, wdentry); + ovl_cleanup_locked(ofs, workdir->d_inode, wdentry); ovl_parent_unlock(workdir); return 0; @@ -149,14 +150,14 @@ int ovl_cleanup_and_whiteout(struct ovl_fs *ofs, struct dentry *dir, if (err) goto kill_whiteout; if (flags) - ovl_cleanup_unlocked(ofs, ofs->workdir, dentry); + ovl_cleanup(ofs, ofs->workdir, dentry); out: dput(whiteout); return err; kill_whiteout: - ovl_cleanup_unlocked(ofs, ofs->workdir, whiteout); + ovl_cleanup(ofs, ofs->workdir, whiteout); goto out; } @@ -351,7 +352,7 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode, return 0; out_cleanup: - ovl_cleanup_unlocked(ofs, upperdir, newdentry); + ovl_cleanup(ofs, upperdir, newdentry); dput(newdentry); return err; } @@ -411,7 +412,7 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry, goto out_cleanup_unlocked; ovl_cleanup_whiteouts(ofs, upper, list); - ovl_cleanup_unlocked(ofs, workdir, upper); + ovl_cleanup(ofs, workdir, upper); /* dentry's upper doesn't match now, get rid of it */ d_drop(dentry); @@ -421,7 +422,7 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry, out_cleanup: unlock_rename(workdir, upperdir); out_cleanup_unlocked: - ovl_cleanup_unlocked(ofs, workdir, opaquedir); + ovl_cleanup(ofs, workdir, opaquedir); dput(opaquedir); out: return ERR_PTR(err); @@ -516,7 +517,7 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, if (err) goto out_cleanup_unlocked; - ovl_cleanup_unlocked(ofs, workdir, upper); + ovl_cleanup(ofs, workdir, upper); } else { err = ovl_do_rename(ofs, workdir, newdentry, upperdir, upper, 0); unlock_rename(workdir, upperdir); @@ -526,7 +527,7 @@ static int ovl_create_over_whiteout(struct dentry *dentry, struct inode *inode, ovl_dir_modified(dentry->d_parent, false); err = ovl_instantiate(dentry, inode, newdentry, hardlink, NULL); if (err) { - ovl_cleanup_unlocked(ofs, upperdir, newdentry); + ovl_cleanup(ofs, upperdir, newdentry); dput(newdentry); } out_dput: @@ -541,7 +542,7 @@ out: out_cleanup: unlock_rename(workdir, upperdir); out_cleanup_unlocked: - ovl_cleanup_unlocked(ofs, workdir, newdentry); + ovl_cleanup(ofs, workdir, newdentry); dput(newdentry); goto out_dput; } @@ -1268,7 +1269,7 @@ static int ovl_rename(struct mnt_idmap *idmap, struct inode *olddir, goto out_revert_creds; if (cleanup_whiteout) - ovl_cleanup_unlocked(ofs, old_upperdir, newdentry); + ovl_cleanup(ofs, old_upperdir, newdentry); if (overwrite && d_inode(new)) { if (new_is_dir) diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h index 4aec0d7aa2dd..ab8f72d33bdf 100644 --- a/fs/overlayfs/overlayfs.h +++ b/fs/overlayfs/overlayfs.h @@ -861,8 +861,7 @@ struct ovl_cattr { struct dentry *ovl_create_real(struct ovl_fs *ofs, struct dentry *parent, struct dentry *newdentry, struct ovl_cattr *attr); -int ovl_cleanup(struct ovl_fs *ofs, struct inode *dir, struct dentry *dentry); -int ovl_cleanup_unlocked(struct ovl_fs *ofs, struct dentry *workdir, struct dentry *dentry); +int ovl_cleanup(struct ovl_fs *ofs, struct dentry *workdir, struct dentry *dentry); struct dentry *ovl_lookup_temp(struct ovl_fs *ofs, struct dentry *workdir); struct dentry *ovl_create_temp(struct ovl_fs *ofs, struct dentry *workdir, struct ovl_cattr *attr); diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c index ecbf39a49d57..b65cdfce31ce 100644 --- a/fs/overlayfs/readdir.c +++ b/fs/overlayfs/readdir.c @@ -1048,7 +1048,7 @@ void ovl_cleanup_whiteouts(struct ovl_fs *ofs, struct dentry *upper, continue; } if (dentry->d_inode) - ovl_cleanup_unlocked(ofs, upper, dentry); + ovl_cleanup(ofs, upper, dentry); dput(dentry); } } @@ -1156,7 +1156,7 @@ int ovl_workdir_cleanup(struct ovl_fs *ofs, struct dentry *parent, int err; if (!d_is_dir(dentry) || level > 1) - return ovl_cleanup_unlocked(ofs, parent, dentry); + return ovl_cleanup(ofs, parent, dentry); err = ovl_parent_lock(parent, dentry); if (err) @@ -1168,7 +1168,7 @@ int ovl_workdir_cleanup(struct ovl_fs *ofs, struct dentry *parent, err = ovl_workdir_cleanup_recurse(ofs, &path, level + 1); if (!err) - err = ovl_cleanup_unlocked(ofs, parent, dentry); + err = ovl_cleanup(ofs, parent, dentry); } return err; @@ -1217,7 +1217,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) goto next; } else if (err == -ESTALE) { /* Cleanup stale index entries */ - err = ovl_cleanup_unlocked(ofs, indexdir, index); + err = ovl_cleanup(ofs, indexdir, index); } else if (err != -ENOENT) { /* * Abort mount to avoid corrupting the index if @@ -1233,7 +1233,7 @@ int ovl_indexdir_cleanup(struct ovl_fs *ofs) err = ovl_cleanup_and_whiteout(ofs, indexdir, index); } else { /* Cleanup orphan index entries */ - err = ovl_cleanup_unlocked(ofs, indexdir, index); + err = ovl_cleanup(ofs, indexdir, index); } if (err) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 9fd0b3acd1a4..4afa91882075 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -600,11 +600,11 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs) /* Best effort cleanup of whiteout and temp file */ if (err) - ovl_cleanup_unlocked(ofs, workdir, whiteout); + ovl_cleanup(ofs, workdir, whiteout); dput(whiteout); cleanup_temp: - ovl_cleanup_unlocked(ofs, workdir, temp); + ovl_cleanup(ofs, workdir, temp); release_dentry_name_snapshot(&name); dput(temp); dput(dest); diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index a63fda9ef6fd..71674b633bc4 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -1123,7 +1123,7 @@ static void ovl_cleanup_index(struct dentry *dentry) indexdir, index); } else { /* Cleanup orphan index entries */ - err = ovl_cleanup_unlocked(ofs, indexdir, index); + err = ovl_cleanup(ofs, indexdir, index); } if (err) goto fail; -- cgit v1.2.3