mbox series

[00/31] btrfs: move extent_io_tree code and cleanups

Message ID cover.1662149276.git.josef@toxicpanda.com (mailing list archive)
Headers show
Series btrfs: move extent_io_tree code and cleanups | expand

Message

Josef Bacik Sept. 2, 2022, 8:16 p.m. UTC
Hello,

In working on extent tree v2 I got really bogged down in trying to sync work
between btrfs-progs and the kernel.  It basically takes me 3x as long, because
there's a lot of different things missing in btrfs-progs, so each patchset has
to be done from scratch and tested completely differently.

Additionally there's just a lot of tech-debt in these areas in general.  So
before tackling the rest of extent-tree-v2 I'm spending some time cleaning up
things we all know are terrible.  This is the first step in that direction,
finishing the separation of the extent_io_tree code from extent_io.c.  I started
this a while ago, but got bogged down in other things.

This has 3 distinct parts

1. Cleanup the io_failure_record code.  This has been tightly integrated into
   the extent_io_tree code forever without much reason for it.  The first part
   of this series moves that handling into it's own tree, and uses our existing
   helpers to reduce the code quite a bit.

2. Move the code out of extent_io.c.  This is easier than previous code moves
   because I did a lot of the prep work earlier.  Unfortunately there is one big
   patch that copy and pastes all the core code, since it all depends on itself
   and would be more annoying to move.  However the independent parts were
   moved piece by piece.

3. A wholesale cleanup of the extent_io_tree code.  We have a ton of helpers
   here, that have all grown a ton of arguments over the years.  I've trimmed
   down the arguments for our core helpers, and hidden the rest internally
   inside of extent-io-tree.c.  Additionally I've cleaned up the lock/unlock
   extent bit helpers so we only have one lock_extent/unlock_extent variant that
   gets used everywhere.

I've tested this locally to make sure I didn't break anything.  This isn't a
simple code move so do please review most of it, the patches that start with
"move X" are pure code move patches, but the rest do change things.  Thanks,

Josef

Josef Bacik (31):
  btrfs: cleanup clean_io_failure
  btrfs: unexport internal failrec functions
  btrfs: stop using extent_io_tree for io_failure_record's
  btrfs: use find_first_extent_bit in btrfs_clean_io_failure
  btrfs: separate out the extent state and extent buffer init code
  btrfs: separate out the eb and extent state leak helpers
  btrfs: temporarily export alloc_extent_state helpers
  btrfs: move extent state init and alloc functions to their own file
  btrfs: convert BUG_ON(EXTENT_BIT_LOCKED) checks to ASSERT's
  btrfs: move simple extent bit helpers out of extent_io.c
  btrfs: export wait_extent_bit
  btrfs: move the core extent_io_tree code into extent-io-tree.c
  btrfs: remove struct tree_entry
  btrfs: use next_state instead of rb_next where we can
  btrfs: make tree_search return struct extent_state
  btrfs: make tree_search_for_insert return extent_state
  btrfs: make tree_search_prev_next return extent_state's
  btrfs: use next_state/prev_state in merge_state
  btrfs: remove temporary exports for extent_state movement
  btrfs: move irrelevant prototypes to their appropriate header
  btrfs: drop exclusive_bits from set_extent_bit
  btrfs: remove the wake argument from clear_extent_bits
  btrfs: remove failed_start argument from set_extent_bit
  btrfs: drop extent_changeset from set_extent_bit
  btrfs: unify the lock/unlock extent variants
  btrfs: get rid of track_uptodate
  btrfs: get rid of ->dirty_bytes
  btrfs: don't clear CTL bits when trying to release extent state
  btrfs: replace delete argument with EXTENT_CLEAR_ALL_BITS
  btrfs: don't init io tree with private data for non inodes
  btrfs: remove is_data_inode() checks in extent-io-tree.c

 fs/btrfs/Makefile                |    2 +-
 fs/btrfs/btrfs_inode.h           |    3 +-
 fs/btrfs/compression.c           |   11 +-
 fs/btrfs/disk-io.c               |   10 +-
 fs/btrfs/extent-io-tree.c        | 1673 ++++++++++++++++++++++++
 fs/btrfs/extent-io-tree.h        |  119 +-
 fs/btrfs/extent_io.c             | 2055 ++----------------------------
 fs/btrfs/extent_io.h             |   14 +-
 fs/btrfs/extent_map.c            |    2 +-
 fs/btrfs/file-item.c             |    2 +-
 fs/btrfs/file.c                  |   48 +-
 fs/btrfs/free-space-cache.c      |   24 +-
 fs/btrfs/inode.c                 |  159 ++-
 fs/btrfs/ioctl.c                 |   24 +-
 fs/btrfs/misc.h                  |   35 +
 fs/btrfs/ordered-data.c          |    4 +-
 fs/btrfs/reflink.c               |   10 +-
 fs/btrfs/relocation.c            |   18 +-
 fs/btrfs/super.c                 |   17 +-
 fs/btrfs/tests/extent-io-tests.c |    6 +-
 fs/btrfs/tests/inode-tests.c     |    8 +-
 fs/btrfs/transaction.c           |    4 +-
 fs/btrfs/tree-log.c              |    8 +-
 include/trace/events/btrfs.h     |    1 -
 24 files changed, 2077 insertions(+), 2180 deletions(-)
 create mode 100644 fs/btrfs/extent-io-tree.c

