Re: [PATCH 04/17] btrfs: handle checksum validation and repair at the storage layer

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2022/9/5 14:48, Christoph Hellwig wrote:
> On Thu, Sep 01, 2022 at 05:04:34PM +0800, Qu Wenruo wrote:
>> But for the verification part, I still don't like the idea of putting
>> the verification code at endio context at all.
>
> Why?

Mostly due to the fact that metadata and data go split ways for
verification.

All the verification for data happens at endio time.

While part of the verification of metadata (bytenr, csum, level,
tree-checker) goes at endio, but transid, checks against parent are all
done at btrfs_read_extent_buffer() time.

This also means, the read-repair happens at different timing.

>
>> This is especially true when data and metadata are still doing different
>> checksum verfication at different timing.
>
> Note that this does not handle the metadata checksum verification at
> all.  Both because it actually works very different and I could not
> verify that we'd actually always read all data that needs to be verified
> together for metadata, but also because there is zero metadata repair
> coverage in xfstests, so I don't dare to touch that code.
>
>> Can we just let the endio function to do the IO, and let the reader to
>> do the verification after all needed data is read out?
>
> What would the benefit be?  It will lead to a lot of duplicate (and thus
> inconsistent) code that is removed here, and make splitting the bios
> under btrfs_submit_bio much more complicated and expensive.

You're right, my initial suggestion is not good at all.


But what about putting all the needed metadata info (first key, level,
transid etc) also into bbio (using a union to take the same space of
data csum), so that all verification and read repair can happen at endio
time, the same timing as data?

Although this may need to force submitting metadata for every tree
block, I guess it's more or less feasible, since metadata read is more
like a random read, other than sequential read.

By this we can also eliminate the duplicated read-repair between meta
and data.

Thanks,
Qu