Re: [PATCH 00/10] btrfs: backref cache cleanups

[Boris Burkov <boris@bur.io>]

On Thu, Oct 03, 2024 at 11:43:02AM -0400, Josef Bacik wrote:
> Hello,
> 
> This is the followup to the relocation fix that I sent out earlier.  This series
> cleans up a lot of the complicated things that exist in backref cache because we
> were keeping track of changes to the file system during relocation.  Now that we
> do not do this we can simplify a lot of the code and make it easier to
> understand.  I've tested this with the horror show of a relocation test I was
> using to trigger the original problem.  I'm running fstests now via the CI, but
> this seems solid.  Hopefully this makes the relocation code a bit easier to
> understand.  Thanks,
> 
> Josef

Sent a few minor comments inline, but all the patches look good to me.
The only ones I would say I didn't deeply understand were 6 and 7 for
the cowonly changes, in case you feel you need a closer look at those.

One high level request (also raised inline on one of them) is just to
add more high level documentation of the functions and general
algorithm design, while it's fresh. The doc covering the backref
cache data structure itself that we have in our dev doc repo is great,
but doesn't do enough to explain the actual relocation operations
carried out with the help of the backref cache.

With that said, thanks for following up (!!) and you can add

Reviewed-by: Boris Burkov <boris@bur.io>

> 
> Josef Bacik (10):
>   btrfs: convert BUG_ON in btrfs_reloc_cow_block to proper error
>     handling
>   btrfs: remove the changed list for backref cache
>   btrfs: add a comment for new_bytenr in bacref_cache_node
>   btrfs: cleanup select_reloc_root
>   btrfs: remove clone_backref_node
>   btrfs: don't build backref tree for cowonly blocks
>   btrfs: do not handle non-shareable roots in backref cache
>   btrfs: simplify btrfs_backref_release_cache
>   btrfs: remove the ->lowest and ->leaves members from backref cache
>   btrfs: remove detached list from btrfs_backref_cache
> 
>  fs/btrfs/backref.c    | 105 +++++-------
>  fs/btrfs/backref.h    |  16 +-
>  fs/btrfs/relocation.c | 362 +++++++++++++++++-------------------------
>  3 files changed, 192 insertions(+), 291 deletions(-)
> 
> -- 
> 2.43.0
>