[00/15] btrfs: snapshot delete cleanups

Message

Josef Bacik April 19, 2024, 6:16 p.m. UTC

Hello,

In light of the recent fix for snapshot delete I looked around at the code to
see if it could be cleaned up.  I still feel like this could be reworked to make
the two stages clearer, but this series brings a lot of cleanups and
re-factoring as well as comments and documentation that hopefully make this code
easier for others to work in.  I've broken up the do_walk_down() function into
discreet peices that are better documented and describe their use.  I've also
taken the opportunity to remove a bunch of BUG_ON()'s in this code.  I've run
this through the CI a few times as I made a couple of errors, but it's passing
cleanly now.  Thanks,

Josef

Josef Bacik (15):
  btrfs: don't do find_extent_buffer in do_walk_down
  btrfs: push ->owner_root check into btrfs_read_extent_buffer
  btrfs: use btrfs_read_extent_buffer in do_walk_down
  btrfs: push lookup_info into walk_control
  btrfs: move the eb uptodate code into it's own helper
  btrfs: remove need_account in do_walk_down
  btrfs: unify logic to decide if we need to walk down into a node
  btrfs: extract the reference dropping code into it's own helper
  btrfs: don't BUG_ON ENOMEM in walk_down_proc
  btrfs: handle errors from ref mods during UPDATE_BACKREF
  btrfs: replace BUG_ON with ASSERT in walk_down_proc
  btrfs: clean up our handling of refs == 0 in snapshot delete
  btrfs: convert correctness BUG_ON()'s to ASSERT()'s in walk_up_proc
  btrfs: handle errors from btrfs_dec_ref properly
  btrfs: add documentation around snapshot delete

 fs/btrfs/ctree.c       |   7 +-
 fs/btrfs/disk-io.c     |   6 +-
 fs/btrfs/extent-tree.c | 507 +++++++++++++++++++++++++++--------------
 3 files changed, 338 insertions(+), 182 deletions(-)

Comments

David Sterba April 24, 2024, 12:31 p.m. UTC | #1

On Fri, Apr 19, 2024 at 02:16:55PM -0400, Josef Bacik wrote:
> Hello,
> 
> In light of the recent fix for snapshot delete I looked around at the code to
> see if it could be cleaned up.  I still feel like this could be reworked to make
> the two stages clearer, but this series brings a lot of cleanups and
> re-factoring as well as comments and documentation that hopefully make this code
> easier for others to work in.  I've broken up the do_walk_down() function into
> discreet peices that are better documented and describe their use.  I've also
> taken the opportunity to remove a bunch of BUG_ON()'s in this code.  I've run
> this through the CI a few times as I made a couple of errors, but it's passing
> cleanly now.  Thanks,

We're past rc5, this series looks generally ok to me so I'd rather get
it merged to for-next. We can do minor refinements later (either still
within for-next or as separate cleanup pass).

There are some comments so I'd not send rev-by for the whole series but
consider it as such. My testing did not hit any problems either.