[PATCH 0/2] Simplify the shash wrappers for the CRC32 library

[PATCH 0/2] Simplify the shash wrappers for the CRC32 library

[Eric Biggers]

This series simplifies how the CRC32 library functions are exposed
through the crypto_shash API.  We'll now have just one shash algorithm
each for "crc32" and "crc32c", and their driver names will just always
be "crc32-lib" and "crc32c-lib" respectively.  This seems to be all
that's actually needed.

As mentioned in patch 2, this does change the content of
/sys/fs/btrfs/$uuid/checksum again, but that should be fine.

This is based on v6.16-rc1, and I'm planning to take these patches
through the crc-next tree.  These supersede
https://lore.kernel.org/r/20250601224441.778374-2-ebiggers@kernel.org/
and
https://lore.kernel.org/r/20250601224441.778374-3-ebiggers@kernel.org/,
and they fix the warning in the full crypto self-tests reported at
https://lore.kernel.org/r/aExLZaoBCg55rZWJ@li-008a6a4c-3549-11b2-a85c-c5cc2836eea2.ibm.com/

Eric Biggers (2):
  btrfs: stop parsing crc32c driver name
  crypto/crc32[c]: register only "-lib" drivers

 crypto/Makefile    |  2 --
 crypto/crc32.c     | 65 +++++----------------------------------------
 crypto/crc32c.c    | 66 ++++------------------------------------------
 crypto/testmgr.c   |  2 ++
 fs/btrfs/disk-io.c |  8 ++----
 5 files changed, 15 insertions(+), 128 deletions(-)


base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
-- 
2.49.0

[PATCH 1/2] btrfs: stop parsing crc32c driver name

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

To determine whether the crc32c implementation is "fast", use
crc32_optimizations() instead of parsing the crypto_shash driver name.
This keeps the code working as intended after the driver name is changed
by the next patch.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/btrfs/disk-io.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 1beb9458f622a..7bb453b69639c 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2024,18 +2024,14 @@ static int btrfs_init_csum_hash(struct btrfs_fs_info *fs_info, u16 csum_type)
 		return PTR_ERR(csum_shash);
 	}
 
 	fs_info->csum_shash = csum_shash;
 
-	/*
-	 * Check if the checksum implementation is a fast accelerated one.
-	 * As-is this is a bit of a hack and should be replaced once the csum
-	 * implementations provide that information themselves.
-	 */
+	/* Check if the checksum implementation is a fast accelerated one. */
 	switch (csum_type) {
 	case BTRFS_CSUM_TYPE_CRC32:
-		if (!strstr(crypto_shash_driver_name(csum_shash), "generic"))
+		if (crc32_optimizations() & CRC32C_OPTIMIZATION)
 			set_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags);
 		break;
 	case BTRFS_CSUM_TYPE_XXHASH:
 		set_bit(BTRFS_FS_CSUM_IMPL_FAST, &fs_info->flags);
 		break;
-- 
2.49.0

[PATCH 2/2] crypto/crc32[c]: register only "-lib" drivers

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

For the "crc32" and "crc32c" shash algorithms, instead of registering
"*-generic" drivers as well as conditionally registering "*-$(ARCH)"
drivers, instead just register "*-lib" drivers.  These just use the
regular library functions crc32_le() and crc32c(), so they just do the
right thing and are fully accelerated when supported by the CPU.

This eliminates the need for the CRC library to export crc32_le_base()
and crc32c_base().  Separate patches make those static functions.

Since this patch removes the "crc32-generic" and "crc32c-generic" driver
names which crypto/testmgr.c expects to exist, update crypto/testmgr.c
accordingly.  This does mean that crypto/testmgr.c will no longer
fuzz-test the "generic" implementation against the "arch" implementation
for crc32 and crc32c, but this was redundant with crc_kunit anyway.

Besides the above, and btrfs_init_csum_hash() which the previous patch
fixed, no code appears to have been relying on the "crc32-generic" or
"crc32c-generic" driver names specifically.

btrfs does export the checksum driver name in
/sys/fs/btrfs/$uuid/checksum.  This patch makes that file contain
"crc32c-lib" instead of "crc32c-generic" or "crc32c-$(ARCH)".  This
should be fine, since in practice the purpose of this file seems to have
been just to allow users to manually check whether they needed to enable
the optimized CRC32C code.  This was needed only because of the bug in
old kernels where the optimized CRC32C code defaulted to off and even
needed to be explicitly added to the ramdisk to be used.  Now that it
just works in Linux 6.14 and later, there's no need for users to take
any action and this file is basically obsolete.  (Also, note that the
contents of this file already changed in 6.14.)

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 crypto/Makefile  |  2 --
 crypto/crc32.c   | 65 +++++------------------------------------------
 crypto/crc32c.c  | 66 ++++--------------------------------------------
 crypto/testmgr.c |  2 ++
 4 files changed, 13 insertions(+), 122 deletions(-)

