Re: [PATCH v3 00/12] btrfs: scrub: use a more reader friendly code to implement scrub_simple_mirror()

[David Sterba <dsterba@suse.cz>]

On Mon, Mar 20, 2023 at 10:12:46AM +0800, Qu Wenruo wrote:
> [TODO]
> 
> - More testing on zoned devices
>   Now the patchset can already pass all scrub/replace groups with
>   regular devices.

I think I noticed some disparity in the old and new code for the zoned
devices. This should be found by testing so I'd add this series to
for-next and see.

> - More cleanup on RAID56 path
>   Now RAID56 still uses some old facility, resulting things like
>   scrub_sector and scrub_bio can not be fully cleaned up.

We can do that incrementally, as long as the raid56 scrub works the
cleanups can be done later, the series changes a lot of code already.

> Qu Wenruo (12):
>   btrfs: scrub: use dedicated super block verification function to scrub
>     one super block
>   btrfs: introduce a new helper to submit bio for scrub
>   btrfs: introduce a new helper to submit write bio for scrub
>   btrfs: scrub: introduce the structure for new BTRFS_STRIPE_LEN based
>     interface
>   btrfs: scrub: introduce a helper to find and fill the sector info for
>     a scrub_stripe
>   btrfs: scrub: introduce a helper to verify one metadata
>   btrfs: scrub: introduce a helper to verify one scrub_stripe
>   btrfs: scrub: introduce the main read repair worker for scrub_stripe
>   btrfs: scrub: introduce a writeback helper for scrub_stripe
>   btrfs: scrub: introduce error reporting functionality for scrub_stripe
>   btrfs: scrub: introduce the helper to queue a stripe for scrub
>   btrfs: scrub: switch scrub_simple_mirror() to scrub_stripe
>     infrastructure

The whole series follows the pattern to first introduce the individual
helpers that can be reviewed separately and then does the switch in one
patch. As the whole scrub IO path is reworked I don't think we can do
much better so clearly separating the old and new logic sounds OK.

One comment I have, the functional switch in the last patch should not
be mixed with deleting of the unused code. As the last patch activates
code from all the previous patches it would also show up in any
bisection as the cause so it would help to narrow the focus only on the
real changes.

There are some minor coding style issues I'll point out under the
patches. I'm assuming that the scrub structure won't change again soon
(the old code is from 3.x times) so let's use this opportunity to make
the style most up to date.