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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mar 8, 2024, at 11:03 AM, Sweet Tea Dorminy <sweettea-kernel@xxxxxxxxxx> 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@xxxxxxx/

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@xxxxxxxxxx>
> ---
> 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





Attachment: signature.asc
Description: Message signed with OpenPGP


[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux