On Wed, Jul 2, 2025 at 4:41 AM NeilBrown <neil@xxxxxxxxxx> wrote:
>
> On Thu, 26 Jun 2025, Amir Goldstein wrote:
> > On Wed, Jun 25, 2025 at 1:07 AM NeilBrown <neil@xxxxxxxxxx> wrote:
> > >
> > > 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.
> > >
> > > Signed-off-by: NeilBrown <neil@xxxxxxxxxx>
> > > ---
> > > fs/overlayfs/super.c | 16 ++++++++--------
> > > 1 file changed, 8 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> > > index 3583e359655f..8331667b8101 100644
> > > --- a/fs/overlayfs/super.c
> > > +++ b/fs/overlayfs/super.c
> > > @@ -554,7 +554,6 @@ static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs,
> > > 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;
> > > @@ -571,19 +570,22 @@ static int ovl_check_rename_whiteout(struct ovl_fs *ofs)
> > > err = PTR_ERR(dest);
> > > if (IS_ERR(dest)) {
> > > dput(temp);
> > > - goto out_unlock;
> > > + unlock_rename(workdir, workdir);
> > > + goto out;
> >
> > dont use unlock_rename hack please
>
> The lock was taken for the purpose of doing a rename. So using
> lock_rename and unlock_rename documents that.
IMO this is not a good excuse for using lock_rename() here
> I can use the less informative "inode_lock" if you prefer.
>
I meant that you should use the new helper that I proposed
in review of patch #2, lock_parent(workdir, temp)
instead of the weird looking lock_rename(workdir, workdir)
BTW, I see that lock_rename_child() effectively has
lock_parent() code in its first part, so maybe factor out this code
as lock_parent().
Thanks,
Amir.
