Re: [PATCH 07/20] btrfs-progs: stop accessing ->csum_root directly

[Josef Bacik <josef@toxicpanda.com>]

On Sat, Nov 06, 2021 at 08:23:46AM +0800, Qu Wenruo wrote:
> 
> 
> On 2021/11/6 04:28, Josef Bacik wrote:
> > With extent tree v2 we will have per-block group checksums,
> 
> I guess you mean per block group checksum tree?
> 
> > so add a
> > helper to access the csum root and rename the fs_info csum_root to
> > _csum_root to catch all the places that are accessing it directly.
> > Convert everybody to use the helper except for internal things.
> > 
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> 
> Currently a lot of locations are still passing 0 to btrfs_csum_root(),
> which would need to be converted later.
> 
> Thus it doesn't look correct to be in the preparation patchset.
> 
> Mind to move this patch into the real extent-tree-v2 patchset?
> 

I explained this in another reply, but I wanted to talk about this specific
style of request here as well.

I did it this way because it's changing a lot of callsites to use a helper.
This is kind of a complex change as a whole, because I'm taking us from having a
fs_info->extent_root to having a rb tree with the extent root, csum root, and
free space tree root.

Once I get to the actual extent tree v2 stuff there's going to be a whole host
of more complicated changes.

So, in order to make it easier to review, I've put this relatively large concept
in the prep patches.  It's easier to look at because its a clear
s/->whatever_root/btrfs_whatever_root/ change.  It's easy to spot check and wrap
your head around.

Then I do the work to actually load it into the rb-tree, and then change the
helpers to access the rb-tree.  Those two changes are contained and easy to wrap
your head around.

Then in the extent-tree-v2 series I actually change the helpers to do the new
crazy stuff, and then go through the places where we do
btrfs_whatever_root(fs_info, 0) and convert those to be compatible with the new
world order.  I do that because each of those spots is it's own deal to convert,
so they need to be reasoned within their context.

I could probably move this around, but we're at like 20 patches per groups of
series, so I did it this way to keep the logical separation as clean as
possible, and drastically reduce the amount of complex things you guys are
needing to look at individually.  Thanks,

Josef