[PATCH i-g-t v2 4/6] lib/kunit: Support writable filter* parameters of kunit module
Kamil Konieczny
kamil.konieczny at linux.intel.com
Thu Feb 1 15:07:41 UTC 2024
Hi Janusz,
On 2024-01-31 at 19:03:51 +0100, Janusz Krzysztofik wrote:
> Instead of wasting resources on reloading the base Kunit module each time
> a different set of filter parameters is needed, try to write the required
> values to sysfs representation of those parameters. If that fails (e.g.
> on older LTS kernels with read-only filter parameters), fall back to
> legacy "no list of test cases available" mode as we do on kernels with
> those module parameters not supported at all.
>
> This modification will also serve as a workaround for the issue of
> impossibility to unload the base Kunit module on Xe platforms, available
> to IGT as soon as the module supports writable filter parameters.
>
> v2: Work around ineffective writes of empty strings to sysfs module
> parameter files (Lucas) by using human readable non-empty strings that
> give the same results as default NULLs,
> - drop fallback to reload of base Kunit module method if assigning new
> values to module parameters via sysfs fails (Lucas), instead use the
> existing fallback to blind execution like if getting a list of test
> cases was not supported at all,
> - split move of open_parameters() helper up in the source file as well
> as cleanup of base KUnit module unloading to separate patches (Kamil),
> - address the issue of the second paragraph of commit description
> suggesting two separate changes combined in one patch (Kamil) by
> rewording the description.
>
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> ---
> lib/igt_kmod.c | 114 ++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 84 insertions(+), 30 deletions(-)
>
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index d2c28d0a64..1d57ea708d 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -811,6 +811,56 @@ static int open_parameters(const char *module_name)
> return open(path, O_RDONLY);
> }
>
> +static const char *kunit_params[3] = {
> + "filter_glob", "filter", "filter_action",
> +};
> +
> +static bool kunit_set_params(const char **values)
> +{
> + /*
> + * Reapplying default NULLs to module parameters via sysfs seems not
> + * possible, and applying empty strings doesn't work as expected,
> + * leaving current values untouched. As a workaround, use
> + * human-readable non-empty strings that exhibit the same behaviour
> + * as if default NULLs were in place. In case of filter_action
> + * parameter there is no such obvious non-empty string, however,
> + * that's not a problem for us since even if it was previously set
> + * to "skip" and we leave it as is, our "module!=none" default filter
> + * guarantees that no test cases are filtered out to be skipped.
> + */
> + const char *defaults[ARRAY_SIZE(kunit_params)] = {
> + /* filter_glob, filter, filter_action */
> + "*", "module!=none", "",
If writing empty string poses a problem, what about writing
one space? Could it work?
One more nit below.
> + };
> + int i, params;
> + bool ret;
> +
> + params = open_parameters("kunit");
> + ret = !igt_debug_on(params < 0);
> +
> + for (i = 0; ret && i < ARRAY_SIZE(kunit_params); i++) {
> + char *param = igt_sysfs_get(params, kunit_params[i]);
> +
> + if (igt_debug_on_f(!param, "param: %s\n", kunit_params[i])) {
> + ret = false;
> + break;
> + }
> +
> + ret = !strcmp(param, values[i] ?: defaults[i]);
> + free(param);
> + if (!ret)
> + ret = !igt_debug_on(!igt_sysfs_set(params,
> + kunit_params[i],
> + values[i] ?:
> + defaults[i]));
> + }
> +
> + if (params >= 0)
> + close(params);
> +
> + return ret;
> +}
> +
> struct modprobe_data {
> struct kmod_module *kmod;
> const char *opts;
> @@ -1100,6 +1150,18 @@ static void kunit_get_tests(struct igt_list_head *tests,
> const char *filter,
> const char *opts)
> {
> + /*
> + * To get a list of test cases provided by a kunit test module, ask the
> + * generic kunit module to respond with SKIP result for each test found.
> + * We could also try to use action=list kunit parameter to get the
> + * listing, however, parsing a KTAP report -- something that we already
> + * can do perfectly -- seems to be more safe than extracting a test case
> + * list of unknown length from /dev/kmsg.
> + */
> + const char *values[ARRAY_SIZE(kunit_params)] = {
> + /* filter_glob, filter, filter_action */
> + NULL, "module=none", "skip",
> + };
> char *suite_name = NULL, *case_name = NULL;
> struct igt_ktap_result *r, *rn;
> struct igt_ktap_results *ktap;
> @@ -1113,27 +1175,21 @@ static void kunit_get_tests(struct igt_list_head *tests,
>
> igt_skip_on(lseek(tst->kmsg, 0, SEEK_END) < 0);
>
> - /*
> - * To get a list of test cases provided by a kunit test module, ask the
> - * generic kunit module to respond with SKIP result for each test found.
> - * We could also use action=list kunit parameter to get the listing,
> - * however, parsing a KTAP report -- something that we already can do
> - * perfectly -- seems to be more safe than extracting a test case list
> - * of unknown length from /dev/kmsg.
> - *
> - * TODO: drop the following workaround, which is required by LTS kernel
> - * v6.1 not capable of listing test cases when built as a module.
> - * If loading the kunit module with required parameters fails then
> - * assume that we are running on a kernel with missing test case listing
> - * capabilities. Dont's skip but just return with empty list of test
> - * cases, that should tell the caller to use a legacy method of
> - * iterating over KTAP results collected from blind execution of all
> - * Kunit test cases provided by a Kunit test module.
> - */
> - igt_ignore_warn(igt_kmod_unload("kunit", KMOD_REMOVE_FORCE));
> - if (igt_debug_on(igt_kmod_load("kunit",
> - "filter=module=none filter_action=skip")))
> + if (igt_debug_on(!kunit_set_params(values))) {
> + /*
> + * TODO: drop the following workaround, required by LTS kernel
> + * v6.1 not capable of listing test cases when built as as
s/as as/as/
Regards,
Kamil
> + * module, when no longer needed.
> + * If updating writable filter parameters of the kunit base
> + * module with required values then assume that we are running
> + * on a kernel with missing test case listing capabilities.
> + * Don't skip but just return with empty list of test cases,
> + * which should tell the caller to use a legacy method of
> + * iterating over KTAP results collected from blind execution
> + * of all Kunit test cases provided by a Kunit test module.
> + */
> return;
> + }
>
> igt_skip_on(modprobe(tst->kmod, opts));
>
> @@ -1200,16 +1256,11 @@ static void __igt_kunit(struct igt_ktest *tst,
> t->case_name) {
>
> if (!modprobe.thread) {
> - /*
> - * Since we have successfully loaded the kunit
> - * base module with non-default parameters in
> - * order to get a list of test cases, now we
> - * have to unload it so it is then automatically
> - * reloaded with default parameter values when
> - * we load the test module again for execution.
> - */
> - igt_skip_on(igt_kmod_unload("kunit",
> - KMOD_REMOVE_FORCE));
> + const char *values[ARRAY_SIZE(kunit_params)] = {
> + NULL, NULL, NULL,
> + };
> +
> + igt_require(kunit_set_params(values));
>
> igt_assert_eq(pthread_mutexattr_init(&attr), 0);
> igt_assert_eq(pthread_mutexattr_setrobust(&attr,
> @@ -1378,6 +1429,9 @@ void igt_kunit(const char *module_name, const char *name, const char *opts)
> igt_assert(igt_list_empty(&tests));
> }
>
> + /* We need the base KUnit module loaded if not built-in */
> + igt_ignore_warn(igt_kmod_load("kunit", NULL));
> +
> /*
> * We need to use igt_subtest here, as otherwise it may crash with:
> * skipping is allowed only in fixtures, subtests or igt_simple_main
> --
> 2.43.0
>
More information about the Intel-xe
mailing list