2022-06-25 05:29:39

by David Gow

[permalink] [raw]
Subject: [PATCH v3 0/5] Rework KUnit test execution in modules

This patch series makes two changes to how KUnit test suites are stored
and executed:
- The .kunit_test_suites section is now used for tests in modules (in
lieu of a module_init funciton), as well as for built-in tests. The
module loader will now trigger test execution. This frees up the
module_init function for other uses.
- Instead of storing an array of arrays of suites, have the
kunit_test_suite() and kunit_test_suites() macros append to one global
(or per-module) list of test suites. This removes a needless layer of
indirection, and removes the need to NULL-terminate suite_sets.

The upshot of this is that it should now be possible to use the
kunit_test_suite() and kunit_test_suites() macros to register test
suites even from within modules which otherwise had module_init
functions. This was proving to be quite a common issue, resulting in
several modules calling into KUnit's private suite execution functions
to run their tests (often introducing incompatibilities with the KUnit
tooling).

This series also fixes the thunderbolt, nitro_enclaves, and
sdhci-of-aspeed tests to use kunit_test_suite() now that it works. This
is required, as otherwise the first two patches may break these tests
entirely.

Huge thanks to Jeremy Kerr, who designed and implemented the module
loader changes, and to Daniel Latypov for pushing the simplification of
the nested arrays in .kunit_test_suites.

I've tested this series both with builtin tests on a number of
architectures, and with modules on x86_64, and it seems good-to-go to
me. More testing (particularly of modules) with more interesting setups
never hurts, though!

Cheers,
-- David

Changes since v2:
https://lore.kernel.org/linux-kselftest/[email protected]/
- Add various Reviewed-by and Acked-by tags.
- Fix the Kconfig for thunderbolt to not allow USB4=y and KUNIT=m with
tests enabled.
- Clean up the sdhci-of-aspeed init a bit more (Thanks Daniel)

Changes since v1:
https://lore.kernel.org/linux-kselftest/[email protected]/
- Fix a compile issue when CONFIG_KUNIT=m (Thanks Christophe)
- No longer NULL-terminate suite_sets.
- Move the thunderbird Kconfig to the correct patch (Thanks Andra)
- Add all the Tested-by and Acked-by tags.

---
Daniel Latypov (1):

Daniel Latypov (1):
kunit: flatten kunit_suite*** to kunit_suite** in .kunit_test_suites

David Gow (3):
thunderbolt: test: Use kunit_test_suite() macro
nitro_enclaves: test: Use kunit_test_suite() macro
mmc: sdhci-of-aspeed: test: Use kunit_test_suite() macro

Jeremy Kerr (1):
kunit: unify module and builtin suite definitions

drivers/mmc/host/Kconfig | 5 +-
drivers/mmc/host/sdhci-of-aspeed-test.c | 8 +-
drivers/mmc/host/sdhci-of-aspeed.c | 34 +----
drivers/thunderbolt/Kconfig | 6 +-
drivers/thunderbolt/domain.c | 3 -
drivers/thunderbolt/tb.h | 8 -
drivers/thunderbolt/test.c | 12 +-
drivers/virt/nitro_enclaves/Kconfig | 5 +-
drivers/virt/nitro_enclaves/ne_misc_dev.c | 27 ----
.../virt/nitro_enclaves/ne_misc_dev_test.c | 5 +-
include/kunit/test.h | 60 ++------
include/linux/module.h | 5 +
kernel/module/main.c | 6 +
lib/kunit/executor.c | 115 ++++----------
lib/kunit/executor_test.c | 144 +++++-------------
lib/kunit/test.c | 54 ++++++-
16 files changed, 154 insertions(+), 343 deletions(-)

--
2.37.0.rc0.161.g10f37bed90-goog


2022-06-25 05:31:14

by David Gow

[permalink] [raw]
Subject: [PATCH v3 5/5] mmc: sdhci-of-aspeed: test: Use kunit_test_suite() macro

