diff mbox series

[1/9] btrfs: send: get rid of i_size logic in send_write()

Message ID 16f28691ff66e8aeb280532cc146c8ee49d6cda4.1597994106.git.osandov@osandov.com (mailing list archive)
State New, archived
Headers show
Series btrfs: implement send/receive of compressed extents without decompressing | expand

Commit Message

Omar Sandoval Aug. 21, 2020, 7:39 a.m. UTC 
From: Omar Sandoval <osandov@fb.com>

send_write()/fill_read_buf() have some logic for avoiding reading past
i_size. However, everywhere that we call
send_write()/send_extent_data(), we've already clamped the length down
to i_size. Get rid of the i_size handling, which simplifies the next
change.

Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 fs/btrfs/send.c | 37 ++++++++++---------------------------
 1 file changed, 10 insertions(+), 27 deletions(-)

Comments

Filipe Manana Aug. 21, 2020, 5:26 p.m. UTC | #1 
On Fri, Aug 21, 2020 at 8:42 AM Omar Sandoval <osandov@osandov.com> wrote:
>
> From: Omar Sandoval <osandov@fb.com>
>
> send_write()/fill_read_buf() have some logic for avoiding reading past
> i_size. However, everywhere that we call
> send_write()/send_extent_data(), we've already clamped the length down
> to i_size. Get rid of the i_size handling, which simplifies the next
> change.
>
> 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 | 37 ++++++++++---------------------------
>  1 file changed, 10 insertions(+), 27 deletions(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 7c7c09fc65e8..8af5e867e4ca 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -4794,7 +4794,7 @@ static int process_all_new_xattrs(struct send_ctx *sctx)
>         return ret;
>  }
>
> -static ssize_t fill_read_buf(struct send_ctx *sctx, u64 offset, u32 len)
> +static int fill_read_buf(struct send_ctx *sctx, u64 offset, u32 len)
>  {
>         struct btrfs_root *root = sctx->send_root;
>         struct btrfs_fs_info *fs_info = root->fs_info;
> @@ -4804,21 +4804,13 @@ static ssize_t fill_read_buf(struct send_ctx *sctx, u64 offset, u32 len)
>         pgoff_t index = offset >> PAGE_SHIFT;
>         pgoff_t last_index;
>         unsigned pg_offset = offset_in_page(offset);
> -       ssize_t ret = 0;
> +       int ret = 0;
> +       size_t read = 0;
>
>         inode = btrfs_iget(fs_info->sb, sctx->cur_ino, root);
>         if (IS_ERR(inode))
>                 return PTR_ERR(inode);
>
> -       if (offset + len > i_size_read(inode)) {
> -               if (offset > i_size_read(inode))
> -                       len = 0;
> -               else
> -                       len = offset - i_size_read(inode);
> -       }
> -       if (len == 0)
> -               goto out;
> -
>         last_index = (offset + len - 1) >> PAGE_SHIFT;
>
>         /* initial readahead */
> @@ -4859,16 +4851,15 @@ static ssize_t fill_read_buf(struct send_ctx *sctx, u64 offset, u32 len)
>                 }
>
>                 addr = kmap(page);
> -               memcpy(sctx->read_buf + ret, addr + pg_offset, cur_len);
> +               memcpy(sctx->read_buf + read, addr + pg_offset, cur_len);
>                 kunmap(page);
>                 unlock_page(page);
>                 put_page(page);
>                 index++;
>                 pg_offset = 0;
>                 len -= cur_len;
> -               ret += cur_len;
> +               read += cur_len;
>         }
> -out:
>         iput(inode);
>         return ret;
>  }
> @@ -4882,7 +4873,6 @@ static int send_write(struct send_ctx *sctx, u64 offset, u32 len)
>         struct btrfs_fs_info *fs_info = sctx->send_root->fs_info;
>         int ret = 0;
>         struct fs_path *p;
> -       ssize_t num_read = 0;
>
>         p = fs_path_alloc();
>         if (!p)
> @@ -4890,12 +4880,9 @@ static int send_write(struct send_ctx *sctx, u64 offset, u32 len)
>
>         btrfs_debug(fs_info, "send_write offset=%llu, len=%d", offset, len);
>
> -       num_read = fill_read_buf(sctx, offset, len);
> -       if (num_read <= 0) {
> -               if (num_read < 0)
> -                       ret = num_read;
> +       ret = fill_read_buf(sctx, offset, len);
> +       if (ret < 0)
>                 goto out;
> -       }
>
>         ret = begin_cmd(sctx, BTRFS_SEND_C_WRITE);
>         if (ret < 0)
> @@ -4907,16 +4894,14 @@ static int send_write(struct send_ctx *sctx, u64 offset, u32 len)
>
>         TLV_PUT_PATH(sctx, BTRFS_SEND_A_PATH, p);
>         TLV_PUT_U64(sctx, BTRFS_SEND_A_FILE_OFFSET, offset);
> -       TLV_PUT(sctx, BTRFS_SEND_A_DATA, sctx->read_buf, num_read);
> +       TLV_PUT(sctx, BTRFS_SEND_A_DATA, sctx->read_buf, len);
>
>         ret = send_cmd(sctx);
>
>  tlv_put_failure:
>  out:
>         fs_path_free(p);
> -       if (ret < 0)
> -               return ret;
> -       return num_read;
> +       return ret;
>  }
>
>  /*
> @@ -5095,9 +5080,7 @@ static int send_extent_data(struct send_ctx *sctx,
>                 ret = send_write(sctx, offset + sent, size);
>                 if (ret < 0)
>                         return ret;
> -               if (!ret)
> -                       break;
> -               sent += ret;
> +               sent += size;
>         }
>         return 0;
>  }
> --
> 2.28.0
>
Josef Bacik Aug. 24, 2020, 5:39 p.m. UTC | #2 
On 8/21/20 3:39 AM, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
> 
> send_write()/fill_read_buf() have some logic for avoiding reading past
> i_size. However, everywhere that we call
> send_write()/send_extent_data(), we've already clamped the length down
> to i_size. Get rid of the i_size handling, which simplifies the next
> change.
> 
> Signed-off-by: Omar Sandoval <osandov@fb.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef
diff mbox series

Patch

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 7c7c09fc65e8..8af5e867e4ca 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -4794,7 +4794,7 @@  static int process_all_new_xattrs(struct send_ctx *sctx)
 	return ret;
 }
 
-static ssize_t fill_read_buf(struct send_ctx *sctx, u64 offset, u32 len)
+static int fill_read_buf(struct send_ctx *sctx, u64 offset, u32 len)
 {
 	struct btrfs_root *root = sctx->send_root;
 	struct btrfs_fs_info *fs_info = root->fs_info;
@@ -4804,21 +4804,13 @@  static ssize_t fill_read_buf(struct send_ctx *sctx, u64 offset, u32 len)
 	pgoff_t index = offset >> PAGE_SHIFT;
 	pgoff_t last_index;
 	unsigned pg_offset = offset_in_page(offset);
-	ssize_t ret = 0;
+	int ret = 0;
+	size_t read = 0;
 
 	inode = btrfs_iget(fs_info->sb, sctx->cur_ino, root);
 	if (IS_ERR(inode))
 		return PTR_ERR(inode);
 
-	if (offset + len > i_size_read(inode)) {
-		if (offset > i_size_read(inode))
-			len = 0;
-		else
-			len = offset - i_size_read(inode);
-	}
-	if (len == 0)
-		goto out;
-
 	last_index = (offset + len - 1) >> PAGE_SHIFT;
 
 	/* initial readahead */
