Re: [PATCH v2] ovl: improving copy-up efficiency for big sparse file

[Vivek Goyal <vgoyal@redhat.com>]

On Thu, Oct 31, 2019 at 08:53:15AM +0200, Amir Goldstein wrote:
[..]
> >  > > +
> >  > > +       if (!error && set_size) {
> >  > > +               inode_lock(new->dentry->d_inode);
> >  > > +               error = ovl_set_size(new->dentry, stat);
> >  > > +               inode_unlock(new->dentry->d_inode);
> >  > > +       }
> >  >
> >  > I see no reason to repeat this code here.
> >  > Two options:
> >  > 1. always set_size at the end of ovl_copy_up_inode()
> >  >     what's the harm in that?
> >
> > I think at least it's not suitable for directory.
> >
> >
> >  > 2. set boolean c->set_size here and check it at the end
> >  >     of ovl_copy_up_inode() instead of checking c->metacopy
> >  >
> >
> > I don't understand why 'c->set_size' can replace 'c->metacopy',
> >
> 
> I did not explain myself well.
> 
> This should be enough IMO:
> 
> @@ -483,7 +483,7 @@ static int ovl_copy_up_inode(struct
> ovl_copy_up_ctx *c, struct dentry *temp)
>         }
> 
>         inode_lock(temp->d_inode);
> -       if (c->metacopy)
> +       if (S_ISREG(c->stat.mode))
>                 err = ovl_set_size(temp, &c->stat);

Hi Amir,

Why do we need this change. c->metacopy is set only for regular files.

ovl_need_meta_copy_up() {
        if (!S_ISREG(mode))
                return false;
}

Even if there is a reason, this change should be part of a separate patch.
What connection does it have to skip holes while copying up.

Thanks
Vivek