The kunit_test_suite() macro is no-longer incompatible with module_add,
so its use can be reinstated.

Since this fixes parsing with builtins and kunit_tool, also enable the
test by default when KUNIT_ALL_TESTS is enabled.

The test can now be run via kunit_tool with:
./tools/testing/kunit/kunit.py run --arch=x86_64 \
--kconfig_add CONFIG_OF=y --kconfig_add CONFIG_OF_ADDRESS=y \
--kconfig_add CONFIG_MMC=y --kconfig_add CONFIG_MMC_SDHCI=y \
--kconfig_add CONFIG_MMC_SDHCI_PLTFM=y \
--kconfig_add CONFIG_MMC_SDHCI_OF_ASPEED=y \
'sdhci-of-aspeed'

(It may be worth adding a .kunitconfig at some point, as there are
enough dependencies to make that command scarily long.)

Acked-by: Daniel Latypov <[email protected]>
Acked-by: Ulf Hansson <[email protected]>
Signed-off-by: David Gow <[email protected]>
---

Changes since v2:
https://lore.kernel.org/linux-kselftest/[email protected]/
- Clean up the aspeed_sdc_init() function to get rid of an obsolete goto
(Thanks Daniel)
- Add Daniel and Ulf's Acked-by tags.

No changes since v1:
https://lore.kernel.org/linux-kselftest/[email protected]/

---
drivers/mmc/host/Kconfig | 5 ++--
drivers/mmc/host/sdhci-of-aspeed-test.c | 8 +-----
drivers/mmc/host/sdhci-of-aspeed.c | 34 +------------------------
3 files changed, 5 insertions(+), 42 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index d6144978e32d..10c563999d3d 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -169,8 +169,9 @@ config MMC_SDHCI_OF_ASPEED
If unsure, say N.

config MMC_SDHCI_OF_ASPEED_TEST
- bool "Tests for the ASPEED SDHCI driver"
- depends on MMC_SDHCI_OF_ASPEED && KUNIT=y
+ bool "Tests for the ASPEED SDHCI driver" if !KUNIT_ALL_TESTS
+ depends on MMC_SDHCI_OF_ASPEED && KUNIT
+ default KUNIT_ALL_TESTS
help
Enable KUnit tests for the ASPEED SDHCI driver. Select this
option only if you will boot the kernel for the purpose of running
diff --git a/drivers/mmc/host/sdhci-of-aspeed-test.c b/drivers/mmc/host/sdhci-of-aspeed-test.c
index 1ed4f86291f2..ecb502606c53 100644
--- a/drivers/mmc/host/sdhci-of-aspeed-test.c
+++ b/drivers/mmc/host/sdhci-of-aspeed-test.c
@@ -96,10 +96,4 @@ static struct kunit_suite aspeed_sdhci_test_suite = {
.test_cases = aspeed_sdhci_test_cases,
};

-static struct kunit_suite *aspeed_sdc_test_suite_array[] = {
- &aspeed_sdhci_test_suite,
- NULL,
-};
-
-static struct kunit_suite **aspeed_sdc_test_suites
- __used __section(".kunit_test_suites") = aspeed_sdc_test_suite_array;
+kunit_test_suite(aspeed_sdhci_test_suite);
diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
index 6e4e132903a6..ba6677bf7372 100644
--- a/drivers/mmc/host/sdhci-of-aspeed.c
+++ b/drivers/mmc/host/sdhci-of-aspeed.c
@@ -606,25 +606,6 @@ static struct platform_driver aspeed_sdc_driver = {

#if defined(CONFIG_MMC_SDHCI_OF_ASPEED_TEST)
#include "sdhci-of-aspeed-test.c"
-
-static inline int aspeed_sdc_tests_init(void)
-{
- return __kunit_test_suites_init(aspeed_sdc_test_suites);
-}
-
-static inline void aspeed_sdc_tests_exit(void)
-{
- __kunit_test_suites_exit(aspeed_sdc_test_suites);
-}
-#else
-static inline int aspeed_sdc_tests_init(void)
-{
- return 0;
-}
-
-static inline void aspeed_sdc_tests_exit(void)
-{
-}
#endif

static int __init aspeed_sdc_init(void)
@@ -637,18 +618,7 @@ static int __init aspeed_sdc_init(void)

rc = platform_driver_register(&aspeed_sdc_driver);
if (rc < 0)
- goto cleanup_sdhci;
-
- rc = aspeed_sdc_tests_init();
- if (rc < 0) {
- platform_driver_unregister(&aspeed_sdc_driver);
- goto cleanup_sdhci;
- }
-
- return 0;
-
-cleanup_sdhci:
- platform_driver_unregister(&aspeed_sdhci_driver);
+ platform_driver_unregister(&aspeed_sdhci_driver);

return rc;
}
@@ -656,8 +626,6 @@ module_init(aspeed_sdc_init);

