For many years, various btrfs users have written programs to discover
the actual disk space used by files, using root-only interfaces.
However, this information is a great fit for fiemap: it is inherently
tied to extent information, all filesystems can use it, and the
capabilities required for FIEMAP make sense for this additional
information also.
Hence, this patchset adds various additional information to fiemap,
and extends filesystems (but not iomap) to return it. This uses some of
the reserved padding in the fiemap extent structure, so programs unaware
of the changes will be unaffected.
This is based on next-20240403. I've tested the btrfs part of this with
the standard btrfs testing matrix locally and manually, and done minimal
testing of the non-btrfs parts.
I'm unsure whether btrfs should be returning the entire physical extent
referenced by a particular logical range, or just the part of the
physical extent referenced by that range. The v2 thread has a discussion
of this.
Changelog:
v3:
- Adapted all the direct users of fiemap, except iomap, to emit
the new fiemap information, as far as I understand the other
filesystems.
v2:
- Adopted PHYS_LEN flag and COMPRESSED flag from the previous version,
as per Andreas Dilger' comment.
https://patchwork.ozlabs.org/project/linux-ext4/patch/4f8d5dc5b51a43efaf16c39398c23a6276e40a30.1386778303.git.dsterba@suse.cz/
- https://lore.kernel.org/linux-fsdevel/[email protected]/T/#t
v1: https://lore.kernel.org/linux-fsdevel/20240315030334.GQ6184@frogsfrogsfrogs/T/#t
Sweet Tea Dorminy (13):
fs: fiemap: add physical_length field to extents
fs: fiemap: update fiemap_fill_next_extent() signature
fs: fiemap: add new COMPRESSED extent state
btrfs: fiemap: emit new COMPRESSED state.
btrfs: fiemap: return extent physical size
nilfs2: fiemap: return correct extent physical length
ext4: fiemap: return correct extent physical length
f2fs: fiemap: add physical length to trace_f2fs_fiemap
f2fs: fiemap: return correct extent physical length
ocfs2: fiemap: return correct extent physical length
bcachefs: fiemap: return correct extent physical length
f2fs: fiemap: emit new COMPRESSED state
bcachefs: fiemap: emit new COMPRESSED state
Documentation/filesystems/fiemap.rst | 35 ++++++++++----
fs/bcachefs/fs.c | 17 +++++--
fs/btrfs/extent_io.c | 72 ++++++++++++++++++----------
fs/ext4/extents.c | 3 +-
fs/f2fs/data.c | 36 +++++++++-----
fs/f2fs/inline.c | 7 +--
fs/ioctl.c | 11 +++--
fs/iomap/fiemap.c | 2 +-
fs/nilfs2/inode.c | 18 ++++---
fs/ntfs3/frecord.c | 7 +--
fs/ocfs2/extent_map.c | 10 ++--
fs/smb/client/smb2ops.c | 1 +
include/linux/fiemap.h | 2 +-
include/trace/events/f2fs.h | 10 ++--
include/uapi/linux/fiemap.h | 34 ++++++++++---
15 files changed, 178 insertions(+), 87 deletions(-)
base-commit: 75e31f66adc4c8d049e8aac1f079c1639294cd65
--
2.43.0
Signed-off-by: Sweet Tea Dorminy <[email protected]>
---
fs/bcachefs/fs.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index f830578a9cd1..d2793bae842d 100644
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -913,15 +913,17 @@ static int bch2_fill_extent(struct bch_fs *c,
flags |= FIEMAP_EXTENT_SHARED;
bkey_for_each_ptr_decode(k.k, ptrs, p, entry) {
- int flags2 = 0;
+ int flags2 = FIEMAP_EXTENT_HAS_PHYS_LEN;
+ u64 phys_len = k.k->size << 9;
u64 offset = p.ptr.offset;
if (p.ptr.unwritten)
flags2 |= FIEMAP_EXTENT_UNWRITTEN;
- if (p.crc.compression_type)
+ if (p.crc.compression_type) {
flags2 |= FIEMAP_EXTENT_ENCODED;
- else
+ phys_len = p.crc.compressed_size << 9;
+ } else
offset += p.crc.offset;
if ((offset & (block_sectors(c) - 1)) ||
@@ -931,7 +933,7 @@ static int bch2_fill_extent(struct bch_fs *c,
ret = fiemap_fill_next_extent(info,
bkey_start_offset(k.k) << 9,
offset << 9,
- k.k->size << 9, 0,
+ k.k->size << 9, phys_len,
flags|flags2);
if (ret)
return ret;
@@ -941,14 +943,18 @@ static int bch2_fill_extent(struct bch_fs *c,
} else if (bkey_extent_is_inline_data(k.k)) {
return fiemap_fill_next_extent(info,
bkey_start_offset(k.k) << 9,
- 0, k.k->size << 9, 0,
+ 0, k.k->size << 9,
+ bkey_inline_data_bytes(k.k),
flags|
+ FIEMAP_EXTENT_HAS_PHYS_LEN|
FIEMAP_EXTENT_DATA_INLINE);
} else if (k.k->type == KEY_TYPE_reservation) {
return fiemap_fill_next_extent(info,
bkey_start_offset(k.k) << 9,
- 0, k.k->size << 9, 0,
+ 0, k.k->size << 9,
+ k.k->size << 9,
flags|
+ FIEMAP_EXTENT_HAS_PHYS_LEN|
FIEMAP_EXTENT_DELALLOC|
FIEMAP_EXTENT_UNWRITTEN);
} else {
--
2.43.0
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 <[email protected]>
---
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;
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
This goes closely with the new physical length field in struct
fiemap_extent, as when physical length is not equal to logical length
the reason is frequently compression.
Signed-off-by: Sweet Tea Dorminy <[email protected]>
---
Documentation/filesystems/fiemap.rst | 4 ++++
fs/ioctl.c | 3 ++-
include/uapi/linux/fiemap.h | 2 ++
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/Documentation/filesystems/fiemap.rst b/Documentation/filesystems/fiemap.rst
index c060bb83f5d8..16bd7faba5e0 100644
--- a/Documentation/filesystems/fiemap.rst
+++ b/Documentation/filesystems/fiemap.rst
@@ -162,6 +162,10 @@ FIEMAP_EXTENT_DATA_ENCRYPTED
This will also set FIEMAP_EXTENT_ENCODED
The data in this extent has been encrypted by the file system.
+FIEMAP_EXTENT_DATA_COMPRESSED
+ This will also set FIEMAP_EXTENT_ENCODED
+ The data in this extent is compressed by the file system.
+
FIEMAP_EXTENT_NOT_ALIGNED
Extent offsets and length are not guaranteed to be block aligned.
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 1830baca532b..b47e2da7ec17 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -126,7 +126,8 @@ int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical,
return 1;
#define SET_UNKNOWN_FLAGS (FIEMAP_EXTENT_DELALLOC)
-#define SET_NO_UNMOUNTED_IO_FLAGS (FIEMAP_EXTENT_DATA_ENCRYPTED)
+#define SET_NO_UNMOUNTED_IO_FLAGS (FIEMAP_EXTENT_DATA_ENCRYPTED|\
+ FIEMAP_EXTENT_DATA_COMPRESSED)
#define SET_NOT_ALIGNED_FLAGS (FIEMAP_EXTENT_DATA_TAIL|FIEMAP_EXTENT_DATA_INLINE)
if (flags & SET_UNKNOWN_FLAGS)
diff --git a/include/uapi/linux/fiemap.h b/include/uapi/linux/fiemap.h
index 3079159b8e94..ea97e33ddbb3 100644
--- a/include/uapi/linux/fiemap.h
+++ b/include/uapi/linux/fiemap.h
@@ -67,6 +67,8 @@ struct fiemap {
* Sets EXTENT_UNKNOWN. */
#define FIEMAP_EXTENT_ENCODED 0x00000008 /* Data can not be read
* while fs is unmounted */
+#define FIEMAP_EXTENT_DATA_COMPRESSED 0x00000040 /* Data is compressed by fs.
+ * Sets EXTENT_ENCODED. */
#define FIEMAP_EXTENT_DATA_ENCRYPTED 0x00000080 /* Data is encrypted by fs.
* Sets EXTENT_NO_BYPASS. */
#define FIEMAP_EXTENT_NOT_ALIGNED 0x00000100 /* Extent offsets may not be
--
2.43.0
Hi,
On 2024/4/3 23:11, Sweet Tea Dorminy wrote:
>
>>
>> I'm not sure why here iomap was excluded technically or I'm missing some
>> previous comments?
>>
>>
>> Could you also make iomap support new FIEMAP physical extent information?
>> since compressed EROFS uses iomap FIEMAP interface to report compressed
>> extents ("z_erofs_iomap_report_ops") but there is no way to return
>> correct compressed lengths, that is unexpected.
>>
>
> I'll add iomap support in v4, I'd skipped it since I was worried it'd be an expansive additional part not necessary initially. Thank you for noting it!
Thanks, I think just fiemap report for iomap seems straight-forward.
Thanks for your work!
Thanks,
Gao Xiang
>
> Sweet Tea
Signed-off-by: Sweet Tea Dorminy <[email protected]>
---
fs/f2fs/data.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 2a3625c10125..e999cb1bb02f 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2014,6 +2014,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
if (compr_cluster) {
flags = FIEMAP_EXTENT_ENCODED |
+ flags = FIEMAP_EXTENT_DATA_COMPRESSED |
FIEMAP_EXTENT_HAS_PHYS_LEN;
count_in_cluster += map.m_len;
if (count_in_cluster == cluster_size) {
--
2.43.0
Signed-off-by: Sweet Tea Dorminy <[email protected]>
---
fs/ext4/extents.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index 2adade3c202a..4874f757e1bd 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2194,7 +2194,7 @@ static int ext4_fill_es_cache_info(struct inode *inode,
while (block <= end) {
next = 0;
- flags = 0;
+ flags = FIEMAP_EXTENT_HAS_PHYS_LEN;
if (!ext4_es_lookup_extent(inode, block, &next, &es))
break;
if (ext4_es_is_unwritten(&es))
--
2.43.0
Signed-off-by: Sweet Tea Dorminy <[email protected]>
---
fs/bcachefs/fs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index d2793bae842d..54f613f977b4 100644
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -921,7 +921,7 @@ static int bch2_fill_extent(struct bch_fs *c,
flags2 |= FIEMAP_EXTENT_UNWRITTEN;
if (p.crc.compression_type) {
- flags2 |= FIEMAP_EXTENT_ENCODED;
+ flags2 |= FIEMAP_EXTENT_DATA_COMPRESSED;
phys_len = p.crc.compressed_size << 9;
} else
offset += p.crc.offset;
--
2.43.0
Signed-off-by: Sweet Tea Dorminy <[email protected]>
---
fs/ocfs2/extent_map.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index eabdf97cd685..229ea45df37b 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -705,7 +705,9 @@ static int ocfs2_fiemap_inline(struct inode *inode, struct buffer_head *di_bh,
unsigned int id_count;
struct ocfs2_dinode *di;
u64 phys;
- u32 flags = FIEMAP_EXTENT_DATA_INLINE|FIEMAP_EXTENT_LAST;
+ u32 flags = (FIEMAP_EXTENT_DATA_INLINE|
+ FIEMAP_EXTENT_HAS_PHYS_LEN|
+ FIEMAP_EXTENT_LAST);
struct ocfs2_inode_info *oi = OCFS2_I(inode);
di = (struct ocfs2_dinode *)di_bh->b_data;
@@ -782,7 +784,7 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
continue;
}
- fe_flags = 0;
+ fe_flags = FIEMAP_EXTENT_HAS_PHYS_LEN;
if (rec.e_flags & OCFS2_EXT_UNWRITTEN)
fe_flags |= FIEMAP_EXTENT_UNWRITTEN;
if (rec.e_flags & OCFS2_EXT_REFCOUNTED)
--
2.43.0
On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <[email protected]> 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 <[email protected]>
> ---
> 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
On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <[email protected]> wrote:
>
> This goes closely with the new physical length field in struct
> fiemap_extent, as when physical length is not equal to logical length
> the reason is frequently compression.
>
> Signed-off-by: Sweet Tea Dorminy <[email protected]>
Looks good.
Reviewed-by: Andreas Dilger <[email protected]>
> ---
> Documentation/filesystems/fiemap.rst | 4 ++++
> fs/ioctl.c | 3 ++-
> include/uapi/linux/fiemap.h | 2 ++
> 3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/filesystems/fiemap.rst b/Documentation/filesystems/fiemap.rst
> index c060bb83f5d8..16bd7faba5e0 100644
> --- a/Documentation/filesystems/fiemap.rst
> +++ b/Documentation/filesystems/fiemap.rst
> @@ -162,6 +162,10 @@ FIEMAP_EXTENT_DATA_ENCRYPTED
> This will also set FIEMAP_EXTENT_ENCODED
> The data in this extent has been encrypted by the file system.
>
> +FIEMAP_EXTENT_DATA_COMPRESSED
> + This will also set FIEMAP_EXTENT_ENCODED
> + The data in this extent is compressed by the file system.
> +
> FIEMAP_EXTENT_NOT_ALIGNED
> Extent offsets and length are not guaranteed to be block aligned.
>
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 1830baca532b..b47e2da7ec17 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -126,7 +126,8 @@ int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical,
> return 1;
>
> #define SET_UNKNOWN_FLAGS (FIEMAP_EXTENT_DELALLOC)
> -#define SET_NO_UNMOUNTED_IO_FLAGS (FIEMAP_EXTENT_DATA_ENCRYPTED)
> +#define SET_NO_UNMOUNTED_IO_FLAGS (FIEMAP_EXTENT_DATA_ENCRYPTED|\
> + FIEMAP_EXTENT_DATA_COMPRESSED)
> #define SET_NOT_ALIGNED_FLAGS (FIEMAP_EXTENT_DATA_TAIL|FIEMAP_EXTENT_DATA_INLINE)
>
> if (flags & SET_UNKNOWN_FLAGS)
> diff --git a/include/uapi/linux/fiemap.h b/include/uapi/linux/fiemap.h
> index 3079159b8e94..ea97e33ddbb3 100644
> --- a/include/uapi/linux/fiemap.h
> +++ b/include/uapi/linux/fiemap.h
> @@ -67,6 +67,8 @@ struct fiemap {
> * Sets EXTENT_UNKNOWN. */
> #define FIEMAP_EXTENT_ENCODED 0x00000008 /* Data can not be read
> * while fs is unmounted */
> +#define FIEMAP_EXTENT_DATA_COMPRESSED 0x00000040 /* Data is compressed by fs.
> + * Sets EXTENT_ENCODED. */
> #define FIEMAP_EXTENT_DATA_ENCRYPTED 0x00000080 /* Data is encrypted by fs.
> * Sets EXTENT_NO_BYPASS. */
> #define FIEMAP_EXTENT_NOT_ALIGNED 0x00000100 /* Extent offsets may not be
> --
> 2.43.0
>
>
Cheers, Andreas
On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <[email protected]> wrote:
>
> Signed-off-by: Sweet Tea Dorminy <[email protected]>
> ---
> fs/bcachefs/fs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
> index d2793bae842d..54f613f977b4 100644
> --- a/fs/bcachefs/fs.c
> +++ b/fs/bcachefs/fs.c
> @@ -921,7 +921,7 @@ static int bch2_fill_extent(struct bch_fs *c,
> flags2 |= FIEMAP_EXTENT_UNWRITTEN;
>
> if (p.crc.compression_type) {
> - flags2 |= FIEMAP_EXTENT_ENCODED;
> + flags2 |= FIEMAP_EXTENT_DATA_COMPRESSED;
(defect) This should *also* set FIEMAP_EXTENT_ENCODED in this case,
along with FIEMAP_EXTENT_DATA_COMPRESSED. Both for compatibility with
older code that doesn't understand FIEMAP_EXTENT_DATA_COMPRESSED, and
because the data still cannot be read directly from the volume when it
is not mounted.
Probably Kent should chime in here with what needs to be done to set
the phys_len properly for bcachefs, or leave this patch out of your
series and let him submit it directly. With proposed wrapper in the
first patch of the series there isn't a hard requirement to change
all of the filesystems in one shot.
Cheers, Andreas
> On Apr 5, 2024, at 1:17 PM, Andreas Dilger <[email protected]> wrote:
>
> On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <[email protected]> wrote:
>>
>> Signed-off-by: Sweet Tea Dorminy <[email protected]>
>> ---
>> fs/bcachefs/fs.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
>> index d2793bae842d..54f613f977b4 100644
>> --- a/fs/bcachefs/fs.c
>> +++ b/fs/bcachefs/fs.c
>> @@ -921,7 +921,7 @@ static int bch2_fill_extent(struct bch_fs *c,
>> flags2 |= FIEMAP_EXTENT_UNWRITTEN;
>>
>> if (p.crc.compression_type) {
>> - flags2 |= FIEMAP_EXTENT_ENCODED;
>> + flags2 |= FIEMAP_EXTENT_DATA_COMPRESSED;
>
> (defect) This should *also* set FIEMAP_EXTENT_ENCODED in this case,
> along with FIEMAP_EXTENT_DATA_COMPRESSED. Both for compatibility with
> older code that doesn't understand FIEMAP_EXTENT_DATA_COMPRESSED, and
> because the data still cannot be read directly from the volume when it
> is not mounted.
>
> Probably Kent should chime in here with what needs to be done to set
> the phys_len properly for bcachefs, or leave this patch out of your
> series and let him submit it directly. With proposed wrapper in the
> first patch of the series there isn't a hard requirement to change
> all of the filesystems in one shot.
Ah, I missed the 11/13 patch that is handling up most of the bcachefs
phys_len changes. I think this should be folded into that patch so
it is clear to the callers that the data is compressed when they see
fe_physical_length is not the same as fe_logical_length.
Cheers, Andreas
On Fri, Apr 05, 2024 at 01:17:45PM -0600, Andreas Dilger wrote:
> On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <[email protected]> wrote:
> >
> > Signed-off-by: Sweet Tea Dorminy <[email protected]>
> > ---
> > fs/bcachefs/fs.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
> > index d2793bae842d..54f613f977b4 100644
> > --- a/fs/bcachefs/fs.c
> > +++ b/fs/bcachefs/fs.c
> > @@ -921,7 +921,7 @@ static int bch2_fill_extent(struct bch_fs *c,
> > flags2 |= FIEMAP_EXTENT_UNWRITTEN;
> >
> > if (p.crc.compression_type) {
> > - flags2 |= FIEMAP_EXTENT_ENCODED;
> > + flags2 |= FIEMAP_EXTENT_DATA_COMPRESSED;
>
> (defect) This should *also* set FIEMAP_EXTENT_ENCODED in this case,
> along with FIEMAP_EXTENT_DATA_COMPRESSED. Both for compatibility with
> older code that doesn't understand FIEMAP_EXTENT_DATA_COMPRESSED, and
> because the data still cannot be read directly from the volume when it
> is not mounted.
>
> Probably Kent should chime in here with what needs to be done to set
> the phys_len properly for bcachefs, or leave this patch out of your
> series and let him submit it directly. With proposed wrapper in the
> first patch of the series there isn't a hard requirement to change
> all of the filesystems in one shot.
You get phys len from crc.compressed_size - that's always guaranteed,
even if it's not compressed
On Wed, Apr 03, 2024 at 03:22:42AM -0400, Sweet Tea Dorminy 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 <[email protected]>
> ---
> 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.
Just out of curiosity, should filesystems set this flag and
fe_physical_length if fe_physical_length == fe_logical_length?
Or just leave both blank?
> 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;
> 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
This #define has global scope; are you sure this isn't going to cause a
weird build problem downstream with some program that declares an
unrelated fe_length symbol?
> +
> 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;
Or why not just leave the field name the same since the "logical length
in bytes" comment is present both here in the header and again in the
documentation?
--D
> + /* 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
>
>
On Apr 9, 2024, at 10:22 AM, Darrick J. Wong <[email protected]> wrote:
>
> On Wed, Apr 03, 2024 at 03:22:42AM -0400, Sweet Tea Dorminy 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 <[email protected]>
>> ---
>> 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.
>
> Just out of curiosity, should filesystems set this flag and
> fe_physical_length if fe_physical_length == fe_logical_length?
> Or just leave both blank?
In the original thread, Dave thought it would be better to always set
fe_physical_length and the flag, so that userspace applications which do
not properly check the flag will not be confused/broken by differences in
filesystem behavior in the future when this is in use.
https://patchwork.ozlabs.org/project/linux-ext4/patch/4f8d5dc5b51a43efaf16c39398c23a6276e40a30.1386778303.git.dsterba@suse.cz/
>
>> 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;
>> 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
>
> This #define has global scope; are you sure this isn't going to cause a
> weird build problem downstream with some program that declares an
> unrelated fe_length symbol?
I guess it's possible. I'm not dead set on this part of the change.
I thought it was cleaner to separate the two in the struct, but I
can see the argument that a UAPI field struct should not change names.
It would be possible to have:
#define fe_logical_length fe_length
which would have much less chance of namespace collisions I think.
New applications can start to use this for some years, before
making a permanent switch, but again not something I'm stuck on...
Cheers, Andreas
>> +
>> 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;
>
> Or why not just leave the field name the same since the "logical length
> in bytes" comment is present both here in the header and again in the
> documentation?
>
> --D
>
>> + /* 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