| Message ID | cover.1712126039.git.sweettea-kernel@dorminy.me (mailing list archive) |
|---|---|
| Headers | show |
| Series | fiemap extension for more physical information | expand |
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
> > 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
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 >
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 > > >
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
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