diff --git a/crypto/Makefile b/crypto/Makefile
index 017df3a2e4bb3..55dd56332dc80 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -152,14 +152,12 @@ obj-$(CONFIG_CRYPTO_CHACHA20) += chacha.o
 CFLAGS_chacha.o += -DARCH=$(ARCH)
 obj-$(CONFIG_CRYPTO_DEFLATE) += deflate.o
 obj-$(CONFIG_CRYPTO_MICHAEL_MIC) += michael_mic.o
 obj-$(CONFIG_CRYPTO_CRC32C) += crc32c-cryptoapi.o
 crc32c-cryptoapi-y := crc32c.o
-CFLAGS_crc32c.o += -DARCH=$(ARCH)
 obj-$(CONFIG_CRYPTO_CRC32) += crc32-cryptoapi.o
 crc32-cryptoapi-y := crc32.o
-CFLAGS_crc32.o += -DARCH=$(ARCH)
 obj-$(CONFIG_CRYPTO_AUTHENC) += authenc.o authencesn.o
 obj-$(CONFIG_CRYPTO_KRB5ENC) += krb5enc.o
 obj-$(CONFIG_CRYPTO_LZO) += lzo.o lzo-rle.o
 obj-$(CONFIG_CRYPTO_LZ4) += lz4.o
 obj-$(CONFIG_CRYPTO_LZ4HC) += lz4hc.o
diff --git a/crypto/crc32.c b/crypto/crc32.c
index cc371d42601fd..489cbed9422e2 100644
--- a/crypto/crc32.c
+++ b/crypto/crc32.c
@@ -57,33 +57,16 @@ static int crc32_init(struct shash_desc *desc)
 static int crc32_update(struct shash_desc *desc, const u8 *data,
 			unsigned int len)
 {
 	u32 *crcp = shash_desc_ctx(desc);
 
-	*crcp = crc32_le_base(*crcp, data, len);
-	return 0;
-}
-
-static int crc32_update_arch(struct shash_desc *desc, const u8 *data,
-			     unsigned int len)
-{
-	u32 *crcp = shash_desc_ctx(desc);
-
 	*crcp = crc32_le(*crcp, data, len);
 	return 0;
 }
 
 /* No final XOR 0xFFFFFFFF, like crc32_le */
-static int __crc32_finup(u32 *crcp, const u8 *data, unsigned int len,
-			 u8 *out)
-{
-	put_unaligned_le32(crc32_le_base(*crcp, data, len), out);
-	return 0;
-}
-
-static int __crc32_finup_arch(u32 *crcp, const u8 *data, unsigned int len,
-			      u8 *out)
+static int __crc32_finup(u32 *crcp, const u8 *data, unsigned int len, u8 *out)
 {
 	put_unaligned_le32(crc32_le(*crcp, data, len), out);
 	return 0;
 }
 
@@ -91,16 +74,10 @@ static int crc32_finup(struct shash_desc *desc, const u8 *data,
 		       unsigned int len, u8 *out)
 {
 	return __crc32_finup(shash_desc_ctx(desc), data, len, out);
 }
 
