Date:   Sat, 10 Dec 2022 00:34:27 +0000
From:   "Elliott, Robert (Servers)" <elliott@....com>
To:     Peter Lafreniere <peter@...jl.ca>
CC:     Herbert Xu <herbert@...dor.apana.org.au>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "tim.c.chen@...ux.intel.com" <tim.c.chen@...ux.intel.com>,
        "ap420073@...il.com" <ap420073@...il.com>,
        "ardb@...nel.org" <ardb@...nel.org>,
        "David.Laight@...lab.com" <David.Laight@...lab.com>,
        "ebiggers@...nel.org" <ebiggers@...nel.org>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v4 10/24] crypto: x86/poly - limit FPU preemption



> -----Original Message-----
> From: Peter Lafreniere <peter@...jl.ca>
> Sent: Tuesday, December 6, 2022 5:06 PM
> To: Elliott, Robert (Servers) <elliott@....com>
> Subject: RE: [PATCH v4 10/24] crypto: x86/poly - limit FPU preemption
> 
> > > Subject: Re: [PATCH v4 10/24] crypto: x86/poly - limit FPU preemption
> > > Perhaps we should try a different approach. How about just limiting
> > > the size to 4K, and then depending on need_resched we break out of
> > > the loop? Something like:
> > >
> > > if (!len)
> > > return 0;
> > >
> > > kernel_fpu_begin();
> > > for (;;) {
> > > unsigned int chunk = min(len, 4096);
> > >
> > > sha1_base_do_update(desc, data, chunk, sha1_xform);
> > >
> > > len -= chunk;
> > > data += chunk;
> > >
> > > if (!len)
> > > break;
> > >
> > > if (need_resched()) {
> > > kernel_fpu_end();
> > > cond_resched();
> > > kernel_fpu_begin();
> > > }
> > > }
> > > kernel_fpu_end();
> >
> >
> > I implemented that conditional approach in the sha algorithms.
> >
> > The results of a boot (using sha512 for module signatures, with
> > crypto extra tests enabled, comparing to sha512 with a 20 KiB
> > fixed limit) are:
> >
> > sha1 cond: 14479 calls; 784256 cycles doing begin/end; longest FPU
> context 35828 cycles
> > sha256 cond: 26763 calls; 1273570 cycles doing begin/end; longest FPU
> context 118612 cycles
> > sha512 cond: 26957 calls; 1680046 cycles doing begin/end; longest FPU
> context 169140982 cycles
> > sha512 20KiB: 161011 calls; 16232280 cycles doing begin/end; longest FPU
> context 4049644 cycles
> >
> > NOTE: I didn't have a patch in place to isolate the counts for each
> variation
> > (ssse3 vs. avx vs. avx2) and
> > - for sha512: sha512 vs. sha384
> > - for sha256: sha256 vs. sha224
> > so the numbers include sha256 and sha512 running twice as many tests
> > as sha1.
> >
> > This approach looks very good:
> > - 16% of the number of begin/end calls
> > - 10% of the CPU cycles spent making the calls
> > - the FPU context is held for a long time (77 ms) but only while
> > it's not needed.
> >
> > That's much more efficient than releasing it every 30 us just in case.
> 
> How recently did you make this change? I implemented this conditional
> approach for ecb_cbc_helpers.h, but saw no changes at all to performance
> on serpent-avx2 and twofish-avx.

The hash functions are the main problem; the skciphers receive
requests already broken into 4 KiB chunks by the SG list helpers.
 
> kernel_fpu_{begin,end} (after the first call to begin) don't do anything
> more than enable/disable preemption and make a few writes to the mxcsr.
> It's likely that the above approach has the tiniest bit less overhead,
> and it will preempt on non CONFIG_PREEMPT kernels, but nothing suggests
> a performance uplift.
> 
> > I'll keep testing this to make sure RCU stalls stay away, and apply
> > the approach to the other algorithms.
> 
> I missed the earlier discussions. Have you seen issues with RCU
> stalls/latency spikes because of crypto routines? If so, what preemption
> model were you running?

While running Wireshark in Fedora, I noticed the top function consuming
CPU cycles (per "perf top") was sha512_generic.

Although Fedora and RHEL have the x86 optimized driver compiled as a
module, nothing in the distro or application space noticed it was there
and loaded it. The only x86 optimized drivers that do get used are the
ones built-in to the kernel.

