2024-04-03 18:18:43

by Kent Overstreet

[permalink] [raw]
Subject: Re: [PATCH v3 00/13] fiemap extension for more physical information

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/[email protected]/T/#t
>
> v1: https://lore.kernel.org/linux-fsdevel/20240315030334.GQ6184@frogsfrogsfrogs/T/#t
>
> Sweet Tea Dorminy (13):
> fs: fiemap: add physical_length field to extents
> fs: fiemap: update fiemap_fill_next_extent() signature
> fs: fiemap: add new COMPRESSED extent state
> btrfs: fiemap: emit new COMPRESSED state.
> btrfs: fiemap: return extent physical size
> nilfs2: fiemap: return correct extent physical length
> ext4: fiemap: return correct extent physical length
> f2fs: fiemap: add physical length to trace_f2fs_fiemap
> f2fs: fiemap: return correct extent physical length
> ocfs2: fiemap: return correct extent physical length
> bcachefs: fiemap: return correct extent physical length
> f2fs: fiemap: emit new COMPRESSED state
> bcachefs: fiemap: emit new COMPRESSED state
>
> Documentation/filesystems/fiemap.rst | 35 ++++++++++----
> fs/bcachefs/fs.c | 17 +++++--
> fs/btrfs/extent_io.c | 72 ++++++++++++++++++----------
> fs/ext4/extents.c | 3 +-
> fs/f2fs/data.c | 36 +++++++++-----
> fs/f2fs/inline.c | 7 +--
> fs/ioctl.c | 11 +++--
> fs/iomap/fiemap.c | 2 +-
> fs/nilfs2/inode.c | 18 ++++---
> fs/ntfs3/frecord.c | 7 +--
> fs/ocfs2/extent_map.c | 10 ++--
> fs/smb/client/smb2ops.c | 1 +
> include/linux/fiemap.h | 2 +-
> include/trace/events/f2fs.h | 10 ++--
> include/uapi/linux/fiemap.h | 34 ++++++++++---
> 15 files changed, 178 insertions(+), 87 deletions(-)
>
>
> base-commit: 75e31f66adc4c8d049e8aac1f079c1639294cd65
> --
> 2.43.0
>


2024-04-03 18:20:38

by Darrick J. Wong

[permalink] [raw]
Subject: Re: [PATCH v3 00/13] fiemap extension for more physical information

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/[email protected]/T/#t
> >
> > v1: https://lore.kernel.org/linux-fsdevel/20240315030334.GQ6184@frogsfrogsfrogs/T/#t
> >
> > Sweet Tea Dorminy (13):
> > fs: fiemap: add physical_length field to extents
> > fs: fiemap: update fiemap_fill_next_extent() signature
> > fs: fiemap: add new COMPRESSED extent state
> > btrfs: fiemap: emit new COMPRESSED state.
> > btrfs: fiemap: return extent physical size
> > nilfs2: fiemap: return correct extent physical length
> > ext4: fiemap: return correct extent physical length
> > f2fs: fiemap: add physical length to trace_f2fs_fiemap
> > f2fs: fiemap: return correct extent physical length
> > ocfs2: fiemap: return correct extent physical length
> > bcachefs: fiemap: return correct extent physical length
> > f2fs: fiemap: emit new COMPRESSED state
> > bcachefs: fiemap: emit new COMPRESSED state
> >
> > Documentation/filesystems/fiemap.rst | 35 ++++++++++----
> > fs/bcachefs/fs.c | 17 +++++--
> > fs/btrfs/extent_io.c | 72 ++++++++++++++++++----------
> > fs/ext4/extents.c | 3 +-
> > fs/f2fs/data.c | 36 +++++++++-----
> > fs/f2fs/inline.c | 7 +--
> > fs/ioctl.c | 11 +++--
> > fs/iomap/fiemap.c | 2 +-
> > fs/nilfs2/inode.c | 18 ++++---
> > fs/ntfs3/frecord.c | 7 +--
> > fs/ocfs2/extent_map.c | 10 ++--
> > fs/smb/client/smb2ops.c | 1 +
> > include/linux/fiemap.h | 2 +-
> > include/trace/events/f2fs.h | 10 ++--
> > include/uapi/linux/fiemap.h | 34 ++++++++++---
> > 15 files changed, 178 insertions(+), 87 deletions(-)
> >
> >
> > base-commit: 75e31f66adc4c8d049e8aac1f079c1639294cd65
> > --
> > 2.43.0
> >
>

2024-04-05 18:24:35

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH v3 00/13] fiemap extension for more physical information

On Apr 3, 2024, at 12:17 PM, Kent Overstreet <[email protected]> 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






Attachments:
signature.asc (890.00 B)
Message signed with OpenPGP