Re: [PATCH v5 03/13] btrfs: introduce a new helper to submit read bio for scrub

From: Qu Wenruo <wqu@suse.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: linux-btrfs@vger.kernel.org, David Sterba <dsterba@suse.com>
Subject: Re: [PATCH v5 03/13] btrfs: introduce a new helper to submit read bio for scrub
Date: Tue, 28 Mar 2023 08:16:38 +0800	[thread overview]
Message-ID: <cd8a91ee-2e30-9829-b50d-599fab3fb490@suse.com> (raw)
In-Reply-To: <ZCIoQLysbLrQW0pX@infradead.org>

On 2023/3/28 07:35, Christoph Hellwig wrote:
> On Tue, Mar 28, 2023 at 07:30:53AM +0800, Qu Wenruo wrote:
>> The new helper, btrfs_submit_scrub_read(), would be mostly a subset of
>> btrfs_submit_bio(), with the following limitations:
>>
>> - Only supports read
>> - @mirror_num must be > 0
>> - No read-time repair nor checksum verification
>> - The @bbio must not cross stripe boundary
>>
>> This would provide the basis for unified read repair for scrub, as we no
>> longer needs to handle RAID56 recovery all by scrub, and RAID56 data
>> stripes scrub can share the same code of read and repair.
>>
>> The repair part would be the same as non-RAID56, as we only need to try
>> the next mirror.
> 
> Stupid question: what do we actually still need this and the write helper
> for now that I think the generic helpers should work just fine without
> bbio->inode?

For read part, as long as we skip all the csum verification and csum 
search, it can go the generic helper.
Although this may needs some coordinate during merge.

But in the future, mostly for the ZNS RST patches, we need special 
handling for the garbage range.

Johannes is already testing his RST branches upon this series, and he is 
hitting the ASSERT() on the mapped length due to RST changes.
In that case, we need to skip certain ranges, and that's specific to 
scrub usage.

Thus for read part, I'd still prefer a scrub specific helper, for the 
incoming RST expansion.

For the write helper, it's very different.

The btrfs_submit_bio() would duplicate the writes to all mirrors, which 
is exactly what I want to avoid for scrub usage.
Thus the scrub specific write helper would always map the write to a 
single stripe, even for RAID56 writes.

So the write helper must stay.

Thanks,
Qu