After making changes to load the x86 sha512 module, I noticed several
boots over the next few weeks reported RCU stalls, all in the sha512_avx2
function. Because the stack traces take a long time to print to the
serial port, these can trigger soft lockups as well. Fedora and RHEL
default to "Voluntary Kernel Preemption (Desktop)": 
    # CONFIG_PREEMPT_NONE is not set
    CONFIG_PREEMPT_VOLUNTARY=y
    # CONFIG_PREEMPT is not set

The reason was that sha512 and all the other x86 crypto hash functions
process the entire data in one kernel_fpu_begin()/end() block, which
blocks preemption. Each boot checks module signatures for about 4000
files, totaling about 2.4 GB. Breaking the loops into smaller chunks
fixes the problem. However, since functions like crc32c are 20x faster
than sha1, one value like 4 KiB is not ideal.

A few non-hash functions have issues too. Although most skciphers are
broken up into 4 KiB chunks by the sg list walking functions, aegis
packs everything inside one kernel_fpu_begin()/end() block. All the
aead functions handle the main data with sg list walking functions,
but handle all the associated data inside one kernel_fpu_begin()/end()
block.

> > In x86, need_resched() has to deal with a PER_CPU variable, so I'm
> > not sure it's worth the hassle to figure out how to do that from
> > assembly code.
> 
> Leave it in c. It'll be more maintainable that way.

I'm testing a new kernel_fpu_yield() utility function that looks nice:

void __sha1_transform_avx2(struct sha1_state *state, const u8 *data, int blocks)
{
        if (blocks <= 0)
                return;

        kernel_fpu_begin();
        for (;;) {
                const int chunks = min(blocks, 4096 / SHA1_BLOCK_SIZE);

                sha1_transform_avx2(state->state, data, chunks);
                blocks -= chunks;

                if (blocks <= 0)
                        break;

                data += chunks * SHA1_BLOCK_SIZE;
                kernel_fpu_yield();
        }
        kernel_fpu_end();
}

This construction also makes it easy to add debug counters to
observe what is happening.

In a boot with preempt=none and the crypto extra self-tests
enabled, two modules benefitted from that new yield call:
/sys/module/sha256_ssse3/parameters/fpu_rescheds:3
/sys/module/sha512_ssse3/parameters/fpu_rescheds:515

10 passes of 1 MiB buffer tests on all the drivers
shows several others benefitting:
/sys/module/aegis128_aesni/parameters/fpu_rescheds:1
/sys/module/aesni_intel/parameters/fpu_rescheds:0
/sys/module/aria_aesni_avx_x86_64/parameters/fpu_rescheds:45
/sys/module/camellia_aesni_avx2/parameters/fpu_rescheds:0
/sys/module/camellia_aesni_avx_x86_64/parameters/fpu_rescheds:0
/sys/module/camellia_x86_64/parameters/fpu_rescheds:0
/sys/module/cast5_avx_x86_64/parameters/fpu_rescheds:0
/sys/module/cast6_avx_x86_64/parameters/fpu_rescheds:0
/sys/module/chacha_x86_64/parameters/fpu_rescheds:0
/sys/module/crc32c_intel/parameters/fpu_rescheds:1
/sys/module/crc32_pclmul/parameters/fpu_rescheds:1
/sys/module/crct10dif_pclmul/parameters/fpu_rescheds:1
/sys/module/ghash_clmulni_intel/parameters/fpu_rescheds:1
/sys/module/libblake2s_x86_64/parameters/fpu_rescheds:0
/sys/module/nhpoly1305_avx2/parameters/fpu_rescheds:1
/sys/module/nhpoly1305_sse2/parameters/fpu_rescheds:1
/sys/module/poly1305_x86_64/parameters/fpu_rescheds:1
/sys/module/polyval_clmulni/parameters/fpu_rescheds:1
/sys/module/serpent_avx2/parameters/fpu_rescheds:0
/sys/module/serpent_avx_x86_64/parameters/fpu_rescheds:0
/sys/module/serpent_sse2_x86_64/parameters/fpu_rescheds:0
/sys/module/sha1_ssse3/parameters/fpu_rescheds:3
/sys/module/sha256_ssse3/parameters/fpu_rescheds:9
/sys/module/sha512_ssse3/parameters/fpu_rescheds:723
/sys/module/sm3_avx_x86_64/parameters/fpu_rescheds:171
/sys/module/sm4_aesni_avx_x86_64/parameters/fpu_rescheds:0
/sys/module/twofish_avx_x86_64/parameters/fpu_rescheds:0
/sys/module/twofish_x86_64_3way/parameters/fpu_rescheds:0


