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. > > 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.
