[PATCH i-g-t 1/2] lib/igt_kmod: Drop legacy kunit fallback

Lucas De Marchi lucas.demarchi at intel.com
Thu Oct 31 00:31:41 UTC 2024


Kunit has support for listing tests with debugfs for quite some time,
there's no need for the fallback to be happening: let's just start
failing the tests if we can't get the list of tests from debugfs.

Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
---
 lib/igt_kmod.c | 278 ++-----------------------------------------------
 1 file changed, 10 insertions(+), 268 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 75a0d057c..4cb9e886d 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -992,121 +992,6 @@ close:
 	return ret;
 }
 
-struct modprobe_data {
-	struct kmod_module *kmod;
-	const char *opts;
-	int err;
-	pthread_t parent;
-	pthread_mutex_t lock;
-	pthread_t thread;
-};
-
-static void *modprobe_task(void *arg)
-{
-	struct modprobe_data *data = arg;
-
-	data->err = modprobe(data->kmod, data->opts);
-
-	if (igt_debug_on(data->err)) {
-		bool once = false;
-		int err;
-
-		while (err = pthread_mutex_trylock(&data->lock),
-		       err && !igt_debug_on(err != EBUSY)) {
-			igt_debug_on(pthread_kill(data->parent, SIGCHLD) &&
-				     !once);
-			once = true;
-		}
-	} else {
-		/* let main thread use mutex to detect modprobe completion */
-		igt_debug_on(pthread_mutex_lock(&data->lock));
-	}
-
-	return NULL;
-}
-
-static void kunit_sigchld_handler(int signal)
-{
-}
-
-static int kunit_kmsg_result_get(struct igt_list_head *results,
-				 struct modprobe_data *modprobe,
-				 int fd, struct igt_ktap_results *ktap)
-{
-	struct sigaction sigchld = { .sa_handler = kunit_sigchld_handler, },
-			 saved;
-	char record[BUF_LEN + 1], *buf;
-	unsigned long taints;
-	int ret;
-
-	do {
-		int err;
-
-		if (igt_debug_on(igt_kernel_tainted(&taints)))
-			return -ENOTRECOVERABLE;
-
-		if (modprobe) {
-			err = igt_debug_on(sigaction(SIGCHLD, &sigchld, &saved));
-			if (err == -1)
-				return -errno;
-			else if (unlikely(err))
-				return err;
-
-			err = pthread_mutex_lock(&modprobe->lock);
-			switch (err) {
-			case EOWNERDEAD:
-				/* leave the mutex unrecoverable */
-				igt_debug_on(pthread_mutex_unlock(&modprobe->lock));
-				__attribute__ ((fallthrough));
-			case ENOTRECOVERABLE:
-				igt_debug_on(sigaction(SIGCHLD, &saved, NULL));
-				if (igt_debug_on(modprobe->err))
-					return modprobe->err;
-				break;
-			case 0:
-				break;
-			default:
-				igt_debug("pthread_mutex_lock() error: %d\n", err);
-				igt_debug_on(sigaction(SIGCHLD, &saved, NULL));
-				return -err;
-			}
-		}
-
-		ret = read(fd, record, BUF_LEN);
-
-		if (modprobe && !err) {	/* pthread_mutex_lock() succeeded */
-			igt_debug_on(pthread_mutex_unlock(&modprobe->lock));
-			igt_debug_on(sigaction(SIGCHLD, &saved, NULL));
-		}
-
-		if (igt_debug_on(!ret))
-			return -ENODATA;
-		if (igt_debug_on(ret == -1))
-			return -errno;
-		if (unlikely(igt_debug_on(ret < 0)))
-			break;
-
-		/* skip kmsg continuation lines */
-		if (igt_debug_on(*record == ' '))
-			continue;
-
-		/* NULL-terminate the record */
-		record[ret] = '\0';
-
-		/* detect start of log message, continue if not found */
-		buf = strchrnul(record, ';');
-		if (igt_debug_on(*buf == '\0'))
-			continue;
-		buf++;
-
-		ret = igt_ktap_parse(buf, ktap);
-		if (!ret || igt_debug_on(ret != -EINPROGRESS))
-			break;
-	} while (igt_list_empty(results));
-
-	return ret;
-}
-
 static void kunit_result_free(struct igt_ktap_result **r,
 			      char **suite_name, char **case_name)
 {
@@ -1181,146 +1066,6 @@ out_fclose:
 	return err;
 }
 
