Re: [PATCH v3 1/2] btrfs: remove extent buffer's redundant `len` member field

[David Sterba <dsterba@suse.cz>]

On Mon, May 05, 2025 at 01:50:54PM +0200, Daniel Vacek wrote:
> Even super block nowadays uses nodesize for eb->len. This is since commits
> 
> 551561c34663 ("btrfs: don't pass nodesize to __alloc_extent_buffer()")
> da17066c4047 ("btrfs: pull node/sector/stripe sizes out of root and into fs_info")
> ce3e69847e3e ("btrfs: sink parameter len to alloc_extent_buffer")
> a83fffb75d09 ("btrfs: sink blocksize parameter to btrfs_find_create_tree_block")
> 
> With these the eb->len is not really useful anymore. Let's use the nodesize
> directly where applicable.

You haven't updated the changelog despite we had a discussion about the
potential drawbacks, so this should be here. But I'm not convinced this
is a good change. The eb size does not change so no better packing in
the slab and the caching of length in the same cacheline is lost.

In the assebly it's clear where the pointer dereference is added, using
an example from btrfs_get_token_8():

  mov    0x8(%rbp),%r9d

vs

  mov    0x18(%rbp),%r10
  mov    0xd38(%r10),%r9d

And there's a new register dependency. The eb operations are most of the
time performed on memory structures and not just in connnection with IO,
I think this could be measurable on a fast device with metadata heavy
workload. So I'd rather see some numbers this will not decrease
performance.