[PATCH 1/3] crypto: skcipher - always yield at end of walk

From: Robert Elliott
Date: Mon Dec 19 2022 - 15:37:52 EST


Always yield to the scheduler at the end of skcipher_walk_done(),
not just if additional bytes are left to be processed.

This avoids soft lockups if drivers are invoked back-to-back
to process data that is an integer multiple of their block
size.

Example: while processing 1 MiB buffers, multiple skciphers run
from 192 s to 218 s without ever yielding to the scheduler,
causing three soft lockup complaints.

The kernel is configured for CONFIG_PREEMPT_NONE=y (or
preempt=none on the kernel command line), so only explicit
cond_resched() calls trigger scheduling - might_resched() and
preempt_enable() do not (see kernel/sched/core.c).

[ 190.865601] tcrypt: PERL my %speeds_skcipher = (
[ 192.041157] tcrypt: PERL "cbc-aes-aesni" => 2396490,
[ 192.373934] tcrypt: PERL "ctr-aes-aesni" => 574888,
[ 193.548967] tcrypt: PERL "cts-cbc-aes-aesni" => 2396018,
[ 193.909077] tcrypt: PERL "ecb-aes-aesni" => 631824,
[ 194.223801] tcrypt: PERL "xctr-aes-aesni" => 535778,
[ 194.608548] tcrypt: PERL "xts-aes-aesni" => 676518,
[ 196.440247] tcrypt: PERL "ctr-aria-avx" => 3804224,
[ 196.675788] tcrypt: PERL "xchacha12-simd" => 368668,
[ 196.988868] tcrypt: PERL "xchacha20-simd" => 535314,
[ 197.301510] tcrypt: PERL "chacha20-simd" => 535142,
[ 198.590113] tcrypt: PERL "ctr-sm4-aesni-avx2" => 2642930,
[ 208.975253] tcrypt: PERL "cfb-sm4-aesni-avx2" => 22499840,
[ 218.187217] watchdog: BUG: soft lockup - CPU#1 stuck for 26s! [modprobe:3433]
[ 246.181238] Modules linked in: tcrypt(+) hctr2 essiv adiantum
...
[ 246.185048] RIP: 0010:measure_skcipher_bigbuf.constprop.0.isra.0+0x282/0x393 [tcrypt]
[ 246.185304] Code: 00 0f 31 ...
...
[ 218.197313] Call Trace:
[ 218.197567] <TASK>
[ 218.197822] ? 0xffffffffc052a000
[ 218.198079] do_test.cold+0x989/0xc7a [tcrypt]
[ 218.198340] ? 0xffffffffc052a000
[ 218.198595] tcrypt_mod_init+0x50/0x1000 [tcrypt]
[ 218.198857] ? 0xffffffffc052a000
[ 218.199112] do_one_initcall+0x41/0x200
...
[ 219.391776] tcrypt: PERL "cbc-sm4-aesni-avx2" => 22528138,
[ 221.560847] tcrypt: PERL "ctr-sm4-aesni-avx" => 4560732,
[ 231.960414] tcrypt: PERL "cfb-sm4-aesni-avx" => 22498380,
[ 242.350070] tcrypt: PERL "cbc-sm4-aesni-avx" => 22527668,
[ 244.471181] tcrypt: PERL "ecb-sm4-aesni-avx" => 4469626,
...
[ 246.181064] watchdog: BUG: soft lockup - CPU#1 stuck for 52s! [modprobe:3433]
...
[ 250.168239] tcrypt: PERL "cbc-camellia-aesni-avx2" => 12202738,
[ 255.840094] tcrypt: PERL "cbc-camellia-aesni" => 12203096,
[ 264.047440] tcrypt: PERL "cbc-cast5-avx" => 17744280,
[ 273.091258] tcrypt: PERL "cbc-cast6-avx" => 19375400,
[ 274.183249] watchdog: BUG: soft lockup - CPU#1 stuck for 78s! [modprobe:3433]
...
[ 283.066260] tcrypt: PERL "cbc-serpent-avx2" => 21454930,
[ 292.983848] tcrypt: PERL "cbc-serpent-avx" => 21452996,
...

By adding a unilateral call to crypto_yield(), which calls
cond_resched() and lets the scheduler use the CPU for another
thread, that no longer happens.

Starting at 2218 s, there is no soft lockup reported at 2244 s:
[ 2217.202692] tcrypt: PERL my %speeds_skcipher = (
[ 2218.450215] tcrypt: PERL "cbc-aes-aesni" => 2179138,
[ 2218.950960] tcrypt: PERL "cbc-aes-aesni" => 538738,
[ 2219.460618] tcrypt: PERL "ctr-aes-aesni" => 575212,
[ 2219.983006] tcrypt: PERL "ctr-aes-aesni" => 574402,
[ 2221.329550] tcrypt: PERL "cts-cbc-aes-aesni" => 2182864,
[ 2221.840599] tcrypt: PERL "cts-cbc-aes-aesni" => 539064,
[ 2222.344290] tcrypt: PERL "ecb-aes-aesni" => 537402,
[ 2222.869201] tcrypt: PERL "ecb-aes-aesni" => 537730,
[ 2223.400315] tcrypt: PERL "xctr-aes-aesni" => 534824,
[ 2223.897915] tcrypt: PERL "xctr-aes-aesni" => 534782,
[ 2224.414956] tcrypt: PERL "xts-aes-aesni" => 539592,
[ 2224.923715] tcrypt: PERL "xts-aes-aesni" => 539356,
[ 2226.740211] tcrypt: PERL "ctr-aria-avx" => 3392444,
[ 2228.545624] tcrypt: PERL "ctr-aria-avx" => 3392068,
[ 2228.869883] tcrypt: PERL "xchacha12-simd" => 368932,
[ 2229.204980] tcrypt: PERL "xchacha12-simd" => 374122,
[ 2229.609975] tcrypt: PERL "xchacha20-simd" => 535596,
[ 2230.022425] tcrypt: PERL "xchacha20-simd" => 537500,
[ 2230.429674] tcrypt: PERL "chacha20-simd" => 535474,
[ 2230.831041] tcrypt: PERL "chacha20-simd" => 534264,
[ 2232.278150] tcrypt: PERL "ctr-sm4-aesni-avx2" => 2640770,
[ 2233.744781] tcrypt: PERL "ctr-sm4-aesni-avx2" => 2642520,
[ 2244.290542] tcrypt: PERL "cfb-sm4-aesni-avx2" => 22497308,
[ 2245.725044] tcrypt: PERL "cfb-sm4-aesni-avx2" => 2604468,
[ 2256.279228] tcrypt: PERL "cbc-sm4-aesni-avx2" => 22526084,
[ 2257.729868] tcrypt: PERL "cbc-sm4-aesni-avx2" => 2600460,
[ 2260.068782] tcrypt: PERL "ctr-sm4-aesni-avx" => 4560650,
[ 2262.414663] tcrypt: PERL "ctr-sm4-aesni-avx" => 4561468,
[ 2272.943000] tcrypt: PERL "cfb-sm4-aesni-avx" => 22496026,
[ 2275.233755] tcrypt: PERL "cfb-sm4-aesni-avx" => 4456984,
[ 2285.779516] tcrypt: PERL "cbc-sm4-aesni-avx" => 22525908,
[ 2288.081160] tcrypt: PERL "cbc-sm4-aesni-avx" => 4457036,
[ 2290.374086] tcrypt: PERL "ecb-sm4-aesni-avx" => 4465790,
[ 2292.677381] tcrypt: PERL "ecb-sm4-aesni-avx" => 4466014,
[ 2298.544718] tcrypt: PERL "cbc-camellia-aesni-avx2" => 12246268,
[ 2299.869611] tcrypt: PERL "cbc-camellia-aesni-avx2" => 2349440,
[ 2305.734078] tcrypt: PERL "cbc-camellia-aesni" => 12246930,
[ 2307.746065] tcrypt: PERL "cbc-camellia-aesni" => 3832992,
[ 2316.127414] tcrypt: PERL "cbc-cast5-avx" => 17737348,
[ 2318.703437] tcrypt: PERL "cbc-cast5-avx" => 5061014,
[ 2327.694881] tcrypt: PERL "cbc-cast6-avx" => 19065488,
[ 2331.672188] tcrypt: PERL "cbc-cast6-avx" => 8145590,
[ 2341.750274] tcrypt: PERL "cbc-serpent-avx2" => 21453172,
[ 2343.209420] tcrypt: PERL "cbc-serpent-avx2" => 2611702,

Fixes: b286d8b1a690 ("crypto: skcipher - Add skcipher walk interface")
Signed-off-by: Robert Elliott <elliott@xxxxxxx>
---
crypto/skcipher.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/crypto/skcipher.c b/crypto/skcipher.c
index 0ecab31cfe79..cdead632117a 100644
--- a/crypto/skcipher.c
+++ b/crypto/skcipher.c
@@ -153,13 +153,20 @@ int skcipher_walk_done(struct skcipher_walk *walk, int err)
scatterwalk_done(&walk->in, 0, nbytes);
scatterwalk_done(&walk->out, 1, nbytes);

- if (nbytes) {
- crypto_yield(walk->flags & SKCIPHER_WALK_SLEEP ?
- CRYPTO_TFM_REQ_MAY_SLEEP : 0);
+ /*
+ * Allow scheduler to use the CPU since it has been busy,
+ * regardless of whether another loop pass is due
+ */
+ crypto_yield(walk->flags & SKCIPHER_WALK_SLEEP ?
+ CRYPTO_TFM_REQ_MAY_SLEEP : 0);
+
+ if (nbytes)
return skcipher_walk_next(walk);
- }

finish:
+ crypto_yield(walk->flags & SKCIPHER_WALK_SLEEP ?
+ CRYPTO_TFM_REQ_MAY_SLEEP : 0);
+
/* Short-circuit for the common/fast path. */
if (!((unsigned long)walk->buffer | (unsigned long)walk->page))
goto out;
--
2.38.1