-static int crc32_finup_arch(struct shash_desc *desc, const u8 *data,
-		       unsigned int len, u8 *out)
-{
-	return __crc32_finup_arch(shash_desc_ctx(desc), data, len, out);
-}
-
 static int crc32_final(struct shash_desc *desc, u8 *out)
 {
 	u32 *crcp = shash_desc_ctx(desc);
 
 	put_unaligned_le32(*crcp, out);
@@ -111,72 +88,42 @@ static int crc32_digest(struct shash_desc *desc, const u8 *data,
 			unsigned int len, u8 *out)
 {
 	return __crc32_finup(crypto_shash_ctx(desc->tfm), data, len, out);
 }
 
-static int crc32_digest_arch(struct shash_desc *desc, const u8 *data,
-			     unsigned int len, u8 *out)
-{
-	return __crc32_finup_arch(crypto_shash_ctx(desc->tfm), data, len, out);
-}
-
-static struct shash_alg algs[] = {{
+static struct shash_alg alg = {
 	.setkey			= crc32_setkey,
 	.init			= crc32_init,
 	.update			= crc32_update,
 	.final			= crc32_final,
 	.finup			= crc32_finup,
 	.digest			= crc32_digest,
 	.descsize		= sizeof(u32),
 	.digestsize		= CHKSUM_DIGEST_SIZE,
 
 	.base.cra_name		= "crc32",
-	.base.cra_driver_name	= "crc32-generic",
+	.base.cra_driver_name	= "crc32-lib",
 	.base.cra_priority	= 100,
 	.base.cra_flags		= CRYPTO_ALG_OPTIONAL_KEY,
 	.base.cra_blocksize	= CHKSUM_BLOCK_SIZE,
 	.base.cra_ctxsize	= sizeof(u32),
 	.base.cra_module	= THIS_MODULE,
 	.base.cra_init		= crc32_cra_init,
-}, {
-	.setkey			= crc32_setkey,
-	.init			= crc32_init,
-	.update			= crc32_update_arch,
-	.final			= crc32_final,
-	.finup			= crc32_finup_arch,
-	.digest			= crc32_digest_arch,
-	.descsize		= sizeof(u32),
-	.digestsize		= CHKSUM_DIGEST_SIZE,
-
-	.base.cra_name		= "crc32",
-	.base.cra_driver_name	= "crc32-" __stringify(ARCH),
-	.base.cra_priority	= 150,
-	.base.cra_flags		= CRYPTO_ALG_OPTIONAL_KEY,
-	.base.cra_blocksize	= CHKSUM_BLOCK_SIZE,
-	.base.cra_ctxsize	= sizeof(u32),
-	.base.cra_module	= THIS_MODULE,
-	.base.cra_init		= crc32_cra_init,
-}};
-
-static int num_algs;
+};
 
 static int __init crc32_mod_init(void)
 {
-	/* register the arch flavor only if it differs from the generic one */
-	num_algs = 1 + ((crc32_optimizations() & CRC32_LE_OPTIMIZATION) != 0);
-
-	return crypto_register_shashes(algs, num_algs);
+	return crypto_register_shash(&alg);
 }
 
 static void __exit crc32_mod_fini(void)
 {
-	crypto_unregister_shashes(algs, num_algs);
+	crypto_unregister_shash(&alg);
 }
 
 module_init(crc32_mod_init);
 module_exit(crc32_mod_fini);
 
 MODULE_AUTHOR("Alexander Boyko <alexander_boyko@xyratex.com>");
 MODULE_DESCRIPTION("CRC32 calculations wrapper for lib/crc32");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS_CRYPTO("crc32");
-MODULE_ALIAS_CRYPTO("crc32-generic");
diff --git a/crypto/crc32c.c b/crypto/crc32c.c
index e5377898414a2..1eff54dde2f74 100644
--- a/crypto/crc32c.c
+++ b/crypto/crc32c.c
@@ -83,19 +83,10 @@ static int chksum_setkey(struct crypto_shash *tfm, const u8 *key,
 static int chksum_update(struct shash_desc *desc, const u8 *data,
 			 unsigned int length)
 {
 	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
 
-	ctx->crc = crc32c_base(ctx->crc, data, length);
-	return 0;
-}
-
-static int chksum_update_arch(struct shash_desc *desc, const u8 *data,
-			      unsigned int length)
-{
-	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
-
 	ctx->crc = crc32c(ctx->crc, data, length);
 	return 0;
 }
 
 static int chksum_final(struct shash_desc *desc, u8 *out)
@@ -105,17 +96,10 @@ static int chksum_final(struct shash_desc *desc, u8 *out)
 	put_unaligned_le32(~ctx->crc, out);
 	return 0;
 }
 
 static int __chksum_finup(u32 *crcp, const u8 *data, unsigned int len, u8 *out)
-{
-	put_unaligned_le32(~crc32c_base(*crcp, data, len), out);
-	return 0;
-}
-
-static int __chksum_finup_arch(u32 *crcp, const u8 *data, unsigned int len,
-			       u8 *out)
 {
 	put_unaligned_le32(~crc32c(*crcp, data, len), out);
 	return 0;
 }
 
@@ -125,98 +109,58 @@ static int chksum_finup(struct shash_desc *desc, const u8 *data,
 	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
 
 	return __chksum_finup(&ctx->crc, data, len, out);
 }
 
-static int chksum_finup_arch(struct shash_desc *desc, const u8 *data,
-			     unsigned int len, u8 *out)
-{
-	struct chksum_desc_ctx *ctx = shash_desc_ctx(desc);
-
-	return __chksum_finup_arch(&ctx->crc, data, len, out);
-}
-
 static int chksum_digest(struct shash_desc *desc, const u8 *data,
 			 unsigned int length, u8 *out)
 {
 	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
 
 	return __chksum_finup(&mctx->key, data, length, out);
 }
 
-static int chksum_digest_arch(struct shash_desc *desc, const u8 *data,
-			      unsigned int length, u8 *out)
-{
-	struct chksum_ctx *mctx = crypto_shash_ctx(desc->tfm);
-
-	return __chksum_finup_arch(&mctx->key, data, length, out);
-}
-
 static int crc32c_cra_init(struct crypto_tfm *tfm)
 {
 	struct chksum_ctx *mctx = crypto_tfm_ctx(tfm);
 
 	mctx->key = ~0;
 	return 0;
 }
 
-static struct shash_alg algs[] = {{
+static struct shash_alg alg = {
 	.digestsize		= CHKSUM_DIGEST_SIZE,
 	.setkey			= chksum_setkey,
 	.init			= chksum_init,
 	.update			= chksum_update,
 	.final			= chksum_final,
 	.finup			= chksum_finup,
 	.digest			= chksum_digest,
 	.descsize		= sizeof(struct chksum_desc_ctx),
 
 	.base.cra_name		= "crc32c",
-	.base.cra_driver_name	= "crc32c-generic",
+	.base.cra_driver_name	= "crc32c-lib",
 	.base.cra_priority	= 100,
 	.base.cra_flags		= CRYPTO_ALG_OPTIONAL_KEY,
 	.base.cra_blocksize	= CHKSUM_BLOCK_SIZE,
 	.base.cra_ctxsize	= sizeof(struct chksum_ctx),
 	.base.cra_module	= THIS_MODULE,
 	.base.cra_init		= crc32c_cra_init,
-}, {
-	.digestsize		= CHKSUM_DIGEST_SIZE,
-	.setkey			= chksum_setkey,
-	.init			= chksum_init,
-	.update			= chksum_update_arch,
-	.final			= chksum_final,
-	.finup			= chksum_finup_arch,
-	.digest			= chksum_digest_arch,
-	.descsize		= sizeof(struct chksum_desc_ctx),
-
-	.base.cra_name		= "crc32c",
-	.base.cra_driver_name	= "crc32c-" __stringify(ARCH),
-	.base.cra_priority	= 150,
-	.base.cra_flags		= CRYPTO_ALG_OPTIONAL_KEY,
-	.base.cra_blocksize	= CHKSUM_BLOCK_SIZE,
-	.base.cra_ctxsize	= sizeof(struct chksum_ctx),
-	.base.cra_module	= THIS_MODULE,
-	.base.cra_init		= crc32c_cra_init,
-}};
-
-static int num_algs;
+};
 
 static int __init crc32c_mod_init(void)
 {
-	/* register the arch flavor only if it differs from the generic one */
-	num_algs = 1 + ((crc32_optimizations() & CRC32C_OPTIMIZATION) != 0);
-
-	return crypto_register_shashes(algs, num_algs);
+	return crypto_register_shash(&alg);
 }
 
 static void __exit crc32c_mod_fini(void)
 {
-	crypto_unregister_shashes(algs, num_algs);
+	crypto_unregister_shash(&alg);
 }
 
 module_init(crc32c_mod_init);
 module_exit(crc32c_mod_fini);
 
 MODULE_AUTHOR("Clay Haapala <chaapala@cisco.com>");
 MODULE_DESCRIPTION("CRC32c (Castagnoli) calculations wrapper for lib/crc32c");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS_CRYPTO("crc32c");
-MODULE_ALIAS_CRYPTO("crc32c-generic");
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 72005074a5c26..3b947b828ff83 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -4544,17 +4544,19 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.suite = {
 			.hash = __VECS(sm4_cmac128_tv_template)
 		}
 	}, {
 		.alg = "crc32",
+		.generic_driver = "crc32-lib",
 		.test = alg_test_hash,
 		.fips_allowed = 1,
 		.suite = {
 			.hash = __VECS(crc32_tv_template)
 		}
 	}, {
 		.alg = "crc32c",
+		.generic_driver = "crc32c-lib",
 		.test = alg_test_crc32c,
 		.fips_allowed = 1,
 		.suite = {
 			.hash = __VECS(crc32c_tv_template)
 		}
-- 
2.49.0

Re: [PATCH 0/2] Simplify the shash wrappers for the CRC32 library

[Eric Biggers]

On Fri, Jun 13, 2025 at 11:37:51AM -0700, Eric Biggers wrote:
> This series simplifies how the CRC32 library functions are exposed
> through the crypto_shash API.  We'll now have just one shash algorithm
> each for "crc32" and "crc32c", and their driver names will just always
> be "crc32-lib" and "crc32c-lib" respectively.  This seems to be all
> that's actually needed.
> 
> As mentioned in patch 2, this does change the content of
> /sys/fs/btrfs/$uuid/checksum again, but that should be fine.
> 
> This is based on v6.16-rc1, and I'm planning to take these patches
> through the crc-next tree.  These supersede
> https://lore.kernel.org/r/20250601224441.778374-2-ebiggers@kernel.org/
> and
> https://lore.kernel.org/r/20250601224441.778374-3-ebiggers@kernel.org/,
> and they fix the warning in the full crypto self-tests reported at
> https://lore.kernel.org/r/aExLZaoBCg55rZWJ@li-008a6a4c-3549-11b2-a85c-c5cc2836eea2.ibm.com/
> 
> Eric Biggers (2):
>   btrfs: stop parsing crc32c driver name
>   crypto/crc32[c]: register only "-lib" drivers

FYI, applied to
https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=crc-next
as per the plan above.  An ack from the btrfs folks on both patches would be
appreciated, though.

- Eric

Re: [PATCH 2/2] crypto/crc32[c]: register only "-lib" drivers

[David Sterba]

On Fri, Jun 13, 2025 at 11:37:53AM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> For the "crc32" and "crc32c" shash algorithms, instead of registering
> "*-generic" drivers as well as conditionally registering "*-$(ARCH)"
> drivers, instead just register "*-lib" drivers.  These just use the
> regular library functions crc32_le() and crc32c(), so they just do the
> right thing and are fully accelerated when supported by the CPU.
> 
> This eliminates the need for the CRC library to export crc32_le_base()
> and crc32c_base().  Separate patches make those static functions.
> 
> Since this patch removes the "crc32-generic" and "crc32c-generic" driver
> names which crypto/testmgr.c expects to exist, update crypto/testmgr.c
> accordingly.  This does mean that crypto/testmgr.c will no longer
> fuzz-test the "generic" implementation against the "arch" implementation
> for crc32 and crc32c, but this was redundant with crc_kunit anyway.
> 
> Besides the above, and btrfs_init_csum_hash() which the previous patch
> fixed, no code appears to have been relying on the "crc32-generic" or
> "crc32c-generic" driver names specifically.
> 
> btrfs does export the checksum driver name in
> /sys/fs/btrfs/$uuid/checksum.  This patch makes that file contain
> "crc32c-lib" instead of "crc32c-generic" or "crc32c-$(ARCH)".  This
> should be fine, since in practice the purpose of this file seems to have
> been just to allow users to manually check whether they needed to enable
> the optimized CRC32C code.  This was needed only because of the bug in
> old kernels where the optimized CRC32C code defaulted to off and even
> needed to be explicitly added to the ramdisk to be used.  Now that it
> just works in Linux 6.14 and later, there's no need for users to take
> any action and this file is basically obsolete.

Well, not the whole file, because it says which checksumming algo is
used for the filesystem, but the implementation part is.

Re: [PATCH 0/2] Simplify the shash wrappers for the CRC32 library

[David Sterba]

On Fri, Jun 13, 2025 at 11:37:51AM -0700, Eric Biggers wrote:
> This series simplifies how the CRC32 library functions are exposed
> through the crypto_shash API.  We'll now have just one shash algorithm
> each for "crc32" and "crc32c", and their driver names will just always
> be "crc32-lib" and "crc32c-lib" respectively.  This seems to be all
> that's actually needed.
> 
> As mentioned in patch 2, this does change the content of
> /sys/fs/btrfs/$uuid/checksum again, but that should be fine.

Yes, this is fine, I don't think any ABI applies here and the
implementation was only informative.

> This is based on v6.16-rc1, and I'm planning to take these patches
> through the crc-next tree.  These supersede
> https://lore.kernel.org/r/20250601224441.778374-2-ebiggers@kernel.org/
> and
> https://lore.kernel.org/r/20250601224441.778374-3-ebiggers@kernel.org/,
> and they fix the warning in the full crypto self-tests reported at
> https://lore.kernel.org/r/aExLZaoBCg55rZWJ@li-008a6a4c-3549-11b2-a85c-c5cc2836eea2.ibm.com/
> 
> Eric Biggers (2):
>   btrfs: stop parsing crc32c driver name
>   crypto/crc32[c]: register only "-lib" drivers

Acked-by: David Sterba <dsterba@suse.com>

Thanks.

Re: [PATCH 2/2] crypto/crc32[c]: register only "-lib" drivers

[Eric Biggers]

On Tue, Jun 17, 2025 at 10:17:48PM +0200, David Sterba wrote:
> On Fri, Jun 13, 2025 at 11:37:53AM -0700, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > For the "crc32" and "crc32c" shash algorithms, instead of registering
> > "*-generic" drivers as well as conditionally registering "*-$(ARCH)"
> > drivers, instead just register "*-lib" drivers.  These just use the
> > regular library functions crc32_le() and crc32c(), so they just do the
> > right thing and are fully accelerated when supported by the CPU.
> > 
> > This eliminates the need for the CRC library to export crc32_le_base()
> > and crc32c_base().  Separate patches make those static functions.
> > 
> > Since this patch removes the "crc32-generic" and "crc32c-generic" driver
> > names which crypto/testmgr.c expects to exist, update crypto/testmgr.c
> > accordingly.  This does mean that crypto/testmgr.c will no longer
> > fuzz-test the "generic" implementation against the "arch" implementation
> > for crc32 and crc32c, but this was redundant with crc_kunit anyway.
> > 
> > Besides the above, and btrfs_init_csum_hash() which the previous patch
> > fixed, no code appears to have been relying on the "crc32-generic" or
> > "crc32c-generic" driver names specifically.
> > 
> > btrfs does export the checksum driver name in
> > /sys/fs/btrfs/$uuid/checksum.  This patch makes that file contain
> > "crc32c-lib" instead of "crc32c-generic" or "crc32c-$(ARCH)".  This
> > should be fine, since in practice the purpose of this file seems to have
> > been just to allow users to manually check whether they needed to enable
> > the optimized CRC32C code.  This was needed only because of the bug in
> > old kernels where the optimized CRC32C code defaulted to off and even
> > needed to be explicitly added to the ramdisk to be used.  Now that it
> > just works in Linux 6.14 and later, there's no need for users to take
> > any action and this file is basically obsolete.
> 
> Well, not the whole file, because it says which checksumming algo is
> used for the filesystem, but the implementation part is.

Oh, right.  It's one of those sysfs files that don't follow the normal sysfs
convention and contain multiple values.  I'll update the paragraph above to
clarify that it's referring to the driver name part of the file.

- Eric

Re: [PATCH 2/2] crypto/crc32[c]: register only "-lib" drivers

[Eric Biggers]

On Tue, Jun 17, 2025 at 01:20:50PM -0700, Eric Biggers wrote:
> On Tue, Jun 17, 2025 at 10:17:48PM +0200, David Sterba wrote:
> > On Fri, Jun 13, 2025 at 11:37:53AM -0700, Eric Biggers wrote:
> > > From: Eric Biggers <ebiggers@google.com>
> > > 
> > > For the "crc32" and "crc32c" shash algorithms, instead of registering
> > > "*-generic" drivers as well as conditionally registering "*-$(ARCH)"
> > > drivers, instead just register "*-lib" drivers.  These just use the
> > > regular library functions crc32_le() and crc32c(), so they just do the
> > > right thing and are fully accelerated when supported by the CPU.
> > > 
> > > This eliminates the need for the CRC library to export crc32_le_base()
> > > and crc32c_base().  Separate patches make those static functions.
> > > 
> > > Since this patch removes the "crc32-generic" and "crc32c-generic" driver
> > > names which crypto/testmgr.c expects to exist, update crypto/testmgr.c
> > > accordingly.  This does mean that crypto/testmgr.c will no longer
> > > fuzz-test the "generic" implementation against the "arch" implementation
> > > for crc32 and crc32c, but this was redundant with crc_kunit anyway.
> > > 
> > > Besides the above, and btrfs_init_csum_hash() which the previous patch
> > > fixed, no code appears to have been relying on the "crc32-generic" or
> > > "crc32c-generic" driver names specifically.
> > > 
> > > btrfs does export the checksum driver name in
> > > /sys/fs/btrfs/$uuid/checksum.  This patch makes that file contain
> > > "crc32c-lib" instead of "crc32c-generic" or "crc32c-$(ARCH)".  This
> > > should be fine, since in practice the purpose of this file seems to have
> > > been just to allow users to manually check whether they needed to enable
> > > the optimized CRC32C code.  This was needed only because of the bug in
> > > old kernels where the optimized CRC32C code defaulted to off and even
> > > needed to be explicitly added to the ramdisk to be used.  Now that it
> > > just works in Linux 6.14 and later, there's no need for users to take
> > > any action and this file is basically obsolete.
> > 
> > Well, not the whole file, because it says which checksumming algo is
> > used for the filesystem, but the implementation part is.
> 
> Oh, right.  It's one of those sysfs files that don't follow the normal sysfs
> convention and contain multiple values.  I'll update the paragraph above to
> clarify that it's referring to the driver name part of the file.

I revised it to:

btrfs does export the checksum name and checksum driver name in
/sys/fs/btrfs/$uuid/checksum.  This commit makes the driver name portion
of that file contain "crc32c-lib" instead of "crc32c-generic" or
"crc32c-$(ARCH)".  This should be fine, since in practice the purpose of
the driver name portion of this file seems to have been just to allow
users to manually check whether they needed to enable the optimized
CRC32C code.  This was needed only because of the bug in old kernels
where the optimized CRC32C code defaulted to off and even needed to be
explicitly added to the ramdisk to be used.  Now that it just works in
Linux 6.14 and later, there's no need for users to take any action and
the driver name portion of this is basically obsolete.  (Also, note that
the crc32c driver name already changed in 6.14.)

Re: [PATCH 2/2] crypto/crc32[c]: register only "-lib" drivers

[David Sterba]

On Tue, Jun 17, 2025 at 01:47:56PM -0700, Eric Biggers wrote:
> On Tue, Jun 17, 2025 at 01:20:50PM -0700, Eric Biggers wrote:
> > On Tue, Jun 17, 2025 at 10:17:48PM +0200, David Sterba wrote:
> > > On Fri, Jun 13, 2025 at 11:37:53AM -0700, Eric Biggers wrote:
> > > > From: Eric Biggers <ebiggers@google.com>
> > > > 
> > > > For the "crc32" and "crc32c" shash algorithms, instead of registering
> > > > "*-generic" drivers as well as conditionally registering "*-$(ARCH)"
> > > > drivers, instead just register "*-lib" drivers.  These just use the
> > > > regular library functions crc32_le() and crc32c(), so they just do the
> > > > right thing and are fully accelerated when supported by the CPU.
> > > > 
> > > > This eliminates the need for the CRC library to export crc32_le_base()
> > > > and crc32c_base().  Separate patches make those static functions.
> > > > 
> > > > Since this patch removes the "crc32-generic" and "crc32c-generic" driver
> > > > names which crypto/testmgr.c expects to exist, update crypto/testmgr.c
> > > > accordingly.  This does mean that crypto/testmgr.c will no longer
> > > > fuzz-test the "generic" implementation against the "arch" implementation
> > > > for crc32 and crc32c, but this was redundant with crc_kunit anyway.
> > > > 
> > > > Besides the above, and btrfs_init_csum_hash() which the previous patch
> > > > fixed, no code appears to have been relying on the "crc32-generic" or
> > > > "crc32c-generic" driver names specifically.
> > > > 
> > > > btrfs does export the checksum driver name in
> > > > /sys/fs/btrfs/$uuid/checksum.  This patch makes that file contain
> > > > "crc32c-lib" instead of "crc32c-generic" or "crc32c-$(ARCH)".  This
> > > > should be fine, since in practice the purpose of this file seems to have
> > > > been just to allow users to manually check whether they needed to enable
> > > > the optimized CRC32C code.  This was needed only because of the bug in
> > > > old kernels where the optimized CRC32C code defaulted to off and even
> > > > needed to be explicitly added to the ramdisk to be used.  Now that it
> > > > just works in Linux 6.14 and later, there's no need for users to take
> > > > any action and this file is basically obsolete.
> > > 
> > > Well, not the whole file, because it says which checksumming algo is
> > > used for the filesystem, but the implementation part is.
> > 
> > Oh, right.  It's one of those sysfs files that don't follow the normal sysfs
> > convention and contain multiple values.  I'll update the paragraph above to
> > clarify that it's referring to the driver name part of the file.
> 
> I revised it to:
> 
> btrfs does export the checksum name and checksum driver name in
> /sys/fs/btrfs/$uuid/checksum.  This commit makes the driver name portion
> of that file contain "crc32c-lib" instead of "crc32c-generic" or
> "crc32c-$(ARCH)".  This should be fine, since in practice the purpose of
> the driver name portion of this file seems to have been just to allow
> users to manually check whether they needed to enable the optimized
> CRC32C code.  This was needed only because of the bug in old kernels
> where the optimized CRC32C code defaulted to off and even needed to be
> explicitly added to the ramdisk to be used.  Now that it just works in
> Linux 6.14 and later, there's no need for users to take any action and
> the driver name portion of this is basically obsolete.  (Also, note that
> the crc32c driver name already changed in 6.14.)

This is OK, thanks.

Re: [PATCH 2/2] crypto/crc32[c]: register only "-lib" drivers

[Daniel Vacek]

On Tue, 17 Jun 2025 at 22:48, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Tue, Jun 17, 2025 at 01:20:50PM -0700, Eric Biggers wrote:
> > On Tue, Jun 17, 2025 at 10:17:48PM +0200, David Sterba wrote:
> > > On Fri, Jun 13, 2025 at 11:37:53AM -0700, Eric Biggers wrote:
> > > > From: Eric Biggers <ebiggers@google.com>
> > > >
> > > > For the "crc32" and "crc32c" shash algorithms, instead of registering
> > > > "*-generic" drivers as well as conditionally registering "*-$(ARCH)"
> > > > drivers, instead just register "*-lib" drivers.  These just use the
> > > > regular library functions crc32_le() and crc32c(), so they just do the
> > > > right thing and are fully accelerated when supported by the CPU.
> > > >
> > > > This eliminates the need for the CRC library to export crc32_le_base()
> > > > and crc32c_base().  Separate patches make those static functions.
> > > >
> > > > Since this patch removes the "crc32-generic" and "crc32c-generic" driver
> > > > names which crypto/testmgr.c expects to exist, update crypto/testmgr.c
> > > > accordingly.  This does mean that crypto/testmgr.c will no longer
> > > > fuzz-test the "generic" implementation against the "arch" implementation
> > > > for crc32 and crc32c, but this was redundant with crc_kunit anyway.
> > > >
> > > > Besides the above, and btrfs_init_csum_hash() which the previous patch
> > > > fixed, no code appears to have been relying on the "crc32-generic" or
> > > > "crc32c-generic" driver names specifically.
> > > >
> > > > btrfs does export the checksum driver name in
> > > > /sys/fs/btrfs/$uuid/checksum.  This patch makes that file contain
> > > > "crc32c-lib" instead of "crc32c-generic" or "crc32c-$(ARCH)".  This
> > > > should be fine, since in practice the purpose of this file seems to have
> > > > been just to allow users to manually check whether they needed to enable
> > > > the optimized CRC32C code.  This was needed only because of the bug in
> > > > old kernels where the optimized CRC32C code defaulted to off and even
> > > > needed to be explicitly added to the ramdisk to be used.  Now that it
> > > > just works in Linux 6.14 and later, there's no need for users to take
> > > > any action and this file is basically obsolete.
> > >
> > > Well, not the whole file, because it says which checksumming algo is
> > > used for the filesystem, but the implementation part is.
> >
> > Oh, right.  It's one of those sysfs files that don't follow the normal sysfs
> > convention and contain multiple values.  I'll update the paragraph above to
> > clarify that it's referring to the driver name part of the file.
>
> I revised it to:
>
> btrfs does export the checksum name and checksum driver name in
> /sys/fs/btrfs/$uuid/checksum.  This commit makes the driver name portion
> of that file contain "crc32c-lib" instead of "crc32c-generic" or
> "crc32c-$(ARCH)".  This should be fine, since in practice the purpose of
> the driver name portion of this file seems to have been just to allow
> users to manually check whether they needed to enable the optimized
> CRC32C code.  This was needed only because of the bug in old kernels
> where the optimized CRC32C code defaulted to off and even needed to be
> explicitly added to the ramdisk to be used.  Now that it just works in
> Linux 6.14 and later, there's no need for users to take any action and
> the driver name portion of this is basically obsolete.  (Also, note that
> the crc32c driver name already changed in 6.14.)

How about instead removing that part since it's useless now?

Re: [PATCH 2/2] crypto/crc32[c]: register only "-lib" drivers

[David Sterba]

On Thu, Jun 19, 2025 at 12:27:13PM +0200, Daniel Vacek wrote:
> > I revised it to:
> >
> > btrfs does export the checksum name and checksum driver name in
> > /sys/fs/btrfs/$uuid/checksum.  This commit makes the driver name portion
> > of that file contain "crc32c-lib" instead of "crc32c-generic" or
> > "crc32c-$(ARCH)".  This should be fine, since in practice the purpose of
> > the driver name portion of this file seems to have been just to allow
> > users to manually check whether they needed to enable the optimized
> > CRC32C code.  This was needed only because of the bug in old kernels
> > where the optimized CRC32C code defaulted to off and even needed to be
> > explicitly added to the ramdisk to be used.  Now that it just works in
> > Linux 6.14 and later, there's no need for users to take any action and
> > the driver name portion of this is basically obsolete.  (Also, note that
> > the crc32c driver name already changed in 6.14.)
> 
> How about instead removing that part since it's useless now?

There's no best answer, removing it makes sense but could break
somebody's scripts parsing that file. The information put to "(...)"
might be useful in the future again. It's less harm to leave it there
than to deal with potential fallout if it's removed.