On 2023/3/8 06:21, Qu Wenruo wrote:
On 2023/3/7 22:41, Christoph Hellwig wrote:
On Tue, Mar 07, 2023 at 09:44:32AM +0800, Qu Wenruo wrote:
With my recent restart on scrub rework, this patch makes me wonder,
what if
scrub wants to use btrfs_bio, but don't want to pass a valid btrfs_inode
pointer?
The full inode is only really needed for the data repair code. But a lot
of code uses the fs_info, which would have to be added as a separate
counter. The other usage is the sync_writers counter, which is a bit
odd and should probably be keyed off the REQ_SYNC flag instead.
E.g. scrub code just wants to read certain mirror of a logical bytenr.
This can simplify the handling of RAID56, as for data stripes the repair
path is the same, just try the next mirror(s).
Furthermore most of the new btrfs_bio code is handling data reads by
triggering read-repair automatically.
This can be unnecessary for scrub.
This sounds like you don't want to use the btrfs_bio at all as you
don't rely on any of the functionality from it.
Well, to me the proper mirror_num based read is the core of btrfs_bio,
not the read-repair thing.
Thus I'm not that convinced fully automatic read-repair integrated into
btrfs_bio is a good idea.
BTW, I also checked if I can craft a scrub specific version of
btrfs_submit_bio().
The result doesn't look good at all.
Without a btrfs_bio structure, it's already pretty hard to properly put
bioc, decrease the bio counter.
Or I need to create a scrub_bio, and re-implement all the needed endio
function handling.
So please really consider the simplest case, one just wants to
read/write some data using logical + mirror_num, without any btrfs inode
nor csum verification.
Thanks,
Qu
Thanks,
Qu
And since we're here, can we also have btrfs equivalent of on-stack bio?
As scrub can benefit a lot from that, as for sector-by-sector read,
we want
to avoid repeating allocating/freeing a btrfs_bio just for reading one
sector.
(The existing behavior is using on-stack bio with bio_init/bio_uninit
inside scrub_recheck_block())
You can do that right now by declaring a btrfs_bio on-stack and then
calling bio_init on the embedded bio followed by a btrfs_bio_init on
the btrfs_bio. But I don't think doing this will actually be a win
for the scrub code in terms of speed or code size.
