btrfs: add send/receive support for reading/writing compressed data

PATCH Superseded Omar Sandoval 2 participants 29 messages Mar 17, 2022 – Mar 30, 2022

Subsystems (2)

Documentation Btrfs file system

lore

Share & tools

lore

Omar Sandoval

author

2022-03-17 17:26 UTC #0 lore raw

v14 cover btrfs: add send/receive support for reading/writing compressed data

From: Omar Sandoval <[email protected]>

This series adds support for sending compressed data via Btrfs send and btrfs-progs support for sending/receiving compressed data and writing it with BTRFS_IOC_ENCODED_WRITE, which was previously merged into misc-next. See the previous posting for more details and benchmarks [1].

Patches 1 and 2 are cleanups for Btrfs send. Patches 3-5 prepare some protocol changes for send stream v2. Patch 6 implements compressed send. Patch 7 enables send stream v2 and compressed send in the send ioctl when requested.

Changes since v13 [2]:

- Rebased on latest misc-next branch.
- Dropped ioctl patches which are already in misc-next.

1: https://lore.kernel.org/linux-btrfs/[email protected]/
2: https://lore.kernel.org/linux-btrfs/[email protected]/

Omar Sandoval (7):
btrfs: send: remove unused send_ctx::{total,cmd}_send_size
btrfs: send: explicitly number commands and attributes
btrfs: add send stream v2 definitions
btrfs: send: write larger chunks when using stream v2
btrfs: send: allocate send buffer with alloc_page() and vmap() for v2
btrfs: send: send compressed extents with encoded writes
btrfs: send: enable support for stream v2 and compressed writes

 fs/btrfs/ctree.h           |   6 +
 fs/btrfs/inode.c           |  13 +-
 fs/btrfs/send.c            | 324 +++++++++++++++++++++++++++++++++----
 fs/btrfs/send.h            | 142 +++++++++-------
 include/uapi/linux/btrfs.h |  10 +-
 5 files changed, 395 insertions(+), 100 deletions(-)

The btrfs-progs patches were written by Boris Burkov with some updates from me. Patches 1-4 are preparation. Patch 5 implements encoded writes. Patch 6 implements the fallback to decompressing. Patches 7 and 8 implement the other commands. Patch 9 adds the new btrfs send options. Patch 10 adds a test case.

Changes since v13:

- Rebased on latest devel branch.
- Updated the btrfs_ioctl_encoded_io_args definition to the version that was merged into misc-next.

Boris Burkov (8):
btrfs-progs: receive: support v2 send stream larger tlv_len
btrfs-progs: receive: dynamically allocate sctx->read_buf
btrfs-progs: receive: support v2 send stream DATA tlv format
btrfs-progs: receive: process encoded_write commands
btrfs-progs: receive: encoded_write fallback to explicit decode and
write
btrfs-progs: receive: process fallocate commands
btrfs-progs: receive: process setflags ioctl commands
btrfs-progs: receive: add tests for basic encoded_write send/receive

Omar Sandoval (2):
btrfs-progs: receive: add send stream v2 cmds and attrs to send.h
btrfs-progs: send: stream v2 ioctl flags

 Documentation/btrfs-receive.rst               |   5 +
 Documentation/btrfs-send.rst                  |  22 ++
 cmds/receive-dump.c                           |  31 +-
 cmds/receive.c                                | 347 +++++++++++++++++-
 cmds/send.c                                   | 100 ++++-
 common/send-stream.c                          | 165 +++++++--
 common/send-stream.h                          |   7 +
 ioctl.h                                       | 151 +++++++-
 kernel-shared/send.h                          | 146 +++++---
 libbtrfs/send-stream.c                        |   2 +-
 .../052-receive-write-encoded/test.sh         | 114 ++++++
 11 files changed, 993 insertions(+), 97 deletions(-)
 create mode 100755 tests/misc-tests/052-receive-write-encoded/test.sh

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #1 lore raw

PATCH v14 1/7 btrfs: send: remove unused send_ctx::{total,cmd}_send_size

From: Omar Sandoval <[email protected]>

We collect these statistics but have never exposed them in any way. I also didn't find any patches that ever attempted to make use of them.

Signed-off-by Omar Sandoval <[email protected]>

---
 fs/btrfs/send.c | 4 ----
 1 file changed, 4 deletions(-)

fs/btrfs/send.c +0 -4

