| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 5 Apr 2024 12:47:40 -0600
From: Andreas Dilger <adilger@...ger.ca>
To: Sweet Tea Dorminy <sweettea-kernel@...miny.me>
Cc: Jonathan Corbet <corbet@....net>,
Kent Overstreet <kent.overstreet@...ux.dev>,
Brian Foster <bfoster@...hat.com>,
Chris Mason <clm@...com>,
Josef Bacik <josef@...icpanda.com>,
David Sterba <dsterba@...e.com>,
Jaegeuk Kim <jaegeuk@...nel.org>,
Chao Yu <chao@...nel.org>,
Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>,
Jan Kara <jack@...e.cz>,
Mickaël Salaün <mic@...ikod.net>,
linux-doc@...r.kernel.org,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-bcachefs@...r.kernel.org,
linux-btrfs <linux-btrfs@...r.kernel.org>,
linux-f2fs-devel@...ts.sourceforge.net,
linux-fsdevel <linux-fsdevel@...r.kernel.org>,
kernel-team@...a.com
Subject: Re: [PATCH v3 01/13] fiemap: add physical_length field to extents
On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <sweettea-kernel@...miny.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@...miny.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
Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)
Powered by blists - more mailing lists