Re: [PATCH 03/34] btrfs: add a btrfs_inode pointer to struct btrfs_bio

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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.



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux