On Fri, Aug 21, 2020 at 8:42 AM Omar Sandoval <osandov@xxxxxxxxxxx> wrote:
>
> From: Omar Sandoval <osandov@xxxxxx>
>
> 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@xxxxxx>
Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>
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
>
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
