Re: [PATCH v2 5/5] btrfs: fiemap: return extent physical size

[Qu Wenruo <wqu@suse.com>]

在 2024/3/28 11:52, Sweet Tea Dorminy 写道:
> Now that fiemap allows returning extent physical size, make btrfs return
> the appropriate extent's actual disk size.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
[...]
> @@ -3221,7 +3239,9 @@ int extent_fiemap(struct btrfs_inode *inode, struct fiemap_extent_info *fieinfo,
>   
>   			ret = emit_fiemap_extent(fieinfo, &cache, key.offset,
>   						 disk_bytenr + extent_offset,
> -						 extent_len, flags);
> +						 extent_len,
> +						 disk_size - extent_offset,

This means, we will emit a entry that uses the end to the physical 
extent end.

Considering a file layout like this:

	item 6 key (257 EXTENT_DATA 0) itemoff 15816 itemsize 53
		generation 7 type 1 (regular)
		extent data disk byte 13631488 nr 65536
		extent data offset 0 nr 4096 ram 65536
		extent compression 0 (none)
	item 7 key (257 EXTENT_DATA 4096) itemoff 15763 itemsize 53
		generation 8 type 1 (regular)
		extent data disk byte 13697024 nr 4096
		extent data offset 0 nr 4096 ram 4096
		extent compression 0 (none)
	item 8 key (257 EXTENT_DATA 8192) itemoff 15710 itemsize 53
		generation 7 type 1 (regular)
		extent data disk byte 13631488 nr 65536
		extent data offset 8192 nr 57344 ram 65536
		extent compression 0 (none)

For fiemap, we would got something like this:

fileoff 0, logical len 4k, phy 13631488, phy len 64K
fileoff 4k, logical len 4k, phy 13697024, phy len 4k
fileoff 8k, logical len 56k, phy 13631488 + 8k, phylen 56k

[HOW TO CALCULATE WASTED SPACE IN USER SPACE]
My concern is on the first entry. It indicates that we have wasted 60K 
(phy len is 64K, while logical len is only 4K)

But that information is not correct, as in reality we only wasted 4K, 
the remaining 56K is still referred by file range [8K, 64K).

Do you mean that user space program should maintain a mapping of each 
utilized physical range, and when handling the reported file range [8K, 
64K), the user space program should find that the physical range covers 
with one existing extent, and do calculation correctly?

[COMPRESSION REPRESENTATION]
The biggest problem other than the complexity in user space is the 
handling of compressed extents.

Should we return the physical bytenr (disk_bytenr of file extent item) 
directly or with the extent offset added?
Either way it doesn't look consistent to me, compared to non-compressed 
extents.

[ALTERNATIVE FORMAT]
The other alternative would be following the btrfs ondisk format, 
providing a unique physical bytenr for any file extent, then the 
offset/referred length inside the uncompressed extent.

That would handle compressed and regular extents more consistent, and a 
little easier for user space tool to handle (really just a tiny bit 
easier, no range overlap check needed), but more complex to represent, 
and I'm not sure if any other filesystem would be happy to accept the 
extra members they don't care.

Thanks,
Qu

> +						 flags);
>   		}
>   
>   		if (ret < 0) {
> @@ -3259,7 +3279,7 @@ int extent_fiemap(struct btrfs_inode *inode, struct fiemap_extent_info *fieinfo,
>   		prev_extent_end = range_end;
>   	}
>   
> -	if (cache.cached && cache.offset + cache.len >= last_extent_end) {
> +	if (cache.cached && cache.offset + cache.log_len >= last_extent_end) {
>   		const u64 i_size = i_size_read(&inode->vfs_inode);
>   
>   		if (prev_extent_end < i_size) {