Messages in this thread | | | From | Andreas Dilger <> | Subject | Re: [PATCH v3 01/13] fiemap: add physical_length field to extents | Date | Fri, 5 Apr 2024 12:47:40 -0600 |
| |
On Apr 3, 2024, at 1:22 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. > > [1] https://github.com/kilobyte/compsize > > Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me> > --- > Documentation/filesystems/fiemap.rst | 28 +++++++++++++++++------- > fs/ioctl.c | 3 ++- > include/uapi/linux/fiemap.h | 32 ++++++++++++++++++++++------ > 3 files changed, 47 insertions(+), 16 deletions(-) > > diff --git a/Documentation/filesystems/fiemap.rst b/Documentation/filesystems/fiemap.rst > index 93fc96f760aa..c2bfa107c8d7 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; > + /* logical length in bytes for this extent */ > + __u64 fe_logical_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 > @@ -175,6 +185,8 @@ FIEMAP_EXTENT_MERGED > userspace would be highly inefficient, the kernel will try to merge most > adjacent blocks into 'extents'. > > +FIEMAP_EXTENT_HAS_PHYS_LEN > + This will be set if the file system populated the physical length field. > > VFS -> File System Implementation > --------------------------------- > diff --git a/fs/ioctl.c b/fs/ioctl.c > index 661b46125669..8afd32e1a27a 100644 > --- a/fs/ioctl.c > +++ b/fs/ioctl.c > @@ -138,7 +138,8 @@ int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical, > memset(&extent, 0, sizeof(extent)); > extent.fe_logical = logical; > extent.fe_physical = phys; > - extent.fe_length = len; > + extent.fe_logical_length = len; > + extent.fe_physical_length = len;
I think Jan mentioned this already, and I agree, that fe_physical_length should be left = 0 initially, and be set only when FIEMAP_EXTENT_HAS_PHYS_LEN is set (either explicitly passed from the filesystem, OR possibly set internally by the common fiemap code along with FIEMAP_EXTENT_HAS_PHYS_LEN if the filesystem didn't set this flag itself.
I don't think it makes sense to set fe_physical length in this patch before FIEMAP_EXTENT_HAS_PHYS_LEN is set, nor in the later filesystem-specific patches that are passing "0" for the physical length instead of "len".
Cheers, Andreas
> 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..3079159b8e94 100644 > --- a/include/uapi/linux/fiemap.h > +++ b/include/uapi/linux/fiemap.h > @@ -14,14 +14,30 @@ > > #include <linux/types.h> > > +/* > + * For backward compatibility, where the member of the struct was called > + * fe_length instead of fe_logical_length. > + */ > +#define fe_length fe_logical_length > + > 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; > + /* logical length in bytes for this extent */ > + __u64 fe_logical_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]; > }; > > @@ -66,5 +82,7 @@ struct fiemap { > * merged for efficiency. */ > #define FIEMAP_EXTENT_SHARED 0x00002000 /* Space shared with other > * files. */ > +#define FIEMAP_EXTENT_HAS_PHYS_LEN 0x00004000 /* Physical length is valid > + * and set by FS. */ > > #endif /* _UAPI_LINUX_FIEMAP_H */ > -- > 2.43.0 > >
Cheers, Andreas
[unhandled content-type:application/pgp-signature] | |