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

[Eric Biggers <ebiggers@kernel.org>]

On Mon, Mar 11, 2024 at 06:22:02PM -0600, 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

Thanks for resurrecting this.  Andreas's suggestions sound good to me.  And yes,
please try to search for any past discussions on this topic.

It may be a good idea to Cc the f2fs mailing list
(linux-f2fs-devel@lists.sourceforge.net), since this will be useful for f2fs
too, since f2fs supports compression.

One use case is that this will make testing the combination of
compression+encryption (e.g. as xfstest f2fs/002 tries to do) easier.

- Eric