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@xxxxxxxxxxx> wrote:
> >
> > From: Omar Sandoval <osandov@xxxxxx>
> >
> > 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@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).
> 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!
