On Wed, Jun 25, 2025 at 7:55 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> On Wed, Jun 25, 2025 at 1:07 AM NeilBrown <neil@xxxxxxxxxx> wrote:
> >
> > 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.
> >
> > Signed-off-by: NeilBrown <neil@xxxxxxxxxx>
> > ---
> > fs/overlayfs/dir.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> > index a51a3dc02bf5..2d67704d641e 100644
> > --- a/fs/overlayfs/dir.c
> > +++ b/fs/overlayfs/dir.c
> > @@ -326,9 +326,10 @@ 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);
> > + inode_unlock(udir);
> > err = PTR_ERR(newdentry);
> > if (IS_ERR(newdentry))
> > - goto out_unlock;
> > + goto out;
> >
> > if (ovl_type_merge(dentry->d_parent) && d_is_dir(newdentry) &&
> > !ovl_allow_offline_changes(ofs)) {
> > @@ -340,14 +341,13 @@ static int ovl_create_upper(struct dentry *dentry, struct inode *inode,
>
> > ovl_dir_modified(dentry->d_parent, false);
>
> inside ovl_dir_modified() =>ovl_dir_version_inc() there is:
> WARN_ON(!inode_is_locked(inode));
>
> so why is this WARN_ON not triggered by this change?
> either there are more changes that fix it later,
> or your tests did not cover this (seems unlikely)
> or you did not look in dmesg and overlay fstests do not check for it?
> some other explanation?
>
The latter - the assertion is on the ovl dir inode lock and you dropped
the upper dir inode lock.
Feel free to add:
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
Thanks,
Amir.
