crypto: avoid unnecessary work when self-tests are disabled
Subsystems (2)
PATCH 1/1 crypto: avoid unnecessary work when self-tests are disabled
From: Eric Biggers <[email protected]>
Currently, registering an algorithm with the crypto API always causes a notification to be posted to the "cryptomgr", which then creates a kthread to self-test the algorithm. However, if self-tests are disabled in the kconfig (as is the default option), then this kthread just notifies waiters that the algorithm has been tested, then exits.
This causes a significant amount of overhead, especially in the kthread creation and destruction, which is not necessary at all. For example, in a quick test I found that booting a "minimum" x86_64 kernel with all the crypto options enabled (except for the self-tests) takes about 400ms until PID 1 can start. Of that, a full 13ms is spent just doing this pointless dance, involving a kthread being created, run, and destroyed over 200 times. That's over 3% of the entire kernel start time.
Fix this by just skipping the creation of the test larval and the posting of the registration notification entirely, when self-tests are disabled. Also compile out the unnecessary code in algboss.c.
While this patch is an optimization and not a "fix" per se, I've marked it as for stable, due to the large improvement it can make to boot time.
--- crypto/algapi.c | 3 ++- crypto/algboss.c | 11 +++++++---- 2 files changed, 9 insertions(+), 5 deletions(-)
crypto/algapi.c +2 -1
@@ -226,7 +226,8 @@ static struct crypto_larval *crypto_alloc_test_larval(struct crypto_alg *alg) {struct crypto_larval *larval; -if (!IS_ENABLED (CONFIG_CRYPTO_MANAGER )) +if (!IS_ENABLED (CONFIG_CRYPTO_MANAGER ) || +IS_ENABLED (CONFIG_CRYPTO_MANAGER_DISABLE_TESTS ))return NULL ; larval = crypto_larval_alloc(alg->cra_name,
crypto/algboss.c +7 -4
@@ -171,16 +171,18 @@ static int cryptomgr_schedule_probe(struct crypto_larval *larval)return NOTIFY_OK ; } +#ifdef CONFIG_CRYPTO_MANAGER_DISABLE_TESTS +static int cryptomgr_schedule_test (struct crypto_alg *alg) +{ +return 0 ; +} +#else static int cryptomgr_test (void *data) {struct crypto_test_param *param = data;u32 type = param->type;int err =0 ; -#ifdef CONFIG_CRYPTO_MANAGER_DISABLE_TESTS - goto skiptest; -#endif -if (type &CRYPTO_ALG_TESTED ) goto skiptest;
@@ -229,6 +231,7 @@ static int cryptomgr_schedule_test(struct crypto_alg *alg) err: return NOTIFY_OK; } +#endif /* !CONFIG_CRYPTO_MANAGER_DISABLE_TESTS */ static int cryptomgr_notify (struct notifier_block *this,unsigned long msg,void *data)
base-commit: f67dd6ce0723ad013395f20a3f79d8a437d3f455
Signature · 1 line
2.38.1
Quoted 26 lines
-----Original Message-----
From: Eric Biggers <[email protected]>
Sent: Wednesday, November 9, 2022 8:38 PM
Subject: [PATCH] crypto: avoid unnecessary work when self-tests are disabledCurrently, registering an algorithm with the crypto API always causes a notification to be posted to the "cryptomgr", which then creates a kthread to self-test the algorithm. However, if self-tests are disabled in the kconfig (as is the default option), then this kthread just notifies waiters that the algorithm has been tested, then exits.
This causes a significant amount of overhead, especially in the kthread creation and destruction, which is not necessary at all. For example, in a quick test I found that booting a "minimum"
x86_64kernel with all the crypto options enabled (except for the self-tests) takes about 400ms until PID 1 can start. Of that, a full 13ms is spent just doing this pointless dance, involving a kthread being created, run, and destroyed over 200 times. That's over 3% of the entire kernel start time.Fix this by just skipping the creation of the test larval and the posting of the registration notification entirely, when self-tests are disabled. Also compile out the unnecessary code in
algboss.c.
...
crypto/algboss.c static int cryptomgr_schedule_probe(struct crypto_larval *larval) · line 179
Quoted 6 lines
crypto/algboss.c +6
-0+#ifdef CONFIG_CRYPTO_MANAGER_DISABLE_TESTS +static int cryptomgr_schedule_test (struct crypto_alg *alg) +{ +return 0 ; +} +#else
The crypto/kdf_sp800108.c init function currently ignores both CONFIG_CRYPTO_MANAGER_DISABLE_TESTS and the cryptomgr module's notests module parameter and always runs its self-test, as described in
https://lore.kernel.org/lkml/MW5PR84MB1842811C4EECC0F4B35B5FB3AB709@MW5PR84MB1842.NAMPRD84.PROD.OUTLOOK.COM/T/#t
Paul reported that taking 262 ms on his system; I measured 1.4 s on my system.
It'd be nice if a patch series improving how DISABLE_TESTS is honored would tackle that module too.
Quoted 38 lines · Robert (Servers)
Quoted 25 lines
-----Original Message-----
From: Eric Biggers <[email protected]>
Sent: Wednesday, November 9, 2022 8:38 PM
Subject: [PATCH] crypto: avoid unnecessary work when self-tests are disabledCurrently, registering an algorithm with the crypto API always causes a notification to be posted to the "cryptomgr", which then creates a kthread to self-test the algorithm. However, if self-tests are disabled in the kconfig (as is the default option), then this kthread just notifies waiters that the algorithm has been tested, then exits.
This causes a significant amount of overhead, especially in the kthread creation and destruction, which is not necessary at all. For example, in a quick test I found that booting a "minimum"
x86_64kernel with all the crypto options enabled (except for the self-tests) takes about 400ms until PID 1 can start. Of that, a full 13ms is spent just doing this pointless dance, involving a kthread being created, run, and destroyed over 200 times. That's over 3% of the entire kernel start time.Fix this by just skipping the creation of the test larval and the posting of the registration notification entirely, when self-tests are disabled. Also compile out the unnecessary code in
algboss.c....
crypto/algboss.c static int cryptomgr_schedule_probe(struct crypto_larval *larval) · line 179
Quoted 6 lines
crypto/algboss.c +6
-0+#ifdef CONFIG_CRYPTO_MANAGER_DISABLE_TESTS +static int cryptomgr_schedule_test (struct crypto_alg *alg) +{ +return 0 ; +} +#else The
crypto/kdf_sp800108.cinit function currently ignores bothCONFIG_CRYPTO_MANAGER_DISABLE_TESTSand the cryptomgr module's notests module parameter and always runs its self-test, as described in
https://lore.kernel.org/lkml/MW5PR84MB1842811C4EECC0F4B35B5FB3AB709@MW5PR84MB1842.NAMPRD84.PROD.OUTLOOK.COM/T/#t
Quoted 5 lines
Paul reported that taking 262 ms on his system; I measured 1.4 s on my system.
It'd be nice if a patch series improving how
DISABLE_TESTSis honored would tackle that module too.
That should be a separate patch, but yes, it should only run the test if !IS_ENABLED(CONFIG_CRYPTO_MANAGER_DISABLE_TESTS), like what everywhere else does.
- Eric
Quoted 20 lines · Eric Biggers
From: Eric Biggers <[email protected]>
Currently, registering an algorithm with the crypto API always causes a notification to be posted to the "cryptomgr", which then creates a kthread to self-test the algorithm. However, if self-tests are disabled in the kconfig (as is the default option), then this kthread just notifies waiters that the algorithm has been tested, then exits.
This causes a significant amount of overhead, especially in the kthread creation and destruction, which is not necessary at all. For example, in a quick test I found that booting a "minimum"
x86_64kernel with all the crypto options enabled (except for the self-tests) takes about 400ms until PID 1 can start. Of that, a full 13ms is spent just doing this pointless dance, involving a kthread being created, run, and destroyed over 200 times. That's over 3% of the entire kernel start time.Fix this by just skipping the creation of the test larval and the posting of the registration notification entirely, when self-tests are disabled. Also compile out the unnecessary code in
algboss.c.
Quoted 5 lines
While this patch is an optimization and not a "fix" per se, I've marked it as for stable, due to the large improvement it can make to boot time.
Signed-off-by Eric Biggers <[email protected]>
Unfortunately, this patch won't work because it breaks templates.
There should still be a solution that at least avoids having to spawn kthreads, though...
- Eric