Re: [LSF/MM/BPF TOPIC] File system checksum offload

[Kanchan Joshi <joshi.k@samsung.com>]

On 2/3/2025 1:17 PM, Johannes Thumshirn wrote:

>>> I wouldn't call this "file system offload".  Enabling the data
>>> integrity feature or whatever you want to call it is really a block
>>> layer issue.  The file system doesn't need to get involved at all.
>>> Indeed, looking the patch, the only reason why the file system is
>>> getting involved is because (a) you've added a mount option, and (b)
>>> the mount option flips a bit in the bio that gets sent to the block
>>> layer.
>> Mount option was only for the RFC. If everything else gets sorted, it
>> would be about choosing whatever is liked by the Btrfs.
>>      > But this could also be done by adding a queue specific flag, at which
>>> point the file system doesn't need to be involved at all.  Why would
>>> you want to enable the data ingregity feature on a per block I/O
>>> basis, if the device supports it?
>> Because I thought users (filesystems) would prefer flexibility. Per-IO
>> control helps to choose different policy for say data and meta. Let me
>> outline the differences.
> But data and metadata checksums are handled differently in btrfs. For
> data we checksum each block and write it into the checksum tree. For
> metadata the checksum is part of the metadata (see 'struct btrfs_header').

Yes, I am aware that btrfs meta checksum is put into an exiting field in 
the B-tree header. It only affects the compute part, and not the 
write-amplification part unlike the data checksums. That is exactly why 
I did not find it worth to optimize in the existing RFC.


>> Block-layer auto integrity
>> - always attaches integrity-payload for each I/O.
>> - it does compute checksum/reftag for each I/O. And this part does not
>> do justice to the label 'offload'.
>>
>> The patches make auto-integrity
>> - attach the integrity-buffer only if the device configuration demands.
>> - never compute checksum/reftag at the block-layer.
>> - keeps the offload choice at per I/O level.
>>
>> Btrfs checksum tree is created only for data blocks, so the patches
>> apply the flag (REQ_INTEGRITY_OFFLOAD) on that. While metadata blocks,
>> which maybe more important, continue to get checksummed at two levels
>> (block and device).
> The thing I don't like with the current RFC patchset is, it breaks
> scrub, repair and device error statistics. It nothing that can't be
> solved though. But as of now it just doesn't make any sense at all to
> me. We at least need the FS to look at the BLK_STS_PROTECTION return and
> handle accordingly in scrub, read repair and statistics.
> 
> And that's only for feature parity. I'd also like to see some
> performance numbers and numbers of reduced WAF, if this is really worth
> the hassle.

Have you seen the numbers mentioned in the cover letter of the RFC?
For direct IO: IOPS increase from 144K to 172K, and extra-writes 
reduction from 66GB to 9GB.

Something similar for buffered IO:
https://lore.kernel.org/linux-block/20250129140207.22718-1-joshi.k@samsung.com/