[PATCH 0/3] btrfs-progs: minor cleanups related to extent buffer

[PATCH 0/3] btrfs-progs: minor cleanups related to extent buffer

[Qu Wenruo]

The first patch is a cleanup to remove the unused @fs_info parameter
from btrfs_csum_data().

The second patch uses btrfs_csum_data() directly to calculate super
block checksum and avoid use extent buffer helpers.

The final patch enhance btrfs_print_leaf() to be called with temporary
extent buffers in mkfs/convert, which have no eb->leaf populated.

Qu Wenruo (3):
  btrfs-progs: remove the unused fs_info parameter for btrfs_csum_data()
  btrfs: mkfs: do not use extent buffer to writeback super block
  btrfs-progs: enhance btrfs_print_leaf() to handle NULL fs_info

 check/main.c                    |  2 +-
 cmds/rescue-chunk-recover.c     |  2 +-
 cmds/rescue-fix-data-checksum.c |  4 ++--
 convert/common.c                |  2 +-
 convert/main.c                  |  2 +-
 kernel-shared/ctree.c           |  4 +---
 kernel-shared/disk-io.c         | 12 +++++-------
 kernel-shared/disk-io.h         |  3 +--
 kernel-shared/file-item.c       |  3 +--
 kernel-shared/print-tree.c      | 34 +++++++++++++++++----------------
 mkfs/common.c                   |  9 +++------
 tune/change-csum.c              |  7 +++----
 12 files changed, 38 insertions(+), 46 deletions(-)

--
2.49.0

[PATCH 1/3] btrfs-progs: remove the unused fs_info parameter for btrfs_csum_data()

[Qu Wenruo]

The parameter @fs_info is not utilized at all, and there are already
several call sites passing NULL as @fs_info.

And there is no counter-part in kernel (we use crypto_shash_* interface
instead), there is no need to keep the parameter list the same.

So just remove the unused parameter.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c                    |  2 +-
 cmds/rescue-chunk-recover.c     |  2 +-
 cmds/rescue-fix-data-checksum.c |  4 ++--
 convert/common.c                |  2 +-
 convert/main.c                  |  2 +-
 kernel-shared/disk-io.c         | 12 +++++-------
 kernel-shared/disk-io.h         |  3 +--
 kernel-shared/file-item.c       |  3 +--
 kernel-shared/print-tree.c      |  4 ++--
 tune/change-csum.c              |  7 +++----
 10 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/check/main.c b/check/main.c
