| Message ID | cover.1715105406.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs: snapshot delete cleanups | expand |
On Tue, May 07, 2024 at 02:12:01PM -0400, Josef Bacik wrote: > v2->v3: > - Fixed the bug pointed out by Dan https://lore.kernel.org/all/96789032-42fb-41c0-b16c-561ac00ca7c3@moroto.mountain/ > > v1->v2: > - Simply removed the btrfs_check_eb_owner() calls as per Qu's suggestion. > - Made the 0 reference count error be more verbose as per Dave's suggestion. > > --- Original email --- > > 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: remove all extra btrfs_check_eb_owner() calls > 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 Reviewed-by: David Sterba <dsterba@suse.com> A lot of good things there, fewer BUG_ONs, comments and refactoring, thanks. I had it in my misc-next for some time, no related problems reported.
On Wed, May 15, 2024 at 08:18:42PM +0200, David Sterba wrote: > On Tue, May 07, 2024 at 02:12:01PM -0400, Josef Bacik wrote: > > v2->v3: > > - Fixed the bug pointed out by Dan https://lore.kernel.org/all/96789032-42fb-41c0-b16c-561ac00ca7c3@moroto.mountain/ > > > > v1->v2: > > - Simply removed the btrfs_check_eb_owner() calls as per Qu's suggestion. > > - Made the 0 reference count error be more verbose as per Dave's suggestion. > > > > --- Original email --- > > > > 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: remove all extra btrfs_check_eb_owner() calls > > 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 > > Reviewed-by: David Sterba <dsterba@suse.com> > > A lot of good things there, fewer BUG_ONs, comments and refactoring, > thanks. I had it in my misc-next for some time, no related problems > reported. For the record, I merged this to for-next a few weeks ago.