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. I can use the less
> informative "inode_lock" if you prefer.
>
> > and why not return err?
>
> Some people like to only have a single "return" in a function. Some are
> comfortable with more. I guess I wasn't sure where you stood.
>
Generally, I think we need to move toward more scoped variables and
less mult-goto-labels in ovl code, which are a common source of bugs,
but I will not require this work from you of course.
Thanks,
Amir.