Comments

David Sterba Sept. 6, 2022, 6:30 p.m. UTC | #1
On Fri, Sep 02, 2022 at 04:16:05PM -0400, Josef Bacik wrote:
> In working on extent tree v2 I got really bogged down in trying to sync work
> between btrfs-progs and the kernel.  It basically takes me 3x as long, because
> there's a lot of different things missing in btrfs-progs, so each patchset has
> to be done from scratch and tested completely differently.
> 
> Additionally there's just a lot of tech-debt in these areas in general.  So
> before tackling the rest of extent-tree-v2 I'm spending some time cleaning up
> things we all know are terrible.  This is the first step in that direction,
> finishing the separation of the extent_io_tree code from extent_io.c.  I started
> this a while ago, but got bogged down in other things.
> 
> This has 3 distinct parts
> 
> 1. Cleanup the io_failure_record code.  This has been tightly integrated into
>    the extent_io_tree code forever without much reason for it.  The first part
>    of this series moves that handling into it's own tree, and uses our existing
>    helpers to reduce the code quite a bit.
> 
> 2. Move the code out of extent_io.c.  This is easier than previous code moves
>    because I did a lot of the prep work earlier.  Unfortunately there is one big
>    patch that copy and pastes all the core code, since it all depends on itself
>    and would be more annoying to move.  However the independent parts were
>    moved piece by piece.
> 
> 3. A wholesale cleanup of the extent_io_tree code.  We have a ton of helpers
>    here, that have all grown a ton of arguments over the years.  I've trimmed
>    down the arguments for our core helpers, and hidden the rest internally
>    inside of extent-io-tree.c.  Additionally I've cleaned up the lock/unlock
>    extent bit helpers so we only have one lock_extent/unlock_extent variant that
>    gets used everywhere.
> 
> I've tested this locally to make sure I didn't break anything.  This isn't a
> simple code move so do please review most of it, the patches that start with
> "move X" are pure code move patches, but the rest do change things.  Thanks,

As this is mostly simple cleanups or straightfowrad changes I think it's
safe to merge before code freeze. I'll send more comments to the
patches, there are some minor issues or missing commit references to
last code that used some structure when it's removed.

