* [PATCH v3 00/13] fiemap extension for more physical information
@ 2024-04-03 7:22 Sweet Tea Dorminy
2024-04-03 7:22 ` [PATCH v3 01/13] fs: fiemap: add physical_length field to extents Sweet Tea Dorminy
` (14 more replies)
0 siblings, 15 replies; 38+ messages in thread
From: Sweet Tea Dorminy @ 2024-04-03 7:22 UTC (permalink / raw
To: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Sweet Tea Dorminy, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
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/cover.1711588701.git.sweettea-kernel@dorminy.me/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
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v3 01/13] fs: fiemap: add physical_length field to extents
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
@ 2024-04-03 7:22 ` Sweet Tea Dorminy
2024-04-03 16:57 ` Brian Foster
` (2 more replies)
2024-04-03 7:22 ` [PATCH v3 02/13] fs: fiemap: update fiemap_fill_next_extent() signature Sweet Tea Dorminy
` (13 subsequent siblings)
14 siblings, 3 replies; 38+ messages in thread
From: Sweet Tea Dorminy @ 2024-04-03 7:22 UTC (permalink / raw
To: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Sweet Tea Dorminy, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
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;
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v3 02/13] fs: fiemap: update fiemap_fill_next_extent() signature
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
2024-04-03 7:22 ` [PATCH v3 01/13] fs: fiemap: add physical_length field to extents Sweet Tea Dorminy
@ 2024-04-03 7:22 ` Sweet Tea Dorminy
2024-04-03 16:58 ` Brian Foster
2024-04-05 19:05 ` [PATCH v3 02/13] " Andreas Dilger
2024-04-03 7:22 ` [PATCH v3 03/13] fs: fiemap: add new COMPRESSED extent state Sweet Tea Dorminy
` (12 subsequent siblings)
14 siblings, 2 replies; 38+ messages in thread
From: Sweet Tea Dorminy @ 2024-04-03 7:22 UTC (permalink / raw
To: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Sweet Tea Dorminy, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
Update the signature of fiemap_fill_next_extent() to allow passing a
physical length. Update all callers to pass a 0 physical length -- since
none set the EXTENT_HAS_PHYS_LEN flag, this value doesn't matter.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
Documentation/filesystems/fiemap.rst | 3 ++-
fs/bcachefs/fs.c | 7 ++++---
fs/btrfs/extent_io.c | 4 ++--
fs/ext4/extents.c | 1 +
fs/f2fs/data.c | 8 +++++---
fs/f2fs/inline.c | 3 ++-
fs/ioctl.c | 9 +++++----
fs/iomap/fiemap.c | 2 +-
fs/nilfs2/inode.c | 6 +++---
fs/ntfs3/frecord.c | 7 ++++---
fs/ocfs2/extent_map.c | 4 ++--
fs/smb/client/smb2ops.c | 1 +
include/linux/fiemap.h | 2 +-
13 files changed, 33 insertions(+), 24 deletions(-)
diff --git a/Documentation/filesystems/fiemap.rst b/Documentation/filesystems/fiemap.rst
index c2bfa107c8d7..c060bb83f5d8 100644
--- a/Documentation/filesystems/fiemap.rst
+++ b/Documentation/filesystems/fiemap.rst
@@ -236,7 +236,8 @@ For each extent in the request range, the file system should call
the helper function, fiemap_fill_next_extent()::
int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
- u64 phys, u64 len, u32 flags, u32 dev);
+ u64 phys, u64 log_len, u64 phys_len, u32 flags,
+ u32 dev);
fiemap_fill_next_extent() will use the passed values to populate the
next free extent in the fm_extents array. 'General' extent flags will
diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index 71013256fc39..f830578a9cd1 100644
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -931,7 +931,8 @@ 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, flags|flags2);
+ k.k->size << 9, 0,
+ flags|flags2);
if (ret)
return ret;
}
@@ -940,13 +941,13 @@ 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, k.k->size << 9, 0,
flags|
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, k.k->size << 9, 0,
flags|
FIEMAP_EXTENT_DELALLOC|
FIEMAP_EXTENT_UNWRITTEN);
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index eceef5ff780b..9e421d99fd5c 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2577,7 +2577,7 @@ static int flush_fiemap_cache(struct fiemap_extent_info *fieinfo,
int ret;
ret = fiemap_fill_next_extent(fieinfo, entry->offset,
- entry->phys, entry->len,
+ entry->phys, entry->len, 0,
entry->flags);
/*
* Ignore 1 (reached max entries) because we keep track of that
@@ -2793,7 +2793,7 @@ static int emit_last_fiemap_cache(struct fiemap_extent_info *fieinfo,
return 0;
ret = fiemap_fill_next_extent(fieinfo, cache->offset, cache->phys,
- cache->len, cache->flags);
+ cache->len, 0, cache->flags);
cache->cached = false;
if (ret > 0)
ret = 0;
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index e57054bdc5fd..2adade3c202a 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -2215,6 +2215,7 @@ static int ext4_fill_es_cache_info(struct inode *inode,
(__u64)es.es_lblk << blksize_bits,
(__u64)es.es_pblk << blksize_bits,
(__u64)es.es_len << blksize_bits,
+ 0,
flags);
if (next == 0)
break;
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index d9494b5fc7c1..87f8d828e038 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1834,7 +1834,8 @@ static int f2fs_xattr_fiemap(struct inode *inode,
if (!xnid)
flags |= FIEMAP_EXTENT_LAST;
- err = fiemap_fill_next_extent(fieinfo, 0, phys, len, flags);
+ err = fiemap_fill_next_extent(
+ fieinfo, 0, phys, len, 0, flags);
trace_f2fs_fiemap(inode, 0, phys, len, flags, err);
if (err)
return err;
@@ -1860,7 +1861,8 @@ static int f2fs_xattr_fiemap(struct inode *inode,
}
if (phys) {
- err = fiemap_fill_next_extent(fieinfo, 0, phys, len, flags);
+ err = fiemap_fill_next_extent(
+ fieinfo, 0, phys, len, 0, flags);
trace_f2fs_fiemap(inode, 0, phys, len, flags, err);
}
@@ -1979,7 +1981,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
flags |= FIEMAP_EXTENT_DATA_ENCRYPTED;
ret = fiemap_fill_next_extent(fieinfo, logical,
- phys, size, flags);
+ phys, size, 0, flags);
trace_f2fs_fiemap(inode, logical, phys, size, flags, ret);
if (ret)
goto out;
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index ac00423f117b..49d2f87fe048 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -806,7 +806,8 @@ int f2fs_inline_data_fiemap(struct inode *inode,
byteaddr = (__u64)ni.blk_addr << inode->i_sb->s_blocksize_bits;
byteaddr += (char *)inline_data_addr(inode, ipage) -
(char *)F2FS_INODE(ipage);
- err = fiemap_fill_next_extent(fieinfo, start, byteaddr, ilen, flags);
+ err = fiemap_fill_next_extent(
+ fieinfo, start, byteaddr, ilen, 0, flags);
trace_f2fs_fiemap(inode, start, byteaddr, ilen, flags, err);
out:
f2fs_put_page(ipage, 1);
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 8afd32e1a27a..1830baca532b 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -99,7 +99,8 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
* @fieinfo: Fiemap context passed into ->fiemap
* @logical: Extent logical start offset, in bytes
* @phys: Extent physical start offset, in bytes
- * @len: Extent length, in bytes
+ * @log_len: Extent logical length, in bytes
+ * @phys_len: Extent physical length, in bytes (optional)
* @flags: FIEMAP_EXTENT flags that describe this extent
*
* Called from file system ->fiemap callback. Will populate extent
@@ -110,7 +111,7 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
* extent that will fit in user array.
*/
int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical,
- u64 phys, u64 len, u32 flags)
+ u64 phys, u64 log_len, u64 phys_len, u32 flags)
{
struct fiemap_extent extent;
struct fiemap_extent __user *dest = fieinfo->fi_extents_start;
@@ -138,8 +139,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_logical_length = len;
- extent.fe_physical_length = len;
+ extent.fe_logical_length = log_len;
+ extent.fe_physical_length = phys_len;
extent.fe_flags = flags;
dest += fieinfo->fi_extents_mapped;
diff --git a/fs/iomap/fiemap.c b/fs/iomap/fiemap.c
index 610ca6f1ec9b..013e843c8d10 100644
--- a/fs/iomap/fiemap.c
+++ b/fs/iomap/fiemap.c
@@ -36,7 +36,7 @@ static int iomap_to_fiemap(struct fiemap_extent_info *fi,
return fiemap_fill_next_extent(fi, iomap->offset,
iomap->addr != IOMAP_NULL_ADDR ? iomap->addr : 0,
- iomap->length, flags);
+ iomap->length, 0, flags);
}
static loff_t iomap_fiemap_iter(const struct iomap_iter *iter,
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 7340a01d80e1..4d3c347c982b 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -1190,7 +1190,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
if (size) {
/* End of the current extent */
ret = fiemap_fill_next_extent(
- fieinfo, logical, phys, size, flags);
+ fieinfo, logical, phys, size, 0, flags);
if (ret)
break;
}
@@ -1240,7 +1240,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
flags |= FIEMAP_EXTENT_LAST;
ret = fiemap_fill_next_extent(
- fieinfo, logical, phys, size, flags);
+ fieinfo, logical, phys, size, 0, flags);
if (ret)
break;
size = 0;
@@ -1256,7 +1256,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
/* Terminate the current extent */
ret = fiemap_fill_next_extent(
fieinfo, logical, phys, size,
- flags);
+ 0, flags);
if (ret || blkoff > end_blkoff)
break;
diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index 7f27382e0ce2..ef0ed913428b 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -1947,7 +1947,7 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,
if (!attr || !attr->non_res) {
err = fiemap_fill_next_extent(
fieinfo, 0, 0,
- attr ? le32_to_cpu(attr->res.data_size) : 0,
+ attr ? le32_to_cpu(attr->res.data_size) : 0, 0,
FIEMAP_EXTENT_DATA_INLINE | FIEMAP_EXTENT_LAST |
FIEMAP_EXTENT_MERGED);
goto out;
@@ -2042,7 +2042,7 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,
flags |= FIEMAP_EXTENT_LAST;
err = fiemap_fill_next_extent(fieinfo, vbo, lbo, dlen,
- flags);
+ 0, flags);
if (err < 0)
break;
if (err == 1) {
@@ -2062,7 +2062,8 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,
if (vbo + bytes >= end)
flags |= FIEMAP_EXTENT_LAST;
- err = fiemap_fill_next_extent(fieinfo, vbo, lbo, bytes, flags);
+ err = fiemap_fill_next_extent(fieinfo, vbo, lbo, bytes, 0,
+ flags);
if (err < 0)
break;
if (err == 1) {
diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
index 70a768b623cf..eabdf97cd685 100644
--- a/fs/ocfs2/extent_map.c
+++ b/fs/ocfs2/extent_map.c
@@ -723,7 +723,7 @@ static int ocfs2_fiemap_inline(struct inode *inode, struct buffer_head *di_bh,
id2.i_data.id_data);
ret = fiemap_fill_next_extent(fieinfo, 0, phys, id_count,
- flags);
+ 0, flags);
if (ret < 0)
return ret;
}
@@ -794,7 +794,7 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
virt_bytes = (u64)le32_to_cpu(rec.e_cpos) << osb->s_clustersize_bits;
ret = fiemap_fill_next_extent(fieinfo, virt_bytes, phys_bytes,
- len_bytes, fe_flags);
+ len_bytes, 0, fe_flags);
if (ret)
break;
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 87b63f6ad2e2..23a193512f96 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -3779,6 +3779,7 @@ static int smb3_fiemap(struct cifs_tcon *tcon,
le64_to_cpu(out_data[i].file_offset),
le64_to_cpu(out_data[i].file_offset),
le64_to_cpu(out_data[i].length),
+ 0,
flags);
if (rc < 0)
goto out;
diff --git a/include/linux/fiemap.h b/include/linux/fiemap.h
index c50882f19235..17a6c32cdf3f 100644
--- a/include/linux/fiemap.h
+++ b/include/linux/fiemap.h
@@ -16,6 +16,6 @@ struct fiemap_extent_info {
int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo,
u64 start, u64 *len, u32 supported_flags);
int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
- u64 phys, u64 len, u32 flags);
+ u64 phys, u64 log_len, u64 phys_len, u32 flags);
#endif /* _LINUX_FIEMAP_H 1 */
--
2.43.0
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v3 03/13] fs: fiemap: add new COMPRESSED extent state
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
2024-04-03 7:22 ` [PATCH v3 01/13] fs: fiemap: add physical_length field to extents Sweet Tea Dorminy
2024-04-03 7:22 ` [PATCH v3 02/13] fs: fiemap: update fiemap_fill_next_extent() signature Sweet Tea Dorminy
@ 2024-04-03 7:22 ` Sweet Tea Dorminy
2024-04-05 19:06 ` [PATCH v3 03/13] " Andreas Dilger
2024-04-03 7:22 ` [PATCH v3 04/13] btrfs: fiemap: emit new COMPRESSED state Sweet Tea Dorminy
` (11 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Sweet Tea Dorminy @ 2024-04-03 7:22 UTC (permalink / raw
To: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Sweet Tea Dorminy, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
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 <sweettea-kernel@dorminy.me>
---
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v3 04/13] btrfs: fiemap: emit new COMPRESSED state.
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
` (2 preceding siblings ...)
2024-04-03 7:22 ` [PATCH v3 03/13] fs: fiemap: add new COMPRESSED extent state Sweet Tea Dorminy
@ 2024-04-03 7:22 ` Sweet Tea Dorminy
2024-04-05 19:10 ` Andreas Dilger
2024-04-03 7:22 ` [PATCH v3 05/13] btrfs: fiemap: return extent physical size Sweet Tea Dorminy
` (10 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Sweet Tea Dorminy @ 2024-04-03 7:22 UTC (permalink / raw
To: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Sweet Tea Dorminy, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/btrfs/extent_io.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 9e421d99fd5c..e9df670ef7d2 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2706,7 +2706,7 @@ static int emit_fiemap_extent(struct fiemap_extent_info *fieinfo,
if (range_end <= cache_end)
return 0;
- if (!(flags & (FIEMAP_EXTENT_ENCODED | FIEMAP_EXTENT_DELALLOC)))
+ if (!(flags & (FIEMAP_EXTENT_DATA_COMPRESSED | FIEMAP_EXTENT_DELALLOC)))
phys += cache_end - offset;
offset = cache_end;
@@ -3236,7 +3236,7 @@ int extent_fiemap(struct btrfs_inode *inode, struct fiemap_extent_info *fieinfo,
}
if (compression != BTRFS_COMPRESS_NONE)
- flags |= FIEMAP_EXTENT_ENCODED;
+ flags |= FIEMAP_EXTENT_DATA_COMPRESSED;
if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
flags |= FIEMAP_EXTENT_DATA_INLINE;
--
2.43.0
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v3 05/13] btrfs: fiemap: return extent physical size
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
` (3 preceding siblings ...)
2024-04-03 7:22 ` [PATCH v3 04/13] btrfs: fiemap: emit new COMPRESSED state Sweet Tea Dorminy
@ 2024-04-03 7:22 ` Sweet Tea Dorminy
2024-04-03 7:22 ` [PATCH v3 06/13] nilfs2: fiemap: return correct extent physical length Sweet Tea Dorminy
` (9 subsequent siblings)
14 siblings, 0 replies; 38+ messages in thread
From: Sweet Tea Dorminy @ 2024-04-03 7:22 UTC (permalink / raw
To: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Sweet Tea Dorminy, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
Now that fiemap allows returning extent physical size, make btrfs return
the appropriate extent's actual disk size.
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/btrfs/extent_io.c | 70 ++++++++++++++++++++++++++++----------------
1 file changed, 45 insertions(+), 25 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index e9df670ef7d2..b631f387cc3c 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2506,7 +2506,8 @@ int try_release_extent_mapping(struct page *page, gfp_t mask)
struct btrfs_fiemap_entry {
u64 offset;
u64 phys;
- u64 len;
+ u64 log_len;
+ u64 phys_len;
u32 flags;
};
@@ -2564,7 +2565,8 @@ struct fiemap_cache {
/* Fields for the cached extent (unsubmitted, not ready, extent). */
u64 offset;
u64 phys;
- u64 len;
+ u64 log_len;
+ u64 phys_len;
u32 flags;
bool cached;
};
@@ -2577,8 +2579,8 @@ static int flush_fiemap_cache(struct fiemap_extent_info *fieinfo,
int ret;
ret = fiemap_fill_next_extent(fieinfo, entry->offset,
- entry->phys, entry->len, 0,
- entry->flags);
+ entry->phys, entry->log_len,
+ entry->phys_len, entry->flags);
/*
* Ignore 1 (reached max entries) because we keep track of that
* ourselves in emit_fiemap_extent().
@@ -2603,7 +2605,8 @@ static int flush_fiemap_cache(struct fiemap_extent_info *fieinfo,
*/
static int emit_fiemap_extent(struct fiemap_extent_info *fieinfo,
struct fiemap_cache *cache,
- u64 offset, u64 phys, u64 len, u32 flags)
+ u64 offset, u64 phys, u64 log_len,
+ u64 phys_len, u32 flags)
{
struct btrfs_fiemap_entry *entry;
u64 cache_end;
@@ -2611,6 +2614,9 @@ static int emit_fiemap_extent(struct fiemap_extent_info *fieinfo,
/* Set at the end of extent_fiemap(). */
ASSERT((flags & FIEMAP_EXTENT_LAST) == 0);
+ /* We always set the correct physical length. */
+ flags |= FIEMAP_EXTENT_HAS_PHYS_LEN;
+
if (!cache->cached)
goto assign;
@@ -2646,7 +2652,7 @@ static int emit_fiemap_extent(struct fiemap_extent_info *fieinfo,
* or equals to what we have in cache->offset. We deal with this as
* described below.
*/
- cache_end = cache->offset + cache->len;
+ cache_end = cache->offset + cache->log_len;
if (cache_end > offset) {
if (offset == cache->offset) {
/*
@@ -2670,10 +2676,10 @@ static int emit_fiemap_extent(struct fiemap_extent_info *fieinfo,
* where a previously found file extent item was split
* due to an ordered extent completing.
*/
- cache->len = offset - cache->offset;
+ cache->log_len = offset - cache->offset;
goto emit;
} else {
- const u64 range_end = offset + len;
+ const u64 range_end = offset + log_len;
/*
* The offset of the file extent item we have just found
@@ -2706,11 +2712,13 @@ static int emit_fiemap_extent(struct fiemap_extent_info *fieinfo,
if (range_end <= cache_end)
return 0;
- if (!(flags & (FIEMAP_EXTENT_DATA_COMPRESSED | FIEMAP_EXTENT_DELALLOC)))
+ if (!(flags & (FIEMAP_EXTENT_DATA_COMPRESSED | FIEMAP_EXTENT_DELALLOC))) {
phys += cache_end - offset;
+ phys_len -= cache_end - offset;
+ }
offset = cache_end;
- len = range_end - cache_end;
+ log_len = range_end - cache_end;
goto emit;
}
}
@@ -2720,15 +2728,17 @@ static int emit_fiemap_extent(struct fiemap_extent_info *fieinfo,
* 1) Their logical addresses are continuous
*
* 2) Their physical addresses are continuous
- * So truly compressed (physical size smaller than logical size)
- * extents won't get merged with each other
*
* 3) Share same flags
+ *
+ * 4) Not compressed
*/
- if (cache->offset + cache->len == offset &&
- cache->phys + cache->len == phys &&
- cache->flags == flags) {
- cache->len += len;
+ if (cache->offset + cache->log_len == offset &&
+ cache->phys + cache->log_len == phys &&
+ cache->flags == flags &&
+ !(flags & FIEMAP_EXTENT_DATA_COMPRESSED)) {
+ cache->log_len += log_len;
+ cache->phys_len += phys_len;
return 0;
}
@@ -2745,7 +2755,7 @@ static int emit_fiemap_extent(struct fiemap_extent_info *fieinfo,
* to miss it.
*/
entry = &cache->entries[cache->entries_size - 1];
- cache->next_search_offset = entry->offset + entry->len;
+ cache->next_search_offset = entry->offset + entry->log_len;
cache->cached = false;
return BTRFS_FIEMAP_FLUSH_CACHE;
@@ -2754,7 +2764,8 @@ static int emit_fiemap_extent(struct fiemap_extent_info *fieinfo,
entry = &cache->entries[cache->entries_pos];
entry->offset = cache->offset;
entry->phys = cache->phys;
- entry->len = cache->len;
+ entry->log_len = cache->log_len;
+ entry->phys_len = cache->phys_len;
entry->flags = cache->flags;
cache->entries_pos++;
cache->extents_mapped++;
@@ -2767,7 +2778,8 @@ static int emit_fiemap_extent(struct fiemap_extent_info *fieinfo,
cache->cached = true;
cache->offset = offset;
cache->phys = phys;
- cache->len = len;
+ cache->log_len = log_len;
+ cache->phys_len = phys_len;
cache->flags = flags;
return 0;
@@ -2793,7 +2805,8 @@ static int emit_last_fiemap_cache(struct fiemap_extent_info *fieinfo,
return 0;
ret = fiemap_fill_next_extent(fieinfo, cache->offset, cache->phys,
- cache->len, 0, cache->flags);
+ cache->log_len, cache->phys_len,
+ cache->flags);
cache->cached = false;
if (ret > 0)
ret = 0;
@@ -2987,13 +3000,15 @@ static int fiemap_process_hole(struct btrfs_inode *inode,
}
ret = emit_fiemap_extent(fieinfo, cache, prealloc_start,
disk_bytenr + extent_offset,
- prealloc_len, prealloc_flags);
+ prealloc_len, prealloc_len,
+ prealloc_flags);
if (ret)
return ret;
extent_offset += prealloc_len;
}
ret = emit_fiemap_extent(fieinfo, cache, delalloc_start, 0,
+ delalloc_end + 1 - delalloc_start,
delalloc_end + 1 - delalloc_start,
FIEMAP_EXTENT_DELALLOC |
FIEMAP_EXTENT_UNKNOWN);
@@ -3034,7 +3049,8 @@ static int fiemap_process_hole(struct btrfs_inode *inode,
}
ret = emit_fiemap_extent(fieinfo, cache, prealloc_start,
disk_bytenr + extent_offset,
- prealloc_len, prealloc_flags);
+ prealloc_len, prealloc_len,
+ prealloc_flags);
if (ret)
return ret;
}
@@ -3180,6 +3196,7 @@ int extent_fiemap(struct btrfs_inode *inode, struct fiemap_extent_info *fieinfo,
u64 extent_offset = 0;
u64 extent_gen;
u64 disk_bytenr = 0;
+ u64 disk_size = 0;
u64 flags = 0;
int extent_type;
u8 compression;
@@ -3242,7 +3259,7 @@ int extent_fiemap(struct btrfs_inode *inode, struct fiemap_extent_info *fieinfo,
flags |= FIEMAP_EXTENT_DATA_INLINE;
flags |= FIEMAP_EXTENT_NOT_ALIGNED;
ret = emit_fiemap_extent(fieinfo, &cache, key.offset, 0,
- extent_len, flags);
+ extent_len, extent_len, flags);
} else if (extent_type == BTRFS_FILE_EXTENT_PREALLOC) {
ret = fiemap_process_hole(inode, fieinfo, &cache,
&delalloc_cached_state,
@@ -3257,6 +3274,7 @@ int extent_fiemap(struct btrfs_inode *inode, struct fiemap_extent_info *fieinfo,
backref_ctx, 0, 0, 0,
key.offset, extent_end - 1);
} else {
+ disk_size = btrfs_file_extent_disk_num_bytes(leaf, ei);
/* We have a regular extent. */
if (fieinfo->fi_extents_max) {
ret = btrfs_is_data_extent_shared(inode,
@@ -3271,7 +3289,9 @@ int extent_fiemap(struct btrfs_inode *inode, struct fiemap_extent_info *fieinfo,
ret = emit_fiemap_extent(fieinfo, &cache, key.offset,
disk_bytenr + extent_offset,
- extent_len, flags);
+ extent_len,
+ disk_size - extent_offset,
+ flags);
}
if (ret < 0) {
@@ -3309,7 +3329,7 @@ int extent_fiemap(struct btrfs_inode *inode, struct fiemap_extent_info *fieinfo,
prev_extent_end = range_end;
}
- if (cache.cached && cache.offset + cache.len >= last_extent_end) {
+ if (cache.cached && cache.offset + cache.log_len >= last_extent_end) {
const u64 i_size = i_size_read(&inode->vfs_inode);
if (prev_extent_end < i_size) {
--
2.43.0
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v3 06/13] nilfs2: fiemap: return correct extent physical length
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
` (4 preceding siblings ...)
2024-04-03 7:22 ` [PATCH v3 05/13] btrfs: fiemap: return extent physical size Sweet Tea Dorminy
@ 2024-04-03 7:22 ` Sweet Tea Dorminy
2024-04-05 19:26 ` Andreas Dilger
2024-04-03 7:22 ` [PATCH v3 07/13] ext4: " Sweet Tea Dorminy
` (8 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Sweet Tea Dorminy @ 2024-04-03 7:22 UTC (permalink / raw
To: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Sweet Tea Dorminy, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/nilfs2/inode.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
index 4d3c347c982b..e3108f2cead7 100644
--- a/fs/nilfs2/inode.c
+++ b/fs/nilfs2/inode.c
@@ -1160,7 +1160,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
{
struct the_nilfs *nilfs = inode->i_sb->s_fs_info;
__u64 logical = 0, phys = 0, size = 0;
- __u32 flags = 0;
+ __u32 flags = FIEMAP_EXTENT_HAS_PHYS_LEN;
loff_t isize;
sector_t blkoff, end_blkoff;
sector_t delalloc_blkoff;
@@ -1197,7 +1197,9 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
if (blkoff > end_blkoff)
break;
- flags = FIEMAP_EXTENT_MERGED | FIEMAP_EXTENT_DELALLOC;
+ flags = FIEMAP_EXTENT_MERGED |
+ FIEMAP_EXTENT_DELALLOC |
+ FIEMAP_EXTENT_HAS_PHYS_LEN;
logical = blkoff << blkbits;
phys = 0;
size = delalloc_blklen << blkbits;
@@ -1261,14 +1263,16 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
break;
/* Start another extent */
- flags = FIEMAP_EXTENT_MERGED;
+ flags = FIEMAP_EXTENT_MERGED |
+ FIEMAP_EXTENT_HAS_PHYS_LEN;
logical = blkoff << blkbits;
phys = blkphy << blkbits;
size = n << blkbits;
}
} else {
/* Start a new extent */
- flags = FIEMAP_EXTENT_MERGED;
+ flags = FIEMAP_EXTENT_MERGED |
+ FIEMAP_EXTENT_HAS_PHYS_LEN;
logical = blkoff << blkbits;
phys = blkphy << blkbits;
size = n << blkbits;
--
2.43.0
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v3 07/13] ext4: fiemap: return correct extent physical length
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
` (5 preceding siblings ...)
2024-04-03 7:22 ` [PATCH v3 06/13] nilfs2: fiemap: return correct extent physical length Sweet Tea Dorminy
@ 2024-04-03 7:22 ` Sweet Tea Dorminy
2024-04-03 11:22 ` Jan Kara
2024-04-03 7:22 ` [PATCH v3 08/13] f2fs: fiemap: add physical length to trace_f2fs_fiemap Sweet Tea Dorminy
` (7 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Sweet Tea Dorminy @ 2024-04-03 7:22 UTC (permalink / raw
To: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Sweet Tea Dorminy, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v3 08/13] f2fs: fiemap: add physical length to trace_f2fs_fiemap
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
` (6 preceding siblings ...)
2024-04-03 7:22 ` [PATCH v3 07/13] ext4: " Sweet Tea Dorminy
@ 2024-04-03 7:22 ` Sweet Tea Dorminy
2024-04-05 19:28 ` Andreas Dilger
2024-04-03 7:22 ` [PATCH v3 09/13] f2fs: fiemap: return correct extent physical length Sweet Tea Dorminy
` (6 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Sweet Tea Dorminy @ 2024-04-03 7:22 UTC (permalink / raw
To: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Sweet Tea Dorminy, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/f2fs/data.c | 6 +++---
fs/f2fs/inline.c | 2 +-
include/trace/events/f2fs.h | 10 +++++++---
3 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 87f8d828e038..34af1673461b 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1836,7 +1836,7 @@ static int f2fs_xattr_fiemap(struct inode *inode,
err = fiemap_fill_next_extent(
fieinfo, 0, phys, len, 0, flags);
- trace_f2fs_fiemap(inode, 0, phys, len, flags, err);
+ trace_f2fs_fiemap(inode, 0, phys, len, 0, flags, err);
if (err)
return err;
}
@@ -1863,7 +1863,7 @@ static int f2fs_xattr_fiemap(struct inode *inode,
if (phys) {
err = fiemap_fill_next_extent(
fieinfo, 0, phys, len, 0, flags);
- trace_f2fs_fiemap(inode, 0, phys, len, flags, err);
+ trace_f2fs_fiemap(inode, 0, phys, len, 0, flags, err);
}
return (err < 0 ? err : 0);
@@ -1982,7 +1982,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
ret = fiemap_fill_next_extent(fieinfo, logical,
phys, size, 0, flags);
- trace_f2fs_fiemap(inode, logical, phys, size, flags, ret);
+ trace_f2fs_fiemap(inode, logical, phys, size, 0, flags, ret);
if (ret)
goto out;
size = 0;
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 49d2f87fe048..235b0d72f6fc 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -808,7 +808,7 @@ int f2fs_inline_data_fiemap(struct inode *inode,
(char *)F2FS_INODE(ipage);
err = fiemap_fill_next_extent(
fieinfo, start, byteaddr, ilen, 0, flags);
- trace_f2fs_fiemap(inode, start, byteaddr, ilen, flags, err);
+ trace_f2fs_fiemap(inode, start, byteaddr, ilen, 0, flags, err);
out:
f2fs_put_page(ipage, 1);
return err;
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index 7ed0fc430dc6..63706eb3a5db 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -2276,9 +2276,10 @@ TRACE_EVENT(f2fs_bmap,
TRACE_EVENT(f2fs_fiemap,
TP_PROTO(struct inode *inode, sector_t lblock, sector_t pblock,
- unsigned long long len, unsigned int flags, int ret),
+ unsigned long long len, unsigned long long phys_len,
+ unsigned int flags, int ret),
- TP_ARGS(inode, lblock, pblock, len, flags, ret),
+ TP_ARGS(inode, lblock, pblock, len, phys_len, flags, ret),
TP_STRUCT__entry(
__field(dev_t, dev)
@@ -2286,6 +2287,7 @@ TRACE_EVENT(f2fs_fiemap,
__field(sector_t, lblock)
__field(sector_t, pblock)
__field(unsigned long long, len)
+ __field(unsigned long long, phys_len)
__field(unsigned int, flags)
__field(int, ret)
),
@@ -2296,16 +2298,18 @@ TRACE_EVENT(f2fs_fiemap,
__entry->lblock = lblock;
__entry->pblock = pblock;
__entry->len = len;
+ __entry->phys_len = phys_len;
__entry->flags = flags;
__entry->ret = ret;
),
TP_printk("dev = (%d,%d), ino = %lu, lblock:%lld, pblock:%lld, "
- "len:%llu, flags:%u, ret:%d",
+ "len:%llu, plen:%llu, flags:%u, ret:%d",
show_dev_ino(__entry),
(unsigned long long)__entry->lblock,
(unsigned long long)__entry->pblock,
__entry->len,
+ __entry->phys_len,
__entry->flags,
__entry->ret)
);
--
2.43.0
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v3 09/13] f2fs: fiemap: return correct extent physical length
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
` (7 preceding siblings ...)
2024-04-03 7:22 ` [PATCH v3 08/13] f2fs: fiemap: add physical length to trace_f2fs_fiemap Sweet Tea Dorminy
@ 2024-04-03 7:22 ` Sweet Tea Dorminy
2024-04-03 7:22 ` [PATCH v3 10/13] ocfs2: " Sweet Tea Dorminy
` (5 subsequent siblings)
14 siblings, 0 replies; 38+ messages in thread
From: Sweet Tea Dorminy @ 2024-04-03 7:22 UTC (permalink / raw
To: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Sweet Tea Dorminy, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
fs/f2fs/data.c | 25 ++++++++++++++++---------
fs/f2fs/inline.c | 2 +-
2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 34af1673461b..2a3625c10125 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1829,7 +1829,9 @@ static int f2fs_xattr_fiemap(struct inode *inode,
f2fs_put_page(page, 1);
- flags = FIEMAP_EXTENT_DATA_INLINE | FIEMAP_EXTENT_NOT_ALIGNED;
+ flags = FIEMAP_EXTENT_DATA_INLINE |
+ FIEMAP_EXTENT_NOT_ALIGNED |
+ FIEMAP_EXTENT_HAS_PHYS_LEN;
if (!xnid)
flags |= FIEMAP_EXTENT_LAST;
@@ -1857,7 +1859,7 @@ static int f2fs_xattr_fiemap(struct inode *inode,
f2fs_put_page(page, 1);
- flags = FIEMAP_EXTENT_LAST;
+ flags = FIEMAP_EXTENT_LAST | FIEMAP_EXTENT_HAS_PHYS_LEN;
}
if (phys) {
@@ -1894,8 +1896,8 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
struct f2fs_map_blocks map;
sector_t start_blk, last_blk;
pgoff_t next_pgofs;
- u64 logical = 0, phys = 0, size = 0;
- u32 flags = 0;
+ u64 logical = 0, phys = 0, size = 0, phys_size = 0;
+ u32 flags = FIEMAP_EXTENT_HAS_PHYS_LEN;
int ret = 0;
bool compr_cluster = false, compr_appended;
unsigned int cluster_size = F2FS_I(inode)->i_cluster_size;
@@ -1981,11 +1983,12 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
flags |= FIEMAP_EXTENT_DATA_ENCRYPTED;
ret = fiemap_fill_next_extent(fieinfo, logical,
- phys, size, 0, flags);
- trace_f2fs_fiemap(inode, logical, phys, size, 0, flags, ret);
+ phys, size, phys_size, flags);
+ trace_f2fs_fiemap(inode, logical, phys, size, phys_size, flags, ret);
if (ret)
goto out;
size = 0;
+ phys_size = 0;
}
if (start_blk > last_blk)
@@ -2006,17 +2009,21 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
phys = __is_valid_data_blkaddr(map.m_pblk) ?
blks_to_bytes(inode, map.m_pblk) : 0;
size = blks_to_bytes(inode, map.m_len);
- flags = 0;
+ phys_size = blks_to_bytes(inode, map.m_len);
+ flags = FIEMAP_EXTENT_HAS_PHYS_LEN;
if (compr_cluster) {
- flags = FIEMAP_EXTENT_ENCODED;
+ flags = FIEMAP_EXTENT_ENCODED |
+ FIEMAP_EXTENT_HAS_PHYS_LEN;
count_in_cluster += map.m_len;
if (count_in_cluster == cluster_size) {
compr_cluster = false;
size += blks_to_bytes(inode, 1);
+ phys_size += blks_to_bytes(inode, 1);
}
} else if (map.m_flags & F2FS_MAP_DELALLOC) {
- flags = FIEMAP_EXTENT_UNWRITTEN;
+ flags = FIEMAP_EXTENT_UNWRITTEN |
+ FIEMAP_EXTENT_HAS_PHYS_LEN;
}
start_blk += bytes_to_blks(inode, size);
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 235b0d72f6fc..c1c804a754de 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -772,7 +772,7 @@ int f2fs_inline_data_fiemap(struct inode *inode,
{
__u64 byteaddr, ilen;
__u32 flags = FIEMAP_EXTENT_DATA_INLINE | FIEMAP_EXTENT_NOT_ALIGNED |
- FIEMAP_EXTENT_LAST;
+ FIEMAP_EXTENT_HAS_PHYS_LEN | FIEMAP_EXTENT_LAST;
struct node_info ni;
struct page *ipage;
int err = 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v3 10/13] ocfs2: fiemap: return correct extent physical length
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
` (8 preceding siblings ...)
2024-04-03 7:22 ` [PATCH v3 09/13] f2fs: fiemap: return correct extent physical length Sweet Tea Dorminy
@ 2024-04-03 7:22 ` Sweet Tea Dorminy
2024-04-03 11:25 ` Jan Kara
2024-04-03 7:22 ` [PATCH v3 11/13] bcachefs: " Sweet Tea Dorminy
` (4 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Sweet Tea Dorminy @ 2024-04-03 7:22 UTC (permalink / raw
To: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Sweet Tea Dorminy, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v3 11/13] bcachefs: fiemap: return correct extent physical length
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
` (9 preceding siblings ...)
2024-04-03 7:22 ` [PATCH v3 10/13] ocfs2: " Sweet Tea Dorminy
@ 2024-04-03 7:22 ` Sweet Tea Dorminy
2024-04-03 17:00 ` Brian Foster
2024-04-03 7:22 ` [PATCH v3 12/13] f2fs: fiemap: emit new COMPRESSED state Sweet Tea Dorminy
` (3 subsequent siblings)
14 siblings, 1 reply; 38+ messages in thread
From: Sweet Tea Dorminy @ 2024-04-03 7:22 UTC (permalink / raw
To: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Sweet Tea Dorminy, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v3 12/13] f2fs: fiemap: emit new COMPRESSED state
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
` (10 preceding siblings ...)
2024-04-03 7:22 ` [PATCH v3 11/13] bcachefs: " Sweet Tea Dorminy
@ 2024-04-03 7:22 ` Sweet Tea Dorminy
2024-04-03 7:22 ` [PATCH v3 13/13] bcachefs: " Sweet Tea Dorminy
` (2 subsequent siblings)
14 siblings, 0 replies; 38+ messages in thread
From: Sweet Tea Dorminy @ 2024-04-03 7:22 UTC (permalink / raw
To: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Sweet Tea Dorminy, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [PATCH v3 13/13] bcachefs: fiemap: emit new COMPRESSED state
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
` (11 preceding siblings ...)
2024-04-03 7:22 ` [PATCH v3 12/13] f2fs: fiemap: emit new COMPRESSED state Sweet Tea Dorminy
@ 2024-04-03 7:22 ` Sweet Tea Dorminy
2024-04-05 19:17 ` Andreas Dilger
2024-04-03 8:29 ` [PATCH v3 00/13] fiemap extension for more physical information Gao Xiang
2024-04-03 18:17 ` Kent Overstreet
14 siblings, 1 reply; 38+ messages in thread
From: Sweet Tea Dorminy @ 2024-04-03 7:22 UTC (permalink / raw
To: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün,
Sweet Tea Dorminy, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
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
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [PATCH v3 00/13] fiemap extension for more physical information
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
` (12 preceding siblings ...)
2024-04-03 7:22 ` [PATCH v3 13/13] bcachefs: " Sweet Tea Dorminy
@ 2024-04-03 8:29 ` Gao Xiang
2024-04-03 15:11 ` Sweet Tea Dorminy
2024-04-03 18:17 ` Kent Overstreet
14 siblings, 1 reply; 38+ messages in thread
From: Gao Xiang @ 2024-04-03 8:29 UTC (permalink / raw
To: Sweet Tea Dorminy, Jonathan Corbet, Kent Overstreet, Brian Foster,
Chris Mason, Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu,
Alexander Viro, Christian Brauner, Jan Kara,
Mickaël Salaün, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
Hi,
On 2024/4/3 15:22, Sweet Tea Dorminy wrote:
> 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.
I'm not sure why here iomap was excluded technically or I'm missing some
previous comments?
>
> 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.
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.
Thanks,
Gao Xiang
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 07/13] ext4: fiemap: return correct extent physical length
2024-04-03 7:22 ` [PATCH v3 07/13] ext4: " Sweet Tea Dorminy
@ 2024-04-03 11:22 ` Jan Kara
0 siblings, 0 replies; 38+ messages in thread
From: Jan Kara @ 2024-04-03 11:22 UTC (permalink / raw
To: Sweet Tea Dorminy
Cc: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
linux-kernel, linux-bcachefs, linux-btrfs, linux-f2fs-devel,
linux-fsdevel, kernel-team
On Wed 03-04-24 03:22:48, Sweet Tea Dorminy wrote:
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
> 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;
OK, but we should then pass (__u64)es.es_len << blksize_bits as physical
extent length below, shouldn't we?
> if (!ext4_es_lookup_extent(inode, block, &next, &es))
> break;
> if (ext4_es_is_unwritten(&es))
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 10/13] ocfs2: fiemap: return correct extent physical length
2024-04-03 7:22 ` [PATCH v3 10/13] ocfs2: " Sweet Tea Dorminy
@ 2024-04-03 11:25 ` Jan Kara
0 siblings, 0 replies; 38+ messages in thread
From: Jan Kara @ 2024-04-03 11:25 UTC (permalink / raw
To: Sweet Tea Dorminy
Cc: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
linux-kernel, linux-bcachefs, linux-btrfs, linux-f2fs-devel,
linux-fsdevel, kernel-team
On Wed 03-04-24 03:22:51, Sweet Tea Dorminy wrote:
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
> 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)
Again, we should be passing non-zero phys_len if we set
FIEMAP_EXTENT_HAS_PHYS_LEN flag AFAIU.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 00/13] fiemap extension for more physical information
2024-04-03 8:29 ` [PATCH v3 00/13] fiemap extension for more physical information Gao Xiang
@ 2024-04-03 15:11 ` Sweet Tea Dorminy
2024-04-04 0:43 ` Gao Xiang
0 siblings, 1 reply; 38+ messages in thread
From: Sweet Tea Dorminy @ 2024-04-03 15:11 UTC (permalink / raw
To: Gao Xiang, Jonathan Corbet, Kent Overstreet, Brian Foster,
Chris Mason, Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu,
Alexander Viro, Christian Brauner, Jan Kara,
Mickaël Salaün, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
>
> 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!
Sweet Tea
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 01/13] fs: fiemap: add physical_length field to extents
2024-04-03 7:22 ` [PATCH v3 01/13] fs: fiemap: add physical_length field to extents Sweet Tea Dorminy
@ 2024-04-03 16:57 ` Brian Foster
2024-04-05 18:47 ` [PATCH v3 01/13] " Andreas Dilger
2024-04-09 16:22 ` [PATCH v3 01/13] fs: " Darrick J. Wong
2 siblings, 0 replies; 38+ messages in thread
From: Brian Foster @ 2024-04-03 16:57 UTC (permalink / raw
To: Sweet Tea Dorminy
Cc: Jonathan Corbet, Kent Overstreet, Chris Mason, Josef Bacik,
David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
linux-kernel, linux-bcachefs, linux-btrfs, linux-f2fs-devel,
linux-fsdevel, kernel-team
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 <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/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;
Nit: Why start this field out as len if the next patch adds the param
and defaults to zero? Not that it matters that much due to the next
patch (which seems logical), but wouldn't it make more sense to set this
to 0 from the start?
Brian
> 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
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 02/13] fs: fiemap: update fiemap_fill_next_extent() signature
2024-04-03 7:22 ` [PATCH v3 02/13] fs: fiemap: update fiemap_fill_next_extent() signature Sweet Tea Dorminy
@ 2024-04-03 16:58 ` Brian Foster
2024-04-05 19:05 ` [PATCH v3 02/13] " Andreas Dilger
1 sibling, 0 replies; 38+ messages in thread
From: Brian Foster @ 2024-04-03 16:58 UTC (permalink / raw
To: Sweet Tea Dorminy
Cc: Jonathan Corbet, Kent Overstreet, Chris Mason, Josef Bacik,
David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
linux-kernel, linux-bcachefs, linux-btrfs, linux-f2fs-devel,
linux-fsdevel, kernel-team
On Wed, Apr 03, 2024 at 03:22:43AM -0400, Sweet Tea Dorminy wrote:
> Update the signature of fiemap_fill_next_extent() to allow passing a
> physical length. Update all callers to pass a 0 physical length -- since
> none set the EXTENT_HAS_PHYS_LEN flag, this value doesn't matter.
>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
> Documentation/filesystems/fiemap.rst | 3 ++-
> fs/bcachefs/fs.c | 7 ++++---
> fs/btrfs/extent_io.c | 4 ++--
> fs/ext4/extents.c | 1 +
> fs/f2fs/data.c | 8 +++++---
> fs/f2fs/inline.c | 3 ++-
> fs/ioctl.c | 9 +++++----
> fs/iomap/fiemap.c | 2 +-
> fs/nilfs2/inode.c | 6 +++---
> fs/ntfs3/frecord.c | 7 ++++---
> fs/ocfs2/extent_map.c | 4 ++--
> fs/smb/client/smb2ops.c | 1 +
> include/linux/fiemap.h | 2 +-
> 13 files changed, 33 insertions(+), 24 deletions(-)
>
...
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 8afd32e1a27a..1830baca532b 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -99,7 +99,8 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
> * @fieinfo: Fiemap context passed into ->fiemap
> * @logical: Extent logical start offset, in bytes
> * @phys: Extent physical start offset, in bytes
> - * @len: Extent length, in bytes
> + * @log_len: Extent logical length, in bytes
> + * @phys_len: Extent physical length, in bytes (optional)
> * @flags: FIEMAP_EXTENT flags that describe this extent
> *
> * Called from file system ->fiemap callback. Will populate extent
> @@ -110,7 +111,7 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
> * extent that will fit in user array.
> */
> int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical,
> - u64 phys, u64 len, u32 flags)
> + u64 phys, u64 log_len, u64 phys_len, u32 flags)
> {
> struct fiemap_extent extent;
> struct fiemap_extent __user *dest = fieinfo->fi_extents_start;
> @@ -138,8 +139,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_logical_length = len;
> - extent.fe_physical_length = len;
> + extent.fe_logical_length = log_len;
> + extent.fe_physical_length = phys_len;
Another nit, but would it simplify things to let this helper set the
_HAS_PHYS_LEN flag on phys_len != 0 or something rather than require
callers to get it right?
Brian
> extent.fe_flags = flags;
>
> dest += fieinfo->fi_extents_mapped;
> diff --git a/fs/iomap/fiemap.c b/fs/iomap/fiemap.c
> index 610ca6f1ec9b..013e843c8d10 100644
> --- a/fs/iomap/fiemap.c
> +++ b/fs/iomap/fiemap.c
> @@ -36,7 +36,7 @@ static int iomap_to_fiemap(struct fiemap_extent_info *fi,
>
> return fiemap_fill_next_extent(fi, iomap->offset,
> iomap->addr != IOMAP_NULL_ADDR ? iomap->addr : 0,
> - iomap->length, flags);
> + iomap->length, 0, flags);
> }
>
> static loff_t iomap_fiemap_iter(const struct iomap_iter *iter,
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index 7340a01d80e1..4d3c347c982b 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -1190,7 +1190,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> if (size) {
> /* End of the current extent */
> ret = fiemap_fill_next_extent(
> - fieinfo, logical, phys, size, flags);
> + fieinfo, logical, phys, size, 0, flags);
> if (ret)
> break;
> }
> @@ -1240,7 +1240,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> flags |= FIEMAP_EXTENT_LAST;
>
> ret = fiemap_fill_next_extent(
> - fieinfo, logical, phys, size, flags);
> + fieinfo, logical, phys, size, 0, flags);
> if (ret)
> break;
> size = 0;
> @@ -1256,7 +1256,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> /* Terminate the current extent */
> ret = fiemap_fill_next_extent(
> fieinfo, logical, phys, size,
> - flags);
> + 0, flags);
> if (ret || blkoff > end_blkoff)
> break;
>
> diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
> index 7f27382e0ce2..ef0ed913428b 100644
> --- a/fs/ntfs3/frecord.c
> +++ b/fs/ntfs3/frecord.c
> @@ -1947,7 +1947,7 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,
> if (!attr || !attr->non_res) {
> err = fiemap_fill_next_extent(
> fieinfo, 0, 0,
> - attr ? le32_to_cpu(attr->res.data_size) : 0,
> + attr ? le32_to_cpu(attr->res.data_size) : 0, 0,
> FIEMAP_EXTENT_DATA_INLINE | FIEMAP_EXTENT_LAST |
> FIEMAP_EXTENT_MERGED);
> goto out;
> @@ -2042,7 +2042,7 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,
> flags |= FIEMAP_EXTENT_LAST;
>
> err = fiemap_fill_next_extent(fieinfo, vbo, lbo, dlen,
> - flags);
> + 0, flags);
> if (err < 0)
> break;
> if (err == 1) {
> @@ -2062,7 +2062,8 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,
> if (vbo + bytes >= end)
> flags |= FIEMAP_EXTENT_LAST;
>
> - err = fiemap_fill_next_extent(fieinfo, vbo, lbo, bytes, flags);
> + err = fiemap_fill_next_extent(fieinfo, vbo, lbo, bytes, 0,
> + flags);
> if (err < 0)
> break;
> if (err == 1) {
> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
> index 70a768b623cf..eabdf97cd685 100644
> --- a/fs/ocfs2/extent_map.c
> +++ b/fs/ocfs2/extent_map.c
> @@ -723,7 +723,7 @@ static int ocfs2_fiemap_inline(struct inode *inode, struct buffer_head *di_bh,
> id2.i_data.id_data);
>
> ret = fiemap_fill_next_extent(fieinfo, 0, phys, id_count,
> - flags);
> + 0, flags);
> if (ret < 0)
> return ret;
> }
> @@ -794,7 +794,7 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> virt_bytes = (u64)le32_to_cpu(rec.e_cpos) << osb->s_clustersize_bits;
>
> ret = fiemap_fill_next_extent(fieinfo, virt_bytes, phys_bytes,
> - len_bytes, fe_flags);
> + len_bytes, 0, fe_flags);
> if (ret)
> break;
>
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index 87b63f6ad2e2..23a193512f96 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -3779,6 +3779,7 @@ static int smb3_fiemap(struct cifs_tcon *tcon,
> le64_to_cpu(out_data[i].file_offset),
> le64_to_cpu(out_data[i].file_offset),
> le64_to_cpu(out_data[i].length),
> + 0,
> flags);
> if (rc < 0)
> goto out;
> diff --git a/include/linux/fiemap.h b/include/linux/fiemap.h
> index c50882f19235..17a6c32cdf3f 100644
> --- a/include/linux/fiemap.h
> +++ b/include/linux/fiemap.h
> @@ -16,6 +16,6 @@ struct fiemap_extent_info {
> int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo,
> u64 start, u64 *len, u32 supported_flags);
> int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
> - u64 phys, u64 len, u32 flags);
> + u64 phys, u64 log_len, u64 phys_len, u32 flags);
>
> #endif /* _LINUX_FIEMAP_H 1 */
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 11/13] bcachefs: fiemap: return correct extent physical length
2024-04-03 7:22 ` [PATCH v3 11/13] bcachefs: " Sweet Tea Dorminy
@ 2024-04-03 17:00 ` Brian Foster
2024-04-03 18:15 ` Kent Overstreet
0 siblings, 1 reply; 38+ messages in thread
From: Brian Foster @ 2024-04-03 17:00 UTC (permalink / raw
To: Sweet Tea Dorminy
Cc: Jonathan Corbet, Kent Overstreet, Chris Mason, Josef Bacik,
David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
linux-kernel, linux-bcachefs, linux-btrfs, linux-f2fs-devel,
linux-fsdevel, kernel-team
On Wed, Apr 03, 2024 at 03:22:52AM -0400, Sweet Tea Dorminy wrote:
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
> 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),
Question for Kent perhaps, but what's the functional difference between
bkey_inline_data_bytes() and k->size in this particular case?
FWIW that and the other couple nitty questions aside, this all LGTM.
Thanks.
Brian
> 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
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 11/13] bcachefs: fiemap: return correct extent physical length
2024-04-03 17:00 ` Brian Foster
@ 2024-04-03 18:15 ` Kent Overstreet
0 siblings, 0 replies; 38+ messages in thread
From: Kent Overstreet @ 2024-04-03 18:15 UTC (permalink / raw
To: Brian Foster
Cc: Sweet Tea Dorminy, Jonathan Corbet, Chris Mason, Josef Bacik,
David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
linux-kernel, linux-bcachefs, linux-btrfs, linux-f2fs-devel,
linux-fsdevel, kernel-team
On Wed, Apr 03, 2024 at 01:00:11PM -0400, Brian Foster wrote:
> On Wed, Apr 03, 2024 at 03:22:52AM -0400, Sweet Tea Dorminy wrote:
> > Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> > ---
> > 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),
>
> Question for Kent perhaps, but what's the functional difference between
> bkey_inline_data_bytes() and k->size in this particular case?
Not much - k->size will correspond to the size of the original write -
that is, the writeback write from the pagecache. inline_data_bytes is
the amount of data that wasn't zeroes.
So inline_data_bytes is probably the right thing to use here.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 00/13] fiemap extension for more physical information
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
` (13 preceding siblings ...)
2024-04-03 8:29 ` [PATCH v3 00/13] fiemap extension for more physical information Gao Xiang
@ 2024-04-03 18:17 ` Kent Overstreet
2024-04-03 18:20 ` Darrick J. Wong
2024-04-05 18:20 ` Andreas Dilger
14 siblings, 2 replies; 38+ messages in thread
From: Kent Overstreet @ 2024-04-03 18:17 UTC (permalink / raw
To: Sweet Tea Dorminy
Cc: Jonathan Corbet, Brian Foster, Chris Mason, Josef Bacik,
David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
linux-kernel, linux-bcachefs, linux-btrfs, linux-f2fs-devel,
linux-fsdevel, kernel-team, djwong
On Wed, Apr 03, 2024 at 03:22:41AM -0400, Sweet Tea Dorminy wrote:
> 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.
I believe there was some talk of using the padding for a device ID, so
that fiemap could properly support multi device filesystems. Are we sure
this is the best use of those bytes?
>
> 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/cover.1711588701.git.sweettea-kernel@dorminy.me/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
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 00/13] fiemap extension for more physical information
2024-04-03 18:17 ` Kent Overstreet
@ 2024-04-03 18:20 ` Darrick J. Wong
2024-04-05 18:20 ` Andreas Dilger
1 sibling, 0 replies; 38+ messages in thread
From: Darrick J. Wong @ 2024-04-03 18:20 UTC (permalink / raw
To: Kent Overstreet
Cc: Sweet Tea Dorminy, Jonathan Corbet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
linux-kernel, linux-bcachefs, linux-btrfs, linux-f2fs-devel,
linux-fsdevel, kernel-team
On Wed, Apr 03, 2024 at 02:17:26PM -0400, Kent Overstreet wrote:
> On Wed, Apr 03, 2024 at 03:22:41AM -0400, Sweet Tea Dorminy wrote:
> > 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.
>
> I believe there was some talk of using the padding for a device ID, so
> that fiemap could properly support multi device filesystems. Are we sure
> this is the best use of those bytes?
We still have 5x u32 of empty space in struct fiemap after this series,
so I don't think adding the physical length is going to prohibit future
expansion.
--D
> >
> > 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/cover.1711588701.git.sweettea-kernel@dorminy.me/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
> >
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 00/13] fiemap extension for more physical information
2024-04-03 15:11 ` Sweet Tea Dorminy
@ 2024-04-04 0:43 ` Gao Xiang
0 siblings, 0 replies; 38+ messages in thread
From: Gao Xiang @ 2024-04-04 0:43 UTC (permalink / raw
To: Sweet Tea Dorminy, Jonathan Corbet, Kent Overstreet, Brian Foster,
Chris Mason, Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu,
Alexander Viro, Christian Brauner, Jan Kara,
Mickaël Salaün, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 00/13] fiemap extension for more physical information
2024-04-03 18:17 ` Kent Overstreet
2024-04-03 18:20 ` Darrick J. Wong
@ 2024-04-05 18:20 ` Andreas Dilger
1 sibling, 0 replies; 38+ messages in thread
From: Andreas Dilger @ 2024-04-05 18:20 UTC (permalink / raw
To: Kent Overstreet
Cc: Sweet Tea Dorminy, Jonathan Corbet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
Linux Kernel Mailing List, linux-bcachefs, linux-btrfs,
linux-f2fs-devel, linux-fsdevel, kernel-team, djwong
[-- Attachment #1: Type: text/plain, Size: 2210 bytes --]
On Apr 3, 2024, at 12:17 PM, Kent Overstreet <kent.overstreet@linux.dev> wrote:
>
> On Wed, Apr 03, 2024 at 03:22:41AM -0400, Sweet Tea Dorminy wrote:
>> 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.
>
> I believe there was some talk of using the padding for a device ID, so
> that fiemap could properly support multi device filesystems. Are we sure
> this is the best use of those bytes?
The current (pre-patch) fiemap_extent struct is:
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 */
__u32 fe_reserved[3];
};
and this series is only changing fe_reserved64[0] to fe_phys_length.
There was discussion in the past of using "fe_reserved[0]" for the device
ID, which is still OK.
Cheers, Andreas
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 01/13] fiemap: add physical_length field to extents
2024-04-03 7:22 ` [PATCH v3 01/13] fs: fiemap: add physical_length field to extents Sweet Tea Dorminy
2024-04-03 16:57 ` Brian Foster
@ 2024-04-05 18:47 ` Andreas Dilger
2024-04-09 16:22 ` [PATCH v3 01/13] fs: " Darrick J. Wong
2 siblings, 0 replies; 38+ messages in thread
From: Andreas Dilger @ 2024-04-05 18:47 UTC (permalink / raw
To: Sweet Tea Dorminy
Cc: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
Linux Kernel Mailing List, linux-bcachefs, linux-btrfs,
linux-f2fs-devel, linux-fsdevel, kernel-team
[-- Attachment #1: Type: text/plain, Size: 6034 bytes --]
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
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 02/13] fiemap: update fiemap_fill_next_extent() signature
2024-04-03 7:22 ` [PATCH v3 02/13] fs: fiemap: update fiemap_fill_next_extent() signature Sweet Tea Dorminy
2024-04-03 16:58 ` Brian Foster
@ 2024-04-05 19:05 ` Andreas Dilger
2024-04-05 19:06 ` Kent Overstreet
1 sibling, 1 reply; 38+ messages in thread
From: Andreas Dilger @ 2024-04-05 19:05 UTC (permalink / raw
To: Sweet Tea Dorminy
Cc: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
Linux Kernel Mailing List, linux-bcachefs, linux-btrfs,
linux-f2fs-devel, linux-fsdevel, kernel-team
[-- Attachment #1: Type: text/plain, Size: 13666 bytes --]
On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <sweettea-kernel@dorminy.me> wrote:
>
> Update the signature of fiemap_fill_next_extent() to allow passing a
> physical length. Update all callers to pass a 0 physical length -- since
> none set the EXTENT_HAS_PHYS_LEN flag, this value doesn't matter.
Patch-structure-wise, it doesn't make sense to me to change all of the callers
to pass "0" as the argument to this function, and then submit a whole series
of per-filesystem patches that sets only FIEMAP_EXTENT_HAS_PHYS_LEN (but also
passes phys_len = 0, which is wrong AFAICS).
A cleaner approach would be to rename the existing fiemap_fill_next_extent()
to fiemap_fill_next_extent_phys() that takes phys_len as an argument, and then
add a simple wrapper until all of the filesystems are updated:
#define fiemap_fill_next_extent(info, logical, phys, log_len, flags, dev) \
fiemap_fill_next_extent_phys(info, logical, phys, log_len, 0, flags, dev)
Then the per-filesystem patches would involve changing over the callers to
use fiemap_fill_next_extent_phys() and setting FIEMAP_EXTENT_HAS_PHYS_LEN.
It would be possible in fiemap_fill_next_extent_phys() to add something like
the following in this patch that is adding the phys_len argument:
if (phys_len == 0 && !(flags & FIEMAP_EXTENT_HAS_PHYS_LEN)) {
phys_len = log_len;
flags |= FIEMAP_EXTENT_HAS_PHYS_LEN;
}
which would immediately enable the fe_phyisical_length argument for all of
the callers. This would reduce the chance of errors by the filesystems, or
avoid the need to change the filesystems at all unless they want to set
phys_len and log_len differently for compression.
Cheers, Andreas
>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
> Documentation/filesystems/fiemap.rst | 3 ++-
> fs/bcachefs/fs.c | 7 ++++---
> fs/btrfs/extent_io.c | 4 ++--
> fs/ext4/extents.c | 1 +
> fs/f2fs/data.c | 8 +++++---
> fs/f2fs/inline.c | 3 ++-
> fs/ioctl.c | 9 +++++----
> fs/iomap/fiemap.c | 2 +-
> fs/nilfs2/inode.c | 6 +++---
> fs/ntfs3/frecord.c | 7 ++++---
> fs/ocfs2/extent_map.c | 4 ++--
> fs/smb/client/smb2ops.c | 1 +
> include/linux/fiemap.h | 2 +-
> 13 files changed, 33 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/filesystems/fiemap.rst b/Documentation/filesystems/fiemap.rst
> index c2bfa107c8d7..c060bb83f5d8 100644
> --- a/Documentation/filesystems/fiemap.rst
> +++ b/Documentation/filesystems/fiemap.rst
> @@ -236,7 +236,8 @@ For each extent in the request range, the file system should call
> the helper function, fiemap_fill_next_extent()::
>
> int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
> - u64 phys, u64 len, u32 flags, u32 dev);
> + u64 phys, u64 log_len, u64 phys_len, u32 flags,
> + u32 dev);
>
> fiemap_fill_next_extent() will use the passed values to populate the
> next free extent in the fm_extents array. 'General' extent flags will
> diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
> index 71013256fc39..f830578a9cd1 100644
> --- a/fs/bcachefs/fs.c
> +++ b/fs/bcachefs/fs.c
> @@ -931,7 +931,8 @@ 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, flags|flags2);
> + k.k->size << 9, 0,
> + flags|flags2);
> if (ret)
> return ret;
> }
> @@ -940,13 +941,13 @@ 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, k.k->size << 9, 0,
> flags|
> 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, k.k->size << 9, 0,
> flags|
> FIEMAP_EXTENT_DELALLOC|
> FIEMAP_EXTENT_UNWRITTEN);
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index eceef5ff780b..9e421d99fd5c 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2577,7 +2577,7 @@ static int flush_fiemap_cache(struct fiemap_extent_info *fieinfo,
> int ret;
>
> ret = fiemap_fill_next_extent(fieinfo, entry->offset,
> - entry->phys, entry->len,
> + entry->phys, entry->len, 0,
> entry->flags);
> /*
> * Ignore 1 (reached max entries) because we keep track of that
> @@ -2793,7 +2793,7 @@ static int emit_last_fiemap_cache(struct fiemap_extent_info *fieinfo,
> return 0;
>
> ret = fiemap_fill_next_extent(fieinfo, cache->offset, cache->phys,
> - cache->len, cache->flags);
> + cache->len, 0, cache->flags);
> cache->cached = false;
> if (ret > 0)
> ret = 0;
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index e57054bdc5fd..2adade3c202a 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -2215,6 +2215,7 @@ static int ext4_fill_es_cache_info(struct inode *inode,
> (__u64)es.es_lblk << blksize_bits,
> (__u64)es.es_pblk << blksize_bits,
> (__u64)es.es_len << blksize_bits,
> + 0,
> flags);
> if (next == 0)
> break;
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index d9494b5fc7c1..87f8d828e038 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1834,7 +1834,8 @@ static int f2fs_xattr_fiemap(struct inode *inode,
> if (!xnid)
> flags |= FIEMAP_EXTENT_LAST;
>
> - err = fiemap_fill_next_extent(fieinfo, 0, phys, len, flags);
> + err = fiemap_fill_next_extent(
> + fieinfo, 0, phys, len, 0, flags);
> trace_f2fs_fiemap(inode, 0, phys, len, flags, err);
> if (err)
> return err;
> @@ -1860,7 +1861,8 @@ static int f2fs_xattr_fiemap(struct inode *inode,
> }
>
> if (phys) {
> - err = fiemap_fill_next_extent(fieinfo, 0, phys, len, flags);
> + err = fiemap_fill_next_extent(
> + fieinfo, 0, phys, len, 0, flags);
> trace_f2fs_fiemap(inode, 0, phys, len, flags, err);
> }
>
> @@ -1979,7 +1981,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> flags |= FIEMAP_EXTENT_DATA_ENCRYPTED;
>
> ret = fiemap_fill_next_extent(fieinfo, logical,
> - phys, size, flags);
> + phys, size, 0, flags);
> trace_f2fs_fiemap(inode, logical, phys, size, flags, ret);
> if (ret)
> goto out;
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index ac00423f117b..49d2f87fe048 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -806,7 +806,8 @@ int f2fs_inline_data_fiemap(struct inode *inode,
> byteaddr = (__u64)ni.blk_addr << inode->i_sb->s_blocksize_bits;
> byteaddr += (char *)inline_data_addr(inode, ipage) -
> (char *)F2FS_INODE(ipage);
> - err = fiemap_fill_next_extent(fieinfo, start, byteaddr, ilen, flags);
> + err = fiemap_fill_next_extent(
> + fieinfo, start, byteaddr, ilen, 0, flags);
> trace_f2fs_fiemap(inode, start, byteaddr, ilen, flags, err);
> out:
> f2fs_put_page(ipage, 1);
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 8afd32e1a27a..1830baca532b 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -99,7 +99,8 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
> * @fieinfo: Fiemap context passed into ->fiemap
> * @logical: Extent logical start offset, in bytes
> * @phys: Extent physical start offset, in bytes
> - * @len: Extent length, in bytes
> + * @log_len: Extent logical length, in bytes
> + * @phys_len: Extent physical length, in bytes (optional)
> * @flags: FIEMAP_EXTENT flags that describe this extent
> *
> * Called from file system ->fiemap callback. Will populate extent
> @@ -110,7 +111,7 @@ static int ioctl_fibmap(struct file *filp, int __user *p)
> * extent that will fit in user array.
> */
> int fiemap_fill_next_extent(struct fiemap_extent_info *fieinfo, u64 logical,
> - u64 phys, u64 len, u32 flags)
> + u64 phys, u64 log_len, u64 phys_len, u32 flags)
> {
> struct fiemap_extent extent;
> struct fiemap_extent __user *dest = fieinfo->fi_extents_start;
> @@ -138,8 +139,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_logical_length = len;
> - extent.fe_physical_length = len;
> + extent.fe_logical_length = log_len;
> + extent.fe_physical_length = phys_len;
> extent.fe_flags = flags;
>
> dest += fieinfo->fi_extents_mapped;
> diff --git a/fs/iomap/fiemap.c b/fs/iomap/fiemap.c
> index 610ca6f1ec9b..013e843c8d10 100644
> --- a/fs/iomap/fiemap.c
> +++ b/fs/iomap/fiemap.c
> @@ -36,7 +36,7 @@ static int iomap_to_fiemap(struct fiemap_extent_info *fi,
>
> return fiemap_fill_next_extent(fi, iomap->offset,
> iomap->addr != IOMAP_NULL_ADDR ? iomap->addr : 0,
> - iomap->length, flags);
> + iomap->length, 0, flags);
> }
>
> static loff_t iomap_fiemap_iter(const struct iomap_iter *iter,
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index 7340a01d80e1..4d3c347c982b 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -1190,7 +1190,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> if (size) {
> /* End of the current extent */
> ret = fiemap_fill_next_extent(
> - fieinfo, logical, phys, size, flags);
> + fieinfo, logical, phys, size, 0, flags);
> if (ret)
> break;
> }
> @@ -1240,7 +1240,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> flags |= FIEMAP_EXTENT_LAST;
>
> ret = fiemap_fill_next_extent(
> - fieinfo, logical, phys, size, flags);
> + fieinfo, logical, phys, size, 0, flags);
> if (ret)
> break;
> size = 0;
> @@ -1256,7 +1256,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> /* Terminate the current extent */
> ret = fiemap_fill_next_extent(
> fieinfo, logical, phys, size,
> - flags);
> + 0, flags);
> if (ret || blkoff > end_blkoff)
> break;
>
> diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
> index 7f27382e0ce2..ef0ed913428b 100644
> --- a/fs/ntfs3/frecord.c
> +++ b/fs/ntfs3/frecord.c
> @@ -1947,7 +1947,7 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,
> if (!attr || !attr->non_res) {
> err = fiemap_fill_next_extent(
> fieinfo, 0, 0,
> - attr ? le32_to_cpu(attr->res.data_size) : 0,
> + attr ? le32_to_cpu(attr->res.data_size) : 0, 0,
> FIEMAP_EXTENT_DATA_INLINE | FIEMAP_EXTENT_LAST |
> FIEMAP_EXTENT_MERGED);
> goto out;
> @@ -2042,7 +2042,7 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,
> flags |= FIEMAP_EXTENT_LAST;
>
> err = fiemap_fill_next_extent(fieinfo, vbo, lbo, dlen,
> - flags);
> + 0, flags);
> if (err < 0)
> break;
> if (err == 1) {
> @@ -2062,7 +2062,8 @@ int ni_fiemap(struct ntfs_inode *ni, struct fiemap_extent_info *fieinfo,
> if (vbo + bytes >= end)
> flags |= FIEMAP_EXTENT_LAST;
>
> - err = fiemap_fill_next_extent(fieinfo, vbo, lbo, bytes, flags);
> + err = fiemap_fill_next_extent(fieinfo, vbo, lbo, bytes, 0,
> + flags);
> if (err < 0)
> break;
> if (err == 1) {
> diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c
> index 70a768b623cf..eabdf97cd685 100644
> --- a/fs/ocfs2/extent_map.c
> +++ b/fs/ocfs2/extent_map.c
> @@ -723,7 +723,7 @@ static int ocfs2_fiemap_inline(struct inode *inode, struct buffer_head *di_bh,
> id2.i_data.id_data);
>
> ret = fiemap_fill_next_extent(fieinfo, 0, phys, id_count,
> - flags);
> + 0, flags);
> if (ret < 0)
> return ret;
> }
> @@ -794,7 +794,7 @@ int ocfs2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> virt_bytes = (u64)le32_to_cpu(rec.e_cpos) << osb->s_clustersize_bits;
>
> ret = fiemap_fill_next_extent(fieinfo, virt_bytes, phys_bytes,
> - len_bytes, fe_flags);
> + len_bytes, 0, fe_flags);
> if (ret)
> break;
>
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> index 87b63f6ad2e2..23a193512f96 100644
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -3779,6 +3779,7 @@ static int smb3_fiemap(struct cifs_tcon *tcon,
> le64_to_cpu(out_data[i].file_offset),
> le64_to_cpu(out_data[i].file_offset),
> le64_to_cpu(out_data[i].length),
> + 0,
> flags);
> if (rc < 0)
> goto out;
> diff --git a/include/linux/fiemap.h b/include/linux/fiemap.h
> index c50882f19235..17a6c32cdf3f 100644
> --- a/include/linux/fiemap.h
> +++ b/include/linux/fiemap.h
> @@ -16,6 +16,6 @@ struct fiemap_extent_info {
> int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo,
> u64 start, u64 *len, u32 supported_flags);
> int fiemap_fill_next_extent(struct fiemap_extent_info *info, u64 logical,
> - u64 phys, u64 len, u32 flags);
> + u64 phys, u64 log_len, u64 phys_len, u32 flags);
>
> #endif /* _LINUX_FIEMAP_H 1 */
> --
> 2.43.0
>
>
Cheers, Andreas
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 03/13] fiemap: add new COMPRESSED extent state
2024-04-03 7:22 ` [PATCH v3 03/13] fs: fiemap: add new COMPRESSED extent state Sweet Tea Dorminy
@ 2024-04-05 19:06 ` Andreas Dilger
0 siblings, 0 replies; 38+ messages in thread
From: Andreas Dilger @ 2024-04-05 19:06 UTC (permalink / raw
To: Sweet Tea Dorminy
Cc: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
Linux Kernel Mailing List, linux-bcachefs, linux-btrfs,
linux-f2fs-devel, linux-fsdevel, kernel-team
[-- Attachment #1: Type: text/plain, Size: 2612 bytes --]
On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <sweettea-kernel@dorminy.me> 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 <sweettea-kernel@dorminy.me>
Looks good.
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> ---
> 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
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 02/13] fiemap: update fiemap_fill_next_extent() signature
2024-04-05 19:05 ` [PATCH v3 02/13] " Andreas Dilger
@ 2024-04-05 19:06 ` Kent Overstreet
0 siblings, 0 replies; 38+ messages in thread
From: Kent Overstreet @ 2024-04-05 19:06 UTC (permalink / raw
To: Andreas Dilger
Cc: Sweet Tea Dorminy, Jonathan Corbet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
Linux Kernel Mailing List, linux-bcachefs, linux-btrfs,
linux-f2fs-devel, linux-fsdevel, kernel-team
On Fri, Apr 05, 2024 at 01:05:01PM -0600, Andreas Dilger wrote:
> On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <sweettea-kernel@dorminy.me> wrote:
> >
> > Update the signature of fiemap_fill_next_extent() to allow passing a
> > physical length. Update all callers to pass a 0 physical length -- since
> > none set the EXTENT_HAS_PHYS_LEN flag, this value doesn't matter.
>
> Patch-structure-wise, it doesn't make sense to me to change all of the callers
> to pass "0" as the argument to this function, and then submit a whole series
> of per-filesystem patches that sets only FIEMAP_EXTENT_HAS_PHYS_LEN (but also
> passes phys_len = 0, which is wrong AFAICS).
>
> A cleaner approach would be to rename the existing fiemap_fill_next_extent()
> to fiemap_fill_next_extent_phys() that takes phys_len as an argument, and then
> add a simple wrapper until all of the filesystems are updated:
>
> #define fiemap_fill_next_extent(info, logical, phys, log_len, flags, dev) \
> fiemap_fill_next_extent_phys(info, logical, phys, log_len, 0, flags, dev)
>
> Then the per-filesystem patches would involve changing over the callers to
> use fiemap_fill_next_extent_phys() and setting FIEMAP_EXTENT_HAS_PHYS_LEN.
Cleaner still would be to just have the callers initiaize and pass a
struct fiemap_extent instead of passing around a whole bunch of integer
parameters.
You get more explicit naming, better typesafety - functions with a bunch
of integer parametrs are not great from a type safety POV - and you can
add and pass fields to fiemap_extent without having to update callers
that aren't using it.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 04/13] btrfs: fiemap: emit new COMPRESSED state.
2024-04-03 7:22 ` [PATCH v3 04/13] btrfs: fiemap: emit new COMPRESSED state Sweet Tea Dorminy
@ 2024-04-05 19:10 ` Andreas Dilger
0 siblings, 0 replies; 38+ messages in thread
From: Andreas Dilger @ 2024-04-05 19:10 UTC (permalink / raw
To: Sweet Tea Dorminy
Cc: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
linux-kernel, linux-bcachefs, linux-btrfs, linux-f2fs-devel,
linux-fsdevel, kernel-team
[-- Attachment #1: Type: text/plain, Size: 1487 bytes --]
On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <sweettea-kernel@dorminy.me> wrote:
>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
I would recommend to merge this with the 05/13 patch that is setting the btrfs
fe_physical_length field. Otherwise, by itself it would be confusing that the
DATA_COMPRESSED flag is set on the extent without fe_physical_length being set
to be able to do anything with that information.
Cheers, Andreas
> ---
> fs/btrfs/extent_io.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 9e421d99fd5c..e9df670ef7d2 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2706,7 +2706,7 @@ static int emit_fiemap_extent(struct fiemap_extent_info *fieinfo,
> if (range_end <= cache_end)
> return 0;
>
> - if (!(flags & (FIEMAP_EXTENT_ENCODED | FIEMAP_EXTENT_DELALLOC)))
> + if (!(flags & (FIEMAP_EXTENT_DATA_COMPRESSED | FIEMAP_EXTENT_DELALLOC)))
> phys += cache_end - offset;
>
> offset = cache_end;
> @@ -3236,7 +3236,7 @@ int extent_fiemap(struct btrfs_inode *inode, struct fiemap_extent_info *fieinfo,
> }
>
> if (compression != BTRFS_COMPRESS_NONE)
> - flags |= FIEMAP_EXTENT_ENCODED;
> + flags |= FIEMAP_EXTENT_DATA_COMPRESSED;
>
> if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
> flags |= FIEMAP_EXTENT_DATA_INLINE;
> --
> 2.43.0
>
>
Cheers, Andreas
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 13/13] bcachefs: fiemap: emit new COMPRESSED state
2024-04-03 7:22 ` [PATCH v3 13/13] bcachefs: " Sweet Tea Dorminy
@ 2024-04-05 19:17 ` Andreas Dilger
2024-04-05 19:34 ` Andreas Dilger
2024-04-06 5:20 ` Kent Overstreet
0 siblings, 2 replies; 38+ messages in thread
From: Andreas Dilger @ 2024-04-05 19:17 UTC (permalink / raw
To: Sweet Tea Dorminy, Kent Overstreet
Cc: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
Linux Kernel Mailing List, linux-bcachefs, linux-btrfs,
linux-f2fs-devel, linux-fsdevel, kernel-team
[-- Attachment #1: Type: text/plain, Size: 1275 bytes --]
On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <sweettea-kernel@dorminy.me> wrote:
>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
> 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
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 06/13] nilfs2: fiemap: return correct extent physical length
2024-04-03 7:22 ` [PATCH v3 06/13] nilfs2: fiemap: return correct extent physical length Sweet Tea Dorminy
@ 2024-04-05 19:26 ` Andreas Dilger
0 siblings, 0 replies; 38+ messages in thread
From: Andreas Dilger @ 2024-04-05 19:26 UTC (permalink / raw
To: Sweet Tea Dorminy
Cc: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
Linux Kernel Mailing List, linux-bcachefs, linux-btrfs,
linux-f2fs-devel, linux-fsdevel, kernel-team
[-- Attachment #1: Type: text/plain, Size: 2413 bytes --]
On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <sweettea-kernel@dorminy.me> wrote:
>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
> fs/nilfs2/inode.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index 4d3c347c982b..e3108f2cead7 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -1160,7 +1160,7 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> {
> struct the_nilfs *nilfs = inode->i_sb->s_fs_info;
> __u64 logical = 0, phys = 0, size = 0;
> - __u32 flags = 0;
> + __u32 flags = FIEMAP_EXTENT_HAS_PHYS_LEN;
>
> loff_t isize;
> sector_t blkoff, end_blkoff;
> sector_t delalloc_blkoff;
> @@ -1197,7 +1197,9 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> if (blkoff > end_blkoff)
> break;
>
> - flags = FIEMAP_EXTENT_MERGED | FIEMAP_EXTENT_DELALLOC;
> + flags = FIEMAP_EXTENT_MERGED |
> + FIEMAP_EXTENT_DELALLOC |
> + FIEMAP_EXTENT_HAS_PHYS_LEN;
IMHO, rather than setting "flags = FIEMAP..." here, it would be better to
initialize "flags |= FIEMAP_HAS_PHYS_LEN" right after fiemap_fill_next_extent()
is called, and use "flags |= FIEMAP_EXTENT_MERGED | FIEMAP_EXTENT_DELALLOC" here.
That makes it more clear that MERGED|DELALLOC are "add-on" flags beyond the
base flags, and if more flags are added in the future (e.g. COMPRESSED) then
the flag management will be simpler (more on this below).
> @@ -1261,14 +1263,16 @@ int nilfs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> break;
>
> /* Start another extent */
> - flags = FIEMAP_EXTENT_MERGED;
> + flags = FIEMAP_EXTENT_MERGED |
> + FIEMAP_EXTENT_HAS_PHYS_LEN;
Strictly speaking, this new extent should not have FIEMAP_EXTENT_MERGED set,
and start out with only FIEMAP_EXTENT_HAS_PHYS_LEN, since it has not actually
been merged with anything.
> logical = blkoff << blkbits;
> phys = blkphy << blkbits;
> size = n << blkbits;
> }
> } else {
> /* Start a new extent */
> - flags = FIEMAP_EXTENT_MERGED;
> + flags = FIEMAP_EXTENT_MERGED |
> + FIEMAP_EXTENT_HAS_PHYS_LEN;
Then this should be "flags |= FIEMAP_EXTENT_MERGED" only once a second
block has been merged into the prior one.
Cheers, Andreas
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 08/13] f2fs: fiemap: add physical length to trace_f2fs_fiemap
2024-04-03 7:22 ` [PATCH v3 08/13] f2fs: fiemap: add physical length to trace_f2fs_fiemap Sweet Tea Dorminy
@ 2024-04-05 19:28 ` Andreas Dilger
0 siblings, 0 replies; 38+ messages in thread
From: Andreas Dilger @ 2024-04-05 19:28 UTC (permalink / raw
To: Sweet Tea Dorminy
Cc: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
Linux Kernel Mailing List, linux-bcachefs, linux-btrfs,
linux-f2fs-devel, linux-fsdevel, kernel-team
[-- Attachment #1: Type: text/plain, Size: 3831 bytes --]
On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <sweettea-kernel@dorminy.me> wrote:
>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
With the compat macro in the first patch in the series, IMHO this one
should be merged into the next patch that actually sets the phys_len,
since passing "0" for the phys_len isn't super useful by itself IMHO.
Cheers, Andreas
> ---
> fs/f2fs/data.c | 6 +++---
> fs/f2fs/inline.c | 2 +-
> include/trace/events/f2fs.h | 10 +++++++---
> 3 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 87f8d828e038..34af1673461b 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1836,7 +1836,7 @@ static int f2fs_xattr_fiemap(struct inode *inode,
>
> err = fiemap_fill_next_extent(
> fieinfo, 0, phys, len, 0, flags);
> - trace_f2fs_fiemap(inode, 0, phys, len, flags, err);
> + trace_f2fs_fiemap(inode, 0, phys, len, 0, flags, err);
> if (err)
> return err;
> }
> @@ -1863,7 +1863,7 @@ static int f2fs_xattr_fiemap(struct inode *inode,
> if (phys) {
> err = fiemap_fill_next_extent(
> fieinfo, 0, phys, len, 0, flags);
> - trace_f2fs_fiemap(inode, 0, phys, len, flags, err);
> + trace_f2fs_fiemap(inode, 0, phys, len, 0, flags, err);
> }
>
> return (err < 0 ? err : 0);
> @@ -1982,7 +1982,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>
> ret = fiemap_fill_next_extent(fieinfo, logical,
> phys, size, 0, flags);
> - trace_f2fs_fiemap(inode, logical, phys, size, flags, ret);
> + trace_f2fs_fiemap(inode, logical, phys, size, 0, flags, ret);
> if (ret)
> goto out;
> size = 0;
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index 49d2f87fe048..235b0d72f6fc 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -808,7 +808,7 @@ int f2fs_inline_data_fiemap(struct inode *inode,
> (char *)F2FS_INODE(ipage);
> err = fiemap_fill_next_extent(
> fieinfo, start, byteaddr, ilen, 0, flags);
> - trace_f2fs_fiemap(inode, start, byteaddr, ilen, flags, err);
> + trace_f2fs_fiemap(inode, start, byteaddr, ilen, 0, flags, err);
> out:
> f2fs_put_page(ipage, 1);
> return err;
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index 7ed0fc430dc6..63706eb3a5db 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -2276,9 +2276,10 @@ TRACE_EVENT(f2fs_bmap,
> TRACE_EVENT(f2fs_fiemap,
>
> TP_PROTO(struct inode *inode, sector_t lblock, sector_t pblock,
> - unsigned long long len, unsigned int flags, int ret),
> + unsigned long long len, unsigned long long phys_len,
> + unsigned int flags, int ret),
>
> - TP_ARGS(inode, lblock, pblock, len, flags, ret),
> + TP_ARGS(inode, lblock, pblock, len, phys_len, flags, ret),
>
> TP_STRUCT__entry(
> __field(dev_t, dev)
> @@ -2286,6 +2287,7 @@ TRACE_EVENT(f2fs_fiemap,
> __field(sector_t, lblock)
> __field(sector_t, pblock)
> __field(unsigned long long, len)
> + __field(unsigned long long, phys_len)
> __field(unsigned int, flags)
> __field(int, ret)
> ),
> @@ -2296,16 +2298,18 @@ TRACE_EVENT(f2fs_fiemap,
> __entry->lblock = lblock;
> __entry->pblock = pblock;
> __entry->len = len;
> + __entry->phys_len = phys_len;
> __entry->flags = flags;
> __entry->ret = ret;
> ),
>
> TP_printk("dev = (%d,%d), ino = %lu, lblock:%lld, pblock:%lld, "
> - "len:%llu, flags:%u, ret:%d",
> + "len:%llu, plen:%llu, flags:%u, ret:%d",
> show_dev_ino(__entry),
> (unsigned long long)__entry->lblock,
> (unsigned long long)__entry->pblock,
> __entry->len,
> + __entry->phys_len,
> __entry->flags,
> __entry->ret)
> );
> --
> 2.43.0
>
>
Cheers, Andreas
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 13/13] bcachefs: fiemap: emit new COMPRESSED state
2024-04-05 19:17 ` Andreas Dilger
@ 2024-04-05 19:34 ` Andreas Dilger
2024-04-06 5:20 ` Kent Overstreet
1 sibling, 0 replies; 38+ messages in thread
From: Andreas Dilger @ 2024-04-05 19:34 UTC (permalink / raw
To: Sweet Tea Dorminy, Kent Overstreet
Cc: Jonathan Corbet, Brian Foster, Chris Mason, Josef Bacik,
David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
Linux Kernel Mailing List, linux-bcachefs, linux-btrfs,
linux-f2fs-devel, linux-fsdevel, kernel-team
[-- Attachment #1: Type: text/plain, Size: 1666 bytes --]
> On Apr 5, 2024, at 1:17 PM, Andreas Dilger <adilger@dilger.ca> wrote:
>
> On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <sweettea-kernel@dorminy.me> wrote:
>>
>> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
>> ---
>> 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
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 13/13] bcachefs: fiemap: emit new COMPRESSED state
2024-04-05 19:17 ` Andreas Dilger
2024-04-05 19:34 ` Andreas Dilger
@ 2024-04-06 5:20 ` Kent Overstreet
1 sibling, 0 replies; 38+ messages in thread
From: Kent Overstreet @ 2024-04-06 5:20 UTC (permalink / raw
To: Andreas Dilger
Cc: Sweet Tea Dorminy, Jonathan Corbet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
Linux Kernel Mailing List, linux-bcachefs, linux-btrfs,
linux-f2fs-devel, linux-fsdevel, kernel-team
On Fri, Apr 05, 2024 at 01:17:45PM -0600, Andreas Dilger wrote:
> On Apr 3, 2024, at 1:22 AM, Sweet Tea Dorminy <sweettea-kernel@dorminy.me> wrote:
> >
> > Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> > ---
> > 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
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 01/13] fs: fiemap: add physical_length field to extents
2024-04-03 7:22 ` [PATCH v3 01/13] fs: fiemap: add physical_length field to extents Sweet Tea Dorminy
2024-04-03 16:57 ` Brian Foster
2024-04-05 18:47 ` [PATCH v3 01/13] " Andreas Dilger
@ 2024-04-09 16:22 ` Darrick J. Wong
2024-04-09 19:50 ` Andreas Dilger
2 siblings, 1 reply; 38+ messages in thread
From: Darrick J. Wong @ 2024-04-09 16:22 UTC (permalink / raw
To: Sweet Tea Dorminy
Cc: Jonathan Corbet, Kent Overstreet, Brian Foster, Chris Mason,
Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu, Alexander Viro,
Christian Brauner, Jan Kara, Mickaël Salaün, linux-doc,
linux-kernel, linux-bcachefs, linux-btrfs, linux-f2fs-devel,
linux-fsdevel, kernel-team
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 <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.
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
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v3 01/13] fs: fiemap: add physical_length field to extents
2024-04-09 16:22 ` [PATCH v3 01/13] fs: " Darrick J. Wong
@ 2024-04-09 19:50 ` Andreas Dilger
0 siblings, 0 replies; 38+ messages in thread
From: Andreas Dilger @ 2024-04-09 19:50 UTC (permalink / raw
To: Darrick J. Wong
Cc: Sweet Tea Dorminy, Jonathan Corbet, Kent Overstreet, Brian Foster,
Chris Mason, Josef Bacik, David Sterba, Jaegeuk Kim, Chao Yu,
Alexander Viro, Christian Brauner, Jan Kara,
Mickaël Salaün, linux-doc, linux-kernel, linux-bcachefs,
linux-btrfs, linux-f2fs-devel, linux-fsdevel, kernel-team
[-- Attachment #1: Type: text/plain, Size: 7078 bytes --]
On Apr 9, 2024, at 10:22 AM, Darrick J. Wong <djwong@kernel.org> 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 <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.
>
> 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
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2024-04-09 19:50 UTC | newest]
Thread overview: 38+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-03 7:22 [PATCH v3 00/13] fiemap extension for more physical information Sweet Tea Dorminy
2024-04-03 7:22 ` [PATCH v3 01/13] fs: fiemap: add physical_length field to extents Sweet Tea Dorminy
2024-04-03 16:57 ` Brian Foster
2024-04-05 18:47 ` [PATCH v3 01/13] " Andreas Dilger
2024-04-09 16:22 ` [PATCH v3 01/13] fs: " Darrick J. Wong
2024-04-09 19:50 ` Andreas Dilger
2024-04-03 7:22 ` [PATCH v3 02/13] fs: fiemap: update fiemap_fill_next_extent() signature Sweet Tea Dorminy
2024-04-03 16:58 ` Brian Foster
2024-04-05 19:05 ` [PATCH v3 02/13] " Andreas Dilger
2024-04-05 19:06 ` Kent Overstreet
2024-04-03 7:22 ` [PATCH v3 03/13] fs: fiemap: add new COMPRESSED extent state Sweet Tea Dorminy
2024-04-05 19:06 ` [PATCH v3 03/13] " Andreas Dilger
2024-04-03 7:22 ` [PATCH v3 04/13] btrfs: fiemap: emit new COMPRESSED state Sweet Tea Dorminy
2024-04-05 19:10 ` Andreas Dilger
2024-04-03 7:22 ` [PATCH v3 05/13] btrfs: fiemap: return extent physical size Sweet Tea Dorminy
2024-04-03 7:22 ` [PATCH v3 06/13] nilfs2: fiemap: return correct extent physical length Sweet Tea Dorminy
2024-04-05 19:26 ` Andreas Dilger
2024-04-03 7:22 ` [PATCH v3 07/13] ext4: " Sweet Tea Dorminy
2024-04-03 11:22 ` Jan Kara
2024-04-03 7:22 ` [PATCH v3 08/13] f2fs: fiemap: add physical length to trace_f2fs_fiemap Sweet Tea Dorminy
2024-04-05 19:28 ` Andreas Dilger
2024-04-03 7:22 ` [PATCH v3 09/13] f2fs: fiemap: return correct extent physical length Sweet Tea Dorminy
2024-04-03 7:22 ` [PATCH v3 10/13] ocfs2: " Sweet Tea Dorminy
2024-04-03 11:25 ` Jan Kara
2024-04-03 7:22 ` [PATCH v3 11/13] bcachefs: " Sweet Tea Dorminy
2024-04-03 17:00 ` Brian Foster
2024-04-03 18:15 ` Kent Overstreet
2024-04-03 7:22 ` [PATCH v3 12/13] f2fs: fiemap: emit new COMPRESSED state Sweet Tea Dorminy
2024-04-03 7:22 ` [PATCH v3 13/13] bcachefs: " Sweet Tea Dorminy
2024-04-05 19:17 ` Andreas Dilger
2024-04-05 19:34 ` Andreas Dilger
2024-04-06 5:20 ` Kent Overstreet
2024-04-03 8:29 ` [PATCH v3 00/13] fiemap extension for more physical information Gao Xiang
2024-04-03 15:11 ` Sweet Tea Dorminy
2024-04-04 0:43 ` Gao Xiang
2024-04-03 18:17 ` Kent Overstreet
2024-04-03 18:20 ` Darrick J. Wong
2024-04-05 18:20 ` Andreas Dilger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).