| Message ID | 0b423d44538f3827a255f1f842b57b4a768b7629.1709918025.git.sweettea-kernel@dorminy.me (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fiemap extension to add physical extent length | expand |
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 > [1] https://github.com/kilobyte/compsize > > Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me> > --- > Documentation/filesystems/fiemap.rst | 26 ++++++++++++++++++-------- > fs/ioctl.c | 1 + > include/uapi/linux/fiemap.h | 24 +++++++++++++++++------- > 3 files changed, 36 insertions(+), 15 deletions(-) > > diff --git a/Documentation/filesystems/fiemap.rst b/Documentation/filesystems/fiemap.rst > index 93fc96f760aa..e3e84573b087 100644 > --- a/Documentation/filesystems/fiemap.rst > +++ b/Documentation/filesystems/fiemap.rst > @@ -80,14 +80,24 @@ Each extent is described by a single fiemap_extent structure as > returned in fm_extents:: > > struct fiemap_extent { > - __u64 fe_logical; /* logical offset in bytes for the start of > - * the extent */ > - __u64 fe_physical; /* physical offset in bytes for the start > - * of the extent */ > - __u64 fe_length; /* length in bytes for the extent */ > - __u64 fe_reserved64[2]; > - __u32 fe_flags; /* FIEMAP_EXTENT_* flags for this extent */ > - __u32 fe_reserved[3]; > + /* > + * logical offset in bytes for the start of > + * the extent from the beginning of the file > + */ > + __u64 fe_logical; > + /* > + * physical offset in bytes for the start > + * of the extent from the beginning of the disk > + */ > + __u64 fe_physical; > + /* length in bytes for this extent */ > + __u64 fe_length; > + /* physical length in bytes for this extent */ > + __u64 fe_physical_length; > + __u64 fe_reserved64[1]; > + /* FIEMAP_EXTENT_* flags for this extent */ > + __u32 fe_flags; > + __u32 fe_reserved[3]; > }; > > All offsets and lengths are in bytes and mirror those on disk. It is valid > diff --git a/fs/ioctl.c b/fs/ioctl.c > index 1d5abfdf0f22..f8e5d6dfc62d 100644 > --- a/fs/ioctl.c > +++ b/fs/ioctl.c > @@ -139,6 +139,7 @@ int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical, > extent.fe_logical = logical; > extent.fe_physical = phys; > extent.fe_length = len; > + extent.fe_physical_length = len; > extent.fe_flags = flags; > > dest += fieinfo->fi_extents_mapped; > diff --git a/include/uapi/linux/fiemap.h b/include/uapi/linux/fiemap.h > index 24ca0c00cae3..fd3c7d380666 100644 > --- a/include/uapi/linux/fiemap.h > +++ b/include/uapi/linux/fiemap.h > @@ -15,13 +15,23 @@ > #include <linux/types.h> > > struct fiemap_extent { > - __u64 fe_logical; /* logical offset in bytes for the start of > - * the extent from the beginning of the file */ > - __u64 fe_physical; /* physical offset in bytes for the start > - * of the extent from the beginning of the disk */ > - __u64 fe_length; /* length in bytes for this extent */ > - __u64 fe_reserved64[2]; > - __u32 fe_flags; /* FIEMAP_EXTENT_* flags for this extent */ > + /* > + * logical offset in bytes for the start of > + * the extent from the beginning of the file > + */ > + __u64 fe_logical; > + /* > + * physical offset in bytes for the start > + * of the extent from the beginning of the disk > + */ > + __u64 fe_physical; > + /* length in bytes for this extent */ > + __u64 fe_length; > + /* physical length in bytes for this extent */ > + __u64 fe_physical_length; > + __u64 fe_reserved64[1]; > + /* FIEMAP_EXTENT_* flags for this extent */ > + __u32 fe_flags; > __u32 fe_reserved[3]; > }; > > -- > 2.44.0 > > Cheers, Andreas
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
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.
diff --git a/Documentation/filesystems/fiemap.rst b/Documentation/filesystems/fiemap.rst index 93fc96f760aa..e3e84573b087 100644 --- a/Documentation/filesystems/fiemap.rst +++ b/Documentation/filesystems/fiemap.rst @@ -80,14 +80,24 @@ Each extent is described by a single fiemap_extent structure as returned in fm_extents:: struct fiemap_extent { - __u64 fe_logical; /* logical offset in bytes for the start of - * the extent */ - __u64 fe_physical; /* physical offset in bytes for the start - * of the extent */ - __u64 fe_length; /* length in bytes for the extent */ - __u64 fe_reserved64[2]; - __u32 fe_flags; /* FIEMAP_EXTENT_* flags for this extent */ - __u32 fe_reserved[3]; + /* + * logical offset in bytes for the start of + * the extent from the beginning of the file + */ + __u64 fe_logical; + /* + * physical offset in bytes for the start + * of the extent from the beginning of the disk + */ + __u64 fe_physical; + /* length in bytes for this extent */ + __u64 fe_length; + /* physical length in bytes for this extent */ + __u64 fe_physical_length; + __u64 fe_reserved64[1]; + /* FIEMAP_EXTENT_* flags for this extent */ + __u32 fe_flags; + __u32 fe_reserved[3]; }; All offsets and lengths are in bytes and mirror those on disk. It is valid diff --git a/fs/ioctl.c b/fs/ioctl.c index 1d5abfdf0f22..f8e5d6dfc62d 100644 --- a/fs/ioctl.c +++ b/fs/ioctl.c @@ -139,6 +139,7 @@ int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical, extent.fe_logical = logical; extent.fe_physical = phys; extent.fe_length = len; + extent.fe_physical_length = len; extent.fe_flags = flags; dest += fieinfo->fi_extents_mapped; diff --git a/include/uapi/linux/fiemap.h b/include/uapi/linux/fiemap.h index 24ca0c00cae3..fd3c7d380666 100644 --- a/include/uapi/linux/fiemap.h +++ b/include/uapi/linux/fiemap.h @@ -15,13 +15,23 @@ #include <linux/types.h> struct fiemap_extent { - __u64 fe_logical; /* logical offset in bytes for the start of - * the extent from the beginning of the file */ - __u64 fe_physical; /* physical offset in bytes for the start - * of the extent from the beginning of the disk */ - __u64 fe_length; /* length in bytes for this extent */ - __u64 fe_reserved64[2]; - __u32 fe_flags; /* FIEMAP_EXTENT_* flags for this extent */ + /* + * logical offset in bytes for the start of + * the extent from the beginning of the file + */ + __u64 fe_logical; + /* + * physical offset in bytes for the start + * of the extent from the beginning of the disk + */ + __u64 fe_physical; + /* length in bytes for this extent */ + __u64 fe_length; + /* physical length in bytes for this extent */ + __u64 fe_physical_length; + __u64 fe_reserved64[1]; + /* FIEMAP_EXTENT_* flags for this extent */ + __u32 fe_flags; __u32 fe_reserved[3]; };
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. [1] https://github.com/kilobyte/compsize Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me> --- Documentation/filesystems/fiemap.rst | 26 ++++++++++++++++++-------- fs/ioctl.c | 1 + include/uapi/linux/fiemap.h | 24 +++++++++++++++++------- 3 files changed, 36 insertions(+), 15 deletions(-)