Re: [PATCH 2/9] btrfs: send: avoid copying file data

[Omar Sandoval <osandov@osandov.com>]

On Fri, Aug 21, 2020 at 06:29:30PM +0100, Filipe Manana wrote:
> On Fri, Aug 21, 2020 at 8:42 AM Omar Sandoval <osandov@osandov.com> wrote:
> >
> > From: Omar Sandoval <osandov@fb.com>
> >
> > send_write() currently copies from the page cache to sctx->read_buf, and
> > then from sctx->read_buf to sctx->send_buf. Similarly, send_hole()
> > zeroes sctx->read_buf and then copies from sctx->read_buf to
> > sctx->send_buf. However, if we write the TLV header manually, we can
> > copy to sctx->send_buf directly and get rid of sctx->read_buf.
> >
> > 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).
> Only one minor thing below, but it's really subjective and doesn't
> make much of a difference.
> 
> Thanks.
> 
> > ---
> >  fs/btrfs/send.c | 65 +++++++++++++++++++++++++++++--------------------
> >  fs/btrfs/send.h |  1 -
> >  2 files changed, 39 insertions(+), 27 deletions(-)
> >
> > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> > index 8af5e867e4ca..e70f5ceb3261 100644
> > --- a/fs/btrfs/send.c
> > +++ b/fs/btrfs/send.c
> > @@ -122,8 +122,6 @@ struct send_ctx {
> >
> >         struct file_ra_state ra;
> >
> > -       char *read_buf;
> > -
> >         /*
> >          * We process inodes by their increasing order, so if before an
> >          * incremental send we reverse the parent/child relationship of
> > @@ -4794,7 +4792,25 @@ static int process_all_new_xattrs(struct send_ctx *sctx)
> >         return ret;
> >  }
> >
> > -static int fill_read_buf(struct send_ctx *sctx, u64 offset, u32 len)
> > +static u64 max_send_read_size(struct send_ctx *sctx)
> 
> We could make this inline, since it's so small and trivial, and
> constify the argument too.

Good point, fixed. Thanks, Filipe!