I'll keep experimenting with all the preempt modes, heavier
workloads, and shorter RCU timeouts to confirm this solution
is robust. It might even be appropriate for the generic
drivers, if they suffer from the problems that sm4 shows here.

> This brings us back to this question: should crypto routines be
> preempted under PREEMPT_VOLUNTARY or not?

I think so. The RCU stall and soft lockup detectors aren't disabled,
so there is still an expectation of sharing the CPUs even in
PREEMPT=none mode.

1 MiB tests under CONFIG_PREEMPT=none triggered soft lockups while
running CBC mode for SM4, Camellia, and Serpent:

[  208.975253] tcrypt: PERL "cfb-sm4-aesni-avx2" => 22499840,
[  218.187217] watchdog: BUG: soft lockup - CPU#1 stuck for 26s! [modprobe:3433]
...
[  219.391776] tcrypt: PERL "cbc-sm4-aesni-avx2" => 22528138,

[  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,


[  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,

SM4 falls back to the generic driver for encryption; it only has
optimized decryption functions. Therefore, it doesn't make any
kernel_fpu_end() calls and thus makes no rescheduling calls.

This shows the CPU cycles for 1 MiB of encrypt and decrypt for
each algorithm (no soft lockups this time). SM4, Serpent, Cast5,
and Cast6 encryption in CBC mode are the slowest by far.

[ 2233.362748] tcrypt: PERL my %speeds_skcipher = (
[ 2234.427387] tcrypt: PERL            "cbc-aes-aesni" =>  2178586,
[ 2234.738823] tcrypt: PERL            "cbc-aes-aesni" =>   538752,
[ 2235.064335] tcrypt: PERL            "ctr-aes-aesni" =>   574026,
[ 2235.389427] tcrypt: PERL            "ctr-aes-aesni" =>   574060,
[ 2236.451594] tcrypt: PERL        "cts-cbc-aes-aesni" =>  2178946,
[ 2236.762174] tcrypt: PERL        "cts-cbc-aes-aesni" =>   540066,
[ 2237.070371] tcrypt: PERL            "ecb-aes-aesni" =>   536970,
[ 2237.379549] tcrypt: PERL            "ecb-aes-aesni" =>   538012,
[ 2237.686137] tcrypt: PERL           "xctr-aes-aesni" =>   534690,
[ 2237.993315] tcrypt: PERL           "xctr-aes-aesni" =>   534632,
[ 2238.304077] tcrypt: PERL            "xts-aes-aesni" =>   542590,
[ 2238.615057] tcrypt: PERL            "xts-aes-aesni" =>   541296,
[ 2240.233298] tcrypt: PERL             "ctr-aria-avx" =>  3393212,
[ 2241.849000] tcrypt: PERL             "ctr-aria-avx" =>  3391982,
[ 2242.081296] tcrypt: PERL           "xchacha12-simd" =>   370794,
[ 2242.316868] tcrypt: PERL           "xchacha12-simd" =>   373788,
[ 2242.626165] tcrypt: PERL           "xchacha20-simd" =>   536310,
[ 2242.936646] tcrypt: PERL           "xchacha20-simd" =>   537094,
[ 2243.250356] tcrypt: PERL            "chacha20-simd" =>   540542,
[ 2243.559396] tcrypt: PERL            "chacha20-simd" =>   536604,
[ 2244.831594] tcrypt: PERL       "ctr-sm4-aesni-avx2" =>  2642674,
[ 2246.106143] tcrypt: PERL       "ctr-sm4-aesni-avx2" =>  2640350,
[ 2256.475661] tcrypt: PERL       "cfb-sm4-aesni-avx2" => 22496346,
[ 2257.732511] tcrypt: PERL       "cfb-sm4-aesni-avx2" =>  2604932,
[ 2268.123821] tcrypt: PERL       "cbc-sm4-aesni-avx2" => 22528268,
[ 2269.378028] tcrypt: PERL       "cbc-sm4-aesni-avx2" =>  2601090,
[ 2271.533556] tcrypt: PERL        "ctr-sm4-aesni-avx" =>  4559648,
[ 2273.688772] tcrypt: PERL        "ctr-sm4-aesni-avx" =>  4561300,
[ 2284.073187] tcrypt: PERL        "cfb-sm4-aesni-avx" => 22499496,
[ 2286.177732] tcrypt: PERL        "cfb-sm4-aesni-avx" =>  4457588,
[ 2296.569751] tcrypt: PERL        "cbc-sm4-aesni-avx" => 22529182,
[ 2298.677312] tcrypt: PERL        "cbc-sm4-aesni-avx" =>  4457226,
[ 2300.789931] tcrypt: PERL        "ecb-sm4-aesni-avx" =>  4464282,
[ 2302.899974] tcrypt: PERL        "ecb-sm4-aesni-avx" =>  4466052,
[ 2308.589365] tcrypt: PERL  "cbc-camellia-aesni-avx2" => 12260426,
[ 2309.737064] tcrypt: PERL  "cbc-camellia-aesni-avx2" =>  2350988,
[ 2315.433319] tcrypt: PERL       "cbc-camellia-aesni" => 12248986,
[ 2317.262589] tcrypt: PERL       "cbc-camellia-aesni" =>  3814202,
[ 2325.460542] tcrypt: PERL            "cbc-cast5-avx" => 17739828,
[ 2327.856127] tcrypt: PERL            "cbc-cast5-avx" =>  5061992,
[ 2336.668992] tcrypt: PERL            "cbc-cast6-avx" => 19066440,
[ 2340.470787] tcrypt: PERL            "cbc-cast6-avx" =>  8147336,
[ 2350.376676] tcrypt: PERL         "cbc-serpent-avx2" => 21466002,
[ 2351.646295] tcrypt: PERL         "cbc-serpent-avx2" =>  2611362,
[ 2361.562736] tcrypt: PERL          "cbc-serpent-avx" => 21471118,
[ 2364.019693] tcrypt: PERL          "cbc-serpent-avx" =>  5201506,
[ 2373.930747] tcrypt: PERL         "cbc-serpent-sse2" => 21465594,
[ 2376.697210] tcrypt: PERL         "cbc-serpent-sse2" =>  5855766,
[ 2380.944596] tcrypt: PERL          "cbc-twofish-avx" =>  9058090,
[ 2383.308215] tcrypt: PERL          "cbc-twofish-avx" =>  4989064,
[ 2384.904158] tcrypt: PERL             "ecb-aria-avx" =>  3299260,
[ 2386.498365] tcrypt: PERL             "ecb-aria-avx" =>  3297534,
[ 2387.625226] tcrypt: PERL  "ecb-camellia-aesni-avx2" =>  2306326,
[ 2388.757749] tcrypt: PERL  "ecb-camellia-aesni-avx2" =>  2312876,
[ 2390.549340] tcrypt: PERL       "ecb-camellia-aesni" =>  3752534,
[ 2392.335240] tcrypt: PERL       "ecb-camellia-aesni" =>  3751896,
[ 2394.724956] tcrypt: PERL            "ecb-cast5-avx" =>  5032914,
[ 2397.116268] tcrypt: PERL            "ecb-cast5-avx" =>  5041908,
[ 2400.935093] tcrypt: PERL            "ecb-cast6-avx" =>  8148418,
[ 2404.754816] tcrypt: PERL            "ecb-cast6-avx" =>  8150448,
[ 2406.025861] tcrypt: PERL         "ecb-serpent-avx2" =>  2613024,
[ 2407.286682] tcrypt: PERL         "ecb-serpent-avx2" =>  2602556,
[ 2409.732474] tcrypt: PERL          "ecb-serpent-avx" =>  5191944,
[ 2412.161829] tcrypt: PERL          "ecb-serpent-avx" =>  5165230,
[ 2414.678835] tcrypt: PERL         "ecb-serpent-sse2" =>  5345630,
[ 2417.217632] tcrypt: PERL         "ecb-serpent-sse2" =>  5331110,
[ 2419.545136] tcrypt: PERL          "ecb-twofish-avx" =>  4917424,
[ 2421.870457] tcrypt: PERL          "ecb-twofish-avx" =>  4915194,
[ 2421.870564] tcrypt: PERL );

Powered by blists - more mailing lists