@@ -4859,16 +4851,15 @@  static ssize_t fill_read_buf(struct send_ctx *sctx, u64 offset, u32 len)
 		}
 
 		addr = kmap(page);
-		memcpy(sctx->read_buf + ret, addr + pg_offset, cur_len);
+		memcpy(sctx->read_buf + read, addr + pg_offset, cur_len);
 		kunmap(page);
 		unlock_page(page);
 		put_page(page);
 		index++;
 		pg_offset = 0;
 		len -= cur_len;
-		ret += cur_len;
+		read += cur_len;
 	}
-out:
 	iput(inode);
 	return ret;
 }
@@ -4882,7 +4873,6 @@  static int send_write(struct send_ctx *sctx, u64 offset, u32 len)
 	struct btrfs_fs_info *fs_info = sctx->send_root->fs_info;
 	int ret = 0;
 	struct fs_path *p;
-	ssize_t num_read = 0;
 
 	p = fs_path_alloc();
 	if (!p)
@@ -4890,12 +4880,9 @@  static int send_write(struct send_ctx *sctx, u64 offset, u32 len)
 
 	btrfs_debug(fs_info, "send_write offset=%llu, len=%d", offset, len);
 
-	num_read = fill_read_buf(sctx, offset, len);
-	if (num_read <= 0) {
-		if (num_read < 0)
-			ret = num_read;
+	ret = fill_read_buf(sctx, offset, len);
+	if (ret < 0)
 		goto out;
-	}
 
 	ret = begin_cmd(sctx, BTRFS_SEND_C_WRITE);
 	if (ret < 0)
@@ -4907,16 +4894,14 @@  static int send_write(struct send_ctx *sctx, u64 offset, u32 len)
 
 	TLV_PUT_PATH(sctx, BTRFS_SEND_A_PATH, p);
 	TLV_PUT_U64(sctx, BTRFS_SEND_A_FILE_OFFSET, offset);
-	TLV_PUT(sctx, BTRFS_SEND_A_DATA, sctx->read_buf, num_read);
+	TLV_PUT(sctx, BTRFS_SEND_A_DATA, sctx->read_buf, len);
 
 	ret = send_cmd(sctx);
 
 tlv_put_failure:
 out:
 	fs_path_free(p);
-	if (ret < 0)
-		return ret;
-	return num_read;
+	return ret;
 }
 
 /*
@@ -5095,9 +5080,7 @@  static int send_extent_data(struct send_ctx *sctx,
 		ret = send_write(sctx, offset + sent, size);
 		if (ret < 0)
 			return ret;
-		if (!ret)
-			break;
-		sent += ret;
+		sent += size;
 	}
 	return 0;
 }