[PATCH i-g-t v4 4/8] lib/kunit: Support writable filter* parameters of kunit module

[Janusz Krzysztofik]

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.

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.

v4: Return early when opening module parameters directory fails (Lucas),
  - use error unwind pattern on errors from sysfs writes,
  - replace "human-readable" with "non-NULL" in comments about strings
    used as equivalents of default NULLs,
  - use empty string as filter_action NULL equivalent, but verify if
    successfully applied when critical,
  - refresh commit description by removing a statement that addressed an
    update to a comment on legacy path, no longer in scope of this patch.
v3: Don't read-compare-write, just write the values (Lucas),
  - skip calling igt_sysfs_set() when the string to be written to
    filter_action is empty (Lucas),
  - warn if we leave the filter_action set to "skip" while setting a non-
    default value of the filter parameter,
  - transform generic kunit_set_params() to kunit_set_filtering().
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 | 75 +++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 62 insertions(+), 13 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 2a1b09c0bd..5931121944 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -811,6 +811,63 @@ static int open_parameters(const char *module_name)
 	return open(path, O_RDONLY);
 }
 
+static bool kunit_set_filtering(const char *filter_glob, const char *filter,
+				const char *filter_action)
+{
+	int params;
+	bool ret;
+
+	params = open_parameters("kunit");
+	if (igt_debug_on(params < 0))
+		return false;
+
+	/*
+	 * Default values of the KUnit base module filtering parameters
+	 * are all NULLs.  Reapplying those NULLs over sysfs once
+	 * overwritten with non-NULL strings seems not possible.
+	 * As a workaround, we use non-NULL strings that exhibit
+	 * the same behaviour as if default NULLs were in place.
+	 */
+	ret = !igt_debug_on(!igt_sysfs_set(params, "filter_glob",
+					   filter_glob ?: "*"));
+	if (!ret)
+		goto close;
+
+	ret = !igt_debug_on(!igt_sysfs_set(params, "filter",
+					   filter ?: "module!=none"));
+	if (!ret)
+		goto close;
+
+	ret = !igt_debug_on(!igt_sysfs_set(params, "filter_action",
+					   filter_action ?: ""));
+
+	/*
+	 * TODO: Drop the extra check below as soon as igt_sysfs_set()
+	 *	 can correctly process empty strings which we are using
+	 *	 as filter_action NULL equivalent.
+	 *
+	 * We need this check only when NULL is requested for "filter_action"
+	 * and not for "filter" parameter, otherwise, even if "filter_action"
+	 * was previously set to "skip", we don't care since our
+	 * "module!=none" default filter guarantees that no test cases are
+	 * filtered out to be processed as "filter_action" says.
+	 */
+	if (ret && !filter_action && filter) {
+		filter_action = igt_sysfs_get(params, "filter_action");
+
+		ret = !(igt_debug_on_f(!filter_action,
+				       "open() failed\n") ||
+			igt_debug_on_f(strlen(filter_action),
+				       "empty string not applied\n"));
+		free((char *)filter_action);
+	}
+
+close:
+	close(params);
+
+	return ret;
+}
+
 struct modprobe_data {
 	struct kmod_module *kmod;
 	const char *opts;
@@ -1121,9 +1178,7 @@ static bool kunit_get_tests(struct igt_list_head *tests,
 	 * perfectly -- seems to be more safe than extracting a test case list
 	 * of unknown length from /dev/kmsg.
 	 */
-	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_filtering(NULL, "module=none", "skip")))
 		return false;
 
 	igt_skip_on(modprobe(tst->kmod, opts));
@@ -1192,16 +1247,7 @@ 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));
+				igt_require(kunit_set_filtering(NULL, NULL, NULL));
 
 				igt_assert_eq(pthread_mutexattr_init(&attr), 0);
 				igt_assert_eq(pthread_mutexattr_setrobust(&attr,
@@ -1370,6 +1416,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