[4/4] crypto: x86/sha-mb - decrease priority of multibuffer algorithms

Commit Message

Eric Biggers June 30, 2018, 12:01 a.m. UTC

From: Eric Biggers <[e-mail]>

With all the crypto modules enabled on x86, and with a CPU that supports
AVX-2 but not SHA-NI instructions (e.g. Haswell, Broadwell, Skylake),
the "multibuffer" implementations of SHA-1, SHA-256, and SHA-512 are the
highest priority.  However, these implementations only perform well when
many hash requests are being submitted concurrently, filling all 8 AVX-2
lanes.  Otherwise, they are incredibly slow, as they waste time waiting
for more requests to arrive before proceeding to execute each request.

For example, here are the speeds I see hashing 4096-byte buffers with a
single thread on a Haswell-based processor:

            generic            avx2          mb (multibuffer)
            -------            --------      ----------------
sha1        602 MB/s           997 MB/s      0.61 MB/s
sha256      228 MB/s           412 MB/s      0.61 MB/s
sha512      312 MB/s           559 MB/s      0.61 MB/s

So, the multibuffer implementation is 500 to 1000 times slower than the
other implementations.  Note that with smaller buffers or more update()s
per digest, the difference would be even greater.

I believe the vast majority of people are in the boat where the
multibuffer code is much slower, and only a small minority are doing the
highly parallel, hashing-intensive, latency-flexible workloads (maybe
IPsec on servers?) where the multibuffer code may be beneficial.  Yet,
people often aren't familiar with all the crypto config options and so
the multibuffer code may inadvertently be built into the kernel.

Also the multibuffer code apparently hasn't been very well tested,
seeing as it was sometimes computing the wrong SHA-256 digest.

So, let's make the multibuffer algorithms low priority.  Users who want
to use them can either request them explicitly by driver name, or use
NETLINK_CRYPTO (crypto_user) to increase their priority at runtime.

Signed-off-by: Eric Biggers <[e-mail]>
---
 arch/x86/crypto/sha1-mb/sha1_mb.c     | 9 ++++++++-
 arch/x86/crypto/sha256-mb/sha256_mb.c | 9 ++++++++-
 arch/x86/crypto/sha512-mb/sha512_mb.c | 9 ++++++++-
 3 files changed, 24 insertions(+), 3 deletions(-)

Patch

diff --git a/arch/x86/crypto/sha1-mb/sha1_mb.c b/arch/x86/crypto/sha1-mb/sha1_mb.c
index e17655ffde79..4b2430274935 100644
--- a/arch/x86/crypto/sha1-mb/sha1_mb.c
+++ b/arch/x86/crypto/sha1-mb/sha1_mb.c
@@ -871,7 +871,14 @@  static struct ahash_alg sha1_mb_async_alg = {
 		.base = {
 			.cra_name               = "sha1",
 			.cra_driver_name        = "sha1_mb",
-			.cra_priority           = 200,
+			/*
+			 * Low priority, since with few concurrent hash requests
+			 * this is extremely slow due to the flush delay.  Users
+			 * whose workloads would benefit from this can request
+			 * it explicitly by driver name, or can increase its
+			 * priority at runtime using NETLINK_CRYPTO.
+			 */
+			.cra_priority           = 50,
 			.cra_flags              = CRYPTO_ALG_TYPE_AHASH | CRYPTO_ALG_ASYNC,
 			.cra_blocksize          = SHA1_BLOCK_SIZE,
 			.cra_type               = &crypto_ahash_type,
diff --git a/arch/x86/crypto/sha256-mb/sha256_mb.c b/arch/x86/crypto/sha256-mb/sha256_mb.c
index 4c46ac1b6653..4c07f6c12c37 100644
--- a/arch/x86/crypto/sha256-mb/sha256_mb.c
+++ b/arch/x86/crypto/sha256-mb/sha256_mb.c
@@ -870,7 +870,14 @@  static struct ahash_alg sha256_mb_async_alg = {
 		.base = {
 			.cra_name               = "sha256",
 			.cra_driver_name        = "sha256_mb",
-			.cra_priority           = 200,
+			/*
+			 * Low priority, since with few concurrent hash requests
+			 * this is extremely slow due to the flush delay.  Users
+			 * whose workloads would benefit from this can request
+			 * it explicitly by driver name, or can increase its
+			 * priority at runtime using NETLINK_CRYPTO.
+			 */
+			.cra_priority           = 50,
 			.cra_flags              = CRYPTO_ALG_TYPE_AHASH |
 							CRYPTO_ALG_ASYNC,
 			.cra_blocksize          = SHA256_BLOCK_SIZE,
diff --git a/arch/x86/crypto/sha512-mb/sha512_mb.c b/arch/x86/crypto/sha512-mb/sha512_mb.c
index 39e2bbdc1836..6a8c31581604 100644
--- a/arch/x86/crypto/sha512-mb/sha512_mb.c
+++ b/arch/x86/crypto/sha512-mb/sha512_mb.c
@@ -904,7 +904,14 @@  static struct ahash_alg sha512_mb_async_alg = {
 		.base = {
 			.cra_name               = "sha512",
 			.cra_driver_name        = "sha512_mb",
-			.cra_priority           = 200,
+			/*
+			 * Low priority, since with few concurrent hash requests
+			 * this is extremely slow due to the flush delay.  Users
+			 * whose workloads would benefit from this can request
+			 * it explicitly by driver name, or can increase its
+			 * priority at runtime using NETLINK_CRYPTO.
+			 */
+			.cra_priority           = 50,
 			.cra_flags              = CRYPTO_ALG_TYPE_AHASH |
 							CRYPTO_ALG_ASYNC,
 			.cra_blocksize          = SHA512_BLOCK_SIZE,