Re: [PATCH v2] btrfs: switch to library APIs for checksums

[Neal Gompa]

On Fri, Dec 5, 2025 at 2:21 AM Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
>
> Make btrfs use the library APIs instead of crypto_shash, for all
> checksum computations.  This has many benefits:
>
> - Allows future checksum types, e.g. XXH3 or CRC64, to be more easily
>   supported.  Only a library API will be needed, not crypto_shash too.
>
> - Eliminates the overhead of the generic crypto layer, including an
>   indirect call for every function call and other API overhead.  A
>   microbenchmark of btrfs_check_read_bio() with crc32c checksums shows a
>   speedup from 658 cycles to 608 cycles per 4096-byte block.
>
> - Decreases the stack usage of btrfs by reducing the size of checksum
>   contexts from 384 bytes to 240 bytes, and by eliminating the need for
>   some functions to declare a checksum context at all.
>
> - Increases reliability.  The library functions always succeed and
>   return void.  In contrast, crypto_shash can fail and return errors.
>   Also, the library functions are guaranteed to be available when btrfs
>   is loaded; there's no longer any need to use module softdeps to try to
>   work around the crypto modules sometimes not being loaded.
>
> - Fixes a bug where blake2b checksums didn't work on kernels booted with
>   fips=1.  Since btrfs checksums are for integrity only, it's fine for
>   them to use non-FIPS-approved algorithms.
>
> Note that with having to handle 4 algorithms instead of just 1-2, this
> commit does result in a slightly positive diffstat.  That being said,
> this wouldn't have been the case if btrfs had actually checked for
> errors from crypto_shash, which technically it should have been doing.
>
> Reviewed-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> ---
>
> v2: rebased onto latest mainline, now that both the crypto library and
>     btrfs pull requests for 6.19 have been merged
>
>  fs/btrfs/Kconfig       |  8 ++--
>  fs/btrfs/compression.c |  1 -
>  fs/btrfs/disk-io.c     | 68 ++++++++----------------------
>  fs/btrfs/file-item.c   |  4 --
>  fs/btrfs/fs.c          | 96 ++++++++++++++++++++++++++++++++++++------
>  fs/btrfs/fs.h          | 23 +++++++---
>  fs/btrfs/inode.c       | 10 ++---
>  fs/btrfs/scrub.c       | 16 +++----
>  fs/btrfs/super.c       |  4 --
>  fs/btrfs/sysfs.c       |  6 +--
>  10 files changed, 133 insertions(+), 103 deletions(-)
>

This patch looks reasonable to me and seems to work as expected.

Reviewed-by: Neal Gompa <neal@xxxxxxxxx>


-- 
真実はいつも一つ！/ Always, there's only one truth!