Re: [PATCH 03/10] btrfs: split btrfs_submit_data_bio

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

On 2022/4/25 19:40, Qu Wenruo wrote:
>
>
> On 2022/4/25 19:34, Christoph Hellwig wrote:
>> On Mon, Apr 25, 2022 at 07:31:08PM +0800, Qu Wenruo wrote:
>>> Then it comes against the btrfs read time repair.
>>>
>>> Currently we split bio to make sure we never need to split bio at
>>> btrfs_map_bio() time.
>>>
>>> But this is against common layer separation.
>>>
>>> And we really want the ability to read a partially corrupted bio (some
>>> part matches csum, some doesn't), no matter if the bio is cloned or not.
>>>
>>> Especially, we already have cloned bio which needs repair (for dio).
>>
>> I have a barely working version based on your patches to split the
>> bio in btrfs_bio_map that solves this problem.  But the next step
>> only removed the save iter for writes, where the only user is
>> index_one_bio.  And the fix for that is pretty trivial :)
>
> That's only for RAID56, aren't you going to remove btrfs_bio usage
> completely for all write (including buffered, non-compressing write)?

Wait, are you going to use some methods like this to avoid
chained/cloned bio while still split the bio?

	page = grab_page_from_existing_bio();
	pgoff = grab_pgoff_from_existing_bio()
	new_bio = bio_alloc()
	bio_add_page(new_bio, page, sectorsize, pgoff);

So that you can create a regular new, non-cloned bio, but still using
the same page/pgoff from an existing bio...

Thanks,
Qu
>
> Thanks,
> Qu
>>
>> ---
>>  From c8fe61748ebc583a7f57c8e5de79f92428e5717c Mon Sep 17 00:00:00 2001
>> From: Christoph Hellwig <hch@lst.de>
>> Date: Mon, 25 Apr 2022 13:23:54 +0200
>> Subject: btrfs: stop looking at btrfs_bio->iter in index_one_bio
>>
>> All the bios that index_one_bio operates on are the bios submitted by the
>> upper layer.  These are never resubmitted to an actual device by the
>> raid56 code, and thus the iter never changes from the initial state.
>> Thus we can always just use bi_iter directly as it will be the same as
>> the saved copy.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> ---
>>   fs/btrfs/raid56.c | 3 ---
>>   1 file changed, 3 deletions(-)
>>
>> diff --git a/fs/btrfs/raid56.c b/fs/btrfs/raid56.c
>> index 1a3c1a9b10d0b..8b40353bb89db 100644
>> --- a/fs/btrfs/raid56.c
>> +++ b/fs/btrfs/raid56.c
>> @@ -1218,9 +1218,6 @@ static void index_one_bio(struct btrfs_raid_bio
>> *rbio, struct bio *bio)
>>       u32 offset = (bio->bi_iter.bi_sector << SECTOR_SHIFT) -
>>                rbio->bioc->raid_map[0];
>> -    if (bio_flagged(bio, BIO_CLONED))
>> -        bio->bi_iter = btrfs_bio(bio)->iter;
>> -
>>       bio_for_each_segment(bvec, bio, iter) {
>>           u32 bvec_offset;
>