Re: [PATCH 3/9] btrfs: send: use btrfs_file_extent_end() in send_write_or_clone()

[Filipe Manana <fdmanana@gmail.com>]

On Fri, Aug 21, 2020 at 8:42 AM Omar Sandoval <osandov@osandov.com> wrote:
>
> From: Omar Sandoval <osandov@fb.com>
>
> send_write_or_clone() basically has an open-coded copy of
> btrfs_file_extent_end() except that it (incorrectly) aligns to PAGE_SIZE
> instead of sectorsize. Fix and simplify the code by using
> btrfs_file_extent_end().
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Looks good, and it passed some long duration tests with both full and
incremental sends here (with and without compression, no-holes, etc).

Thanks.

> ---
>  fs/btrfs/send.c | 44 +++++++++++---------------------------------
>  1 file changed, 11 insertions(+), 33 deletions(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index e70f5ceb3261..37ce21361782 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -5400,51 +5400,29 @@ static int send_write_or_clone(struct send_ctx *sctx,
>                                struct clone_root *clone_root)
>  {
>         int ret = 0;
> -       struct btrfs_file_extent_item *ei;
>         u64 offset = key->offset;
> -       u64 len;
> -       u8 type;
> +       u64 end;
>         u64 bs = sctx->send_root->fs_info->sb->s_blocksize;
>
> -       ei = btrfs_item_ptr(path->nodes[0], path->slots[0],
> -                       struct btrfs_file_extent_item);
> -       type = btrfs_file_extent_type(path->nodes[0], ei);
> -       if (type == BTRFS_FILE_EXTENT_INLINE) {
> -               len = btrfs_file_extent_ram_bytes(path->nodes[0], ei);
> -               /*
> -                * it is possible the inline item won't cover the whole page,
> -                * but there may be items after this page.  Make
> -                * sure to send the whole thing
> -                */
> -               len = PAGE_ALIGN(len);
> -       } else {
> -               len = btrfs_file_extent_num_bytes(path->nodes[0], ei);
> -       }
> -
> -       if (offset >= sctx->cur_inode_size) {
> -               ret = 0;
> -               goto out;
> -       }
> -       if (offset + len > sctx->cur_inode_size)
> -               len = sctx->cur_inode_size - offset;
> -       if (len == 0) {
> -               ret = 0;
> -               goto out;
> -       }
> +       end = min(btrfs_file_extent_end(path), sctx->cur_inode_size);
> +       if (offset >= end)
> +               return 0;
>
> -       if (clone_root && IS_ALIGNED(offset + len, bs)) {
> +       if (clone_root && IS_ALIGNED(end, bs)) {
> +               struct btrfs_file_extent_item *ei;
>                 u64 disk_byte;
>                 u64 data_offset;
>
> +               ei = btrfs_item_ptr(path->nodes[0], path->slots[0],
> +                                   struct btrfs_file_extent_item);
>                 disk_byte = btrfs_file_extent_disk_bytenr(path->nodes[0], ei);
>                 data_offset = btrfs_file_extent_offset(path->nodes[0], ei);
>                 ret = clone_range(sctx, clone_root, disk_byte, data_offset,
> -                                 offset, len);
> +                                 offset, end - offset);
>         } else {
> -               ret = send_extent_data(sctx, offset, len);
> +               ret = send_extent_data(sctx, offset, end - offset);
>         }
> -       sctx->cur_inode_next_write_offset = offset + len;
> -out:
> +       sctx->cur_inode_next_write_offset = end;
>         return ret;
>  }
>
> --
> 2.28.0
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”