-static void __igt_kunit_legacy(struct igt_ktest *tst,
-			       const char *subtest,
-			       const char *opts)
-{
-	struct modprobe_data modprobe = { tst->kmod, opts, 0, pthread_self(), };
-	char *suite_name = NULL, *case_name = NULL;
-	struct igt_ktap_results *ktap;
-	struct igt_ktap_result *r;
-	pthread_mutexattr_t attr;
-	IGT_LIST_HEAD(results);
-	unsigned long taints;
-	int flags, ret;
-
-	igt_skip_on_f(tst->kmsg < 0, "Could not open /dev/kmsg\n");
-
-	igt_skip_on((flags = fcntl(tst->kmsg, F_GETFL, 0), flags < 0));
-	igt_skip_on_f(fcntl(tst->kmsg, F_SETFL, flags & ~O_NONBLOCK) == -1,
-		      "Could not set /dev/kmsg to blocking mode\n");
-
-	igt_skip_on(lseek(tst->kmsg, 0, SEEK_END) < 0);
-
-	igt_skip_on(pthread_mutexattr_init(&attr));
-	igt_skip_on(pthread_mutexattr_setrobust(&attr, PTHREAD_MUTEX_ROBUST));
-	igt_skip_on(pthread_mutex_init(&modprobe.lock, &attr));
-
-	ktap = igt_ktap_alloc(&results);
-	igt_require(ktap);
-
-	if (igt_debug_on(pthread_create(&modprobe.thread, NULL,
-					modprobe_task, &modprobe))) {
-		igt_ktap_free(&ktap);
-		igt_skip("Failed to create a modprobe thread\n");
-	}
-
-	do {
-		ret = kunit_kmsg_result_get(&results, &modprobe,
-					    tst->kmsg, ktap);
-		if (igt_debug_on(ret && ret != -EINPROGRESS))
-			break;
-
-		if (igt_debug_on(igt_list_empty(&results)))
-			break;
-
-		r = igt_list_first_entry(&results, r, link);
-
-		igt_dynamic_f("%s%s%s",
-			      strcmp(r->suite_name, subtest) ?  r->suite_name : "",
-			      strcmp(r->suite_name, subtest) ? "-" : "",
-			      r->case_name) {
-
-			if (r->code == IGT_EXIT_INVALID) {
-				/* parametrized test case, get actual result */
-				kunit_result_free(&r, &suite_name, &case_name);
-
-				igt_assert(igt_list_empty(&results));
-
-				ret = kunit_kmsg_result_get(&results, &modprobe,
-							    tst->kmsg, ktap);
-				if (ret != -EINPROGRESS)
-					igt_fail_on(ret);
-
-				igt_fail_on(igt_list_empty(&results));
-
-				r = igt_list_first_entry(&results, r, link);
-
-				igt_fail_on_f(strcmp(r->suite_name, suite_name),
-					      "suite_name expected: %s, got: %s\n",
-					      suite_name, r->suite_name);
-				igt_fail_on_f(strcmp(r->case_name, case_name),
-					      "case_name expected: %s, got: %s\n",
-					      case_name, r->case_name);
-			}
-
-			igt_assert_neq(r->code, IGT_EXIT_INVALID);
-
-			if (r->msg && *r->msg) {
-				igt_skip_on_f(r->code == IGT_EXIT_SKIP,
-					      "%s\n", r->msg);
-				igt_fail_on_f(r->code == IGT_EXIT_FAILURE,
-					      "%s\n", r->msg);
-				igt_abort_on_f(r->code == IGT_EXIT_ABORT,
-					      "%s\n", r->msg);
-			} else {
-				igt_skip_on(r->code == IGT_EXIT_SKIP);
-				igt_fail_on(r->code == IGT_EXIT_FAILURE);
-				if (r->code == IGT_EXIT_ABORT)
-					igt_fail(r->code);
-			}
-			igt_assert_eq(r->code, IGT_EXIT_SUCCESS);
-
-			switch (pthread_mutex_lock(&modprobe.lock)) {
-			case 0:
-				igt_debug_on(pthread_mutex_unlock(&modprobe.lock));
-				break;
-			case EOWNERDEAD:
-				/* leave the mutex unrecoverable */
-				igt_debug_on(pthread_mutex_unlock(&modprobe.lock));
-				__attribute__ ((fallthrough));
-			case ENOTRECOVERABLE:
-				igt_assert_eq(modprobe.err, 0);
-				break;
-			default:
-				igt_debug("pthread_mutex_lock() failed\n");
-				break;
-			}
-
-			igt_assert_eq(igt_kernel_tainted(&taints), 0);
-		}
-
-		kunit_result_free(&r, &suite_name, &case_name);
-
-	} while (ret == -EINPROGRESS);
-
-	kunit_results_free(&results, &suite_name, &case_name);
-
-	switch (pthread_mutex_lock(&modprobe.lock)) {
-	case 0:
-		igt_debug_on(pthread_cancel(modprobe.thread));
-		igt_debug_on(pthread_mutex_unlock(&modprobe.lock));
-		igt_debug_on(pthread_join(modprobe.thread, NULL));
-		break;
-	case EOWNERDEAD:
-		/* leave the mutex unrecoverable */
-		igt_debug_on(pthread_mutex_unlock(&modprobe.lock));
-		break;
-	case ENOTRECOVERABLE:
-		break;
-	default:
-		igt_debug("pthread_mutex_lock() failed\n");
-		igt_debug_on(pthread_join(modprobe.thread, NULL));
-		break;
-	}
-
-	igt_ktap_free(&ktap);
-
-	igt_skip_on(modprobe.err);
-	igt_skip_on(igt_kernel_tainted(&taints));
-	igt_skip_on_f(ret, "KTAP parser failed\n");
-}
-
 static bool kunit_get_tests(struct igt_list_head *tests,
 			    struct igt_ktest *tst,
 			    const char *suite,
@@ -1523,25 +1268,22 @@ void igt_kunit(const char *module_name, const char *suite, const char *opts)
 
 	/*
 	 * We need to use igt_subtest here, as otherwise it may crash with:
-	 *  skipping is allowed only in fixtures, subtests or igt_simple_main
+	 * "skipping is allowed only in fixtures, subtests or igt_simple_main"
 	 * if used on igt_main. This is also needed in order to provide
 	 * proper namespace for dynamic subtests, with is required for CI
 	 * and for documentation.
 	 */
 	igt_subtest_with_dynamic(subtest) {
-		/*
-		 * TODO: As soon as no longer needed by major Linux
-		 *	 distributions, replace the fallback to
-		 *	 __igt_kunit_legacy() processing path, required by
-		 *	 LTS kernels not capable of using KUnit filters for
-		 *	 listing test cases in KTAP format, with igt_require.
-		 */
+		bool has_tests;
+
 		kunit_debugfs_path(debugfs_path);
-		if (igt_debug_on(!*debugfs_path) ||
-		    !kunit_get_tests(&tests, &tst, suite, opts, debugfs_path, &debugfs_dir, &ktap))
-			__igt_kunit_legacy(&tst, subtest, opts);
-		else
-			__igt_kunit(&tst, subtest, opts, debugfs_path, &tests, &ktap);
+		igt_assert(*debugfs_path);
+
+		has_tests = kunit_get_tests(&tests, &tst, suite, opts,
+					    debugfs_path, &debugfs_dir, &ktap);
+		igt_assert(has_tests);
+
+		__igt_kunit(&tst, subtest, opts, debugfs_path, &tests, &ktap);
 	}
 
 	igt_fixture {
-- 
2.47.0



More information about the igt-dev mailing list