On Wed, May 26, 2021 at 06:31:39PM +0200, David Sterba wrote:

quoted

quoted

quoted

+	header = page_address(eb->pages[0]) +
+		 get_eb_offset_in_page(eb, offsetof(struct btrfs_leaf, header));

It takes me near a minute to figure why it's not just
"get_eb_offset_in_page(eb, 0)".

I'm not sure if we really need that explicit way to just get 0,
especially most of us (and even some advanced users) know that csum
comes at the very beginning of a tree block.

And the mention of btrfs_leave can sometimes be confusing, especially
when we could have tree nodes passed in.

Ah right, I wanted to use the offsetof for clarity but that it could be
used with nodes makes it confusing again. What if it's replaced by

	get_eb_offset_in_page(eb, offsetof(struct btrfs_header, csum));

This makes it clear that it's the checksum and from the experience we
know it's at offset 0. I'd rather avoid magic constants and offsets but
you're right that everybody knows that the checksum is at the beginning
of btree block.

--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c

@@ -584,7 +584,7 @@ static int validate_extent_buffer(struct extent_buffer *eb)
         const u32 csum_size = fs_info->csum_size;
         u8 found_level;
         u8 result[BTRFS_CSUM_SIZE];
-       const struct btrfs_header *header;
+       const u8 *header_csum;
         int ret = 0;

         found_start = btrfs_header_bytenr(eb);

@@ -609,14 +609,14 @@ static int validate_extent_buffer(struct extent_buffer *eb)
         }

         csum_tree_block(eb, result);
-       header = page_address(eb->pages[0]) +
-                get_eb_offset_in_page(eb, offsetof(struct btrfs_leaf, header));
+       header_csum = page_address(eb->pages[0]) +
+               get_eb_offset_in_page(eb, offsetof(struct btrfs_header, csum));