On Wed, Jun 25, 2025 at 1:07 AM NeilBrown <neil@xxxxxxxxxx> wrote:
>
> In ovl_copy_up_workdir() unlock immediately after the rename, and then
> use ovl_cleanup_unlocked() with separate locking rather than using the
> same lock to protect both.
>
> This makes way for future changes where locks are taken on individual
> dentries rather than the whole directory.
>
> Signed-off-by: NeilBrown <neil@xxxxxxxxxx>
FYI I am not reviewing this one because the code was made too much
spaghetti to my taste by patch 2, so I will wait for a better version
> ---
> fs/overlayfs/copy_up.c | 25 ++++++++++---------------
> 1 file changed, 10 insertions(+), 15 deletions(-)
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 7a21ad1f2b6e..884c738b67ff 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -763,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;
> @@ -793,8 +792,10 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
> */
> path.dentry = temp;
> err = ovl_copy_up_data(c, &path);
> - if (err)
> - goto cleanup_write_unlocked;
> + if (err) {
> + ovl_start_write(c->dentry);
> + 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
> @@ -814,9 +815,9 @@ static int ovl_copy_up_workdir(struct ovl_copy_up_ctx *c)
> /* temp or workdir moved underneath us? abort without cleanup */
> dput(temp);
> err = -EIO;
> - if (IS_ERR(trap))
> - goto out;
> - goto unlock;
> + if (!IS_ERR(trap))
> + unlock_rename(c->workdir, c->destdir);
> + goto out;
> }
>
> err = ovl_copy_up_metadata(c, temp);
> @@ -830,9 +831,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)
> @@ -846,20 +848,13 @@ 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);
>
> return err;
>
> cleanup:
> - ovl_cleanup(ofs, wdir, temp);
> - dput(temp);
> - goto unlock;
> -
> -cleanup_write_unlocked:
> - ovl_start_write(c->dentry);
> + unlock_rename(c->workdir, c->destdir);
> cleanup_unlocked:
> ovl_cleanup_unlocked(ofs, c->workdir, temp);
> dput(temp);
> --
> 2.49.0
>
