[PATCH i-g-t 1/3] lib/kunit: Support writable filter* parameters of kunit module
Kamil Konieczny
kamil.konieczny at linux.intel.com
Fri Jan 26 09:37:28 UTC 2024
Hi Janusz,
On 2024-01-25 at 17:52:10 +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
> reloading the module.
>
> This change also provides a workaround for the issue of impossibility to
> unload the base Kunit module on Xe platforms, available as soon as the
> module supports writable filter parameters.
>
> Since the base Kunit module is now unloaded from kunit_set_params() when
> needed, drop other attempts to unload it.
>
> To reuse an existing open_parameters() helper, moved it up in the source
> file to avoid a forward declaration.
Could you split your patch into smaller parts? You described four
actions you did, maybe four patches?
Regards,
Kamil
>
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
> ---
> lib/igt_kmod.c | 147 +++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 107 insertions(+), 40 deletions(-)
>
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index 250ab2107b..4f96dafe01 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -803,6 +803,78 @@ void igt_kselftest_get_tests(struct kmod_module *kmod,
> kmod_module_info_free_list(pre);
> }
>
> +static int open_parameters(const char *module_name)
> +{
> + char path[256];
> +
> + snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
> + return open(path, O_RDONLY);
> +}
> +
> +static bool kunit_set_params(const char **name, const char **value, int count)
> +{
> + static bool writable = true;
> + int i, ret, params;
> + char *opts = NULL;
> +
> + if (!writable) {
> + if (count)
> + if (igt_debug_on(asprintf(&opts, "%s=%s",
> + name[0], value[0]) < 0)) {
> + free(opts);
> + return false;
> + }
> +
> + for (i = 1; i < count; i++) {
> + char *head = opts;
> +
> + opts = NULL;
> + ret = asprintf(&opts, "%s %s=%s", head, name[i], value[i]);
> + free(head);
> + if (igt_debug_on(ret < 0)) {
> + free(opts);
> + return false;
> + }
> + }
> +
> + igt_ignore_warn(igt_kmod_unload("kunit", KMOD_REMOVE_FORCE));
> + }
> +
> + ret = igt_debug_on(igt_kmod_load("kunit", opts));
> + free(opts);
> +
> + if (ret && !writable)
> + return false;
> +
> + params = open_parameters("kunit");
> + ret = igt_debug_on(params < 0);
> +
> + for (i = 0; !ret && i < count; i++) {
> + char *param = igt_sysfs_get(params, name[i]);
> +
> + if (igt_debug_on_f(!param, "param: %s\n", name[i])) {
> + ret = -1;
> + continue;
> + }
> +
> + ret = strcmp(param, value[i]);
> + free(param);
> + if (ret)
> + ret = !igt_sysfs_set(params, name[i], value[i]);
> + }
> +
> + if (params >= 0)
> + close(params);
> +
> + if (!ret)
> + return true;
> + if (!writable)
> + return false;
> +
> + writable = false;
> + return kunit_set_params(name, value, count);
> +}
> +
> struct modprobe_data {
> struct kmod_module *kmod;
> const char *opts;
> @@ -814,11 +886,17 @@ struct modprobe_data {
>
> static void *modprobe_task(void *arg)
> {
> + const char *param[3] = { "filter_glob", "filter", "filter_action", };
> + const char *value[3] = { "*", "", "", };
> struct modprobe_data *data = arg;
> + bool ret;
> +
> + ret = kunit_set_params(param, value, ARRAY_SIZE(param));
>
> - data->err = modprobe(data->kmod, data->opts);
> + if (ret)
> + data->err = modprobe(data->kmod, data->opts);
>
> - if (igt_debug_on(data->err)) {
> + if (igt_debug_on(!ret || data->err)) {
> bool once = false;
> int err;
>
> @@ -1092,9 +1170,20 @@ 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 *param[3] = { "filter_glob", "filter", "filter_action", };
> + const char *value[3] = { "*", "module=none", "skip", };
> char *suite_name = NULL, *case_name = NULL;
> struct igt_ktap_result *r, *rn;
> struct igt_ktap_results *ktap;
> + unsigned long taints;
> int flags, err;
>
> igt_skip_on_f(tst->kmsg < 0, "Could not open /dev/kmsg\n");
> @@ -1105,26 +1194,23 @@ 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.
> - */
> - if (igt_debug_on(igt_kmod_load("kunit",
> - "filter=module=none filter_action=skip")))
> + if (igt_debug_on(!kunit_set_params(param, value, ARRAY_SIZE(param)))) {
> + /*
> + * TODO: drop the following workaround, required by LTS kernel
> + * v6.1 not capable of listing test cases when built as as
> + * module, when no longer needed.
> + * If updating writable filter parameters of the kunit base
> + * module with required values or reloading that module with
> + * those parameters specified fails then assume that we are
> + * running on a kernel with missing test case listing
> + * capabilities. Dont 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));
>
> @@ -1159,7 +1245,6 @@ static void kunit_get_tests(struct igt_list_head *tests,
> }
>
> igt_skip_on(kmod_module_remove_module(tst->kmod, KMOD_REMOVE_FORCE));
> - igt_skip_on(igt_kmod_unload("kunit", KMOD_REMOVE_FORCE));
>
> igt_skip_on_f(err,
> "KTAP parser failed while getting a list of test cases\n");
> @@ -1355,15 +1440,6 @@ void igt_kunit(const char *module_name, const char *name, const char *opts)
> igt_skip_on(igt_ktest_init(&tst, module_name));
> igt_skip_on(igt_ktest_begin(&tst));
>
> - /*
> - * Since we need to load kunit base module with specific
> - * options in order to get a list of test cases, make
> - * sure that the module is not loaded. However, since
> - * unload may fail if kunit base module is not loaded,
> - * ignore any failures, we'll fail later if still loaded.
> - */
> - igt_ignore_warn(igt_kmod_unload("kunit", KMOD_REMOVE_FORCE));
> -
> igt_assert(igt_list_empty(&tests));
> }
>
> @@ -1395,20 +1471,11 @@ void igt_kunit(const char *module_name, const char *name, const char *opts)
> kunit_results_free(&tests, &suite_name, &case_name);
>
> igt_ktest_end(&tst);
> - igt_debug_on(igt_kmod_unload("kunit", KMOD_REMOVE_FORCE));
> }
>
> igt_ktest_fini(&tst);
> }
>
> -static int open_parameters(const char *module_name)
> -{
> - char path[256];
> -
> - snprintf(path, sizeof(path), "/sys/module/%s/parameters", module_name);
> - return open(path, O_RDONLY);
> -}
> -
> int igt_ktest_init(struct igt_ktest *tst,
> const char *module_name)
> {
> --
> 2.43.0
>
More information about the igt-dev
mailing list