On Fri, 11 Jul 2025, Amir Goldstein wrote:
> On Fri, Jul 11, 2025 at 1:21 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>
> > ---
> > fs/overlayfs/copy_up.c | 18 +++++++++---------
> > 1 file changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> > index eafb46686854..7b84a39c081f 100644
> > --- a/fs/overlayfs/copy_up.c
> > +++ b/fs/overlayfs/copy_up.c
> > @@ -765,7 +765,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;
> > @@ -816,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;
>
> I now see that this bit was missing from my proposed patch 1
> variant, but with this in patch 1, this patch becomes trivial.
I missed that too :-)
As you say - nice and trivial here now.
NeilBrown
>
> Thanks,
> Amir.
>
> > }
> >
> > err = ovl_copy_up_metadata(c, temp);
> > @@ -832,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)
> > @@ -848,17 +848,17 @@ 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);
> > + unlock_rename(c->workdir, c->destdir);
> > +cleanup_unlocked:
> > + ovl_cleanup_unlocked(ofs, c->workdir, temp);
> > dput(temp);
> > - goto unlock;
> > + goto out;
> >
> > cleanup_need_write:
> > ovl_start_write(c->dentry);
> > --
> > 2.49.0
> >
>
