Re: [PATCH 4/5] ovl: prepare for lazy lookup of lowerdata inode

[Alexander Larsson <alexl@redhat.com>]

On Wed, 2023-04-12 at 16:54 +0300, Amir Goldstein wrote:
> Make the code handle the case of numlower > 1 and missing lowerdata
> dentry gracefully.
> 
> Missing lowerdata dentry is an indication for lazy lookup of
> lowerdata
> and in that case the lowerdata_redirect path is stored in ovl_inode.
> 
> Following commits will defer lookup and perform the lazy lookup on
> acccess.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Reviewed-by: Alexander Larsson <alexl@redhat.com>

> ---
>  fs/overlayfs/export.c |  2 +-
>  fs/overlayfs/file.c   |  7 +++++++
>  fs/overlayfs/inode.c  | 18 ++++++++++++++----
>  fs/overlayfs/super.c  |  3 +++
>  fs/overlayfs/util.c   |  2 +-
>  5 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
> index 9951c504fb8d..2498fa8311e3 100644
> --- a/fs/overlayfs/export.c
> +++ b/fs/overlayfs/export.c
> @@ -343,7 +343,7 @@ static struct dentry *ovl_dentry_real_at(struct
> dentry *dentry, int idx)
>         if (!idx)
>                 return ovl_dentry_upper(dentry);
>  
> -       for (i = 0; i < ovl_numlower(oe); i++) {
> +       for (i = 0; i < ovl_numlower(oe) && lowerstack[i].layer; i++)
> {
>                 if (lowerstack[i].layer->idx == idx)
>                         return lowerstack[i].dentry;
>         }
> diff --git a/fs/overlayfs/file.c b/fs/overlayfs/file.c
> index 7c04f033aadd..951683a66ff6 100644
> --- a/fs/overlayfs/file.c
> +++ b/fs/overlayfs/file.c
> @@ -115,6 +115,9 @@ static int ovl_real_fdget_meta(const struct file
> *file, struct fd *real,
>                 ovl_path_real(dentry, &realpath);
>         else
>                 ovl_path_realdata(dentry, &realpath);
> +       /* TODO: lazy lookup of lowerdata */
> +       if (!realpath.dentry)
> +               return -EIO;
>  
>         /* Has it been copied up since we'd opened it? */
>         if (unlikely(file_inode(real->file) !=
> d_inode(realpath.dentry))) {
> @@ -158,6 +161,10 @@ static int ovl_open(struct inode *inode, struct
> file *file)
>         file->f_flags &= ~(O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC);
>  
>         ovl_path_realdata(dentry, &realpath);
> +       /* TODO: lazy lookup of lowerdata */
> +       if (!realpath.dentry)
> +               return -EIO;
> +
>         realfile = ovl_open_realfile(file, &realpath);
>         if (IS_ERR(realfile))
>                 return PTR_ERR(realfile);
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 35d51a6dced7..c29cbd9db64a 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -240,15 +240,22 @@ int ovl_getattr(struct mnt_idmap *idmap, const
> struct path *path,
>                         /*
>                          * If lower is not same as lowerdata or if
> there was
>                          * no origin on upper, we can end up here.
> +                        * With lazy lowerdata lookup, guess
> lowerdata blocks
> +                        * from size to avoid lowerdata lookup on
> stat(2).
>                          */
>                         struct kstat lowerdatastat;
>                         u32 lowermask = STATX_BLOCKS;
>  
>                         ovl_path_lowerdata(dentry, &realpath);
> -                       err = vfs_getattr(&realpath, &lowerdatastat,
> -                                         lowermask, flags);
> -                       if (err)
> -                               goto out;
> +                       if (realpath.dentry) {
> +                               err = vfs_getattr(&realpath,
> &lowerdatastat,
> +                                                 lowermask, flags);
> +                               if (err)
> +                                       goto out;
> +                       } else {
> +                               lowerdatastat.blocks =
> +                                       round_up(stat->size, stat-
> >blksize) >> 9;
> +                       }
>                         stat->blocks = lowerdatastat.blocks;
>                 }
>         }
> @@ -710,6 +717,9 @@ static int ovl_fiemap(struct inode *inode, struct
> fiemap_extent_info *fieinfo,
>         struct inode *realinode = ovl_inode_realdata(inode);
>         const struct cred *old_cred;
>  
> +       if (!realinode)
> +               return -EIO;
> +
>
>         if (!realinode->i_op->fiemap)
>                 return -EOPNOTSUPP;
>  
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 3484f39a8f27..ef78abc21998 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -103,6 +103,9 @@ static int ovl_revalidate_real(struct dentry *d,
> unsigned int flags, bool weak)
>  {
>         int ret = 1;
>  
> +       if (!d)
> +               return 1;
> +
>         if (weak) {
>                 if (d->d_flags & DCACHE_OP_WEAK_REVALIDATE)
>                         ret =  d->d_op->d_weak_revalidate(d, flags);
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index fe2e5a8b216b..284b5ba4fcf6 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -179,7 +179,7 @@ void ovl_dentry_init_flags(struct dentry *dentry,
> struct dentry *upperdentry,
>  
>         if (upperdentry)
>                 flags |= upperdentry->d_flags;
> -       for (i = 0; i < ovl_numlower(oe); i++)
> +       for (i = 0; i < ovl_numlower(oe) && lowerstack[i].dentry;
> i++)
>                 flags |= lowerstack[i].dentry->d_flags;
>  
>         spin_lock(&dentry->d_lock);

-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
=-=-=
 Alexander Larsson                                            Red Hat,
Inc 
       alexl@redhat.com            alexander.larsson@gmail.com 
He's a time-tossed Amish vagrant who knows the secret of the alien 
invasion. She's a high-kicking gypsy barmaid from a family of eight
older 
brothers. They fight crime!