static void __exit aspeed_sdc_exit(void)
{
- aspeed_sdc_tests_exit();
-
platform_driver_unregister(&aspeed_sdc_driver);
platform_driver_unregister(&aspeed_sdhci_driver);
}
--
2.37.0.rc0.161.g10f37bed90-goog

2022-06-25 05:31:17

by David Gow

[permalink] [raw]
Subject: [PATCH v3 3/5] thunderbolt: test: Use kunit_test_suite() macro

The new implementation of kunit_test_suite() for modules no longer
conflicts with module_init, so can now be used by the thunderbolt tests.

Also update the Kconfig entry to enable the test when KUNIT_ALL_TESTS is
enabled.

This means that kunit_tool can now successfully run and parse the test
results with, for example:
./tools/testing/kunit/kunit.py run --arch=x86_64 \
--kconfig_add CONFIG_PCI=y --kconfig_add CONFIG_USB4=y \
'thunderbolt'

Acked-by: Mika Westerberg <[email protected]>
Acked-by: Daniel Latypov <[email protected]>
Signed-off-by: David Gow <[email protected]>
---

Changes since v2:
https://lore.kernel.org/linux-kselftest/[email protected]/
- Don't permit USB4_KUNIT_TESTS to be enabled when USB4=y and KUNIT=m
i.e., add a dependency on (USB4=m || KUNIT=y)
This would result in undefined kunit symbols being used, otherwise.
- Add Daniel's Acked-by

Changes since v1:
https://lore.kernel.org/linux-kselftest/[email protected]/
- Actually include the Kconfig changes, which were mistakenly added to
the next patch in the series in v1.
- Add Acked-by tag from Mika Westerberg

---
drivers/thunderbolt/Kconfig | 6 ++++--
drivers/thunderbolt/domain.c | 3 ---
drivers/thunderbolt/tb.h | 8 --------
drivers/thunderbolt/test.c | 12 +-----------
4 files changed, 5 insertions(+), 24 deletions(-)

diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
index 4bfec8a28064..e76a6c173637 100644
--- a/drivers/thunderbolt/Kconfig
+++ b/drivers/thunderbolt/Kconfig
@@ -28,8 +28,10 @@ config USB4_DEBUGFS_WRITE
this for production systems or distro kernels.

config USB4_KUNIT_TEST
- bool "KUnit tests"
- depends on KUNIT=y
+ bool "KUnit tests" if !KUNIT_ALL_TESTS
+ depends on (USB4=m || KUNIT=y)
+ depends on KUNIT
+ default KUNIT_ALL_TESTS

