linux-unionfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Amir Goldstein <amir73il@gmail.com>
To: Chengguang Xu <cgxu519@mykernel.net>
Cc: Miklos Szeredi <miklos@szeredi.hu>,
	overlayfs <linux-unionfs@vger.kernel.org>
Subject: Re: [PATCH] ovl: copy-up optimization for truncate
Date: Mon, 8 Mar 2021 14:09:37 +0200	[thread overview]
Message-ID: <CAOQ4uxjsQ90D5ts=bwOA+KZ8m5ycdiyH49c2g+WLOnJXSa9NvA@mail.gmail.com> (raw)
In-Reply-To: <20210308111717.2027030-1-cgxu519@mykernel.net>

On Mon, Mar 8, 2021 at 1:25 PM Chengguang Xu <cgxu519@mykernel.net> wrote:
>
> Currently copy-up will copy whole lower file to upper
> regardless of the data range which is needed for further
> operation. This patch avoids unnecessary copy when truncate
> size is smaller than the file size.
>
> Signed-off-by: Chengguang Xu <cgxu519@mykernel.net>

Nice!

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

This reminds me. I think this comment in ovl_nlink_start() can be removed:

         * TODO: implement metadata only index copy up when called with
         *       ovl_copy_up_flags(dentry, O_PATH).

It was added in 2017 and not removed when metacopy was implemented.
Technically, I think that truncate() could also be a metacopy, but it seems
like a corner case that is not worth the trouble...

Thanks,
Amir.


> ---
>  fs/overlayfs/copy_up.c   | 16 +++++++++-------
>  fs/overlayfs/inode.c     |  4 +++-
>  fs/overlayfs/overlayfs.h |  2 +-
>  3 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 0b2891c6c71e..b426a3f59636 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -866,7 +866,7 @@ static int ovl_copy_up_meta_inode_data(struct ovl_copy_up_ctx *c)
>  }
>
>  static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
> -                          int flags)
> +                          int flags, loff_t size)
>  {
>         int err;
>         DEFINE_DELAYED_CALL(done);
> @@ -903,6 +903,8 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
>         /* maybe truncate regular file. this has no effect on dirs */
>         if (flags & O_TRUNC)
>                 ctx.stat.size = 0;
> +       if (size)
> +               ctx.stat.size = size;
>
>         if (S_ISLNK(ctx.stat.mode)) {
>                 ctx.link = vfs_get_link(ctx.lowerpath.dentry, &done);
> @@ -929,7 +931,7 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry,
>         return err;
>  }
>
> -static int ovl_copy_up_flags(struct dentry *dentry, int flags)
> +static int ovl_copy_up_flags(struct dentry *dentry, int flags, loff_t size)
>  {
>         int err = 0;
>         const struct cred *old_cred = ovl_override_creds(dentry->d_sb);
> @@ -962,7 +964,7 @@ static int ovl_copy_up_flags(struct dentry *dentry, int flags)
>                         next = parent;
>                 }
>
> -               err = ovl_copy_up_one(parent, next, flags);
> +               err = ovl_copy_up_one(parent, next, flags, size);
>
>                 dput(parent);
>                 dput(next);
> @@ -994,7 +996,7 @@ int ovl_maybe_copy_up(struct dentry *dentry, int flags)
>         if (ovl_open_need_copy_up(dentry, flags)) {
>                 err = ovl_want_write(dentry);
>                 if (!err) {
> -                       err = ovl_copy_up_flags(dentry, flags);
> +                       err = ovl_copy_up_flags(dentry, flags, 0);
>                         ovl_drop_write(dentry);
>                 }
>         }
> @@ -1002,12 +1004,12 @@ int ovl_maybe_copy_up(struct dentry *dentry, int flags)
>         return err;
>  }
>
> -int ovl_copy_up_with_data(struct dentry *dentry)
> +int ovl_copy_up_with_data(struct dentry *dentry, loff_t size)
>  {
> -       return ovl_copy_up_flags(dentry, O_WRONLY);
> +       return ovl_copy_up_flags(dentry, O_WRONLY, size);
>  }
>
>  int ovl_copy_up(struct dentry *dentry)
>  {
> -       return ovl_copy_up_flags(dentry, 0);
> +       return ovl_copy_up_flags(dentry, 0, 0);
>  }
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 003cf83bf78a..5eb99e4c3c73 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -44,7 +44,9 @@ int ovl_setattr(struct user_namespace *mnt_userns, struct dentry *dentry,
>         if (!full_copy_up)
>                 err = ovl_copy_up(dentry);
>         else
> -               err = ovl_copy_up_with_data(dentry);
> +               err = ovl_copy_up_with_data(dentry,
> +                               attr->ia_size < i_size_read(d_inode(dentry)) ?
> +                               attr->ia_size : 0);
>         if (!err) {
>                 struct inode *winode = NULL;
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 95cff83786a5..1bc17ca87158 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -524,7 +524,7 @@ long ovl_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
>
>  /* copy_up.c */
>  int ovl_copy_up(struct dentry *dentry);
> -int ovl_copy_up_with_data(struct dentry *dentry);
> +int ovl_copy_up_with_data(struct dentry *dentry, loff_t size);
>  int ovl_maybe_copy_up(struct dentry *dentry, int flags);
>  int ovl_copy_xattr(struct super_block *sb, struct dentry *old,
>                    struct dentry *new);
> --
> 2.27.0
>
>

  reply	other threads:[~2021-03-08 12:10 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-08 11:17 [PATCH] ovl: copy-up optimization for truncate Chengguang Xu
2021-03-08 12:09 ` Amir Goldstein [this message]
2021-03-29 15:13 ` Miklos Szeredi
2021-03-30  7:17   ` Chengguang Xu
2021-04-01 11:15   ` Chengguang Xu
2021-04-01 11:31     ` Miklos Szeredi
2021-04-01 12:39       ` Chengguang Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAOQ4uxjsQ90D5ts=bwOA+KZ8m5ycdiyH49c2g+WLOnJXSa9NvA@mail.gmail.com' \
    --to=amir73il@gmail.com \
    --cc=cgxu519@mykernel.net \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).