index bf250c41eb07..b78eb59d0c50 100644
--- a/check/main.c
+++ b/check/main.c
@@ -5831,7 +5831,7 @@ static int check_extent_csums(struct btrfs_root *root, u64 bytenr,
 			while (data_checked < read_len) {
 				tmp = offset + data_checked;
 
-				btrfs_csum_data(gfs_info, csum_type, data + tmp,
+				btrfs_csum_data(csum_type, data + tmp,
 						result, gfs_info->sectorsize);
 
 				csum_offset = leaf_offset +
diff --git a/cmds/rescue-chunk-recover.c b/cmds/rescue-chunk-recover.c
index 60a705817c80..1a5a2a39c640 100644
--- a/cmds/rescue-chunk-recover.c
+++ b/cmds/rescue-chunk-recover.c
@@ -1896,7 +1896,7 @@ static int check_one_csum(int fd, u64 start, u32 len, u32 tree_csum,
 	}
 	ret = 0;
 	put_unaligned_le32(tree_csum, expected_csum);
-	btrfs_csum_data(NULL, csum_type, (u8 *)data, result, len);
+	btrfs_csum_data(csum_type, (u8 *)data, result, len);
 	if (memcmp(result, expected_csum, csum_size) != 0)
 		ret = 1;
 out:
diff --git a/cmds/rescue-fix-data-checksum.c b/cmds/rescue-fix-data-checksum.c
index 23b59fffe2f7..0e3b5cdbb217 100644
--- a/cmds/rescue-fix-data-checksum.c
+++ b/cmds/rescue-fix-data-checksum.c
@@ -135,7 +135,7 @@ static int verify_one_data_block(struct btrfs_fs_info *fs_info,
 				break;
 		}
 		/* Verify the data checksum. */
-		btrfs_csum_data(fs_info, fs_info->csum_type, buf, csum, blocksize);
+		btrfs_csum_data(fs_info->csum_type, buf, csum, blocksize);
 		read_extent_buffer(leaf, csum_expected, leaf_offset, csum_size);
 		if (memcmp(csum_expected, csum, csum_size) != 0) {
 			ret = add_corrupted_block(fs_info, logical, mirror, num_mirrors);
@@ -364,7 +364,7 @@ static int update_csum_item(struct btrfs_fs_info *fs_info, u64 logical,
 		btrfs_abort_transaction(trans, ret);
 		goto out;
 	}
-	btrfs_csum_data(fs_info, fs_info->csum_type, buf, csum, fs_info->sectorsize);
+	btrfs_csum_data(fs_info->csum_type, buf, csum, fs_info->sectorsize);
 	write_extent_buffer(path.nodes[0], csum, (unsigned long)citem, fs_info->csum_size);
 	btrfs_release_path(&path);
 	ret = btrfs_commit_transaction(trans, csum_root);
diff --git a/convert/common.c b/convert/common.c
index 2cbe45180ce9..8c64c2ce80ff 100644
--- a/convert/common.c
+++ b/convert/common.c
@@ -80,7 +80,7 @@ static inline int write_temp_super(int fd, struct btrfs_super_block *sb,
        u16 csum_type = btrfs_super_csum_type(sb);
        int ret;
 
-       btrfs_csum_data(NULL, csum_type, (u8 *)sb + BTRFS_CSUM_SIZE,
+       btrfs_csum_data(csum_type, (u8 *)sb + BTRFS_CSUM_SIZE,
 		       result, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
        memcpy(&sb->csum[0], result, BTRFS_CSUM_SIZE);
        ret = pwrite(fd, sb, BTRFS_SUPER_INFO_SIZE, sb_bytenr);
diff --git a/convert/main.c b/convert/main.c
index 0dc75c9eb1c6..8784192fefc5 100644
--- a/convert/main.c
+++ b/convert/main.c
@@ -1060,7 +1060,7 @@ static int migrate_super_block(int fd, u64 old_bytenr)
 	BUG_ON(btrfs_super_bytenr(&super) != old_bytenr);
 	btrfs_set_super_bytenr(&super, BTRFS_SUPER_INFO_OFFSET);
 
-	btrfs_csum_data(NULL, btrfs_super_csum_type(&super),
+	btrfs_csum_data(btrfs_super_csum_type(&super),
 			(u8 *)&super + BTRFS_CSUM_SIZE, result,
 			BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
 	memcpy(&super.csum[0], result, BTRFS_CSUM_SIZE);
diff --git a/kernel-shared/disk-io.c b/kernel-shared/disk-io.c
index b7d478007ae8..95e1504fd07e 100644
--- a/kernel-shared/disk-io.c
+++ b/kernel-shared/disk-io.c
@@ -162,8 +162,7 @@ static void print_tree_block_error(struct btrfs_fs_info *fs_info,
 	}
 }
 
-int btrfs_csum_data(struct btrfs_fs_info *fs_info, u16 csum_type, const u8 *data,
-		    u8 *out, size_t len)
+int btrfs_csum_data(u16 csum_type, const u8 *data, u8 *out, size_t len)
 {
 	memset(out, 0, BTRFS_CSUM_SIZE);
 
@@ -191,8 +190,7 @@ static int __csum_tree_block_size(struct extent_buffer *buf, u16 csum_size,
 	u32 len;
 
 	len = buf->len - BTRFS_CSUM_SIZE;
-	btrfs_csum_data(buf->fs_info, csum_type, (u8 *)buf->data + BTRFS_CSUM_SIZE,
-			result, len);
+	btrfs_csum_data(csum_type, (u8 *)buf->data + BTRFS_CSUM_SIZE, result, len);
 
 	if (verify) {
 		if (buf->fs_info && buf->fs_info->skip_csum_check) {
@@ -1770,7 +1768,7 @@ int btrfs_check_super(struct btrfs_super_block *sb, unsigned sbflags)
 	}
 	csum_size = btrfs_super_csum_size(sb);
 
-	btrfs_csum_data(NULL, csum_type, (u8 *)sb + BTRFS_CSUM_SIZE,
+	btrfs_csum_data(csum_type, (u8 *)sb + BTRFS_CSUM_SIZE,
 			result, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
 
 	if (memcmp(result, sb->csum, csum_size)) {
@@ -2028,7 +2026,7 @@ static int write_dev_supers(struct btrfs_fs_info *fs_info,
 	}
 	if (fs_info->super_bytenr != BTRFS_SUPER_INFO_OFFSET) {
 		btrfs_set_super_bytenr(sb, fs_info->super_bytenr);
-		btrfs_csum_data(fs_info, csum_type, (u8 *)sb + BTRFS_CSUM_SIZE,
+		btrfs_csum_data(csum_type, (u8 *)sb + BTRFS_CSUM_SIZE,
 				result, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
 		memcpy(&sb->csum[0], result, BTRFS_CSUM_SIZE);
 
@@ -2061,7 +2059,7 @@ static int write_dev_supers(struct btrfs_fs_info *fs_info,
 
 		btrfs_set_super_bytenr(sb, bytenr);
 
-		btrfs_csum_data(fs_info, csum_type, (u8 *)sb + BTRFS_CSUM_SIZE,
+		btrfs_csum_data(csum_type, (u8 *)sb + BTRFS_CSUM_SIZE,
 				result, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
 		memcpy(&sb->csum[0], result, BTRFS_CSUM_SIZE);
 
diff --git a/kernel-shared/disk-io.h b/kernel-shared/disk-io.h
index f57bc1b18ad1..dd2dabdf3731 100644
--- a/kernel-shared/disk-io.h
+++ b/kernel-shared/disk-io.h
@@ -223,8 +223,7 @@ void btrfs_mark_buffer_dirty(struct extent_buffer *buf);
 int btrfs_buffer_uptodate(struct extent_buffer *buf, u64 parent_transid,
 			  int atomic);
 int btrfs_set_buffer_uptodate(struct extent_buffer *buf);
-int btrfs_csum_data(struct btrfs_fs_info *fs_info, u16 csum_type, const u8 *data,
-		    u8 *out, size_t len);
+int btrfs_csum_data(u16 csum_type, const u8 *data, u8 *out, size_t len);
 
 int btrfs_open_device(struct btrfs_device *dev);
 int csum_tree_block_size(struct extent_buffer *buf, u16 csum_sectorsize,
diff --git a/kernel-shared/file-item.c b/kernel-shared/file-item.c
index 503ad657c661..6b7c47ce642b 100644
--- a/kernel-shared/file-item.c
+++ b/kernel-shared/file-item.c
@@ -298,8 +298,7 @@ csum:
 	item = (struct btrfs_csum_item *)((unsigned char *)item +
 					  csum_offset * csum_size);
 found:
-	btrfs_csum_data(root->fs_info, csum_type, (u8 *)data, csum_result,
-			sectorsize);
+	btrfs_csum_data(csum_type, (u8 *)data, csum_result, sectorsize);
 	write_extent_buffer(leaf, csum_result, (unsigned long)item,
 			    csum_size);
 	btrfs_mark_buffer_dirty(path->nodes[0]);
diff --git a/kernel-shared/print-tree.c b/kernel-shared/print-tree.c
index 06c50ad8db09..6ed1191948d8 100644
--- a/kernel-shared/print-tree.c
+++ b/kernel-shared/print-tree.c
@@ -1392,7 +1392,7 @@ static void print_header_info(struct extent_buffer *eb, unsigned int mode)
 		printf("%02hhx", (int)(eb->data[i]));
 	printf("\n");
 	memset(csum, 0, sizeof(csum));
-	btrfs_csum_data(fs_info, btrfs_super_csum_type(fs_info->super_copy),
+	btrfs_csum_data(btrfs_super_csum_type(fs_info->super_copy),
 			(u8 *)eb->data + BTRFS_CSUM_SIZE,
 			csum, fs_info->nodesize - BTRFS_CSUM_SIZE);
 	printf("checksum calced ");
@@ -1902,7 +1902,7 @@ static int check_csum_sblock(void *sb, int csum_size, u16 csum_type)
 {
 	u8 result[BTRFS_CSUM_SIZE];
 
-	btrfs_csum_data(NULL, csum_type, (u8 *)sb + BTRFS_CSUM_SIZE,
+	btrfs_csum_data(csum_type, (u8 *)sb + BTRFS_CSUM_SIZE,
 			result, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
 
 	return !memcmp(sb, result, csum_size);
diff --git a/tune/change-csum.c b/tune/change-csum.c
index 46aa96237960..1efb6fd60758 100644
--- a/tune/change-csum.c
+++ b/tune/change-csum.c
@@ -158,8 +158,7 @@ static int read_verify_one_data_sector(struct btrfs_fs_info *fs_info,
 			error("failed to read logical %llu: %m", logical);
 			continue;
 		}
-		btrfs_csum_data(fs_info, fs_info->csum_type, data_buf, csum_has,
-				sectorsize);
+		btrfs_csum_data(fs_info->csum_type, data_buf, csum_has, sectorsize);
 		if (memcmp(csum_has, old_csums, fs_info->csum_size) == 0) {
 			found_good = true;
 			break;
@@ -577,9 +576,9 @@ static int rewrite_tree_block_csum(struct btrfs_fs_info *fs_info, u64 logical,
 	}
 
 	/* Verify the csum first. */
-	btrfs_csum_data(fs_info, fs_info->csum_type, (u8 *)eb->data + BTRFS_CSUM_SIZE,
+	btrfs_csum_data(fs_info->csum_type, (u8 *)eb->data + BTRFS_CSUM_SIZE,
 			result_old, fs_info->nodesize - BTRFS_CSUM_SIZE);
-	btrfs_csum_data(fs_info, new_csum_type, (u8 *)eb->data + BTRFS_CSUM_SIZE,
+	btrfs_csum_data(new_csum_type, (u8 *)eb->data + BTRFS_CSUM_SIZE,
 			result_new, fs_info->nodesize - BTRFS_CSUM_SIZE);
 
 	/* Matches old csum, rewrite. */
-- 
2.49.0

[PATCH 2/3] btrfs: mkfs: do not use extent buffer to writeback super block

[Qu Wenruo]

There is a long history that we used extent_buffer helpers to modify a
superblock, but we're moving out of that behavior, and migrate to use
the on stack helpers to modify super blocks.

In the function make_btrfs() we use a dummy extent buffer just to
calculate the checksum for the super block.
We do not need such workaround, and can manually call btrfs_csum_data()
on @super, and pass @super directly to sbwrite(), completely avoid the
need of a dummy extent buffer.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 mkfs/common.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/mkfs/common.c b/mkfs/common.c
index bb5a2ad46f4f..f785d8962a06 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -754,12 +754,9 @@ int make_btrfs(int fd, struct btrfs_mkfs_config *cfg)
 	}
 
 	/* and write out the super block */
-	memset(buf->data, 0, BTRFS_SUPER_INFO_SIZE);
-	memcpy(buf->data, &super, sizeof(super));
-	buf->len = BTRFS_SUPER_INFO_SIZE;
-	csum_tree_block_size(buf, btrfs_csum_type_size(cfg->csum_type), 0,
-			     cfg->csum_type);
-	ret = sbwrite(fd, buf->data, BTRFS_SUPER_INFO_OFFSET);
+	btrfs_csum_data(cfg->csum_type, (u8 *)&super + BTRFS_CSUM_SIZE,
+			super.csum, sizeof(super) - BTRFS_CSUM_SIZE);
+	ret = sbwrite(fd, &super, BTRFS_SUPER_INFO_OFFSET);
 	if (ret != BTRFS_SUPER_INFO_SIZE) {
 		ret = (ret < 0 ? -errno : -EIO);
 		goto out;
-- 
2.49.0

[PATCH 3/3] btrfs-progs: enhance btrfs_print_leaf() to handle NULL fs_info

[Qu Wenruo]

For mkfs and convert, we need to create a temporary fs without an
fs_info.

This makes debugging much harder that we can not use btrfs_print_leaf()
to print the temporary tree blocks.

There are only two things causing problems for btrfs_print_leaf() if
eb->fs_info is NULL:

- print_header_info()
  Which needs to grab the checksum type from eb->fs_info.

  This can be avoided by completely skipping checksum output if
  eb->fs_info is NULL.

- btrfs_leaf_free_space()
  Which have two BUG_ON()s checking eb->fs_info, and finally calling
  BTRFS_LEAF_DATA_SIZE().

  Which can be avoided by removing the two BUG_ON()s, and use
  __BTRFS_LEAF_DATA_SIZE(eb->len) to grab the same leaf data size.

  Thankfully all call sites inside mkfs and convert are setting eb->len
  to nodesize correctly.

- __btrfs_print_leaf()
  Which calls BTRFS_LEAF_DATA_SIZE(eb->fs_info).

  Can be avoided by the same method above.

With those changes, we can call btrfs_print_leaf() inside a debugger
for temporary fses created by mkfs and convert.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 kernel-shared/ctree.c      |  4 +---
 kernel-shared/print-tree.c | 32 +++++++++++++++++---------------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/kernel-shared/ctree.c b/kernel-shared/ctree.c
index f90de606e7b1..7815101b2195 100644
--- a/kernel-shared/ctree.c
+++ b/kernel-shared/ctree.c
@@ -1888,9 +1888,7 @@ int btrfs_leaf_free_space(const struct extent_buffer *leaf)
 	u32 leaf_data_size;
 	int ret;
 
-	BUG_ON(!leaf->fs_info);
-	BUG_ON(leaf->fs_info->nodesize != leaf->len);
-	leaf_data_size = BTRFS_LEAF_DATA_SIZE(leaf->fs_info);
+	leaf_data_size = __BTRFS_LEAF_DATA_SIZE(leaf->len);
 	ret = leaf_data_size - leaf_space_used(leaf, 0 ,nritems);
 	if (ret < 0) {
 		printk("leaf free space ret %d, leaf data size %u, used %d nritems %d\n",
diff --git a/kernel-shared/print-tree.c b/kernel-shared/print-tree.c
index 6ed1191948d8..1ca2cb90273e 100644
--- a/kernel-shared/print-tree.c
+++ b/kernel-shared/print-tree.c
@@ -1345,7 +1345,7 @@ static void print_header_info(struct extent_buffer *eb, unsigned int mode)
 	u8 backref_rev;
 	char csum_str[2 * BTRFS_CSUM_SIZE + 8 /* strlen(" csum 0x") */ + 1];
 	int i;
-	int csum_size = fs_info->csum_size;
+	int csum_size = fs_info ? fs_info->csum_size : 0;
 
 	flags = btrfs_header_flags(eb) & ~BTRFS_BACKREF_REV_MASK;
 	backref_rev = btrfs_header_flags(eb) >> BTRFS_BACKREF_REV_SHIFT;
@@ -1387,18 +1387,20 @@ static void print_header_info(struct extent_buffer *eb, unsigned int mode)
 	       csum_str);
 
 #if EXPERIMENTAL
-	printf("checksum stored ");
-	for (i = 0; i < csum_size; i++)
-		printf("%02hhx", (int)(eb->data[i]));
-	printf("\n");
-	memset(csum, 0, sizeof(csum));
-	btrfs_csum_data(btrfs_super_csum_type(fs_info->super_copy),
-			(u8 *)eb->data + BTRFS_CSUM_SIZE,
-			csum, fs_info->nodesize - BTRFS_CSUM_SIZE);
-	printf("checksum calced ");
-	for (i = 0; i < csum_size; i++)
-		printf("%02hhx", (int)(csum[i]));
-	printf("\n");
+	if (fs_info) {
+		printf("checksum stored ");
+		for (i = 0; i < csum_size; i++)
+			printf("%02hhx", (int)(eb->data[i]));
+		printf("\n");
+		memset(csum, 0, sizeof(csum));
+		btrfs_csum_data(btrfs_super_csum_type(fs_info->super_copy),
+				(u8 *)eb->data + BTRFS_CSUM_SIZE,
+				csum, fs_info->nodesize - BTRFS_CSUM_SIZE);
+		printf("checksum calced ");
+		for (i = 0; i < csum_size; i++)
+			printf("%02hhx", (int)(csum[i]));
+		printf("\n");
+	}
 #endif
 
 	print_uuids(eb);
@@ -1478,10 +1480,10 @@ static void print_dev_replace_item(struct extent_buffer *eb, struct btrfs_dev_re
 void __btrfs_print_leaf(struct extent_buffer *eb, unsigned int mode)
 {
 	struct btrfs_disk_key disk_key;
-	u32 leaf_data_size = BTRFS_LEAF_DATA_SIZE(eb->fs_info);
+	u32 leaf_data_size = __BTRFS_LEAF_DATA_SIZE(eb->len);
 	u32 i;
 	u32 nr;
-	const bool print_csum_items = (mode & BTRFS_PRINT_TREE_CSUM_ITEMS);
+	const bool print_csum_items = (mode & BTRFS_PRINT_TREE_CSUM_ITEMS) && eb->fs_info;
 
 	print_header_info(eb, mode);
 	nr = btrfs_header_nritems(eb);
-- 
2.49.0

Re: [PATCH 1/3] btrfs-progs: remove the unused fs_info parameter for btrfs_csum_data()

[David Sterba]

On Mon, May 26, 2025 at 01:32:35PM +0930, Qu Wenruo wrote:
> The parameter @fs_info is not utilized at all, and there are already
> several call sites passing NULL as @fs_info.
> 
> And there is no counter-part in kernel (we use crypto_shash_* interface
> instead), there is no need to keep the parameter list the same.
> 
> So just remove the unused parameter.

This is a bit unfortunate, I need to fs_info parameter for checksumming
functions for the authenticated hashes, I have a prototype and work on
it in spare time.

I'll probably apply this cleanup as I don't have ETA for the other
patchset and some things are still not finalized. It may be feasible to
pass the key context from the global config and not as parameter.

Re: [PATCH 0/3] btrfs-progs: minor cleanups related to extent buffer

[David Sterba]

On Mon, May 26, 2025 at 01:32:34PM +0930, Qu Wenruo wrote:
> The first patch is a cleanup to remove the unused @fs_info parameter
> from btrfs_csum_data().
> 
> The second patch uses btrfs_csum_data() directly to calculate super
> block checksum and avoid use extent buffer helpers.
> 
> The final patch enhance btrfs_print_leaf() to be called with temporary
> extent buffers in mkfs/convert, which have no eb->leaf populated.
> 
> Qu Wenruo (3):
>   btrfs-progs: remove the unused fs_info parameter for btrfs_csum_data()
>   btrfs: mkfs: do not use extent buffer to writeback super block
>   btrfs-progs: enhance btrfs_print_leaf() to handle NULL fs_info

Added to devel, thanks.