[RFC i-g-t] kunit: Remove hard-coded test list from xe_live_ktest
Kamil Konieczny
kamil.konieczny at linux.intel.com
Mon Aug 26 11:46:45 UTC 2024
Hi John.C.Harrison,
On 2024-08-23 at 11:24:18 -0700, John.C.Harrison at Intel.com wrote:
> From: johnharr <johnharr at invalid-email.com>
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Again invalid e-mail here.
>
> The list of supported kunit tests is currently hard coded. Which means
> that adding a new test to the kernel also requires patches to IGT as
> well.
>
> The list of available kunit tests is already exported by the kernel.
> So there is no need to bake a list into the IGT source code. So, add
> support for querying the test list dynamically.
>
> NB: Currently, the test list can only be queried by root but the IGT
> build system queries all tests at compile time. In theory this should
> not be a problem. However, the kunit helper code is all written to run
> inside a test and not inside the prep code, which means that when it
> fails to open the root only interfaces, it calls 'skip'. And skip is
> not allowed outside of running a test. Hence the build fails with:
> skipping is allowed only in fixtures, subtests or igt_simple_main
Looks like we should fix it, move out skips from kunit libs.
>
> And of course, putting all the query code inside an igt_fixture block
> means it doesn't get run as part of --show-testlist or --list-subtests.
>
> Reworking the kunit code to fail cleanly rather than skipping
> everywhere seems much more invasive. It is also not clear why the
> kunit code is considered 'library' when it is execlusively used from
> the kunit test alone!? So posting this as an RFC before going too far
> down the possible rabbit hole.
>
As I see this, it is a lib because some other vendor (like amdpgu)
could use it.
+cc Janusz
Cc: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
btw could you split this into two patches, one for lib/* and
one for tests/intel/xe_live_ktest ?
> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> ---
> lib/igt_kmod.c | 68 +++++++++++++++++++++++++++++++++++++
> lib/igt_kmod.h | 6 ++++
> tests/intel/xe_live_ktest.c | 60 ++++++++++++++++----------------
> 3 files changed, 103 insertions(+), 31 deletions(-)
>
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index 464c0dcf484a..69c1254b397c 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -1414,6 +1414,74 @@ static void __igt_kunit(struct igt_ktest *tst,
> }
> }
>
> +/**
> + * igt_kunit:
> + * @module_name: the name of the module
> + * @suite: the name of test suite to be executed, also used as subtest name;
> + * if NULL then test cases from all test suites provided by the module
> + * are executed as dynamic sub-subtests of one IGT subtest, which name
> + * is derived from the module name by cutting off its optional trailing
> + * _test or _kunit suffix
> + * @opts: options to load the module
> + *
> + * Loads the test module, parses its (k)tap dmesg output, then unloads it
> + */
> +void igt_kunit_get_test_names(const char *module_name, const char *opts, struct igt_list_head *names)
> +{
> + char debugfs_path[PATH_MAX] = { '\0', };
> + struct igt_ktest tst = { .kmsg = -1, };
> + struct igt_ktap_results *ktap = NULL;
> + const char *subtest;
> + char *suite_name = NULL, *case_name = NULL;
> + DIR *debugfs_dir = NULL;
> + IGT_LIST_HEAD(tests);
> +
> + subtest = strdup(module_name);
> + if (!igt_debug_on(!subtest)) {
> + char *suffix = strstr(subtest, "_test");
> +
> + if (!suffix)
> + suffix = strstr(subtest, "_kunit");
> +
> + if (suffix)
> + *suffix = '\0';
> + }
> +
> + igt_require(subtest);
> +
> + igt_require(!igt_ktest_init(&tst, module_name));
> + igt_require(!igt_ktest_begin(&tst));
> +
> + igt_ignore_warn(igt_kmod_load("kunit", NULL));
> +
> + kunit_debugfs_path(debugfs_path);
> + igt_info("kunit path: %s\n", debugfs_path);
> + if (igt_debug_on(!*debugfs_path) ||
> + !kunit_get_tests(&tests, &tst, NULL, opts, debugfs_path, &debugfs_dir, &ktap)) {
> + igt_info("kunit - no tests found!?\n");
> + } else {
> + struct igt_ktap_result *t;
> + igt_info("kunit tests:\n");
> + igt_list_for_each_entry(t, &tests, link) {
> + struct igt_kunit_names *name = malloc(sizeof(*name));
> + name->suite = strdup(t->suite_name);
> + name->sub = strdup(t->case_name);
> + igt_info(" %s / %s\n", t->suite_name, t->case_name);
> + igt_list_add(&name->link, names);
> + }
> + }
> +
> + igt_ktap_free(&ktap);
> +
> + kunit_results_free(&tests, &suite_name, &case_name);
> +
> + if (debugfs_dir)
> + closedir(debugfs_dir);
> +
> + igt_ktest_end(&tst);
> + igt_ktest_fini(&tst);
> +}
> +
> /**
> * igt_kunit:
> * @module_name: the name of the module
> diff --git a/lib/igt_kmod.h b/lib/igt_kmod.h
> index efb46da128d4..2f608eb69d48 100644
> --- a/lib/igt_kmod.h
> +++ b/lib/igt_kmod.h
> @@ -71,7 +71,13 @@ static inline int igt_xe_driver_unload(void)
> int igt_amdgpu_driver_load(const char *opts);
> int igt_amdgpu_driver_unload(void);
>
> +struct igt_kunit_names {
> + char *suite;
> + char *sub;
> + struct igt_list_head link;
> +};
> void igt_kunit(const char *module_name, const char *name, const char *opts);
> +void igt_kunit_get_test_names(const char *module_name, const char *opts, struct igt_list_head *names);
>
> void igt_kselftests(const char *module_name,
> const char *module_options,
> diff --git a/tests/intel/xe_live_ktest.c b/tests/intel/xe_live_ktest.c
> index 4376d5df7751..13265e87ba9d 100644
> --- a/tests/intel/xe_live_ktest.c
> +++ b/tests/intel/xe_live_ktest.c
> @@ -9,40 +9,38 @@
> * Sub-category: kunit
> * Functionality: kunit test
> * Test category: functionality test
> - *
> - * SUBTEST: xe_bo
> - * Description:
> - * Kernel dynamic selftests to check if GPU buffer objects are
> - * being handled properly.
> - * Functionality: bo
> - *
> - * SUBTEST: xe_dma_buf
> - * Description: Kernel dynamic selftests for dmabuf functionality.
> - * Functionality: dmabuf test
> - *
> - * SUBTEST: xe_migrate
> - * Description:
> - * Kernel dynamic selftests to check if page table migrations
> - * are working properly.
> - * Functionality: migrate
> - *
> - * SUBTEST: xe_mocs
> - * Description:
> - * Kernel dynamic selftests to check mocs configuration.
> - * Functionality: mocs configuration
> */
>
Does that mean we will not have any subtests here? The point
into moving subtests names into documentation was to have a testlist
generated during compilation, so it will not depend on running
test with --list option. Adding also Katarzyna to cc.
Cc: Katarzyna Piecielska <katarzyna.piecielska at intel.com>
Regards,
Kamil
> -static const char *live_tests[] = {
> - "xe_bo",
> - "xe_dma_buf",
> - "xe_migrate",
> - "xe_mocs",
> -};
> -
> igt_main
> {
> - int i;
> + IGT_LIST_HEAD(names);
> + IGT_LIST_HEAD(done);
> + struct igt_kunit_names *name, *cmp;
> +
> + igt_kunit_get_test_names("xe_live_test", NULL, &names);
> +
> + while (!igt_list_empty(&names)) {
> + bool found = false;
> +
> + name = igt_list_first_entry(&names, name, link);
> + igt_list_del(&name->link);
> +
> + /*
> + * Retuned list is sub-tests.
> + * So, need filter out duplicated top level names.
> + */
> + igt_list_for_each_entry(cmp, &done, link) {
> + if (strcmp(name->suite, cmp->suite) != 0)
> + continue;
> +
> + found = true;
> + }
> +
> + if (found)
> + continue;
> +
> + igt_list_add(&name->link, &done);
>
> - for (i = 0; i < ARRAY_SIZE(live_tests); i++)
> - igt_kunit("xe_live_test", live_tests[i], NULL);
> + igt_kunit("xe_live_test", name->suite, NULL);
> + }
> }
> --
> 2.46.0
>
More information about the igt-dev
mailing list