Re: [PATCH v11 10/14] btrfs: add send stream v2 definitions

Omar Sandoval
2021-10-18T18:58:08Z

On Mon, Oct 18, 2021 at 06:11:54PM +0300, Nikolay Borisov wrote:
>
>
> On 1.09.21 г. 20:01, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> >
> > This adds the definitions of the new commands for send stream version 2
> > and their respective attributes: fallocate, FS_IOC_SETFLAGS (a.k.a.
> > chattr), and encoded writes. It also documents two changes to the send
> > stream format in v2: the receiver shouldn't assume a maximum command
> > size, and the DATA attribute is encoded differently to allow for writes
> > larger than 64k. These will be implemented in subsequent changes, and
> > then the ioctl will accept the new flags.
> >
> > Reviewed-by: Josef Bacik <josef@toxicpanda.com>
> > Signed-off-by: Omar Sandoval <osandov@fb.com>
> > ---
> > fs/btrfs/send.c | 2 +-
> > fs/btrfs/send.h | 30 +++++++++++++++++++++++++++++-
> > include/uapi/linux/btrfs.h | 13 +++++++++++++
> > 3 files changed, 43 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> > index afdcbe7844e0..2ec07943f173 100644
> > --- a/fs/btrfs/send.c
> > +++ b/fs/btrfs/send.c
> > @@ -7287,7 +7287,7 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
> >
> > sctx->clone_roots_cnt = arg->clone_sources_count;
> >
> > - sctx->send_max_size = BTRFS_SEND_BUF_SIZE;
> > + sctx->send_max_size = BTRFS_SEND_BUF_SIZE_V1;
> > sctx->send_buf = kvmalloc(sctx->send_max_size, GFP_KERNEL);
> > if (!sctx->send_buf) {
> > ret = -ENOMEM;
> > diff --git a/fs/btrfs/send.h b/fs/btrfs/send.h
> > index de91488b7cd0..9f4f7b96b1eb 100644
> > --- a/fs/btrfs/send.h
> > +++ b/fs/btrfs/send.h
> > @@ -12,7 +12,11 @@
> > #define BTRFS_SEND_STREAM_MAGIC "btrfs-stream"
> > #define BTRFS_SEND_STREAM_VERSION 1
> >
> > -#define BTRFS_SEND_BUF_SIZE SZ_64K
> > +/*
> > + * In send stream v1, no command is larger than 64k. In send stream v2, no limit
> > + * should be assumed.
> > + */
> > +#define BTRFS_SEND_BUF_SIZE_V1 SZ_64K
> >
> > enum btrfs_tlv_type {
> > BTRFS_TLV_U8,
> > @@ -76,6 +80,13 @@ enum btrfs_send_cmd {
> >
> > BTRFS_SEND_C_END,
> > BTRFS_SEND_C_UPDATE_EXTENT,
> > +
> > + /* The following commands were added in send stream v2. */
> > +
> > + BTRFS_SEND_C_FALLOCATE,
> > + BTRFS_SEND_C_SETFLAGS,
> > + BTRFS_SEND_C_ENCODED_WRITE,
> > +
> > __BTRFS_SEND_C_MAX,
> > };
> > #define BTRFS_SEND_C_MAX (__BTRFS_SEND_C_MAX - 1)
> > @@ -106,6 +117,11 @@ enum {
> > BTRFS_SEND_A_PATH_LINK,
> >
> > BTRFS_SEND_A_FILE_OFFSET,
> > + /*
> > + * In send stream v2, this attribute is special: it must be the last
> > + * attribute in a command, its header contains only the type, and its
> > + * length is implicitly the remaining length of the command.
> > + */
> > BTRFS_SEND_A_DATA,
>
> Now that I think more about this, it would be best if this logic is
> actually codified in the code. I.e first set of SEND_A_DATA would set
> some bool/flag in the sctx and subsequent calls would be able to
> ASSERT/WARN ?

I suppose I could do something like this, is that what you had in mind?

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 90ca915fed78..46443d80b431 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -81,6 +81,7 @@ struct send_ctx {
char *send_buf;
u32 send_size;
u32 send_max_size;
+ bool put_data;
struct page **send_buf_pages;
u64 total_send_size;
u64 cmd_send_size[BTRFS_SEND_C_MAX + 1];
@@ -575,6 +576,9 @@ static int tlv_put(struct send_ctx *sctx, u16 attr, const void *data, int len)
int total_len = sizeof(*hdr) + len;
int left = sctx->send_max_size - sctx->send_size;

+ if (WARN_ON(sctx->put_data))
+ return -EINVAL;
+
if (unlikely(left < total_len))
return -EOVERFLOW;

@@ -718,6 +722,7 @@ static int send_cmd(struct send_ctx *sctx)
sctx->total_send_size += sctx->send_size;
sctx->cmd_send_size[get_unaligned_le16(&hdr->cmd)] += sctx->send_size;
sctx->send_size = 0;
+ sctx->put_data = false;

return ret;
}
@@ -4913,6 +4918,9 @@ static inline u64 max_send_read_size(const struct send_ctx *sctx)

static int put_data_header(struct send_ctx *sctx, u32 len)
{
+ if (WARN_ON(sctx->put_data))
+ return -EINVAL;
+ sctx->put_data = true;
if (sctx->flags & BTRFS_SEND_FLAG_STREAM_V2) {
/*
* In v2, the data attribute header doesn't include a length; it
@@ -5368,6 +5376,7 @@ static int send_encoded_extent(struct send_ctx *sctx, struct btrfs_path *path,
sctx->cmd_send_size[le16_to_cpu(hdr->cmd)] +=
sctx->send_size + block_len;
sctx->send_size = 0;
+ sctx->put_data = false;