Re: [PATCH 1/3] add physical_length field to fiemap extents

[Sweet Tea Dorminy <sweettea-kernel@dorminy.me>]

On 2024-03-11 20:22, Andreas Dilger wrote:
> On Mar 8, 2024, at 11:03 AM, Sweet Tea Dorminy
> <sweettea-kernel@dorminy.me> wrote:
>> 
>> Some filesystems support compressed extents which have a larger 
>> logical
>> size than physical, and for those filesystems, it can be useful for
>> userspace to know how much space those extents actually use. For
>> instance, the compsize [1] tool for btrfs currently uses 
>> btrfs-internal,
>> root-only ioctl to find the actual disk space used by a file; it would
>> be better and more useful for this information to require fewer
>> privileges and to be usable on more filesystems. Therefore, use one of
>> the padding u64s in the fiemap extent structure to return the actual
>> physical length; and, for now, return this as equal to the logical
>> length.
> 
> Thank you for working on this patch.  Note that there was a patch from
> David Sterba and a lengthy discussion about exactly this functionality
> several years ago.  If you haven't already read the details, it would 
> be
> useful to do so. I think the thread had mostly come to good 
> conclusions,
> but the patch never made it into the kernel.
> 
> https://patchwork.ozlabs.org/project/linux-ext4/patch/4f8d5dc5b51a43efaf16c39398c23a6276e40a30.1386778303.git.dsterba@suse.cz/
> 
> One of those conclusions was that the kernel should always fill in the
> fe_physical_length field in the returned extent, and set a flag:
> 
> #define FIEMAP_EXTENT_PHYS_LENGTH      0x00000010
> 
> to indicate to userspace that the physical length field is valid.
> 
> There should also be a separate flag for extents that are compressed:
> 
> #define FIEMAP_EXTENT_DATA_COMPRESSED  0x00000040
> 
> Rename fe_length to fe_logical_length and #define fe_length 
> fe_logical_length
> so that it is more clear which field is which in the data structure, 
> but
> does not break compatibility.
> 
> I think this patch gets most of this right, except the presence of the
> flags to indicate the PHYS_LENGTH and DATA_COMPRESSED state in the 
> extent.
> 
> Cheers, Andreas
> 
I had not seen that; thank you for the pointers, I'll add the flags in 
v2.