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