config USB4_DMA_TEST
tristate "DMA traffic test driver"
diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index 2889a214dadc..99211f35a5cd 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -872,7 +872,6 @@ int tb_domain_init(void)
{
int ret;

- tb_test_init();
tb_debugfs_init();
tb_acpi_init();

@@ -890,7 +889,6 @@ int tb_domain_init(void)
err_acpi:
tb_acpi_exit();
tb_debugfs_exit();
- tb_test_exit();

return ret;
}
@@ -903,5 +901,4 @@ void tb_domain_exit(void)
tb_xdomain_exit();
tb_acpi_exit();
tb_debugfs_exit();
- tb_test_exit();
}
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 4602c69913fa..a831faa50f65 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -1271,12 +1271,4 @@ static inline void tb_service_debugfs_init(struct tb_service *svc) { }
static inline void tb_service_debugfs_remove(struct tb_service *svc) { }
#endif

-#ifdef CONFIG_USB4_KUNIT_TEST
-int tb_test_init(void);
-void tb_test_exit(void);
-#else
-static inline int tb_test_init(void) { return 0; }
-static inline void tb_test_exit(void) { }
-#endif
-
#endif
diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
index ee37f8b58f50..24c06e7354cd 100644
--- a/drivers/thunderbolt/test.c
+++ b/drivers/thunderbolt/test.c
@@ -2817,14 +2817,4 @@ static struct kunit_suite tb_test_suite = {
.test_cases = tb_test_cases,
};

-static struct kunit_suite *tb_test_suites[] = { &tb_test_suite, NULL };
-
-int tb_test_init(void)
-{
- return __kunit_test_suites_init(tb_test_suites);
-}
-
-void tb_test_exit(void)
-{
- return __kunit_test_suites_exit(tb_test_suites);
-}
+kunit_test_suite(tb_test_suite);
--
2.37.0.rc0.161.g10f37bed90-goog

2022-07-06 21:35:08

by Brendan Higgins

[permalink] [raw]
Subject: Re: [PATCH v3 5/5] mmc: sdhci-of-aspeed: test: Use kunit_test_suite() macro

On Sat, Jun 25, 2022 at 1:11 AM 'David Gow' via KUnit Development
<[email protected]> wrote:
>
> The kunit_test_suite() macro is no-longer incompatible with module_add,
> so its use can be reinstated.
>
> Since this fixes parsing with builtins and kunit_tool, also enable the
> test by default when KUNIT_ALL_TESTS is enabled.
>
> The test can now be run via kunit_tool with:
> ./tools/testing/kunit/kunit.py run --arch=x86_64 \
> --kconfig_add CONFIG_OF=y --kconfig_add CONFIG_OF_ADDRESS=y \
> --kconfig_add CONFIG_MMC=y --kconfig_add CONFIG_MMC_SDHCI=y \
> --kconfig_add CONFIG_MMC_SDHCI_PLTFM=y \
> --kconfig_add CONFIG_MMC_SDHCI_OF_ASPEED=y \
> 'sdhci-of-aspeed'
>
> (It may be worth adding a .kunitconfig at some point, as there are
> enough dependencies to make that command scarily long.)
>
> Acked-by: Daniel Latypov <[email protected]>
> Acked-by: Ulf Hansson <[email protected]>
> Signed-off-by: David Gow <[email protected]>

Acked-by: Brendan Higgins <[email protected]>

2022-07-06 22:10:47

by Brendan Higgins

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] thunderbolt: test: Use kunit_test_suite() macro

On Sat, Jun 25, 2022 at 1:10 AM David Gow <[email protected]> wrote:
>
> The new implementation of kunit_test_suite() for modules no longer
> conflicts with module_init, so can now be used by the thunderbolt tests.
>
> Also update the Kconfig entry to enable the test when KUNIT_ALL_TESTS is
> enabled.
>
> This means that kunit_tool can now successfully run and parse the test
> results with, for example:
> ./tools/testing/kunit/kunit.py run --arch=x86_64 \
> --kconfig_add CONFIG_PCI=y --kconfig_add CONFIG_USB4=y \
> 'thunderbolt'
>
> Acked-by: Mika Westerberg <[email protected]>
> Acked-by: Daniel Latypov <[email protected]>
> Signed-off-by: David Gow <[email protected]>

Acked-by: Brendan Higgins <[email protected]>