index cf86f1eafcb7..6d36dee1505f 100644
@@ -82,8 +82,6 @@ struct send_ctx {
 	char *send_buf;
 	u32 send_size;
 	u32 send_max_size;
-	u64 total_send_size;
-	u64 cmd_send_size[BTRFS_SEND_C_MAX + 1];
 	u64 flags;	/* 'flags' member of btrfs_ioctl_send_args is u64 */
 	/* Protocol version compatibility requested */
 	u32 proto;

@@ -727,8 +725,6 @@ static int send_cmd(struct send_ctx *sctx)
 	ret = write_buf(sctx->send_filp, sctx->send_buf, sctx->send_size,
 					&sctx->send_off);
 
-	sctx->total_send_size += sctx->send_size;
-	sctx->cmd_send_size[get_unaligned_le16(&hdr->cmd)] += sctx->send_size;
 	sctx->send_size = 0;
 
 	return ret;

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #2 lore raw

PATCH v14 2/7 btrfs: send: explicitly number commands and attributes

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

Commit e77fbf990316 ("btrfs: send: prepare for v2 protocol") added _BTRFS_SEND_C_MAX_V* macros equal to the maximum command number for the version plus 1, but as written this creates gaps in the number space. The maximum command number is currently 22, and __BTRFS_SEND_C_MAX_V1 is accordingly 23. But then __BTRFS_SEND_C_MAX_V2 is 24, suggesting that v2 has a command numbered 23, and __BTRFS_SEND_C_MAX is 25, suggesting that 23 and 24 are valid commands.

Instead, let's explicitly number all of the commands, attributes, and sentinel MAX constants.

Signed-off-by Omar Sandoval <[email protected]>

---
 fs/btrfs/send.c |   4 +-
 fs/btrfs/send.h | 106 ++++++++++++++++++++++++------------------------
 2 files changed, 54 insertions(+), 56 deletions(-)

fs/btrfs/send.c +2 -2

index 6d36dee1505f..9363f625fa17 100644
@@ -326,8 +326,8 @@ __maybe_unused
 static bool proto_cmd_ok(const struct send_ctx *sctx, int cmd)
 {
 	switch (sctx->proto) {
-	case 1:	 return cmd < __BTRFS_SEND_C_MAX_V1;
-	case 2:	 return cmd < __BTRFS_SEND_C_MAX_V2;
+	case 1:	 return cmd <= BTRFS_SEND_C_MAX_V1;
+	case 2:	 return cmd <= BTRFS_SEND_C_MAX_V2;
 	default: return false;
 	}
 }

fs/btrfs/send.h +52 ~~-54~~

136 lines not on this page · raw message

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #4 lore raw

PATCH v14 3/7 btrfs: add send stream v2 definitions

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

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 version and flag.

Reviewed-by Josef Bacik <[email protected]>

Signed-off-by Omar Sandoval <[email protected]>

---
 fs/btrfs/send.c            |  2 +-
 fs/btrfs/send.h            | 40 ++++++++++++++++++++++++++++++++++----
 include/uapi/linux/btrfs.h |  7 +++++++
 3 files changed, 44 insertions(+), 5 deletions(-)

fs/btrfs/send.c +1 -1

index 9363f625fa17..1f141de3a7d6 100644
@@ -7459,7 +7459,7 @@ long btrfs_ioctl_send(struct inode *inode, 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;

fs/btrfs/send.h +36 -4

index 67721e0281ba..805d8095209a 100644
@@ -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,

@@ -80,16 +84,20 @@ enum btrfs_send_cmd {
 	BTRFS_SEND_C_MAX_V1 = 22,
 
 	/* Version 2 */
-	BTRFS_SEND_C_MAX_V2 = 22,
+	BTRFS_SEND_C_FALLOCATE = 23,
+	BTRFS_SEND_C_SETFLAGS = 24,
+	BTRFS_SEND_C_ENCODED_WRITE = 25,
+	BTRFS_SEND_C_MAX_V2 = 25,
 
 	/* End */
-	BTRFS_SEND_C_MAX = 22,
+	BTRFS_SEND_C_MAX = 25,
 };
 
 /* attributes in send stream */
 enum {
 	BTRFS_SEND_A_UNSPEC = 0,
 
+	/* Version 1 */
 	BTRFS_SEND_A_UUID = 1,
 	BTRFS_SEND_A_CTRANSID = 2,

@@ -112,6 +120,11 @@ enum {
 	BTRFS_SEND_A_PATH_LINK = 17,
 
 	BTRFS_SEND_A_FILE_OFFSET = 18,
+	/*
+	 * As of 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 = 19,
 
 	BTRFS_SEND_A_CLONE_UUID = 20,

@@ -120,7 +133,26 @@ enum {
 	BTRFS_SEND_A_CLONE_OFFSET = 23,
 	BTRFS_SEND_A_CLONE_LEN = 24,
 
-	BTRFS_SEND_A_MAX = 24,
+	BTRFS_SEND_A_MAX_V1 = 24,
+
+	/* Version 2 */
+	BTRFS_SEND_A_FALLOCATE_MODE = 25,
+
+	BTRFS_SEND_A_SETFLAGS_FLAGS = 26,
+
+	BTRFS_SEND_A_UNENCODED_FILE_LEN = 27,
+	BTRFS_SEND_A_UNENCODED_LEN = 28,
+	BTRFS_SEND_A_UNENCODED_OFFSET = 29,
+	/*
+	 * COMPRESSION and ENCRYPTION default to NONE (0) if omitted from
+	 * BTRFS_SEND_C_ENCODED_WRITE.
+	 */
+	BTRFS_SEND_A_COMPRESSION = 30,
+	BTRFS_SEND_A_ENCRYPTION = 31,
+	BTRFS_SEND_A_MAX_V2 = 31,
+
+	/* End */
+	BTRFS_SEND_A_MAX = 31,
 };
 
 #ifdef __KERNEL__

include/uapi/linux/btrfs.h +7 -0

index d956b2993970..b6f26a434b10 100644
@@ -777,6 +777,13 @@ struct btrfs_ioctl_received_subvol_args {
  */
 #define BTRFS_SEND_FLAG_VERSION			0x8
 
+/*
+ * Send compressed data using the ENCODED_WRITE command instead of decompressing
+ * the data and sending it with the WRITE command. This requires protocol
+ * version >= 2.
+ */
+#define BTRFS_SEND_FLAG_COMPRESSED		0x10
+
 #define BTRFS_SEND_FLAG_MASK \
 	(BTRFS_SEND_FLAG_NO_FILE_DATA | \
 	 BTRFS_SEND_FLAG_OMIT_STREAM_HEADER | \

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #5 lore raw

PATCH v14 4/7 btrfs: send: write larger chunks when using stream v2

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

The length field of the send stream TLV header is 16 bits. This means that the maximum amount of data that can be sent for one write is 64k minus one. However, encoded writes must be able to send the maximum compressed extent (128k) in one command. To support this, send stream version 2 encodes the DATA attribute differently: it has no length field, and the length is implicitly up to the end of containing command (which has a 32-bit length field). Although this is necessary for encoded writes, normal writes can benefit from it, too.

Also add a check to enforce that the DATA attribute is last. It is only strictly necessary for v2, but we might as well make v1 consistent with it.

For v2, let's bump up the send buffer to the maximum compressed extent size plus 16k for the other metadata (144k total). Since this will most likely be vmalloc'd (and always will be after the next commit), we round it up to the next page since we might as well use the rest of the page on systems with >16k pages.

Reviewed-by Nikolay Borisov <[email protected]>

Signed-off-by Omar Sandoval <[email protected]>

---
 fs/btrfs/send.c | 42 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

fs/btrfs/send.c +34 -8

index 1f141de3a7d6..02053fff80ca 100644
@@ -82,6 +82,7 @@ struct send_ctx {
 	char *send_buf;
 	u32 send_size;
 	u32 send_max_size;
+	bool put_data;
 	u64 flags;	/* 'flags' member of btrfs_ioctl_send_args is u64 */
 	/* Protocol version compatibility requested */
 	u32 proto;

@@ -589,6 +590,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_ONCE(sctx->put_data))
+		return -EINVAL;
+
 	if (unlikely(left < total_len))
 		return -EOVERFLOW;

@@ -726,6 +730,7 @@ static int send_cmd(struct send_ctx *sctx)
 					&sctx->send_off);
 
 	sctx->send_size = 0;
+	sctx->put_data = false;
 
 	return ret;
 }

@@ -4853,14 +4858,30 @@ static inline u64 max_send_read_size(const struct send_ctx *sctx)
 
 static int put_data_header(struct send_ctx *sctx, u32 len)
 {
-	struct btrfs_tlv_header *hdr;
+	if (WARN_ON_ONCE(sctx->put_data))
+		return -EINVAL;
+	sctx->put_data = true;
+	if (sctx->proto >= 2) {
+		/*
+		 * In v2, the data attribute header doesn't include a length; it
+		 * is implicitly to the end of the command.
+		 */
+		if (sctx->send_max_size - sctx->send_size < 2 + len)
+			return -EOVERFLOW;
+		put_unaligned_le16(BTRFS_SEND_A_DATA,
+				   sctx->send_buf + sctx->send_size);
+		sctx->send_size += 2;
+	} else {
+		struct btrfs_tlv_header *hdr;
 
-	if (sctx->send_max_size - sctx->send_size < sizeof(*hdr) + len)
-		return -EOVERFLOW;
-	hdr = (struct btrfs_tlv_header *)(sctx->send_buf + sctx->send_size);
-	put_unaligned_le16(BTRFS_SEND_A_DATA, &hdr->tlv_type);
-	put_unaligned_le16(len, &hdr->tlv_len);
-	sctx->send_size += sizeof(*hdr);
+		if (sctx->send_max_size - sctx->send_size < sizeof(*hdr) + len)
+			return -EOVERFLOW;
+		hdr = (struct btrfs_tlv_header *)(sctx->send_buf +
+						  sctx->send_size);
+		put_unaligned_le16(BTRFS_SEND_A_DATA, &hdr->tlv_type);
+		put_unaligned_le16(len, &hdr->tlv_len);
+		sctx->send_size += sizeof(*hdr);
+	}
 	return 0;
 }

@@ -7459,7 +7480,12 @@ long btrfs_ioctl_send(struct inode *inode, struct btrfs_ioctl_send_args *arg)
 
 	sctx->clone_roots_cnt = arg->clone_sources_count;
 
-	sctx->send_max_size = BTRFS_SEND_BUF_SIZE_V1;
+	if (sctx->proto >= 2) {
+		sctx->send_max_size = ALIGN(SZ_16K + BTRFS_MAX_COMPRESSED,
+					    PAGE_SIZE);
+	} else {
+		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;

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #8 lore raw

PATCH v14 5/7 btrfs: send: allocate send buffer with alloc_page() and vmap() for v2

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

For encoded writes, we need the raw pages for reading compressed data directly via a bio. So, replace kvmalloc() with vmap() so we have access to the raw pages. 144k is large enough that it usually gets allocated with vmalloc(), anyways.

Reviewed-by Nikolay Borisov <[email protected]>

Signed-off-by Omar Sandoval <[email protected]>

---
 fs/btrfs/send.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

fs/btrfs/send.c +31 -2

index 02053fff80ca..ac2a1297027a 100644
@@ -83,6 +83,7 @@ struct send_ctx {
 	u32 send_size;
 	u32 send_max_size;
 	bool put_data;
+	struct page **send_buf_pages;
 	u64 flags;	/* 'flags' member of btrfs_ioctl_send_args is u64 */
 	/* Protocol version compatibility requested */
 	u32 proto;

@@ -7392,6 +7393,7 @@ long btrfs_ioctl_send(struct inode *inode, struct btrfs_ioctl_send_args *arg)
 	struct btrfs_root *clone_root;
 	struct send_ctx *sctx = NULL;
 	u32 i;
+	u32 send_buf_num_pages = 0;
 	u64 *clone_sources_tmp = NULL;
 	int clone_sources_to_rollback = 0;
 	size_t alloc_size;

@@ -7483,10 +7485,28 @@ long btrfs_ioctl_send(struct inode *inode, struct btrfs_ioctl_send_args *arg)
 	if (sctx->proto >= 2) {
 		sctx->send_max_size = ALIGN(SZ_16K + BTRFS_MAX_COMPRESSED,
 					    PAGE_SIZE);
+		send_buf_num_pages = sctx->send_max_size >> PAGE_SHIFT;
+		sctx->send_buf_pages = kcalloc(send_buf_num_pages,
+					       sizeof(*sctx->send_buf_pages),
+					       GFP_KERNEL);
+		if (!sctx->send_buf_pages) {
+			send_buf_num_pages = 0;
+			ret = -ENOMEM;
+			goto out;
+		}
+		for (i = 0; i < send_buf_num_pages; i++) {
+			sctx->send_buf_pages[i] = alloc_page(GFP_KERNEL);
+			if (!sctx->send_buf_pages[i]) {
+				ret = -ENOMEM;
+				goto out;
+			}
+		}
+		sctx->send_buf = vmap(sctx->send_buf_pages, send_buf_num_pages,
+				      VM_MAP, PAGE_KERNEL);
 	} else {
 		sctx->send_max_size = BTRFS_SEND_BUF_SIZE_V1;
+		sctx->send_buf = kvmalloc(sctx->send_max_size, GFP_KERNEL);
 	}
-	sctx->send_buf = kvmalloc(sctx->send_max_size, GFP_KERNEL);
 	if (!sctx->send_buf) {
 		ret = -ENOMEM;
 		goto out;

@@ -7679,7 +7699,16 @@ long btrfs_ioctl_send(struct inode *inode, struct btrfs_ioctl_send_args *arg)
 			fput(sctx->send_filp);
 
 		kvfree(sctx->clone_roots);
-		kvfree(sctx->send_buf);
+		if (sctx->proto >= 2) {
+			vunmap(sctx->send_buf);
+			for (i = 0; i < send_buf_num_pages; i++) {
+				if (sctx->send_buf_pages[i])
+					__free_page(sctx->send_buf_pages[i]);
+			}
+			kfree(sctx->send_buf_pages);
+		} else {
+			kvfree(sctx->send_buf);
+		}
 
 		name_cache_free(sctx);

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #16 lore raw

PATCH v14 6/7 btrfs: send: send compressed extents with encoded writes

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

Now that all of the pieces are in place, we can use the ENCODED_WRITE command to send compressed extents when appropriate.

Signed-off-by Omar Sandoval <[email protected]>

---
 fs/btrfs/ctree.h |   6 ++
 fs/btrfs/inode.c |  13 +--
 fs/btrfs/send.c  | 234 +++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 228 insertions(+), 25 deletions(-)

fs/btrfs/ctree.h +6 -0

index 09b7b0b2d016..92bfee5b3ded 100644
@@ -3355,6 +3355,12 @@ int btrfs_writepage_cow_fixup(struct page *page);
 void btrfs_writepage_endio_finish_ordered(struct btrfs_inode *inode,
 					  struct page *page, u64 start,
 					  u64 end, bool uptodate);
+int btrfs_encoded_io_compression_from_extent(struct btrfs_fs_info *fs_info,
+					     int compress_type);
+int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
+					  u64 file_offset, u64 disk_bytenr,
+					  u64 disk_io_size,
+					  struct page **pages);
 ssize_t btrfs_encoded_read(struct kiocb *iocb, struct iov_iter *iter,
 			   struct btrfs_ioctl_encoded_io_args *encoded);
 ssize_t btrfs_do_encoded_write(struct kiocb *iocb, struct iov_iter *from,

fs/btrfs/inode.c +5 -8

index 78a5145353e1..6b8c0f026545 100644
@@ -10114,9 +10114,8 @@ void btrfs_set_range_writeback(struct btrfs_inode *inode, u64 start, u64 end)
 	}
 }
 
-static int btrfs_encoded_io_compression_from_extent(
-				struct btrfs_fs_info *fs_info,
-				int compress_type)
+int btrfs_encoded_io_compression_from_extent(struct btrfs_fs_info *fs_info,
+					     int compress_type)
 {
 	switch (compress_type) {
 	case BTRFS_COMPRESS_NONE:

@@ -10321,11 +10320,9 @@ static void btrfs_encoded_read_endio(struct bio *bio)
 	bio_put(bio);
 }
 
-static int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
-						 u64 file_offset,
-						 u64 disk_bytenr,
-						 u64 disk_io_size,
-						 struct page **pages)
+int btrfs_encoded_read_regular_fill_pages(struct btrfs_inode *inode,
+					  u64 file_offset, u64 disk_bytenr,
+					  u64 disk_io_size, struct page **pages)
 {
 	struct btrfs_fs_info *fs_info = inode->root->fs_info;
 	struct btrfs_encoded_read_private priv = {

fs/btrfs/send.c +217 ~~-17~~

296 lines not on this page · raw message

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #17 lore raw

PATCH v14 7/7 btrfs: send: enable support for stream v2 and compressed writes

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

Now that the new support is implemented, allow the ioctl to accept v2 and the compressed flag, and update the version in sysfs.

Signed-off-by Omar Sandoval <[email protected]>

---
 fs/btrfs/send.c            | 7 +++++--
 fs/btrfs/send.h            | 2 +-
 include/uapi/linux/btrfs.h | 3 ++-
 3 files changed, 8 insertions(+), 4 deletions(-)

fs/btrfs/send.c +5 -2

index b0560be3053b..4567271ce642 100644
@@ -690,8 +690,7 @@ static int send_header(struct send_ctx *sctx)
 	struct btrfs_stream_header hdr;
 
 	strcpy(hdr.magic, BTRFS_SEND_STREAM_MAGIC);
-	hdr.version = cpu_to_le32(BTRFS_SEND_STREAM_VERSION);
-
+	hdr.version = cpu_to_le32(sctx->proto);
 	return write_buf(sctx->send_filp, &hdr, sizeof(hdr),
 					&sctx->send_off);
 }

@@ -7663,6 +7662,10 @@ long btrfs_ioctl_send(struct inode *inode, struct btrfs_ioctl_send_args *arg)
 	} else {
 		sctx->proto = 1;
 	}
+	if ((arg->flags & BTRFS_SEND_FLAG_COMPRESSED) && sctx->proto < 2) {
+		ret = -EINVAL;
+		goto out;
+	}
 
 	sctx->send_filp = fget(arg->send_fd);
 	if (!sctx->send_filp) {

fs/btrfs/send.h +1 -1

index 805d8095209a..50a2aceae929 100644
@@ -10,7 +10,7 @@
 #include "ctree.h"
 
 #define BTRFS_SEND_STREAM_MAGIC "btrfs-stream"
-#define BTRFS_SEND_STREAM_VERSION 1
+#define BTRFS_SEND_STREAM_VERSION 2
 
 /*
  * In send stream v1, no command is larger than 64k. In send stream v2, no limit

include/uapi/linux/btrfs.h +2 -1

index b6f26a434b10..f54dc91e4025 100644
@@ -788,7 +788,8 @@ struct btrfs_ioctl_received_subvol_args {
 	(BTRFS_SEND_FLAG_NO_FILE_DATA | \
 	 BTRFS_SEND_FLAG_OMIT_STREAM_HEADER | \
 	 BTRFS_SEND_FLAG_OMIT_END_CMD | \
-	 BTRFS_SEND_FLAG_VERSION)
+	 BTRFS_SEND_FLAG_VERSION | \
+	 BTRFS_SEND_FLAG_COMPRESSED)
 
 struct btrfs_ioctl_send_args {
 	__s64 send_fd;			/* in */

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #18 lore raw

PATCH v14 1/10 btrfs-progs: receive: support v2 send stream larger tlv_len

In reply to Omar Sandoval

From: Boris Burkov <[email protected]>

An encoded extent can be up to 128K in length, which exceeds the largest value expressible by the current send stream format's 16 bit tlv_len field. Since encoded writes cannot be split into multiple writes by btrfs send, the send stream format must change to accommodate encoded writes.

Supporting this changed format requires retooling how we store the commands we have processed. We currently store pointers to the struct btrfs_tlv_headers in the command buffer. This is not sufficient to represent the new BTRFS_SEND_A_DATA format. Instead, parse the attribute headers and store them in a new struct btrfs_send_attribute which has a 32-bit length field. This is transparent to users of the various TLV_GET macros.

Reviewed-by Nikolay Borisov <[email protected]>

Signed-off-by Boris Burkov <[email protected]>

---
 common/send-stream.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

common/send-stream.c +25 -9

index e9be922b..7d182238 100644
@@ -24,13 +24,23 @@
 #include "crypto/crc32c.h"
 #include "common/utils.h"
 
+struct btrfs_send_attribute {
+	u16 tlv_type;
+	/*
+	 * Note: in btrfs_tlv_header, this is __le16, but we need 32 bits for
+	 * attributes with file data as of version 2 of the send stream format
+	 */
+	u32 tlv_len;
+	char *data;
+};
+
 struct btrfs_send_stream {
 	char read_buf[BTRFS_SEND_BUF_SIZE];
 	int fd;
 
 	int cmd;
 	struct btrfs_cmd_header *cmd_hdr;
-	struct btrfs_tlv_header *cmd_attrs[BTRFS_SEND_A_MAX + 1];
+	struct btrfs_send_attribute cmd_attrs[BTRFS_SEND_A_MAX + 1];
 	u32 version;
 
 	/*

@@ -152,6 +162,7 @@ static int read_cmd(struct btrfs_send_stream *sctx)
 		struct btrfs_tlv_header *tlv_hdr;
 		u16 tlv_type;
 		u16 tlv_len;
+		struct btrfs_send_attribute *send_attr;
 
 		tlv_hdr = (struct btrfs_tlv_header *)data;
 		tlv_type = le16_to_cpu(tlv_hdr->tlv_type);

@@ -164,10 +175,15 @@ static int read_cmd(struct btrfs_send_stream *sctx)
 			goto out;
 		}
 
-		sctx->cmd_attrs[tlv_type] = tlv_hdr;
+		send_attr = &sctx->cmd_attrs[tlv_type];
+		send_attr->tlv_type = tlv_type;
+		send_attr->tlv_len = tlv_len;
+		pos += sizeof(*tlv_hdr);
+		data += sizeof(*tlv_hdr);
 
-		data += sizeof(*tlv_hdr) + tlv_len;
-		pos += sizeof(*tlv_hdr) + tlv_len;
+		send_attr->data = data;
+		pos += send_attr->tlv_len;
+		data += send_attr->tlv_len;
 	}
 
 	sctx->cmd = cmd;

@@ -180,7 +196,7 @@ out:
 static int tlv_get(struct btrfs_send_stream *sctx, int attr, void **data, int *len)
 {
 	int ret;
-	struct btrfs_tlv_header *hdr;
+	struct btrfs_send_attribute *send_attr;
 
 	if (attr <= 0 || attr > BTRFS_SEND_A_MAX) {
 		error("invalid attribute requested, attr = %d", attr);

@@ -188,15 +204,15 @@ static int tlv_get(struct btrfs_send_stream *sctx, int attr, void **data, int *l
 		goto out;
 	}
 
-	hdr = sctx->cmd_attrs[attr];
-	if (!hdr) {
+	send_attr = &sctx->cmd_attrs[attr];
+	if (!send_attr->data) {
 		error("attribute %d requested but not present", attr);
 		ret = -ENOENT;
 		goto out;
 	}
 
-	*len = le16_to_cpu(hdr->tlv_len);
-	*data = hdr + 1;
+	*len = send_attr->tlv_len;
+	*data = send_attr->data;
 
 	ret = 0;

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #19 lore raw

PATCH v14 2/10 btrfs-progs: receive: dynamically allocate sctx->read_buf

In reply to Omar Sandoval

From: Boris Burkov <[email protected]>

In send stream v2, write commands can now be an arbitrary size. For that reason, we can no longer allocate a fixed array in sctx for read_cmd. Instead, read_cmd dynamically allocates sctx->read_buf. To avoid needless reallocations, we reuse read_buf between read_cmd calls by also keeping track of the size of the allocated buffer in sctx->read_buf_sz.

We do the first allocation of the old default size at the start of processing the stream, and we only reallocate if we encounter a command that needs a larger buffer.

Signed-off-by Boris Burkov <[email protected]>

---
 common/send-stream.c   | 56 ++++++++++++++++++++++++++++--------------
 kernel-shared/send.h   |  6 ++++-
 libbtrfs/send-stream.c |  2 +-
 3 files changed, 43 insertions(+), 21 deletions(-)

common/send-stream.c +37 ~~-19~~

index 7d182238..421cd1bb 100644
@@ -35,11 +35,11 @@ struct btrfs_send_attribute {
 };
 
 struct btrfs_send_stream {
-	char read_buf[BTRFS_SEND_BUF_SIZE];
+	char *read_buf;
+	size_t read_buf_sz;
 	int fd;
 
 	int cmd;
-	struct btrfs_cmd_header *cmd_hdr;
 	struct btrfs_send_attribute cmd_attrs[BTRFS_SEND_A_MAX + 1];
 	u32 version;

@@ -111,11 +111,12 @@ static int read_cmd(struct btrfs_send_stream *sctx)
 	u32 pos;
 	u32 crc;
 	u32 crc2;
+	struct btrfs_cmd_header *cmd_hdr;
+	size_t buf_len;
 
 	memset(sctx->cmd_attrs, 0, sizeof(sctx->cmd_attrs));
 
-	ASSERT(sizeof(*sctx->cmd_hdr) <= sizeof(sctx->read_buf));
-	ret = read_buf(sctx, sctx->read_buf, sizeof(*sctx->cmd_hdr));
+	ret = read_buf(sctx, sctx->read_buf, sizeof(*cmd_hdr));
 	if (ret < 0)
 		goto out;
 	if (ret) {

@@ -124,18 +125,25 @@ static int read_cmd(struct btrfs_send_stream *sctx)
 		goto out;
 	}
 
-	sctx->cmd_hdr = (struct btrfs_cmd_header *)sctx->read_buf;
-	cmd = le16_to_cpu(sctx->cmd_hdr->cmd);
-	cmd_len = le32_to_cpu(sctx->cmd_hdr->len);
+	cmd_hdr = (struct btrfs_cmd_header *)sctx->read_buf;
+	cmd_len = le32_to_cpu(cmd_hdr->len);
+	cmd = le16_to_cpu(cmd_hdr->cmd);
+	buf_len = sizeof(*cmd_hdr) + cmd_len;
+	if (sctx->read_buf_sz < buf_len) {
+		void *new_read_buf;
 
-	if (cmd_len + sizeof(*sctx->cmd_hdr) >= sizeof(sctx->read_buf)) {
-		ret = -EINVAL;
-		error("command length %u too big for buffer %zu",
-				cmd_len, sizeof(sctx->read_buf));
-		goto out;
+		new_read_buf = realloc(sctx->read_buf, buf_len);
+		if (!new_read_buf) {
+			ret = -ENOMEM;
+			error("failed to reallocate read buffer for cmd");
+			goto out;
+		}
+		sctx->read_buf = new_read_buf;
+		sctx->read_buf_sz = buf_len;
+		/* We need to reset cmd_hdr after realloc of sctx->read_buf */
+		cmd_hdr = (struct btrfs_cmd_header *)sctx->read_buf;
 	}
-
-	data = sctx->read_buf + sizeof(*sctx->cmd_hdr);
+	data = sctx->read_buf + sizeof(*cmd_hdr);
 	ret = read_buf(sctx, data, cmd_len);
 	if (ret < 0)
 		goto out;

@@ -145,11 +153,12 @@ static int read_cmd(struct btrfs_send_stream *sctx)
 		goto out;
 	}
 
-	crc = le32_to_cpu(sctx->cmd_hdr->crc);
-	sctx->cmd_hdr->crc = 0;
+	crc = le32_to_cpu(cmd_hdr->crc);
+	/* in send, crc is computed with header crc = 0, replicate that */
+	cmd_hdr->crc = 0;
 
 	crc2 = crc32c(0, (unsigned char*)sctx->read_buf,
-			sizeof(*sctx->cmd_hdr) + cmd_len);
+			sizeof(*cmd_hdr) + cmd_len);
 
 	if (crc != crc2) {
 		ret = -EINVAL;

@@ -537,19 +546,28 @@ int btrfs_read_and_process_send_stream(int fd,
 		goto out;
 	}
 
+	sctx.read_buf = malloc(BTRFS_SEND_BUF_SIZE_V1);
+	if (!sctx.read_buf) {
+		ret = -ENOMEM;
+		error("unable to allocate send stream read buffer");
+		goto out;
+	}
+	sctx.read_buf_sz = BTRFS_SEND_BUF_SIZE_V1;
+
 	while (1) {
 		ret = read_and_process_cmd(&sctx);
 		if (ret < 0) {
 			last_err = ret;
 			errors++;
 			if (max_errors > 0 && errors >= max_errors)
-				goto out;
+				break;
 		} else if (ret > 0) {
 			if (!honor_end_cmd)
 				ret = 0;
-			goto out;
+			break;
 		}
 	}
+	free(sctx.read_buf);
 
 out:
 	if (last_err && !ret)

kernel-shared/send.h +5 -1

index e73f09df..e986b6c8 100644
@@ -33,7 +33,11 @@ extern "C" {
 #define BTRFS_SEND_STREAM_MAGIC "btrfs-stream"
 #define BTRFS_SEND_STREAM_VERSION 1
 
-#define BTRFS_SEND_BUF_SIZE  (64 * 1024)
+/*
+ * 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 (64 * 1024)
 #define BTRFS_SEND_READ_SIZE (1024 * 48)
 
 enum btrfs_tlv_type {

libbtrfs/send-stream.c +1 -1

index 2b21d846..39cbb3ed 100644
@@ -22,7 +22,7 @@
 #include "crypto/crc32c.h"
 
 struct btrfs_send_stream {
-	char read_buf[BTRFS_SEND_BUF_SIZE];
+	char read_buf[BTRFS_SEND_BUF_SIZE_V1];
 	int fd;
 
 	int cmd;

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #20 lore raw

PATCH v14 3/10 btrfs-progs: receive: support v2 send stream DATA tlv format

In reply to Omar Sandoval

From: Boris Burkov <[email protected]>

The new format privileges the BTRFS_SEND_A_DATA attribute by guaranteeing it will always be the last attribute in any command that needs it, and by implicitly encoding the data length as the difference between the total command length in the command header and the sizes of the rest of the attributes (and of course the tlv_type identifying the DATA attribute). To parse the new stream, we must read the tlv_type and if it is not DATA, we proceed normally, but if it is DATA, we don't parse a tlv_len but simply compute the length.

In addition, we add some bounds checking when parsing each chunk of data, as well as for the tlv_len itself.

Reviewed-by Nikolay Borisov <[email protected]>

Signed-off-by Boris Burkov <[email protected]>

---
 common/send-stream.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

common/send-stream.c +26 ~~-10~~

index 421cd1bb..81a830d9 100644
@@ -168,28 +168,44 @@ static int read_cmd(struct btrfs_send_stream *sctx)
 
 	pos = 0;
 	while (pos < cmd_len) {
-		struct btrfs_tlv_header *tlv_hdr;
 		u16 tlv_type;
-		u16 tlv_len;
 		struct btrfs_send_attribute *send_attr;
 
-		tlv_hdr = (struct btrfs_tlv_header *)data;
-		tlv_type = le16_to_cpu(tlv_hdr->tlv_type);
-		tlv_len = le16_to_cpu(tlv_hdr->tlv_len);
+		if (cmd_len - pos < sizeof(__le16)) {
+			error("send stream is truncated");
+			ret = -EINVAL;
+			goto out;
+		}
+		tlv_type = le16_to_cpu(*(__le16 *)data);
 
 		if (tlv_type == 0 || tlv_type > BTRFS_SEND_A_MAX) {
-			error("invalid tlv in cmd tlv_type = %hu, tlv_len = %hu",
-					tlv_type, tlv_len);
+			error("invalid tlv in cmd tlv_type = %hu", tlv_type);
 			ret = -EINVAL;
 			goto out;
 		}
 
 		send_attr = &sctx->cmd_attrs[tlv_type];
 		send_attr->tlv_type = tlv_type;
-		send_attr->tlv_len = tlv_len;
-		pos += sizeof(*tlv_hdr);
-		data += sizeof(*tlv_hdr);
 
+		pos += sizeof(tlv_type);
+		data += sizeof(tlv_type);
+		if (sctx->version >= 2 && tlv_type == BTRFS_SEND_A_DATA) {
+			send_attr->tlv_len = cmd_len - pos;
+		} else {
+			if (cmd_len - pos < sizeof(__le16)) {
+				error("send stream is truncated");
+				ret = -EINVAL;
+				goto out;
+			}
+			send_attr->tlv_len = le16_to_cpu(*(__le16 *)data);
+			pos += sizeof(__le16);
+			data += sizeof(__le16);
+		}
+		if (cmd_len - pos < send_attr->tlv_len) {
+			error("send stream is truncated");
+			ret = -EINVAL;
+			goto out;
+		}
 		send_attr->data = data;
 		pos += send_attr->tlv_len;
 		data += send_attr->tlv_len;

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #21 lore raw

PATCH v14 4/10 btrfs-progs: receive: add send stream v2 cmds and attrs to send.h

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

Update our copy of send.h from the kernel. This adds the new commands and attributes for v2 as well as explicit enum numbering.

Reviewed-by Nikolay Borisov <[email protected]>

Signed-off-by Boris Burkov <[email protected]>

Signed-off-by Omar Sandoval <[email protected]>

---
 kernel-shared/send.h | 138 ++++++++++++++++++++++++++-----------------
 1 file changed, 85 insertions(+), 53 deletions(-)

kernel-shared/send.h +85 ~~-53~~

index e986b6c8..b902d054 100644
@@ -38,7 +38,6 @@ extern "C" {
  * should be assumed.
  */
 #define BTRFS_SEND_BUF_SIZE_V1 (64 * 1024)
-#define BTRFS_SEND_READ_SIZE (1024 * 48)
 
 enum btrfs_tlv_type {
 	BTRFS_TLV_U8,

@@ -72,77 +71,110 @@ struct btrfs_tlv_header {
 
 /* commands */
 enum btrfs_send_cmd {
-	BTRFS_SEND_C_UNSPEC,
+	BTRFS_SEND_C_UNSPEC = 0,
 
-	BTRFS_SEND_C_SUBVOL,
-	BTRFS_SEND_C_SNAPSHOT,
+	/* Version 1 */
+	BTRFS_SEND_C_SUBVOL = 1,
+	BTRFS_SEND_C_SNAPSHOT = 2,
 
-	BTRFS_SEND_C_MKFILE,
-	BTRFS_SEND_C_MKDIR,
-	BTRFS_SEND_C_MKNOD,
-	BTRFS_SEND_C_MKFIFO,
-	BTRFS_SEND_C_MKSOCK,
-	BTRFS_SEND_C_SYMLINK,
+	BTRFS_SEND_C_MKFILE = 3,
+	BTRFS_SEND_C_MKDIR = 4,
+	BTRFS_SEND_C_MKNOD = 5,
+	BTRFS_SEND_C_MKFIFO = 6,
+	BTRFS_SEND_C_MKSOCK = 7,
+	BTRFS_SEND_C_SYMLINK = 8,
 
-	BTRFS_SEND_C_RENAME,
-	BTRFS_SEND_C_LINK,
-	BTRFS_SEND_C_UNLINK,
-	BTRFS_SEND_C_RMDIR,
+	BTRFS_SEND_C_RENAME = 9,
+	BTRFS_SEND_C_LINK = 10,
+	BTRFS_SEND_C_UNLINK = 11,
+	BTRFS_SEND_C_RMDIR = 12,
 
-	BTRFS_SEND_C_SET_XATTR,
-	BTRFS_SEND_C_REMOVE_XATTR,
+	BTRFS_SEND_C_SET_XATTR = 13,
+	BTRFS_SEND_C_REMOVE_XATTR = 14,
 
-	BTRFS_SEND_C_WRITE,
-	BTRFS_SEND_C_CLONE,
+	BTRFS_SEND_C_WRITE = 15,
+	BTRFS_SEND_C_CLONE = 16,
 
-	BTRFS_SEND_C_TRUNCATE,
-	BTRFS_SEND_C_CHMOD,
-	BTRFS_SEND_C_CHOWN,
-	BTRFS_SEND_C_UTIMES,
+	BTRFS_SEND_C_TRUNCATE = 17,
+	BTRFS_SEND_C_CHMOD = 18,
+	BTRFS_SEND_C_CHOWN = 19,
+	BTRFS_SEND_C_UTIMES = 20,
 
-	BTRFS_SEND_C_END,
-	BTRFS_SEND_C_UPDATE_EXTENT,
-	__BTRFS_SEND_C_MAX,
+	BTRFS_SEND_C_END = 21,
+	BTRFS_SEND_C_UPDATE_EXTENT = 22,
+	BTRFS_SEND_C_MAX_V1 = 22,
+
+	/* Version 2 */
+	BTRFS_SEND_C_FALLOCATE = 23,
+	BTRFS_SEND_C_SETFLAGS = 24,
+	BTRFS_SEND_C_ENCODED_WRITE = 25,
+	BTRFS_SEND_C_MAX_V2 = 25,
+
+	/* End */
+	BTRFS_SEND_C_MAX = 25,
 };
-#define BTRFS_SEND_C_MAX (__BTRFS_SEND_C_MAX - 1)
 
 /* attributes in send stream */
 enum {
-	BTRFS_SEND_A_UNSPEC,
+	BTRFS_SEND_A_UNSPEC = 0,
 
-	BTRFS_SEND_A_UUID,
-	BTRFS_SEND_A_CTRANSID,
+	/* Version 1 */
+	BTRFS_SEND_A_UUID = 1,
+	BTRFS_SEND_A_CTRANSID = 2,
 
-	BTRFS_SEND_A_INO,
-	BTRFS_SEND_A_SIZE,
-	BTRFS_SEND_A_MODE,
-	BTRFS_SEND_A_UID,
-	BTRFS_SEND_A_GID,
-	BTRFS_SEND_A_RDEV,
-	BTRFS_SEND_A_CTIME,
-	BTRFS_SEND_A_MTIME,
-	BTRFS_SEND_A_ATIME,
-	BTRFS_SEND_A_OTIME,
+	BTRFS_SEND_A_INO = 3,
+	BTRFS_SEND_A_SIZE = 4,
+	BTRFS_SEND_A_MODE = 5,
+	BTRFS_SEND_A_UID = 6,
+	BTRFS_SEND_A_GID = 7,
+	BTRFS_SEND_A_RDEV = 8,
+	BTRFS_SEND_A_CTIME = 9,
+	BTRFS_SEND_A_MTIME = 10,
+	BTRFS_SEND_A_ATIME = 11,
+	BTRFS_SEND_A_OTIME = 12,
 
-	BTRFS_SEND_A_XATTR_NAME,
-	BTRFS_SEND_A_XATTR_DATA,
+	BTRFS_SEND_A_XATTR_NAME = 13,
+	BTRFS_SEND_A_XATTR_DATA = 14,
 
-	BTRFS_SEND_A_PATH,
-	BTRFS_SEND_A_PATH_TO,
-	BTRFS_SEND_A_PATH_LINK,
+	BTRFS_SEND_A_PATH = 15,
+	BTRFS_SEND_A_PATH_TO = 16,
+	BTRFS_SEND_A_PATH_LINK = 17,
 
-	BTRFS_SEND_A_FILE_OFFSET,
-	BTRFS_SEND_A_DATA,
+	BTRFS_SEND_A_FILE_OFFSET = 18,
+	/*
+	 * As of 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 = 19,
 
-	BTRFS_SEND_A_CLONE_UUID,
-	BTRFS_SEND_A_CLONE_CTRANSID,
-	BTRFS_SEND_A_CLONE_PATH,
-	BTRFS_SEND_A_CLONE_OFFSET,
-	BTRFS_SEND_A_CLONE_LEN,
+	BTRFS_SEND_A_CLONE_UUID = 20,
+	BTRFS_SEND_A_CLONE_CTRANSID = 21,
+	BTRFS_SEND_A_CLONE_PATH = 22,
+	BTRFS_SEND_A_CLONE_OFFSET = 23,
+	BTRFS_SEND_A_CLONE_LEN = 24,
 
-	__BTRFS_SEND_A_MAX,
+	BTRFS_SEND_A_MAX_V1 = 24,
+
+	/* Version 2 */
+	BTRFS_SEND_A_FALLOCATE_MODE = 25,
+
+	BTRFS_SEND_A_SETFLAGS_FLAGS = 26,
+
+	BTRFS_SEND_A_UNENCODED_FILE_LEN = 27,
+	BTRFS_SEND_A_UNENCODED_LEN = 28,
+	BTRFS_SEND_A_UNENCODED_OFFSET = 29,
+	/*
+	 * COMPRESSION and ENCRYPTION default to NONE (0) if omitted from
+	 * BTRFS_SEND_C_ENCODED_WRITE.
+	 */
+	BTRFS_SEND_A_COMPRESSION = 30,
+	BTRFS_SEND_A_ENCRYPTION = 31,
+	BTRFS_SEND_A_MAX_V2 = 31,
+
+	/* End */
+	BTRFS_SEND_A_MAX = 31,
 };
-#define BTRFS_SEND_A_MAX (__BTRFS_SEND_A_MAX - 1)
 
 #ifdef __cplusplus
 }

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #22 lore raw

PATCH v14 5/10 btrfs-progs: receive: process encoded_write commands

In reply to Omar Sandoval

From: Boris Burkov <[email protected]>

Add a new btrfs_send_op and support for both dumping and proper receive processing which does actual encoded writes.

Encoded writes are only allowed on a file descriptor opened with an extra flag that allows encoded writes, so we also add support for this flag when opening or reusing a file for writing.

Signed-off-by Boris Burkov <[email protected]>

---
 cmds/receive-dump.c  |  16 +++++-
 cmds/receive.c       |  48 ++++++++++++++++
 common/send-stream.c |  29 ++++++++++
 common/send-stream.h |   4 ++
 ioctl.h              | 132 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 228 insertions(+), 1 deletion(-)

cmds/receive-dump.c +15 -1

index 00ad4fd1..83701b62 100644
@@ -318,6 +318,19 @@ static int print_update_extent(const char *path, u64 offset, u64 len,
 			  offset, len);
 }
 
+static int print_encoded_write(const char *path, const void *data, u64 offset,
+			       u64 len, u64 unencoded_file_len,
+			       u64 unencoded_len, u64 unencoded_offset,
+			       u32 compression, u32 encryption, void *user)
+{
+	return PRINT_DUMP(user, path, "encoded_write",
+			  "offset=%llu len=%llu, unencoded_file_len=%llu, "
+			  "unencoded_len=%llu, unencoded_offset=%llu, "
+			  "compression=%u, encryption=%u",
+			  offset, len, unencoded_file_len, unencoded_len,
+			  unencoded_offset, compression, encryption);
+}
+
 struct btrfs_send_ops btrfs_print_send_ops = {
 	.subvol = print_subvol,
 	.snapshot = print_snapshot,

@@ -339,5 +352,6 @@ struct btrfs_send_ops btrfs_print_send_ops = {
 	.chmod = print_chmod,
 	.chown = print_chown,
 	.utimes = print_utimes,
-	.update_extent = print_update_extent
+	.update_extent = print_update_extent,
+	.encoded_write = print_encoded_write,
 };

cmds/receive.c +48 -0

78 lines not on this page · raw message

common/send-stream.c +29 -0

index 81a830d9..ce7c40f5 100644
@@ -357,6 +357,8 @@ static int read_and_process_cmd(struct btrfs_send_stream *sctx)
 	struct timespec mt;
 	u8 uuid[BTRFS_UUID_SIZE];
 	u8 clone_uuid[BTRFS_UUID_SIZE];
+	u32 compression;
+	u32 encryption;
 	u64 tmp;
 	u64 tmp2;
 	u64 ctransid;

@@ -366,6 +368,9 @@ static int read_and_process_cmd(struct btrfs_send_stream *sctx)
 	u64 clone_offset;
 	u64 offset;
 	u64 ino;
+	u64 unencoded_file_len;
+	u64 unencoded_len;
+	u64 unencoded_offset;
 	int len;
 	int xattr_len;

@@ -452,6 +457,30 @@ static int read_and_process_cmd(struct btrfs_send_stream *sctx)
 		TLV_GET(sctx, BTRFS_SEND_A_DATA, &data, &len);
 		ret = sctx->ops->write(path, data, offset, len, sctx->user);
 		break;
+	case BTRFS_SEND_C_ENCODED_WRITE:
+		TLV_GET_STRING(sctx, BTRFS_SEND_A_PATH, &path);
+		TLV_GET_U64(sctx, BTRFS_SEND_A_FILE_OFFSET, &offset);
+		TLV_GET_U64(sctx, BTRFS_SEND_A_UNENCODED_FILE_LEN,
+			    &unencoded_file_len);
+		TLV_GET_U64(sctx, BTRFS_SEND_A_UNENCODED_LEN, &unencoded_len);
+		TLV_GET_U64(sctx, BTRFS_SEND_A_UNENCODED_OFFSET,
+			    &unencoded_offset);
+		/* Compression and encryption default to none if omitted. */
+		if (sctx->cmd_attrs[BTRFS_SEND_A_COMPRESSION].data)
+			TLV_GET_U32(sctx, BTRFS_SEND_A_COMPRESSION, &compression);
+		else
+			compression = BTRFS_ENCODED_IO_COMPRESSION_NONE;
+		if (sctx->cmd_attrs[BTRFS_SEND_A_ENCRYPTION].data)
+			TLV_GET_U32(sctx, BTRFS_SEND_A_ENCRYPTION, &encryption);
+		else
+			encryption = BTRFS_ENCODED_IO_ENCRYPTION_NONE;
+		TLV_GET(sctx, BTRFS_SEND_A_DATA, &data, &len);
+		ret = sctx->ops->encoded_write(path, data, offset, len,
+					       unencoded_file_len,
+					       unencoded_len, unencoded_offset,
+					       compression, encryption,
+					       sctx->user);
+		break;
 	case BTRFS_SEND_C_CLONE:
 		TLV_GET_STRING(sctx, BTRFS_SEND_A_PATH, &path);
 		TLV_GET_U64(sctx, BTRFS_SEND_A_FILE_OFFSET, &offset);

common/send-stream.h +4 -0

index 2de51eac..44abbc9d 100644
@@ -53,6 +53,10 @@ struct btrfs_send_ops {
 		      struct timespec *mt, struct timespec *ct,
 		      void *user);
 	int (*update_extent)(const char *path, u64 offset, u64 len, void *user);
+	int (*encoded_write)(const char *path, const void *data, u64 offset,
+			     u64 len, u64 unencoded_file_len, u64 unencoded_len,
+			     u64 unencoded_offset, u32 compression,
+			     u32 encryption, void *user);
 };
 
 int btrfs_read_and_process_send_stream(int fd,

ioctl.h +132 -0

144 lines not on this page · raw message

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #24 lore raw

PATCH v14 7/10 btrfs-progs: receive: process fallocate commands

In reply to Omar Sandoval

From: Boris Burkov <[email protected]>

Send stream v2 can emit fallocate commands, so receive must support them as well. The implementation simply passes along the arguments to the syscall. Note that mode is encoded as a u32 in send stream but fallocate takes an int, so there is a unsigned->signed conversion there.

Reviewed-by Nikolay Borisov <[email protected]>

Signed-off-by Boris Burkov <[email protected]>

---
 cmds/receive-dump.c  |  9 +++++++++
 cmds/receive.c       | 25 +++++++++++++++++++++++++
 common/send-stream.c |  9 +++++++++
 common/send-stream.h |  2 ++
 4 files changed, 45 insertions(+)

cmds/receive-dump.c +9 -0

index 83701b62..fa397bcf 100644
@@ -331,6 +331,14 @@ static int print_encoded_write(const char *path, const void *data, u64 offset,
 			  unencoded_offset, compression, encryption);
 }
 
+static int print_fallocate(const char *path, int mode, u64 offset, u64 len,
+			   void *user)
+{
+	return PRINT_DUMP(user, path, "fallocate",
+			  "mode=%d offset=%llu len=%llu",
+			  mode, offset, len);
+}
+
 struct btrfs_send_ops btrfs_print_send_ops = {
 	.subvol = print_subvol,
 	.snapshot = print_snapshot,

@@ -354,4 +362,5 @@ struct btrfs_send_ops btrfs_print_send_ops = {
 	.utimes = print_utimes,
 	.update_extent = print_update_extent,
 	.encoded_write = print_encoded_write,
+	.fallocate = print_fallocate,
 };

cmds/receive.c +25 -0

index 5fd939ce..4893d693 100644
@@ -1260,6 +1260,30 @@ static int process_encoded_write(const char *path, const void *data, u64 offset,
 				    compression);
 }
 
+static int process_fallocate(const char *path, int mode, u64 offset, u64 len,
+			     void *user)
+{
+	int ret;
+	struct btrfs_receive *rctx = user;
+	char full_path[PATH_MAX];
+
+	ret = path_cat_out(full_path, rctx->full_subvol_path, path);
+	if (ret < 0) {
+		error("fallocate: path invalid: %s", path);
+		return ret;
+	}
+	ret = open_inode_for_write(rctx, full_path);
+	if (ret < 0)
+		return ret;
+	ret = fallocate(rctx->write_fd, mode, offset, len);
+	if (ret < 0) {
+		ret = -errno;
+		error("fallocate: fallocate on %s failed: %m", path);
+		return ret;
+	}
+	return 0;
+}
+
 static struct btrfs_send_ops send_ops = {
 	.subvol = process_subvol,
 	.snapshot = process_snapshot,

@@ -1283,6 +1307,7 @@ static struct btrfs_send_ops send_ops = {
 	.utimes = process_utimes,
 	.update_extent = process_update_extent,
 	.encoded_write = process_encoded_write,
+	.fallocate = process_fallocate,
 };
 
 static int do_receive(struct btrfs_receive *rctx, const char *tomnt,

common/send-stream.c +9 -0

index ce7c40f5..2d0aa624 100644
@@ -373,6 +373,7 @@ static int read_and_process_cmd(struct btrfs_send_stream *sctx)
 	u64 unencoded_offset;
 	int len;
 	int xattr_len;
+	int fallocate_mode;
 
 	ret = read_cmd(sctx);
 	if (ret)

@@ -537,6 +538,14 @@ static int read_and_process_cmd(struct btrfs_send_stream *sctx)
 	case BTRFS_SEND_C_END:
 		ret = 1;
 		break;
+	case BTRFS_SEND_C_FALLOCATE:
+		TLV_GET_STRING(sctx, BTRFS_SEND_A_PATH, &path);
+		TLV_GET_U32(sctx, BTRFS_SEND_A_FALLOCATE_MODE, &fallocate_mode);
+		TLV_GET_U64(sctx, BTRFS_SEND_A_FILE_OFFSET, &offset);
+		TLV_GET_U64(sctx, BTRFS_SEND_A_SIZE, &tmp);
+		ret = sctx->ops->fallocate(path, fallocate_mode, offset, tmp,
+					   sctx->user);
+		break;
 	}
 
 tlv_get_failed:

common/send-stream.h +2 -0

index 44abbc9d..61a88d3d 100644
@@ -57,6 +57,8 @@ struct btrfs_send_ops {
 			     u64 len, u64 unencoded_file_len, u64 unencoded_len,
 			     u64 unencoded_offset, u32 compression,
 			     u32 encryption, void *user);
+	int (*fallocate)(const char *path, int mode, u64 offset, u64 len,
+			 void *user);
 };
 
 int btrfs_read_and_process_send_stream(int fd,

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #23 lore raw

PATCH v14 6/10 btrfs-progs: receive: encoded_write fallback to explicit decode and write

In reply to Omar Sandoval

From: Boris Burkov <[email protected]>

An encoded_write can fail if the file system it is being applied to does not support encoded writes or if it can't find enough contiguous space to accommodate the encoded extent. In those cases, we can likely still process an encoded_write by explicitly decoding the data and doing a normal write.

Add the necessary fallback path for decoding data compressed with zlib, lzo, or zstd. zlib and zstd have reusable decoding context data structures which we cache in the receive context so that we don't have to recreate them on every encoded_write.

Finally, add a command line flag for force-decompress which causes receive to always use the fallback path rather than first attempting the encoded write.

Signed-off-by Boris Burkov <[email protected]>

---
 Documentation/btrfs-receive.rst |   5 +
 cmds/receive.c                  | 261 +++++++++++++++++++++++++++++++-
 2 files changed, 259 insertions(+), 7 deletions(-)

Documentation/btrfs-receive.rst +5 -0

index 86ffdcc6..b9a3cad6 100644
@@ -57,6 +57,11 @@ A subvolume is made read-only after the receiving process finishes successfully
         If */proc* is not accessible, eg. in a chroot environment, use this option to
         tell us where this filesystem is mounted.
 
+--force-decompress
+        if the stream contains compressed data (see *--compressed-data* in
+        ``btrfs-send(8)``), always decompress it instead of writing it with
+        encoded I/O
+
 --dump
         dump the stream metadata, one line per operation

cmds/receive.c +254 -7

315 lines not on this page · raw message

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #25 lore raw

PATCH v14 8/10 btrfs-progs: receive: process setflags ioctl commands

In reply to Omar Sandoval

From: Boris Burkov <[email protected]>

In send stream v2, send can emit a command for setting inode flags via the setflags ioctl. Pass the flags attribute through to the ioctl call in receive.

Reviewed-by Nikolay Borisov <[email protected]>

Signed-off-by Boris Burkov <[email protected]>

---
 cmds/receive-dump.c  |  6 ++++++
 cmds/receive.c       | 25 +++++++++++++++++++++++++
 common/send-stream.c |  7 +++++++
 common/send-stream.h |  1 +
 4 files changed, 39 insertions(+)

cmds/receive-dump.c +6 -0

index fa397bcf..df5991e1 100644
@@ -339,6 +339,11 @@ static int print_fallocate(const char *path, int mode, u64 offset, u64 len,
 			  mode, offset, len);
 }
 
+static int print_setflags(const char *path, int flags, void *user)
+{
+	return PRINT_DUMP(user, path, "setflags", "flags=%d", flags);
+}
+
 struct btrfs_send_ops btrfs_print_send_ops = {
 	.subvol = print_subvol,
 	.snapshot = print_snapshot,

@@ -363,4 +368,5 @@ struct btrfs_send_ops btrfs_print_send_ops = {
 	.update_extent = print_update_extent,
 	.encoded_write = print_encoded_write,
 	.fallocate = print_fallocate,
+	.setflags = print_setflags,
 };

cmds/receive.c +25 -0

index 4893d693..7f76a04f 100644
@@ -38,6 +38,7 @@
 #include <sys/types.h>
 #include <sys/uio.h>
 #include <sys/xattr.h>
+#include <linux/fs.h>
 #include <uuid/uuid.h>
 
 #include <lzo/lzo1x.h>

@@ -1284,6 +1285,29 @@ static int process_fallocate(const char *path, int mode, u64 offset, u64 len,
 	return 0;
 }
 
+static int process_setflags(const char *path, int flags, void *user)
+{
+	int ret;
+	struct btrfs_receive *rctx = user;
+	char full_path[PATH_MAX];
+
+	ret = path_cat_out(full_path, rctx->full_subvol_path, path);
+	if (ret < 0) {
+		error("setflags: path invalid: %s", path);
+		return ret;
+	}
+	ret = open_inode_for_write(rctx, full_path);
+	if (ret < 0)
+		return ret;
+	ret = ioctl(rctx->write_fd, FS_IOC_SETFLAGS, &flags);
+	if (ret < 0) {
+		ret = -errno;
+		error("setflags: setflags ioctl on %s failed: %m", path);
+		return ret;
+	}
+	return 0;
+}
+
 static struct btrfs_send_ops send_ops = {
 	.subvol = process_subvol,
 	.snapshot = process_snapshot,

@@ -1308,6 +1332,7 @@ static struct btrfs_send_ops send_ops = {
 	.update_extent = process_update_extent,
 	.encoded_write = process_encoded_write,
 	.fallocate = process_fallocate,
+	.setflags = process_setflags,
 };
 
 static int do_receive(struct btrfs_receive *rctx, const char *tomnt,

common/send-stream.c +7 -0

index 2d0aa624..21295cbb 100644
@@ -374,6 +374,7 @@ static int read_and_process_cmd(struct btrfs_send_stream *sctx)
 	int len;
 	int xattr_len;
 	int fallocate_mode;
+	int setflags_flags;
 
 	ret = read_cmd(sctx);
 	if (ret)

@@ -546,8 +547,14 @@ static int read_and_process_cmd(struct btrfs_send_stream *sctx)
 		ret = sctx->ops->fallocate(path, fallocate_mode, offset, tmp,
 					   sctx->user);
 		break;
+	case BTRFS_SEND_C_SETFLAGS:
+		TLV_GET_STRING(sctx, BTRFS_SEND_A_PATH, &path);
+		TLV_GET_U32(sctx, BTRFS_SEND_A_SETFLAGS_FLAGS, &setflags_flags);
+		ret = sctx->ops->setflags(path, setflags_flags, sctx->user);
+		break;
 	}
 
+
 tlv_get_failed:
 out:
 	free(path);

common/send-stream.h +1 -0

index 61a88d3d..3189f889 100644
@@ -59,6 +59,7 @@ struct btrfs_send_ops {
 			     u32 encryption, void *user);
 	int (*fallocate)(const char *path, int mode, u64 offset, u64 len,
 			 void *user);
+	int (*setflags)(const char *path, int flags, void *user);
 };
 
 int btrfs_read_and_process_send_stream(int fd,

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #26 lore raw

PATCH v14 9/10 btrfs-progs: send: stream v2 ioctl flags

In reply to Omar Sandoval

From: Omar Sandoval <[email protected]>

First, add a --proto option to allow specifying the desired send protocol version. It defaults to one, the original version. In a couple of releases once people are aware that protocol revisions are happening, we can change it to default to zero, which means the latest version supported by the kernel. This is based on Dave Sterba's patch.

Also add a --compressed-data flag to instruct the kernel to use encoded_write commands for compressed extents. This requires an explicit opt in separate from the protocol version because:

1. The user may not want compression on the receiving side, or may want a different compression algorithm/level on the receiving side.
2. It has a soft requirement for kernel support on the receiving side (btrfs-progs can fall back to decompressing and writing if the kernel doesn't support BTRFS_IOC_ENCODED_WRITE, but the user may not be prepared to pay that CPU cost). Going forward, since it's easier to update progs than the kernel, I think we'll want to make new send features that require kernel support opt-in, whereas anything that only requires a progs update can happen automatically.

Signed-off-by Boris Burkov <[email protected]>

Signed-off-by Omar Sandoval <[email protected]>

---
 Documentation/btrfs-send.rst |  22 ++++++++
 cmds/send.c                  | 100 ++++++++++++++++++++++++++++++++++-
 ioctl.h                      |  19 ++++++-
 kernel-shared/send.h         |   2 +-
 4 files changed, 138 insertions(+), 5 deletions(-)

Documentation/btrfs-send.rst +22 -0

index 4526532e..291c537e 100644
@@ -60,6 +60,28 @@ please see section *SUBVOLUME FLAGS* in ``btrfs-subvolume(8)``.
         used to transfer changes. This mode is faster and is useful to show the
         differences in metadata.
 
+--proto <N>
+        use send protocol version N
+
+        The default is 1, which was the original protocol version. Version 2
+        encodes file data slightly more efficiently; it is also required for
+        sending compressed data directly (see *--compressed-data*). Version 2
+        requires at least btrfs-progs 5.18 on both the sender and receiver and
+        at least Linux 5.18 on the sender. Passing 0 means to use the highest
+        version supported by the running kernel.
+
+--compressed-data
+        send data that is compressed on the filesystem directly without
+        decompressing it
+
+        If the receiver supports the *BTRFS_IOC_ENCODED_WRITE* ioctl (added in
+        Linux 5.18), it can also write it directly without decompressing it.
+        Otherwise, the receiver will fall back to decompressing it and writing
+        it normally.
+
+        This requires protocol version 2 or higher. If *--proto* was not used,
+        then *--compressed-data* implies *--proto 2*.
+
 -q|--quiet
         (deprecated) alias for global *-q* option

cmds/send.c +98 -2

161 lines not on this page · raw message

ioctl.h +17 -2

index 8adf63c2..f19695e3 100644
@@ -655,10 +655,24 @@ BUILD_ASSERT(sizeof(struct btrfs_ioctl_received_subvol_args_32) == 192);
  */
 #define BTRFS_SEND_FLAG_OMIT_END_CMD		0x4
 
+/*
+ * Read the protocol version in the structure
+ */
+#define BTRFS_SEND_FLAG_VERSION			0x8
+
+/*
+ * Send compressed data using the ENCODED_WRITE command instead of decompressing
+ * the data and sending it with the WRITE command. This requires protocol
+ * version >= 2.
+ */
+#define BTRFS_SEND_FLAG_COMPRESSED		0x10
+
 #define BTRFS_SEND_FLAG_MASK \
 	(BTRFS_SEND_FLAG_NO_FILE_DATA | \
 	 BTRFS_SEND_FLAG_OMIT_STREAM_HEADER | \
-	 BTRFS_SEND_FLAG_OMIT_END_CMD)
+	 BTRFS_SEND_FLAG_OMIT_END_CMD | \
+	 BTRFS_SEND_FLAG_VERSION | \
+	 BTRFS_SEND_FLAG_COMPRESSED)
 
 struct btrfs_ioctl_send_args {
 	__s64 send_fd;			/* in */

@@ -666,7 +680,8 @@ struct btrfs_ioctl_send_args {
 	__u64 __user *clone_sources;	/* in */
 	__u64 parent_root;		/* in */
 	__u64 flags;			/* in */
-	__u64 reserved[4];		/* in */
+	__u32 version;			/* in */
+	__u8 reserved[28];		/* in */
 };
 /*
  * Size of structure depends on pointer width, was not caught in the early

kernel-shared/send.h +1 -1

index b902d054..1f20d01a 100644
@@ -31,7 +31,7 @@ extern "C" {
 #endif
 
 #define BTRFS_SEND_STREAM_MAGIC "btrfs-stream"
-#define BTRFS_SEND_STREAM_VERSION 1
+#define BTRFS_SEND_STREAM_VERSION 2
 
 /*
  * In send stream v1, no command is larger than 64k. In send stream v2, no limit

Signature · 1 line

2.35.1

Omar Sandoval

author

2022-03-17 17:26 UTC #27 lore raw

PATCH v14 10/10 btrfs-progs: receive: add tests for basic encoded_write send/receive

In reply to Omar Sandoval

From: Boris Burkov <[email protected]>

Adapt the existing send/receive tests by passing '-o compress-force' to the mount commands in a new test. After writing a few files in the various compression formats, send/receive them with and without --force-decompress to test both the encoded_write path and the fallback to decode+write.

Signed-off-by Boris Burkov <[email protected]>

---
 .../052-receive-write-encoded/test.sh         | 114 ++++++++++++++++++
 1 file changed, 114 insertions(+)
 create mode 100755 tests/misc-tests/052-receive-write-encoded/test.sh

tests/misc-tests/052-receive-write-encoded/test.sh new file +114 -0

new file mode 100755
index 00000000..47330281
@@ -0,0 +1,114 @@
+#!/bin/bash
+#
+# test that we can send and receive encoded writes for three modes of
+# transparent compression: zlib, lzo, and zstd.
+
+source "$TEST_TOP/common"
+
+check_prereq mkfs.btrfs
+check_prereq btrfs
+
+setup_root_helper
+prepare_test_dev
+
+here=`pwd`
+
+# assumes the filesystem exists, and does mount, write, snapshot, send, unmount
+# for the specified encoding option
+send_one() {
+	local str
+	local subv
+	local snap
+
+	algorithm="$1"
+	shift
+	str="$1"
+	shift
+
+	subv="subv-$algorithm"
+	snap="snap-$algorithm"
+
+	run_check_mount_test_dev "-o" "compress-force=$algorithm"
+	cd "$TEST_MNT" || _fail "cannot chdir to TEST_MNT"
+
+	run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "$subv"
+	run_check $SUDO_HELPER dd if=/dev/zero of="$subv/file1" bs=1M count=1
+	run_check $SUDO_HELPER dd if=/dev/zero of="$subv/file2" bs=500K count=1
+	run_check $SUDO_HELPER "$TOP/btrfs" subvolume snapshot -r "$subv" "$snap"
+	run_check $SUDO_HELPER "$TOP/btrfs" send -f "$str" "$snap" "$@"
+
+	cd "$here" || _fail "cannot chdir back to test directory"
+	run_check_umount_test_dev
+}
+
+receive_one() {
+	local str
+	str="$1"
+	shift
+
+	run_check_mkfs_test_dev
+	run_check_mount_test_dev
+	run_check $SUDO_HELPER "$TOP/btrfs" receive "$@" -v -f "$str" "$TEST_MNT"
+	run_check_umount_test_dev
+	run_check rm -f -- "$str"
+}
+
+test_one_write_encoded() {
+	local str
+	local algorithm
+	algorithm="$1"
+	shift
+	str="$here/stream-$algorithm.stream"
+
+	run_check_mkfs_test_dev
+	send_one "$algorithm" "$str" --compressed-data
+	receive_one "$str" "$@"
+}
+
+test_one_stream_v1() {
+	local str
+	local algorithm
+	algorithm="$1"
+	shift
+	str="$here/stream-$algorithm.stream"
+
+	run_check_mkfs_test_dev
+	send_one "$algorithm" "$str" --proto 1
+	receive_one "$str" "$@"
+}
+
+test_mix_write_encoded() {
+	local strzlib
+	local strlzo
+	local strzstd
+	strzlib="$here/stream-zlib.stream"
+	strlzo="$here/stream-lzo.stream"
+	strzstd="$here/stream-zstd.stream"
+
+	run_check_mkfs_test_dev
+
+	send_one "zlib" "$strzlib" --compressed-data
+	send_one "lzo" "$strlzo" --compressed-data
+	send_one "zstd" "$strzstd" --compressed-data
+
+	receive_one "$strzlib"
+	receive_one "$strlzo"
+	receive_one "$strzstd"
+}
+
+test_one_write_encoded "zlib"
+test_one_write_encoded "lzo"
+test_one_write_encoded "zstd"
+
+# with decompression forced
+test_one_write_encoded "zlib" "--force-decompress"
+test_one_write_encoded "lzo" "--force-decompress"
+test_one_write_encoded "zstd" "--force-decompress"
+
+# send stream v1
+test_one_stream_v1 "zlib"
+test_one_stream_v1 "lzo"
+test_one_stream_v1 "zstd"
+
+# files use a mix of compression algorithms
+test_mix_write_encoded

Signature · 1 line

2.35.1

Sweet Tea Dorminy

2022-03-24 17:52 UTC #3 lore raw

Re: [PATCH v14 2/7] btrfs: send: explicitly number commands and attributes

In reply to Omar Sandoval

Quoted 4 lines · Omar Sandoval

From: Omar Sandoval <[email protected]>
Commit e77fbf990316 ("btrfs: send: prepare for v2 protocol") added _BTRFS_SEND_C_MAX_V* macros equal to the maximum command number for the

s/macros/enums/?

Quoted 8 lines

version plus 1, but as written this creates gaps in the number space. The maximum command number is currently 22, and __BTRFS_SEND_C_MAX_V1 is accordingly 23. But then __BTRFS_SEND_C_MAX_V2 is 24, suggesting that v2 has a command numbered 23, and __BTRFS_SEND_C_MAX is 25, suggesting that 23 and 24 are valid commands.
Instead, let's explicitly number all of the commands, attributes, and sentinel MAX constants.

If you were sending out another version, the last sentence explains what the change is doing but not why you like it as a solution to the problem.

Nit: I think it would be slightly more elegant to set the MAX values to the appropriate symbolic value instead of numerical, to emphasize the relationship a tiny bit more. Perhaps e.g.

enum btrfs_send_cmds {
	...
	BTRFS_SEND_C_UPDATE_EXTENT,
	__BTRFS_SEND_CMDS_V1_MAX = BTRFS_SEND_C_UPDATE_EXTENT,
	BTRFS_SEND_C_FALLOCATE,
	...
	BTRFS_SEND_C_ENCODED_WRITE,
	__BTRFS_SEND_CMDS_V2_MAX = BTRFS_SEND_C_ENCODED_WRITE,
	BTRFS_SEND_C_CMDS_MAX = __BTRFS_SEND_CMDS_V2_MAX,
};

(either with or without explicitly setting the numerical values of the individual commands). Or perhaps #define the MAX values instead, still in terms of the symbolic constant?

I have a mild preference for not explicitly setting things to numerical values, so it's harder to duplicate a value by accident later on; but explicit setting does help if one day some cmd needs to be dropped from the middle of the list without renumbering everything, so shrug.

Nit: If you do stick with explicit numerical values everywhere, is there a chance you could line up the = vertically, to make it easier to scan down the list of numbers and verify they are in order and without holes in the sequence? I know vertical alignment is usually unneeded, but in this case I think it does add a bunch to the readability of the table.

Sweet Tea Dorminy

2022-03-24 17:52 UTC #6 lore raw

Re: [PATCH v14 4/7] btrfs: send: write larger chunks when using stream v2

In reply to Omar Sandoval

fs/btrfs/send.c struct send_ctx { · line 85

Quoted 10 lines

fs/btrfs/send.c +1 -0

index 1f141de3a7d6..02053fff80ca 100644
@@ -82,6 +82,7 @@ struct send_ctx {
  	char *send_buf;
  	u32 send_size;
  	u32 send_max_size;
+	bool put_data;

put_data's use seems to be about making sure put_data_header() isn't called more than once, which is not super obvious to me from the name; perhaps one of 'data_header_{set,setup,initialized}' might make it clearer? Or if it's actually about put_file_data, maybe moving the assertion there would make that clearer?

Quoted 25 lines

fs/btrfs/send.c +15 -7

  static int put_data_header(struct send_ctx *sctx, u32 len)
  {
-	struct btrfs_tlv_header *hdr;
+	if (WARN_ON_ONCE(sctx->put_data))
+		return -EINVAL;
+	sctx->put_data = true;
+	if (sctx->proto >= 2) {
+		/*
+		 * In v2, the data attribute header doesn't include a length; it
+		 * is implicitly to the end of the command.
+		 */
+		if (sctx->send_max_size - sctx->send_size < 2 + len)
+			return -EOVERFLOW;
+		put_unaligned_le16(BTRFS_SEND_A_DATA,
+				   sctx->send_buf + sctx->send_size);
+		sctx->send_size += 2;
+	} else {
+		struct btrfs_tlv_header *hdr;
 
-	if (sctx->send_max_size - sctx->send_size < sizeof(*hdr) + len)
-		return -EOVERFLOW;
-	hdr = (struct btrfs_tlv_header *)(sctx->send_buf + sctx->send_size);
-	put_unaligned_le16(BTRFS_SEND_A_DATA, &hdr->tlv_type);
-	put_unaligned_le16(len, &hdr->tlv_len);
-	sctx->send_size += sizeof(*hdr);

fs/btrfs/send.c static inline u64 max_send_read_size(const struct send_ctx *sctx) · line 4886

Quoted 10 lines

fs/btrfs/send.c +8 -0

+		if (sctx->send_max_size - sctx->send_size < sizeof(*hdr) + len)
+			return -EOVERFLOW;
+		hdr = (struct btrfs_tlv_header *)(sctx->send_buf +
+						  sctx->send_size);
+		put_unaligned_le16(BTRFS_SEND_A_DATA, &hdr->tlv_type);
+		put_unaligned_le16(len, &hdr->tlv_len);
+		sctx->send_size += sizeof(*hdr);
+	}
  	return 0;
  }

I wish the 2s were named, and that there were more commonality between the two branches... Might I propose this alternative? It doesn't check the length's suitability until after adding the two fields, but I don't think anything bad happens from delaying the check.

static int put_data_header(struct send_ctx *sctx, u32 len)

{
        struct btrfs_tlv_header *hdr =
                (struct btrfs_tlv_header *)(sctx->send_buf + sctx->send_size);

        if (WARN_ON_ONCE(sctx->put_data))
                return -EINVAL;
        sctx->put_data = true;

        put_unaligned_le16(BTRFS_SEND_A_DATA, &hdr->tlv_type);
        sctx->send_size += sizeof(hdr->tlv_type);
                                                                                                                       
        /*
         * In v2+, the data attribute header doesn't include a length; it is
         * implicitly to the end of the command.
         */
        if (sctx->proto == 1) {
                put_unaligned_le16(len, &hdr->tlv_len);
                sctx->send_size += sizeof(hdr->tlv_len);
        }

        if (sctx->send_max_size - sctx->send_size < len)
                return -EOVERFLOW;

        return 0;
}

Sweet Tea Dorminy

2022-03-24 17:53 UTC #9 lore raw

Re: [PATCH v14 5/7] btrfs: send: allocate send buffer with alloc_page() and vmap() for v2

In reply to Omar Sandoval

Quoted 4 lines · Omar Sandoval

From: Omar Sandoval <[email protected]>
For encoded writes, we need the raw pages for reading compressed data directly via a bio.

Perhaps:
"For encoded writes, the existing btrfs_encoded_read*() functions expect a list of raw pages."

I think it would be a better to continue just vmalloc'ing a large continuous buffer and translating each page in the buffer into its raw page with something like is_vmalloc_addr(data) ? vmalloc_to_page(data) : virt_to_page(data). Vmalloc can request a higher-order allocation, which probably doesn't matter but might slightly improve memory locality. And in terms of readability, I somewhat like the elegance of having a single identical kvmalloc call to allocate and send_buf in both cases, even if we do need to initialize the page list for some v2 commands.

Sweet Tea Dorminy

Reviewed-by on the v14 series

2022-03-24 17:53 UTC #28 lore raw

In reply to Omar Sandoval

For the 7 kernel patches:

Reviewed-by Sweet Tea Dorminy <[email protected]>

Quoted 77 lines · Omar Sandoval

From: Omar Sandoval <[email protected]>
This series adds support for sending compressed data via Btrfs send and btrfs-progs support for sending/receiving compressed data and writing it with BTRFS_IOC_ENCODED_WRITE, which was previously merged into misc-next. See the previous posting for more details and benchmarks [1].
Patches 1 and 2 are cleanups for Btrfs send. Patches 3-5 prepare some protocol changes for send stream v2. Patch 6 implements compressed send. Patch 7 enables send stream v2 and compressed send in the send ioctl when requested.
Changes since v13 [2]:
- Rebased on latest misc-next branch.
- Dropped ioctl patches which are already in misc-next.
1: https://lore.kernel.org/linux-btrfs/[email protected]/
2: https://lore.kernel.org/linux-btrfs/[email protected]/
Omar Sandoval (7):
btrfs: send: remove unused send_ctx::{total,cmd}_send_size
btrfs: send: explicitly number commands and attributes
btrfs: add send stream v2 definitions
btrfs: send: write larger chunks when using stream v2
btrfs: send: allocate send buffer with alloc_page() and vmap() for v2
btrfs: send: send compressed extents with encoded writes
btrfs: send: enable support for stream v2 and compressed writes
  fs/btrfs/ctree.h           |   6 +
  fs/btrfs/inode.c           |  13 +-
  fs/btrfs/send.c            | 324 +++++++++++++++++++++++++++++++++----
  fs/btrfs/send.h            | 142 +++++++++-------
  include/uapi/linux/btrfs.h |  10 +-
  5 files changed, 395 insertions(+), 100 deletions(-)
The btrfs-progs patches were written by Boris Burkov with some updates from me. Patches 1-4 are preparation. Patch 5 implements encoded writes. Patch 6 implements the fallback to decompressing. Patches 7 and 8 implement the other commands. Patch 9 adds the new btrfs send options. Patch 10 adds a test case.
Changes since v13:
- Rebased on latest devel branch.
- Updated the btrfs_ioctl_encoded_io_args definition to the version that was merged into misc-next.
Boris Burkov (8):
btrfs-progs: receive: support v2 send stream larger tlv_len
btrfs-progs: receive: dynamically allocate sctx->read_buf
btrfs-progs: receive: support v2 send stream DATA tlv format
btrfs-progs: receive: process encoded_write commands
btrfs-progs: receive: encoded_write fallback to explicit decode and
write
btrfs-progs: receive: process fallocate commands
btrfs-progs: receive: process setflags ioctl commands
btrfs-progs: receive: add tests for basic encoded_write send/receive
Omar Sandoval (2):
btrfs-progs: receive: add send stream v2 cmds and attrs to send.h
btrfs-progs: send: stream v2 ioctl flags
  Documentation/btrfs-receive.rst               |   5 +
  Documentation/btrfs-send.rst                  |  22 ++
  cmds/receive-dump.c                           |  31 +-
  cmds/receive.c                                | 347 +++++++++++++++++-
  cmds/send.c                                   | 100 ++++-
  common/send-stream.c                          | 165 +++++++--
  common/send-stream.h                          |   7 +
  ioctl.h                                       | 151 +++++++-
  kernel-shared/send.h                          | 146 +++++---
  libbtrfs/send-stream.c                        |   2 +-
  .../052-receive-write-encoded/test.sh         | 114 ++++++
  11 files changed, 993 insertions(+), 97 deletions(-)
create mode 100755 tests/misc-tests/052-receive-write-encoded/test.sh

Omar Sandoval

author

2022-03-30 16:03 UTC #10 lore raw

Re: [PATCH v14 5/7] btrfs: send: allocate send buffer with alloc_page() and vmap() for v2

In reply to Sweet Tea Dorminy

Quoted 11 lines · Sweet Tea Dorminy

Quoted 4 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
For encoded writes, we need the raw pages for reading compressed data directly via a bio.
Perhaps:
"For encoded writes, the existing btrfs_encoded_read*() functions expect a list of raw pages."

Quoted 8 lines

I think it would be a better to continue just vmalloc'ing a large continuous buffer and translating each page in the buffer into its raw page with something like is_vmalloc_addr(data) ? vmalloc_to_page(data) :
virt_to_page(data). Vmalloc can request a higher-order allocation, which probably doesn't matter but might slightly improve memory locality. And in terms of readability, I somewhat like the elegance of having a single identical kvmalloc call to allocate and send_buf in both cases, even if we do need to initialize the page list for some v2 commands.

I like this, but are we guaranteed that kvmalloc() will return a page-aligned buffer? It seems reasonable to me that it would for allocations of at least one page, but I can't find that written down anywhere.

Sweet Tea Dorminy

2022-03-30 16:33 UTC #11 lore raw

Re: [PATCH v14 5/7] btrfs: send: allocate send buffer with alloc_page() and vmap() for v2

Quoted 21 lines · Omar Sandoval

Quoted 11 lines · Sweet Tea Dorminy
Quoted 4 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
For encoded writes, we need the raw pages for reading compressed data directly via a bio.
Perhaps:
"For encoded writes, the existing btrfs_encoded_read*() functions expect a list of raw pages."
Quoted 8 lines
I think it would be a better to continue just vmalloc'ing a large continuous buffer and translating each page in the buffer into its raw page with something like is_vmalloc_addr(data) ? vmalloc_to_page(data) :
virt_to_page(data). Vmalloc can request a higher-order allocation, which probably doesn't matter but might slightly improve memory locality. And in terms of readability, I somewhat like the elegance of having a single identical kvmalloc call to allocate and send_buf in both cases, even if we do need to initialize the page list for some v2 commands.

Quoted 4 lines

I like this, but are we guaranteed that kvmalloc() will return a page-aligned buffer? It seems reasonable to me that it would for allocations of at least one page, but I can't find that written down anywhere.

Since vmalloc allocates whole pages, and kmalloc guarantees alignment to the allocation size for powers of 2 sizes (and PAGE_SIZE is required to be a power of 2), I think that adds up to a guarantee of page alignment both ways?

https://elixir.bootlin.com/linux/v5.17.1/source/include/linux/slab.h#L522 : kmalloc: "For @size of power of two bytes, the alignment is also guaranteed to be at least to the size."
https://elixir.bootlin.com/linux/v5.17.1/source/mm/vmalloc.c#L3180: vmalloc: " Allocate enough pages"...

Omar Sandoval

author

2022-03-30 17:06 UTC #7 lore raw

Re: [PATCH v14 4/7] btrfs: send: write larger chunks when using stream v2

In reply to Sweet Tea Dorminy

Quoted 15 lines · Sweet Tea Dorminy

fs/btrfs/send.c struct send_ctx { · line 85
Quoted 10 lines
fs/btrfs/send.c +1 -0
index 1f141de3a7d6..02053fff80ca 100644
@@ -82,6 +82,7 @@ struct send_ctx {
  	char *send_buf;
  	u32 send_size;
  	u32 send_max_size;
+	bool put_data;
put_data's use seems to be about making sure put_data_header() isn't called more than once, which is not super obvious to me from the name; perhaps one of 'data_header_{set,setup,initialized}' might make it clearer? Or if it's actually about put_file_data, maybe moving the assertion there would make that clearer?

The intention is to prevent adding another attribute after a data attribute, since that's impossible with v2. Notice that it's also checked in tlv_put(). So "put_data" means "was a data attribute already added to this command?"; it's not specifically about the data header or data itself. I'll add a comment to that effect.

Quoted 31 lines

fs/btrfs/send.c +22 -7

  static int put_data_header(struct send_ctx *sctx, u32 len)
  {
-	struct btrfs_tlv_header *hdr;
+	if (WARN_ON_ONCE(sctx->put_data))
+		return -EINVAL;
+	sctx->put_data = true;
+	if (sctx->proto >= 2) {
+		/*
+		 * In v2, the data attribute header doesn't include a length; it
+		 * is implicitly to the end of the command.
+		 */
+		if (sctx->send_max_size - sctx->send_size < 2 + len)
+			return -EOVERFLOW;
+		put_unaligned_le16(BTRFS_SEND_A_DATA,
+				   sctx->send_buf + sctx->send_size);
+		sctx->send_size += 2;
+	} else {
+		struct btrfs_tlv_header *hdr;
-	if (sctx->send_max_size - sctx->send_size < sizeof(*hdr) + len)
-		return -EOVERFLOW;
-	hdr = (struct btrfs_tlv_header *)(sctx->send_buf + sctx->send_size);
-	put_unaligned_le16(BTRFS_SEND_A_DATA, &hdr->tlv_type);
-	put_unaligned_le16(len, &hdr->tlv_len);
-	sctx->send_size += sizeof(*hdr);
+		if (sctx->send_max_size - sctx->send_size < sizeof(*hdr) + len)
+			return -EOVERFLOW;
+		hdr = (struct btrfs_tlv_header *)(sctx->send_buf +
+						  sctx->send_size);
+		put_unaligned_le16(BTRFS_SEND_A_DATA, &hdr->tlv_type);
+		put_unaligned_le16(len, &hdr->tlv_len);
+		sctx->send_size += sizeof(*hdr);

Quoted 8 lines

fs/btrfs/send.c static inline u64 max_send_read_size(const struct send_ctx *sctx) · line 4886
Quoted 3 lines
fs/btrfs/send.c +1 -0
+	}
  	return 0;
  }
I wish the 2s were named, and that there were more commonality between the two branches... Might I propose this alternative? It doesn't check the length's suitability until after adding the two fields, but I don't think anything bad happens from delaying the check.

We need to check that writing the header itself won't write past the end of send_buf, so it'd have to look more like:

static int put_data_header(struct send_ctx *sctx, u32 len)

{
	struct btrfs_tlv_header *hdr =
		(struct btrfs_tlv_header *)(sctx->send_buf + sctx->send_size)

	if (WARN_ON_ONCE(sctx->put_data))
		return -EINVAL;
	sctx->put_data = true;

	if (sctx->send_max_size - sctx->send_size < sizeof(hdr->tlv_type))
		return -EOVERFLOW;
	put_unaligned_le16(BTRFS_SEND_A_DATA, &hdr->tlv_type);
	sctx->send_size += sizeof(hdr->tlv_type);

	/*
	 * In v2, the data attribute header doesn't include a length; it is
	 * implicitly to the end of the command.
	 */
	if (sctx->proto < 2) {
		if (sctx->send_max_size - sctx->send_size <
		    sizeof(hdr->tlv_len))
			return -EOVERFLOW;
		put_unaligned_le16(len, &hdr->tlv_len);
		sctx->send_size += sizeof(hdr->tlv_len);
	}

	if (sctx->send_max_size - sctx->send_size < len)
		return -EOVERFLOW;

	return 0;
}

Now we're checking for overflow in three places, shrug.

One thing that I like about the two separate cases is that it makes it more clear that v2+ doesn't actually have a btrfs_tlv_header; it's just a single __le16. If it's alright with you, I'll stick with my original version, but I will replaced the hard-coded 2s with sizeof(__le16).

Omar Sandoval

author

2022-03-30 17:13 UTC #12 lore raw

Re: [PATCH v14 5/7] btrfs: send: allocate send buffer with alloc_page() and vmap() for v2

In reply to Sweet Tea Dorminy

Quoted 29 lines · Sweet Tea Dorminy

Quoted 21 lines · Omar Sandoval
Quoted 11 lines · Sweet Tea Dorminy
Quoted 4 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
For encoded writes, we need the raw pages for reading compressed data directly via a bio.
Perhaps:
"For encoded writes, the existing btrfs_encoded_read*() functions expect a list of raw pages."
Quoted 8 lines
I think it would be a better to continue just vmalloc'ing a large continuous buffer and translating each page in the buffer into its raw page with something like is_vmalloc_addr(data) ? vmalloc_to_page(data) :
virt_to_page(data). Vmalloc can request a higher-order allocation, which probably doesn't matter but might slightly improve memory locality. And in terms of readability, I somewhat like the elegance of having a single identical kvmalloc call to allocate and send_buf in both cases, even if we do need to initialize the page list for some v2 commands.
Quoted 4 lines
I like this, but are we guaranteed that kvmalloc() will return a page-aligned buffer? It seems reasonable to me that it would for allocations of at least one page, but I can't find that written down anywhere.

Quoted 8 lines

Since vmalloc allocates whole pages, and kmalloc guarantees alignment to the allocation size for powers of 2 sizes (and PAGE_SIZE is required to be a power of 2), I think that adds up to a guarantee of page alignment both ways?
https://elixir.bootlin.com/linux/v5.17.1/source/include/linux/slab.h#L522 :
kmalloc: "For @size of power of two bytes, the alignment is also guaranteed to be at least to the size."

Our allocation size is ALIGN(SZ_16K + BTRFS_MAX_COMPRESSED, PAGE_SIZE), which is 144K for PAGE_SIZE = 4k. If I interpret the kmalloc() comment very literally, since this isn't a power of two, it's not guaranteed to be aligned, right?

Sweet Tea Dorminy

2022-03-30 18:48 UTC #13 lore raw

Re: [PATCH v14 5/7] btrfs: send: allocate send buffer with alloc_page() and vmap() for v2

Quoted 35 lines · Omar Sandoval

Quoted 34 lines · Sweet Tea Dorminy
Quoted 21 lines · Omar Sandoval
Quoted 11 lines · Sweet Tea Dorminy
Quoted 4 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
For encoded writes, we need the raw pages for reading compressed data directly via a bio.
Perhaps:
"For encoded writes, the existing btrfs_encoded_read*() functions expect a list of raw pages."
Quoted 8 lines
I think it would be a better to continue just vmalloc'ing a large continuous buffer and translating each page in the buffer into its raw page with something like is_vmalloc_addr(data) ? vmalloc_to_page(data) :
virt_to_page(data). Vmalloc can request a higher-order allocation, which probably doesn't matter but might slightly improve memory locality. And in terms of readability, I somewhat like the elegance of having a single identical kvmalloc call to allocate and send_buf in both cases, even if we do need to initialize the page list for some v2 commands.
Quoted 4 lines
I like this, but are we guaranteed that kvmalloc() will return a page-aligned buffer? It seems reasonable to me that it would for allocations of at least one page, but I can't find that written down anywhere.
Since vmalloc allocates whole pages, and kmalloc guarantees alignment to the allocation size for powers of 2 sizes (and PAGE_SIZE is required to be a power of 2), I think that adds up to a guarantee of page alignment both ways?

Quoted 8 lines

Quoted 3 lines
https://elixir.bootlin.com/linux/v5.17.1/source/include/linux/slab.h#L522 :
kmalloc: "For @size of power of two bytes, the alignment is also guaranteed to be at least to the size."
Our allocation size is ALIGN(SZ_16K + BTRFS_MAX_COMPRESSED, PAGE_SIZE), which is 144K for PAGE_SIZE = 4k. If I interpret the kmalloc() comment very literally, since this isn't a power of two, it's not guaranteed to be aligned, right?

Ah, an excellent point.

Now that I think about it, the kmalloc path picks a slab to allocate from based on the log_2 of the size: https://elixir.bootlin.com/linux/v5.17.1/source/mm/slab_common.c#L733 so we'd end up wasting 128k-16k space using kmalloc, whether it's aligned or not, I think?

So maybe it should just always use vmalloc and get the page alignment?

Omar Sandoval

author

2022-03-30 20:42 UTC #14 lore raw

Re: [PATCH v14 5/7] btrfs: send: allocate send buffer with alloc_page() and vmap() for v2

Quoted 49 lines · Sweet Tea Dorminy

Quoted 35 lines · Omar Sandoval
Quoted 34 lines · Sweet Tea Dorminy
Quoted 21 lines · Omar Sandoval
Quoted 11 lines · Sweet Tea Dorminy
Quoted 4 lines · Omar Sandoval
From: Omar Sandoval <[email protected]>
For encoded writes, we need the raw pages for reading compressed data directly via a bio.
Perhaps:
"For encoded writes, the existing btrfs_encoded_read*() functions expect a list of raw pages."
Quoted 8 lines
I think it would be a better to continue just vmalloc'ing a large continuous buffer and translating each page in the buffer into its raw page with something like is_vmalloc_addr(data) ? vmalloc_to_page(data) :
virt_to_page(data). Vmalloc can request a higher-order allocation, which probably doesn't matter but might slightly improve memory locality. And in terms of readability, I somewhat like the elegance of having a single identical kvmalloc call to allocate and send_buf in both cases, even if we do need to initialize the page list for some v2 commands.
Quoted 4 lines
I like this, but are we guaranteed that kvmalloc() will return a page-aligned buffer? It seems reasonable to me that it would for allocations of at least one page, but I can't find that written down anywhere.
Since vmalloc allocates whole pages, and kmalloc guarantees alignment to the allocation size for powers of 2 sizes (and PAGE_SIZE is required to be a power of 2), I think that adds up to a guarantee of page alignment both ways?
Quoted 8 lines
Quoted 3 lines
https://elixir.bootlin.com/linux/v5.17.1/source/include/linux/slab.h#L522 :
kmalloc: "For @size of power of two bytes, the alignment is also guaranteed to be at least to the size."
Our allocation size is ALIGN(SZ_16K + BTRFS_MAX_COMPRESSED, PAGE_SIZE), which is 144K for PAGE_SIZE = 4k. If I interpret the kmalloc() comment very literally, since this isn't a power of two, it's not guaranteed to be aligned, right?
Ah, an excellent point.

Quoted 7 lines

Now that I think about it, the kmalloc path picks a slab to allocate from based on the log_2 of the size:
https://elixir.bootlin.com/linux/v5.17.1/source/mm/slab_common.c#L733 so we'd end up wasting 128k-16k space using kmalloc, whether it's aligned or not, I think?
So maybe it should just always use vmalloc and get the page alignment?

Yeah, vmalloc()+vmalloc_to_page() is going to be more or less equivalent to the vmap thing I'm doing here, but a lot cleaner. Replacing this patch with the below patch seems to work:

fs/btrfs/send.c +23 -1

index c0ca45dae6d6..e574d4f4a167 100644
@@ -87,6 +87,7 @@ struct send_ctx {
 	 * command (since protocol v2, data must be the last attribute).
 	 */
 	bool put_data;
+	struct page **send_buf_pages;
 	u64 flags;	/* 'flags' member of btrfs_ioctl_send_args is u64 */
 	/* Protocol version compatibility requested */
 	u32 proto;

@@ -7486,12 +7487,32 @@ long btrfs_ioctl_send(struct inode *inode, struct btrfs_ioctl_send_args *arg)
 	sctx->clone_roots_cnt = arg->clone_sources_count;
 
 	if (sctx->proto >= 2) {
+		u32 send_buf_num_pages;
+
 		sctx->send_max_size = ALIGN(SZ_16K + BTRFS_MAX_COMPRESSED,
 					    PAGE_SIZE);
+		sctx->send_buf = vmalloc(sctx->send_max_size);
+		if (!sctx->send_buf) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		send_buf_num_pages = sctx->send_max_size >> PAGE_SHIFT;
+		sctx->send_buf_pages = kcalloc(send_buf_num_pages,
+					       sizeof(*sctx->send_buf_pages),
+					       GFP_KERNEL);
+		if (!sctx->send_buf_pages) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		for (i = 0; i < send_buf_num_pages; i++) {
+			sctx->send_buf_pages[i] =
+				vmalloc_to_page(sctx->send_buf +
+						(i << PAGE_SHIFT));
+		}
 	} else {
 		sctx->send_max_size = BTRFS_SEND_BUF_SIZE_V1;
+		sctx->send_buf = kvmalloc(sctx->send_max_size, GFP_KERNEL);
 	}
-	sctx->send_buf = kvmalloc(sctx->send_max_size, GFP_KERNEL);
 	if (!sctx->send_buf) {
 		ret = -ENOMEM;
 		goto out;

@@ -7684,6 +7705,7 @@ long btrfs_ioctl_send(struct inode *inode, struct btrfs_ioctl_send_args *arg)
 			fput(sctx->send_filp);
 
 		kvfree(sctx->clone_roots);
+		kfree(sctx->send_buf_pages);
 		kvfree(sctx->send_buf);
 
 		name_cache_free(sctx);

Sweet Tea Dorminy

2022-03-30 21:04 UTC #15 lore raw

Re: [PATCH v14 5/7] btrfs: send: allocate send buffer with alloc_page() and vmap() for v2

Quoted 4 lines

Yeah, vmalloc()+vmalloc_to_page() is going to be more or less equivalent to the vmap thing I'm doing here, but a lot cleaner. Replacing this patch with the below patch seems to work:

Looks great to me - thanks!