| Message ID | cover.1663312786.git.wqu@suse.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs: btrfs_get_extent() cleanup for inline extents | expand |
On Fri, Sep 16, 2022 at 03:28:34PM +0800, Qu Wenruo wrote: > Currently we have some very confusing (and harder to explain) code > inside btrfs_get_extent() for inline extents. > > The TL;DR is, those mess is to support inline extents at non-zero file > offset. > > This is completely impossible now, as tree-checker will reject any > inline file extents at non-zero offset. The extent item check has been in place since 2017, but I don't remember if it was ever possible the create inline uncompressed extent other than at offset 0. I know there is the inconsistency with compressed extent but for plain uncompressed it should not be normally possible. > So this series will: > > - Update the selftest to get rid of the impossible case > > - Simplify the inline extent read > > Since we no longer need to support inline extents at non-zero file > offset, some variables can be removed. > > - Don't reset the inline extent map members > > Those resets are either doing nothing (setting the member to the same > value), or making no difference (the member is not utilized anyway) > > - Remove @new_inline argument for btrfs_extent_item_to_extent_map() > > That argument makes btrfs_get_extent() to skip certain member update. > Previously it makes a difference only for fiemap. > > But now since fiemap no longer relies on extent_map, the remaining > use cases won't bother those members anyway. > > Thus we can remove the bool argument and make > btrfs_extent_item_to_extent_map() to have a consistent behavior. > > - Introduce a helper to do inline extents read > > Now the function is so simple that, extracting the helper can only > reduce indents. > > [CHANGELOG] > v2: > - Do better patch split for bisection with better reason > > - Put the selftest to the first to help bisection > Or we may hit ASSERT() during selftest. > > Qu Wenruo (5): > btrfs: selftests: remove impossible inline extent at non-zero file > offset > btrfs: make inline extent read calculation much simpler > btrfs: do not reset extent map members for inline extents read > btrfs: remove the @new_inline argument from > btrfs_extent_item_to_extent_map() > btrfs: extract the inline extent read code into its own function I did a quick review, code looks ok for inclusion to for-next.
On Tue, Oct 25, 2022 at 04:08:51PM +0200, David Sterba wrote: > > v2: > > - Do better patch split for bisection with better reason > > > > - Put the selftest to the first to help bisection > > Or we may hit ASSERT() during selftest. > > > > Qu Wenruo (5): > > btrfs: selftests: remove impossible inline extent at non-zero file > > offset > > btrfs: make inline extent read calculation much simpler > > btrfs: do not reset extent map members for inline extents read > > btrfs: remove the @new_inline argument from > > btrfs_extent_item_to_extent_map() > > btrfs: extract the inline extent read code into its own function > > I did a quick review, code looks ok for inclusion to for-next. Moved to misc-next, thanks.
Currently we have some very confusing (and harder to explain) code inside btrfs_get_extent() for inline extents. The TL;DR is, those mess is to support inline extents at non-zero file offset. This is completely impossible now, as tree-checker will reject any inline file extents at non-zero offset. So this series will: - Update the selftest to get rid of the impossible case - Simplify the inline extent read Since we no longer need to support inline extents at non-zero file offset, some variables can be removed. - Don't reset the inline extent map members Those resets are either doing nothing (setting the member to the same value), or making no difference (the member is not utilized anyway) - Remove @new_inline argument for btrfs_extent_item_to_extent_map() That argument makes btrfs_get_extent() to skip certain member update. Previously it makes a difference only for fiemap. But now since fiemap no longer relies on extent_map, the remaining use cases won't bother those members anyway. Thus we can remove the bool argument and make btrfs_extent_item_to_extent_map() to have a consistent behavior. - Introduce a helper to do inline extents read Now the function is so simple that, extracting the helper can only reduce indents. [CHANGELOG] v2: - Do better patch split for bisection with better reason - Put the selftest to the first to help bisection Or we may hit ASSERT() during selftest. Qu Wenruo (5): btrfs: selftests: remove impossible inline extent at non-zero file offset btrfs: make inline extent read calculation much simpler btrfs: do not reset extent map members for inline extents read btrfs: remove the @new_inline argument from btrfs_extent_item_to_extent_map() btrfs: extract the inline extent read code into its own function fs/btrfs/ctree.h | 1 - fs/btrfs/extent_map.c | 7 +++ fs/btrfs/file-item.c | 6 +-- fs/btrfs/inode.c | 100 +++++++++++++++++++---------------- fs/btrfs/ioctl.c | 2 +- fs/btrfs/tests/inode-tests.c | 56 +++++++------------- 6 files changed, 83 insertions(+), 89 deletions(-)