As others noted this patchset conflicts with the series from Christoph,
namely the io_failure_tree removal, but that series is more intrusive
and with pending reviews.
David Sterba Sept. 7, 2022, 6:42 p.m. UTC | #2
On Fri, Sep 02, 2022 at 04:16:05PM -0400, Josef Bacik wrote:
> Josef Bacik (31):
>   btrfs: cleanup clean_io_failure
>   btrfs: unexport internal failrec functions
>   btrfs: stop using extent_io_tree for io_failure_record's
>   btrfs: use find_first_extent_bit in btrfs_clean_io_failure
>   btrfs: separate out the extent state and extent buffer init code
>   btrfs: separate out the eb and extent state leak helpers
>   btrfs: temporarily export alloc_extent_state helpers
>   btrfs: move extent state init and alloc functions to their own file
>   btrfs: convert BUG_ON(EXTENT_BIT_LOCKED) checks to ASSERT's
>   btrfs: move simple extent bit helpers out of extent_io.c
>   btrfs: export wait_extent_bit
>   btrfs: move the core extent_io_tree code into extent-io-tree.c
>   btrfs: remove struct tree_entry
>   btrfs: use next_state instead of rb_next where we can
>   btrfs: make tree_search return struct extent_state
>   btrfs: make tree_search_for_insert return extent_state
>   btrfs: make tree_search_prev_next return extent_state's
>   btrfs: use next_state/prev_state in merge_state
>   btrfs: remove temporary exports for extent_state movement
>   btrfs: move irrelevant prototypes to their appropriate header
>   btrfs: drop exclusive_bits from set_extent_bit
>   btrfs: remove the wake argument from clear_extent_bits
>   btrfs: remove failed_start argument from set_extent_bit
>   btrfs: drop extent_changeset from set_extent_bit
>   btrfs: unify the lock/unlock extent variants
>   btrfs: get rid of track_uptodate
>   btrfs: get rid of ->dirty_bytes
>   btrfs: don't clear CTL bits when trying to release extent state
>   btrfs: replace delete argument with EXTENT_CLEAR_ALL_BITS
>   btrfs: don't init io tree with private data for non inodes
>   btrfs: remove is_data_inode() checks in extent-io-tree.c

The self tests don't pass

[   13.933867] Btrfs loaded, crc32c=crc32c-generic, debug=on, assert=on, integrity-checker=on, ref-verify=on, zoned=yes, fsverity=yes                                                                                
[   13.936232] BTRFS: selftest: sectorsize: 4096  nodesize: 4096                                                                                                                                                     
[   13.937186] BTRFS: selftest: running btrfs free space cache tests                                                                                                                                                 
[   13.938314] BTRFS: selftest: running extent only tests                                                                                                                                                            
[   13.939327] BTRFS: selftest: running bitmap only tests                                                                                                                                                            
[   13.940285] BTRFS: selftest: running bitmap and extent tests                                                                                                                                                      
[   13.941288] BTRFS: selftest: running space stealing from bitmap to extent tests                                                                                                                                   
[   13.942627] BTRFS: selftest: running bytes index tests                                                                                                                                                            
[   13.943681] BTRFS: selftest: running extent buffer operation tests                                                                                                                                                
[   13.944868] BTRFS: selftest: running btrfs_split_item tests                                                                                                                                                       
[   13.945641] BTRFS: selftest: running extent I/O tests                                                                                                                                                             
[   13.946662] BTRFS: selftest: running find delalloc tests                                                                                                                                                          
[   14.226329] BTRFS: selftest: running find_first_clear_extent_bit test                                                                                                                                             
[   14.227517] BTRFS: selftest: fs/btrfs/tests/extent-io-tests.c:528 error finding trimmed range: start 67108864 end 33554431                                                                                        
[   14.229160] BTRFS: selftest: io tree content:                                                                                                                                                                     
[   14.229873] BTRFS: selftest:   start=1048576 len=3145728 flags=DIRTY|DEFRAG                                                                                                                                       
[   14.231208] BTRFS: selftest:   start=33554432 len=33554432 flags=DIRTY|DEFRAG