Re: [PATCH v2 03/11] btrfs: introduce new members for extent_map

[Qu Wenruo <wqu@suse.com>]

在 2024/5/10 20:56, Filipe Manana 写道:
[...]
>>
>> Check case 1).
>>
>> Both file extents are referring to the same data extent.
>>
>> Like this:
>>
>> FE 1, file pos 0, num_bytes 4K, disk_bytenr X, disk_num_bytes 16K,
>> offset 0, ram_bytes 16K, compression none
>>
>> FE 2, file pos 4k, num_bytes 4K, disk_bytenr X, disk_num_bytes 16K,
>> offset 4k, ram_bytes 16K, compression none.
>>
>> In that case we should not just sum up the disk_num_bytes at all.
>> Remember disk_num_bytes are for the data extent.
> 
> Yes, but for cases where they refer to different file extent items
> that are contiguous on disk, the max is confusing, and a sum is what
> makes sense. Example:
> 
> FE 1, file pos 0, num_bytes 4K, disk_bytenr X, disk_num_bytes 4K,
> offset 0, ram_bytes 4K, no compression
> 
> FE 2, file pos 4K, num_bytes 4K, disk_bytenr X + 4K, disk_num_bytes
> 4K, offset 0, ram_bytes 4K, no compression
> 
> When merging the corresponding extent maps it's natural to think
> disk_num_bytes is 8K and not 4K.

The max based solution is based on their end bytenr, not disk_num_bytes.

So the merged one would have:

disk_bytenr = min(X, X + 4K) = X
disk_num_bytes = max(X + 4K, X + 4K + 4K) - disk_bytenr = 8K.

So I didn't see why it's not natural.

Furthermore, the max/min based solution can handle both case 1) (same 
data extent, different parts) and case 2) (different data extents) well.

> 
> But as I mentioned before, after merging we really don't use
> disk_num_bytes anywhere.
> It's only used during extent logging, which only happens for extents
> maps that were not merged and can not be before they are logged.
> 
> Before this patchset, when disk_num_bytes was named orig_block_len,
> that was never updated during merging, exactly because we don't use
> cases for it.

But since we want everything to match the definition of the same-named 
member, no matter if they get merged or not, we need to change the values.

If they do not get used, that's still fine.

Thanks,
Qu