Re: [PATCH 00/15] Wire up CRC32 library functions to arch-optimized code

[Ard Biesheuvel <ardb@kernel.org>]

On Mon, 21 Oct 2024 at 02:29, Eric Biggers <ebiggers@kernel.org> wrote:
>
> This patchset is also available in git via:
>
>     git fetch https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git crc32-lib-v1
>
> CRC32 is a family of common non-cryptographic integrity check algorithms
> that are fairly fast with a portable C implementation and become far
> faster still with the CRC32 or carryless multiplication instructions
> that most CPUs have.  9 architectures already have optimized code for at
> least some CRC32 variants; however, except for arm64 this optimized code
> was only accessible through the crypto API, not the library functions.
>
> This patchset fixes that so that the CRC32 library functions use the
> optimized code.  This allows users to just use the library instead of
> the crypto API.  This is much simpler and also improves performance due
> to eliminating the crypto API overhead including an indirect call.  Some
> examples of updating users are included at the end of the patchset.
>
> Note: crc32c() was a weird case.  It was a library function layered on
> top of the crypto API, which in turn is layered on top of the real
> library functions.  So while it was easy to use, it was still subject to
> the crypto API overhead.  This patchset provides CRC32C acceleration in
> the real library functions directly.
>
> The updated CRC32 library design is:
>
> - Each arch's CRC32 code (all variants) is in arch/$ARCH/lib/crc32*.
>   This adopts what arm64 and riscv already did.  Note, the crypto
>   directory is not used because CRC32 is not a cryptographic algorithm.
>
> - Weak symbols are no longer used.  Instead there are crc32*_base() and
>   crc32*_arch(), and the appropriate ones are called based on the
>   kconfig.  This is similar to how the ChaCha20 library code works.
>
> - Each arch's CRC32 code is enabled by default when CRC32 is enabled,
>   but it can now be disabled, controlled by the choice that previously
>   controlled the base implementation only.  It can also now be built as
>   a module if CRC32 is a module too.
>
> - Instead of lots of pointless glue code that wires up each CRC32
>   variant to the crypto API for each architecture, we now just rely on
>   the existing shash algorithms that use the library functions.
>
> - As before, the library functions don't provide access to off-CPU
>   crypto accelerators.  But these appear to have very little, if any,
>   real-world relevance for CRC32 which is very fast on CPUs.
>
> Future work should apply a similar cleanup to crct10dif which is a
> variant of CRC16.
>
> I tested all arches in QEMU using CONFIG_CRC32_SELFTEST and the crypto
> self-tests, except for mips which I couldn't figure out how to do.
>
> This patchset has the following dependencies on recent patches:
>
> - "crypto - move crypto_simd_disabled_for_test to lib"
>   (https://lore.kernel.org/linux-crypto/20241018235343.425758-1-ebiggers@kernel.org/)
> - "crypto: x86/crc32c - jump table elimination and other cleanups"
>   (https://lore.kernel.org/linux-crypto/20241014042447.50197-1-ebiggers@kernel.org/)
> - "arm64: Speed up CRC-32 using PMULL instructions"
>   (https://lore.kernel.org/linux-crypto/20241018075347.2821102-5-ardb+git@google.com/)
> - "crypto: Enable fuzz testing for arch code"
>   (https://lore.kernel.org/linux-crypto/20241016185722.400643-4-ardb+git@google.com/)
> - "crypto: mips/crc32 - fix the CRC32C implementation"
>   (https://lore.kernel.org/linux-crypto/20241020180258.8060-1-ebiggers@kernel.org/)
>
> Everything can be retrieved from git using the command given earlier.
>
> Since this patchset touches many areas, getting it merged may be
> difficult.  One option is a pull request with the whole patchset
> directly to Linus.  Another is to have at least patches 1-2 and the
> above dependencies taken through the crypto tree in v6.13; then the arch
> patches can land separately afterwards, followed by the rest.
>
> Eric Biggers (15):
>   lib/crc32: drop leading underscores from __crc32c_le_base
>   lib/crc32: improve support for arch-specific overrides
>   arm/crc32: expose CRC32 functions through lib
>   loongarch/crc32: expose CRC32 functions through lib
>   mips/crc32: expose CRC32 functions through lib
>   powerpc/crc32: expose CRC32 functions through lib
>   s390/crc32: expose CRC32 functions through lib
>   sparc/crc32: expose CRC32 functions through lib
>   x86/crc32: update prototype for crc_pcl()
>   x86/crc32: update prototype for crc32_pclmul_le_16()
>   x86/crc32: expose CRC32 functions through lib
>   lib/crc32: make crc32c() go directly to lib
>   ext4: switch to using the crc32c library
>   jbd2: switch to using the crc32c library
>   f2fs: switch to using the crc32 library
>
...
>  89 files changed, 1002 insertions(+), 2455 deletions(-)

Very nice cleanup!

For the series:

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>