Re: [PATCH 03/10] btrfs: offload all write I/O completions to a workqueue

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

On 2023/3/25 16:15, Christoph Hellwig wrote:
> On Fri, Mar 24, 2023 at 09:25:07AM -0400, Chris Mason wrote:
>> As you mentioned above, we're currently doing synchronous crcs for
>> metadata when BTRFS_FS_CSUM_IMPL_FAST, and for synchronous writes.
>> We've benchmarked this a few times and I think with modern hardware a
>> better default is just always doing synchronous crcs for data too.
>>
>> Qu or David have you looked synchronous data crcs recently?
> 
> As mentioend in the other mail I have a bit.  But only for crc32
> so far, and only on x86, so the matrix might be a little more
> complicated.

Haven't really looked up the async csum part at all.

In fact I'm not even sure what's the point of generating data checksum 
asynchronously...

I didn't see any extra split to get multiple threads to calculate hash 
on different parts.
Furthermore, I'm not sure even if all the supported hash algos 
(CRC32/SHA256/BLAKE2/XXHASH) can support such split and merge 
multi-threading.
(IIRC SHA256 can not?)

Thus I'm not really sure of the benefit of async csum generation anyway.
(Not to mention asynchronous code itself is not straightforward)

Considering at least we used to do data csum verification in endio 
function synchronously, I see no problem to do the generation path 
synchronously, even without hardware accelerated checksum support.

Thanks,
Qu

> 
>> My preference would be:
>>
>> - crcs are always inline if your hardware is fast
>> - Compression, encryption, slow hardware crcs use the thread_pool_size knob
>> - We don't try to limit the other workers
>>
>> The idea behind the knob is just "how much of your CPU should each btrfs
>> mount consume?"  Obviously we'll silently judge anyone that chooses less
>> than 100% but I guess it's good to give people options.
> 
> Ok.  I'll do a series about the nobs ASAP, and then the inline CRCs
> next.