Re: [PATCH v2 1/9] crypto: tcrypt - remove CRYPTO_TEST from defconfigs

[Geert Uytterhoeven]

Hi Eric,

On Thu, 24 Apr 2025 at 01:20, Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
> On Wed, Apr 23, 2025 at 08:49:24AM +0200, Geert Uytterhoeven wrote:
> > (replaying my response to v1, which I wrote before I noticed there was a v2)
> >
> > On Tue, 22 Apr 2025 at 17:23, Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
> > > From: Eric Biggers <ebiggers@xxxxxxxxxx>
> > >
> > > CONFIG_CRYPTO_TEST enables a benchmarking module that is only really
> > > useful for developers working on the crypto subsystem.  It is in a bunch
> > > of defconfigs.  But as with most of the other crypto options that tend
> > > to be randomly set in defconfigs, it is unlikely that much thought was
> > > put into these, especially when placed in "production" defconfigs.
> > > Clear it out of the defconfigs for now.
> > >
> > > Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>
> >
> > Thanks for your patch!
> >
> > All of these are modular, so I don't think it's a big issue, even on
> > "production" defconfigs. It just means the test is available when
> > someone feels the urge to run it.
> > Hence I try to make all tests available as modules in m68k defconfigs.
>
> Okay, but again note that this option isn't the actual crypto tests.  It's a
> benchmark.  Patch #2 renames CONFIG_CRYPTO_TEST to CONFIG_CRYPTO_BENCHMARK

Benchmarks are a kind of performance tests...

> accordingly.  The actual crypto tests are not modular and are controlled by the
> inverted option CONFIG_CRYPTO_MANAGER_DISABLE_TESTS, changed to
> CONFIG_CRYPTO_SELFTESTS by patch #5.  CONFIG_CRYPTO_TEST did used to be the
> actual tests, but that changed in 2008, and no one ever fixed it.

I guess the KUnit janitors will convert it to a proper KUnit module,
one day...

> Due to the renaming I'd need to update the defconfigs anyway, and I figured just
> clearing out the option is the right choice in most cases.  Hence this patch.
> But if you do understand what this option does and think it should be kept in as
> CONFIG_CRYPTO_BENCHMARK=m, we can do that instead (for all defconfigs that had
> it, presumably?).

I would like to keep it as CONFIG_CRYPTO_BENCHMARK=m.  If the
maintainers of the other architectures disagree, you can keep this
patch as-is.  Then CONFIG_CRYPTO_BENCHMARK=m will be added to the
m68k defconfigs during the customary defconfig update.
(To be removed again after the conversion to KUnit, and the addition
 of "default KUNIT_ALL_TESTS" ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds