Re: [PATCH] crypto: avoid unnecessary work when self-tests are disabled

[Eric Biggers <ebiggers@kernel.org>]

On Thu, Nov 10, 2022 at 04:36:17AM +0000, Elliott, Robert (Servers) wrote:
> 
> > -----Original Message-----
> > From: Eric Biggers <ebiggers@kernel.org>
> > Sent: Wednesday, November 9, 2022 8:38 PM
> > Subject: [PATCH] crypto: avoid unnecessary work when self-tests are
> > disabled
> > 
> > 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.
> > 
> ...
> > +#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.

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