All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH v7 i-g-t 00/10] Introduce KUnit
@ 2023-06-13  7:27 Dominik Karol Piatkowski
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 01/10] lib/igt_kmod: rename kselftest functions to ktest Dominik Karol Piatkowski
                   ` (11 more replies)
  0 siblings, 12 replies; 22+ messages in thread
From: Dominik Karol Piatkowski @ 2023-06-13  7:27 UTC (permalink / raw)
  To: igt-dev

This series is a continuation of Isabella's work on introducing
KUnit to IGT.

Sample drm_buddy output:
        Starting subtest: drm_buddy_test
        [thread:23835] TAP version 1
        [thread:23835] Executing 6 tests in: drm_buddy
        Starting dynamic subtest: drm_buddy-drm_test_buddy_alloc_limit
        Dynamic subtest drm_buddy-drm_test_buddy_alloc_limit: SUCCESS (0.000s)
        Starting dynamic subtest: drm_buddy-drm_test_buddy_alloc_range
        Dynamic subtest drm_buddy-drm_test_buddy_alloc_range: SUCCESS (0.000s)
        Starting dynamic subtest: drm_buddy-drm_test_buddy_alloc_optimistic
        Dynamic subtest drm_buddy-drm_test_buddy_alloc_optimistic: SUCCESS (0.000s)
        Starting dynamic subtest: drm_buddy-drm_test_buddy_alloc_pessimistic
        Dynamic subtest drm_buddy-drm_test_buddy_alloc_pessimistic: SUCCESS (0.000s)
        Starting dynamic subtest: drm_buddy-drm_test_buddy_alloc_smoke
        Dynamic subtest drm_buddy-drm_test_buddy_alloc_smoke: SUCCESS (0.000s)
        Starting dynamic subtest: drm_buddy-drm_test_buddy_alloc_pathological
        Dynamic subtest drm_buddy-drm_test_buddy_alloc_pathological: SUCCESS (0.000s)
        Starting dynamic subtest: drm_buddy
        Dynamic subtest drm_buddy: SUCCESS (0.000s)
        Subtest drm_buddy_test: SUCCESS (0.182s)

The issue of possibility of too many sublevels occurrence is solved
by name concatenation.

Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Cc: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>

Dominik Karol Piatkowski (4):
  Change logic of ktap parser to run on a thread
  KUnit: Remove igt_kselftest fallback
  KUnit: Change subtest name from all-tests to module name
  KUnit: replace abort with graceful skip

Isabella Basso (4):
  lib/igt_kmod: rename kselftest functions to ktest
  lib/igt_kmod.c: check if module is builtin before attempting to unload
    it
  lib/igt_kmod: add compatibility for KUnit
  tests: DRM selftests: switch to KUnit

Mauro Carvalho Chehab (2):
  kunit tests: add an optional name for the selftests
  tests/xe: Add a test that launches the xe driver live kunit tests

 lib/igt_kmod.c           | 148 +++++++++-
 lib/igt_kmod.h           |  14 +-
 lib/igt_ktap.c           | 615 +++++++++++++++++++++++++++++++++++++++
 lib/igt_ktap.h           |  50 ++++
 lib/meson.build          |   1 +
 tests/drm_buddy.c        |   2 +-
 tests/drm_mm.c           |  44 +--
 tests/kms_selftest.c     |  19 +-
 tests/meson.build        |   1 +
 tests/xe/xe_live_ktest.c |  52 ++++
 10 files changed, 905 insertions(+), 41 deletions(-)
 create mode 100644 lib/igt_ktap.c
 create mode 100644 lib/igt_ktap.h
 create mode 100644 tests/xe/xe_live_ktest.c

-- 
2.34.1



^ permalink raw reply	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 01/10] lib/igt_kmod: rename kselftest functions to ktest
  2023-06-13  7:27 [igt-dev] [PATCH v7 i-g-t 00/10] Introduce KUnit Dominik Karol Piatkowski
@ 2023-06-13  7:27 ` Dominik Karol Piatkowski
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 02/10] lib/igt_kmod.c: check if module is builtin before attempting to unload it Dominik Karol Piatkowski
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Dominik Karol Piatkowski @ 2023-06-13  7:27 UTC (permalink / raw)
  To: igt-dev; +Cc: Isabella Basso

From: Isabella Basso <isabbasso@riseup.net>

This aims at making IGT's structure more general to different kernel
testing frameworks such as KUnit, as they use a lot of the same
functionality.

Reviewed-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Signed-off-by: Isabella Basso <isabbasso@riseup.net>

Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
Reviewed-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 lib/igt_kmod.c | 22 +++++++++++-----------
 lib/igt_kmod.h | 12 ++++++------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index ccf0063ca..93fa20067 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -749,8 +749,8 @@ static int open_parameters(const char *module_name)
 	return open(path, O_RDONLY);
 }
 
-int igt_kselftest_init(struct igt_kselftest *tst,
-		       const char *module_name)
+int igt_ktest_init(struct igt_ktest *tst,
+		   const char *module_name)
 {
 	int err;
 
@@ -769,7 +769,7 @@ int igt_kselftest_init(struct igt_kselftest *tst,
 	return 0;
 }
 
-int igt_kselftest_begin(struct igt_kselftest *tst)
+int igt_ktest_begin(struct igt_ktest *tst)
 {
 	int err;
 
@@ -784,7 +784,7 @@ int igt_kselftest_begin(struct igt_kselftest *tst)
 	return 0;
 }
 
-int igt_kselftest_execute(struct igt_kselftest *tst,
+int igt_kselftest_execute(struct igt_ktest *tst,
 			  struct igt_kselftest_list *tl,
 			  const char *options,
 			  const char *result)
@@ -822,13 +822,13 @@ int igt_kselftest_execute(struct igt_kselftest *tst,
 	return err;
 }
 
-void igt_kselftest_end(struct igt_kselftest *tst)
+void igt_ktest_end(struct igt_ktest *tst)
 {
 	kmod_module_remove_module(tst->kmod, KMOD_REMOVE_FORCE);
 	close(tst->kmsg);
 }
 
-void igt_kselftest_fini(struct igt_kselftest *tst)
+void igt_ktest_fini(struct igt_ktest *tst)
 {
 	free(tst->module_name);
 	kmod_module_unref(tst->kmod);
@@ -851,15 +851,15 @@ void igt_kselftests(const char *module_name,
 		    const char *result,
 		    const char *filter)
 {
-	struct igt_kselftest tst;
+	struct igt_ktest tst;
 	IGT_LIST_HEAD(tests);
 	struct igt_kselftest_list *tl, *tn;
 
-	if (igt_kselftest_init(&tst, module_name) != 0)
+	if (igt_ktest_init(&tst, module_name) != 0)
 		return;
 
 	igt_fixture
-		igt_require(igt_kselftest_begin(&tst) == 0);
+		igt_require(igt_ktest_begin(&tst) == 0);
 
 	igt_kselftest_get_tests(tst.kmod, filter, &tests);
 	igt_subtest_with_dynamic(filter ?: "all-tests") {
@@ -878,9 +878,9 @@ void igt_kselftests(const char *module_name,
 	}
 
 	igt_fixture {
-		igt_kselftest_end(&tst);
+		igt_ktest_end(&tst);
 		igt_require(!igt_list_empty(&tests));
 	}
 
-	igt_kselftest_fini(&tst);
+	igt_ktest_fini(&tst);
 }
diff --git a/lib/igt_kmod.h b/lib/igt_kmod.h
index d05af4a69..ff59f1ec1 100644
--- a/lib/igt_kmod.h
+++ b/lib/igt_kmod.h
@@ -76,7 +76,7 @@ void igt_kselftests(const char *module_name,
 		    const char *result_option,
 		    const char *filter);
 
-struct igt_kselftest {
+struct igt_ktest {
 	struct kmod_module *kmod;
 	char *module_name;
 	int kmsg;
@@ -89,19 +89,19 @@ struct igt_kselftest_list {
 	char param[];
 };
 
-int igt_kselftest_init(struct igt_kselftest *tst,
+int igt_ktest_init(struct igt_ktest *tst,
 		       const char *module_name);
-int igt_kselftest_begin(struct igt_kselftest *tst);
+int igt_ktest_begin(struct igt_ktest *tst);
 
 void igt_kselftest_get_tests(struct kmod_module *kmod,
 			     const char *filter,
 			     struct igt_list_head *tests);
-int igt_kselftest_execute(struct igt_kselftest *tst,
+int igt_kselftest_execute(struct igt_ktest *tst,
 			  struct igt_kselftest_list *tl,
 			  const char *module_options,
 			  const char *result);
 
-void igt_kselftest_end(struct igt_kselftest *tst);
-void igt_kselftest_fini(struct igt_kselftest *tst);
+void igt_ktest_end(struct igt_ktest *tst);
+void igt_ktest_fini(struct igt_ktest *tst);
 
 #endif /* IGT_KMOD_H */
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 02/10] lib/igt_kmod.c: check if module is builtin before attempting to unload it
  2023-06-13  7:27 [igt-dev] [PATCH v7 i-g-t 00/10] Introduce KUnit Dominik Karol Piatkowski
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 01/10] lib/igt_kmod: rename kselftest functions to ktest Dominik Karol Piatkowski
@ 2023-06-13  7:27 ` Dominik Karol Piatkowski
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 03/10] lib/igt_kmod: add compatibility for KUnit Dominik Karol Piatkowski
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Dominik Karol Piatkowski @ 2023-06-13  7:27 UTC (permalink / raw)
  To: igt-dev; +Cc: Isabella Basso

From: Isabella Basso <isabbasso@riseup.net>

This change makes `igt_kmod_unload_r` safer as it checks whether the
module can be unloaded before attempting it.

v2 -> v3:
- Fix commit message
- Make return value clearer

Acked-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Signed-off-by: Isabella Basso <isabbasso@riseup.net>

Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
Acked-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 lib/igt_kmod.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 93fa20067..26d58e293 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -259,6 +259,9 @@ static int igt_kmod_unload_r(struct kmod_module *kmod, unsigned int flags)
 	int err, tries;
 	const char *mod_name = kmod_module_get_name(kmod);
 
+	if (kmod_module_get_initstate(kmod) == KMOD_MODULE_BUILTIN)
+		return 0;
+
 	holders = kmod_module_get_holders(kmod);
 	kmod_list_foreach(pos, holders) {
 		struct kmod_module *it = kmod_module_get_module(pos);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 03/10] lib/igt_kmod: add compatibility for KUnit
  2023-06-13  7:27 [igt-dev] [PATCH v7 i-g-t 00/10] Introduce KUnit Dominik Karol Piatkowski
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 01/10] lib/igt_kmod: rename kselftest functions to ktest Dominik Karol Piatkowski
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 02/10] lib/igt_kmod.c: check if module is builtin before attempting to unload it Dominik Karol Piatkowski
@ 2023-06-13  7:27 ` Dominik Karol Piatkowski
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 04/10] tests: DRM selftests: switch to KUnit Dominik Karol Piatkowski
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Dominik Karol Piatkowski @ 2023-06-13  7:27 UTC (permalink / raw)
  To: igt-dev; +Cc: Isabella Basso

From: Isabella Basso <isabbasso@riseup.net>

This adds functions for both executing the tests as well as parsing (K)TAP
kmsg output, as per the KTAP spec [1].

[1] https://www.kernel.org/doc/html/latest/dev-tools/ktap.html

v1 -> v2:
- refactor igt_kunit function and ktap parser so that we have only one
  parser that we call only once (code size is now less than half the
  size as v1)
- add lookup_value helper
- fix parsing problems
v2 -> v3:
- move ktap parsing functions to own file
- rename to ktap_parser
- get rid of unneeded pointers in igt_kunit
- change return values to allow for subsequent call to igt_kselftests if
  needed
- add docs to parsing functions and helpers
- switch to line buffering
- add line buffering logging helper
- fix kunit module handling
- fix parsing of version lines
- use igt_subtest blocks to improve output handling on the CI
- fix output handling during crashes

Signed-off-by: Isabella Basso <isabbasso@riseup.net>

v3 -> v4:
- handle igt_ktap_parser fail with IGT_EXIT_ABORT code

v4 -> v5:
- added missing newlines in igt_warn
- removed setvbuf

Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
Acked-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Cc: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>
---
 lib/igt_kmod.c  |  79 ++++++++++++
 lib/igt_kmod.h  |   2 +
 lib/igt_ktap.c  | 334 ++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_ktap.h  |  31 +++++
 lib/meson.build |   1 +
 5 files changed, 447 insertions(+)
 create mode 100644 lib/igt_ktap.c
 create mode 100644 lib/igt_ktap.h

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 26d58e293..21e801bde 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -29,6 +29,7 @@
 #include "igt_aux.h"
 #include "igt_core.h"
 #include "igt_kmod.h"
+#include "igt_ktap.h"
 #include "igt_sysfs.h"
 #include "igt_taints.h"
 
@@ -744,6 +745,84 @@ void igt_kselftest_get_tests(struct kmod_module *kmod,
 	kmod_module_info_free_list(pre);
 }
 
+/**
+ * igt_kunit:
+ * @module_name: the name of the module
+ * @opts: options to load the module
+ *
+ * Loads the test module, parses its (k)tap dmesg output, then unloads it
+ *
+ * Returns: IGT default codes
+ */
+int igt_kunit(const char *module_name, const char *opts)
+{
+	struct igt_ktest tst;
+	struct kmod_module *kunit_kmod;
+	char record[BUF_LEN + 1];
+	FILE *f;
+	bool is_builtin;
+	int ret;
+
+	ret = IGT_EXIT_INVALID;
+
+	/* get normalized module name */
+	if (igt_ktest_init(&tst, module_name) != 0) {
+		igt_warn("Unable to initialize ktest for %s\n", module_name);
+		return ret;
+	}
+
+	if (igt_ktest_begin(&tst) != 0) {
+		igt_warn("Unable to begin ktest for %s\n", module_name);
+
+		igt_ktest_fini(&tst);
+		return ret;
+	}
+
+	if (tst.kmsg < 0) {
+		igt_warn("Could not open /dev/kmsg\n");
+		goto unload;
+	}
+
+	if (lseek(tst.kmsg, 0, SEEK_END)) {
+		igt_warn("Could not seek the end of /dev/kmsg\n");
+		goto unload;
+	}
+
+	f = fdopen(tst.kmsg, "r");
+
+	if (f == NULL) {
+		igt_warn("Could not turn /dev/kmsg file descriptor into a FILE pointer\n");
+		goto unload;
+	}
+
+	/* The KUnit module is required for running any KUnit tests */
+	if (igt_kmod_load("kunit", NULL) != 0 ||
+	    kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) != 0) {
+		igt_warn("Unable to load KUnit\n");
+		igt_fail(IGT_EXIT_FAILURE);
+	}
+
+	is_builtin = kmod_module_get_initstate(kunit_kmod) == KMOD_MODULE_BUILTIN;
+
+	if (igt_kmod_load(module_name, opts) != 0) {
+		igt_warn("Unable to load %s module\n", module_name);
+		igt_fail(IGT_EXIT_FAILURE);
+	}
+
+	ret = igt_ktap_parser(f, record, is_builtin);
+	if (ret != 0)
+		ret = IGT_EXIT_ABORT;
+unload:
+	igt_ktest_end(&tst);
+
+	igt_ktest_fini(&tst);
+
+	if (ret == 0)
+		igt_success();
+
+	return ret;
+}
+
 static int open_parameters(const char *module_name)
 {
 	char path[256];
diff --git a/lib/igt_kmod.h b/lib/igt_kmod.h
index ff59f1ec1..ce17c714e 100644
--- a/lib/igt_kmod.h
+++ b/lib/igt_kmod.h
@@ -71,6 +71,8 @@ static inline int igt_xe_driver_unload(void)
 int igt_amdgpu_driver_load(const char *opts);
 int igt_amdgpu_driver_unload(void);
 
+int igt_kunit(const char *module_name, const char *opts);
+
 void igt_kselftests(const char *module_name,
 		    const char *module_options,
 		    const char *result_option,
diff --git a/lib/igt_ktap.c b/lib/igt_ktap.c
new file mode 100644
index 000000000..117598faa
--- /dev/null
+++ b/lib/igt_ktap.c
@@ -0,0 +1,334 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2023 Isabella Basso do Amaral <isabbasso@riseup.net>
+ */
+
+#include <ctype.h>
+#include <limits.h>
+
+#include "igt_aux.h"
+#include "igt_core.h"
+#include "igt_ktap.h"
+
+static int log_to_end(enum igt_log_level level, FILE *f,
+		      char *record, const char *format, ...) __attribute__((format(printf, 4, 5)));
+
+/**
+ * log_to_end:
+ * @level: #igt_log_level
+ * @record: record to store the read data
+ * @format: format string
+ * @...: optional arguments used in the format string
+ *
+ * This is an altered version of the generic structured logging helper function
+ * igt_log capable of reading to the end of a given line.
+ *
+ * Returns: 0 for success, or -2 if there's an error reading from the file
+ */
+static int log_to_end(enum igt_log_level level, FILE *f,
+		      char *record, const char *format, ...)
+{
+	va_list args;
+	const char *lend;
+
+	va_start(args, format);
+	igt_vlog(IGT_LOG_DOMAIN, level, format, args);
+	va_end(args);
+
+	lend = strchrnul(record, '\n');
+	while (*lend == '\0') {
+		igt_log(IGT_LOG_DOMAIN, level, "%s", record);
+		if (fgets(record, BUF_LEN, f) == NULL) {
+			igt_warn("kmsg truncated: unknown error (%m)\n");
+			return -2;
+		}
+		lend = strchrnul(record, '\n');
+	}
+	return 0;
+}
+
+/**
+ * lookup_value:
+ * @haystack: the string to search in
+ * @needle: the string to search for
+ *
+ * Returns: the value of the needle in the haystack, or -1 if not found.
+ */
+static long lookup_value(const char *haystack, const char *needle)
+{
+	const char *needle_rptr;
+	char *needle_end;
+	long num;
+
+	needle_rptr = strcasestr(haystack, needle);
+
+	if (needle_rptr == NULL)
+		return -1;
+
+	/* skip search string and whitespaces after it */
+	needle_rptr += strlen(needle);
+
+	num = strtol(needle_rptr, &needle_end, 10);
+
+	if (needle_rptr == needle_end)
+		return -1;
+
+	if (num == LONG_MIN || num == LONG_MAX)
+		return 0;
+
+	return num > 0 ? num : 0;
+}
+
+/**
+ * find_next_tap_subtest:
+ * @fp: FILE pointer
+ * @record: buffer used to read fp
+ * @is_builtin: whether KUnit is built-in or not
+ *
+ * Returns:
+ * 0 if there's missing information
+ * -1 if not found
+ * -2 if there are problems while reading the file.
+ * any other value corresponds to the amount of cases of the next (sub)test
+ */
+static int find_next_tap_subtest(FILE *fp, char *record, bool is_builtin)
+{
+	const char *test_lookup_str, *subtest_lookup_str, *name_rptr, *version_rptr;
+	char test_name[BUF_LEN + 1];
+	long test_count;
+
+	test_name[0] = '\0';
+	test_name[BUF_LEN] = '\0';
+
+	test_lookup_str = " subtest: ";
+	subtest_lookup_str = " test: ";
+
+	/*
+	 * "(K)TAP version XX" should be the first line on all (sub)tests as per
+	 * https://kernel.org/doc/html/latest/dev-tools/ktap.html#version-lines
+	 *
+	 * but actually isn't, as it currently depends on the KUnit module
+	 * being built-in, so we can't rely on it every time
+	 */
+	if (is_builtin) {
+		version_rptr = strcasestr(record, "TAP version ");
+		if (version_rptr == NULL)
+			return -1;
+
+		igt_info("%s", version_rptr);
+
+		if (fgets(record, BUF_LEN, fp) == NULL) {
+			igt_warn("kmsg truncated: unknown error (%m)\n");
+			return -2;
+		}
+	}
+
+	name_rptr = strcasestr(record, test_lookup_str);
+	if (name_rptr != NULL) {
+		name_rptr += strlen(test_lookup_str);
+	} else {
+		name_rptr = strcasestr(record, subtest_lookup_str);
+		if (name_rptr != NULL)
+			name_rptr += strlen(subtest_lookup_str);
+	}
+
+	if (name_rptr == NULL) {
+		if (!is_builtin)
+			/* we've probably found nothing */
+			return -1;
+		igt_info("Missing test name\n");
+	} else {
+		strncpy(test_name, name_rptr, BUF_LEN);
+		if (fgets(record, BUF_LEN, fp) == NULL) {
+			igt_warn("kmsg truncated: unknown error (%m)\n");
+			return -2;
+		}
+		/* now we can be sure we found tests */
+		if (!is_builtin)
+			igt_info("KUnit is not built-in, skipping version check...\n");
+	}
+
+	/*
+	 * total test count will almost always appear as 0..N at the beginning
+	 * of a run, so we use it to reliably identify a new run
+	 */
+	test_count = lookup_value(record, "..");
+
+	if (test_count <= 0) {
+		igt_info("Missing test count\n");
+		if (test_name[0] == '\0')
+			return 0;
+		if (log_to_end(IGT_LOG_INFO, fp, record,
+				"Running some tests in: %s",
+				test_name) < 0)
+			return -2;
+		return 0;
+	} else if (test_name[0] == '\0') {
+		igt_info("Running %ld tests...\n", test_count);
+		return 0;
+	}
+
+	if (log_to_end(IGT_LOG_INFO, fp, record,
+			"Executing %ld tests in: %s",
+			test_count, test_name) < 0)
+		return -2;
+
+	return test_count;
+}
+
+/**
+ * find_next_tap_test:
+ * @fp: FILE pointer
+ * @record: buffer used to read fp
+ * @test_name: buffer to store the test name
+ *
+ * Returns:
+ * 1 if no results were found
+ * 0 if a test succeded
+ * -1 if a test failed
+ * -2 if there are problems reading the file
+ */
+static int parse_kmsg_for_tap(FILE *fp, char *record, char *test_name)
+{
+	const char *lstart, *ok_lookup_str, *nok_lookup_str,
+	      *ok_rptr, *nok_rptr, *comment_start, *value_parse_start;
+	char *test_name_end;
+
+	ok_lookup_str = "ok ";
+	nok_lookup_str = "not ok ";
+
+	lstart = strchrnul(record, ';');
+
+	if (*lstart == '\0') {
+		igt_warn("kmsg truncated: output malformed (%m)\n");
+		return -2;
+	}
+
+	lstart++;
+	while (isspace(*lstart))
+		lstart++;
+
+	nok_rptr = strstr(lstart, nok_lookup_str);
+	if (nok_rptr != NULL) {
+		nok_rptr += strlen(nok_lookup_str);
+		while (isdigit(*nok_rptr) || isspace(*nok_rptr) || *nok_rptr == '-')
+			nok_rptr++;
+		test_name_end = strncpy(test_name, nok_rptr, BUF_LEN);
+		while (!isspace(*test_name_end))
+			test_name_end++;
+		*test_name_end = '\0';
+		if (log_to_end(IGT_LOG_WARN, fp, record,
+			       "%s", lstart) < 0)
+			return -2;
+		return -1;
+	}
+
+	comment_start = strchrnul(lstart, '#');
+
+	/* check if we're still in a subtest */
+	if (*comment_start != '\0') {
+		comment_start++;
+		value_parse_start = comment_start;
+
+		if (lookup_value(value_parse_start, "fail: ") > 0) {
+			if (log_to_end(IGT_LOG_WARN, fp, record,
+				       "%s", lstart) < 0)
+				return -2;
+			return -1;
+		}
+	}
+
+	ok_rptr = strstr(lstart, ok_lookup_str);
+	if (ok_rptr != NULL) {
+		ok_rptr += strlen(ok_lookup_str);
+		while (isdigit(*ok_rptr) || isspace(*ok_rptr) || *ok_rptr == '-')
+			ok_rptr++;
+		test_name_end = strncpy(test_name, ok_rptr, BUF_LEN);
+		while (!isspace(*test_name_end))
+			test_name_end++;
+		*test_name_end = '\0';
+		return 0;
+	}
+
+	return 1;
+}
+
+/**
+ * igt_ktap_parser:
+ * @fp: FILE pointer
+ * @record: buffer used to read fp
+ * @is_builtin: whether the KUnit module is built-in or not
+ *
+ * This function parses the output of a ktap script and prints the test results,
+ * as well as any other output to stdout.
+ *
+ * Returns: IGT default codes
+ */
+int igt_ktap_parser(FILE *fp, char *record, bool is_builtin)
+{
+	char test_name[BUF_LEN + 1];
+	bool failed_tests, found_tests;
+	int sublevel = 0;
+
+	test_name[0] = '\0';
+	test_name[BUF_LEN] = '\0';
+
+	failed_tests = false;
+	found_tests = false;
+
+	while (sublevel >= 0) {
+		if (fgets(record, BUF_LEN, fp) == NULL) {
+			if (!found_tests)
+				igt_warn("kmsg truncated: unknown error (%m)\n");
+			break;
+		}
+
+		switch (find_next_tap_subtest(fp, record, is_builtin)) {
+		case -2:
+			/* no more data to read */
+			return IGT_EXIT_FAILURE;
+		case -1:
+			/* no test found, so we keep parsing */
+			break;
+		case 0:
+			/*
+			 * tests found, but they're missing info, so we might
+			 * have read into test output
+			 */
+			found_tests = true;
+			sublevel++;
+			break;
+		default:
+			if (fgets(record, BUF_LEN, fp) == NULL) {
+				igt_warn("kmsg truncated: unknown error (%m)\n");
+				return -2;
+			}
+			found_tests = true;
+			sublevel++;
+			break;
+		}
+
+		switch (parse_kmsg_for_tap(fp, record, test_name)) {
+		case -2:
+			return IGT_EXIT_FAILURE;
+		case -1:
+			sublevel--;
+			failed_tests = true;
+			igt_subtest(test_name)
+				igt_fail(IGT_EXIT_FAILURE);
+			test_name[0] = '\0';
+			break;
+		case 0: /* fallthrough */
+			igt_subtest(test_name)
+				igt_success();
+			test_name[0] = '\0';
+		default:
+			break;
+		}
+	}
+
+	if (failed_tests || !found_tests)
+		return IGT_EXIT_FAILURE;
+
+	return IGT_EXIT_SUCCESS;
+}
diff --git a/lib/igt_ktap.h b/lib/igt_ktap.h
new file mode 100644
index 000000000..b2f69df2d
--- /dev/null
+++ b/lib/igt_ktap.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright © 2022 Isabella Basso do Amaral <isabbasso@riseup.net>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef IGT_KTAP_H
+#define IGT_KTAP_H
+
+#define BUF_LEN 4096
+
+int igt_ktap_parser(FILE *fp, char *record, bool is_builtin);
+
+#endif /* IGT_KTAP_H */
diff --git a/lib/meson.build b/lib/meson.build
index ad5d999d9..8e9977083 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -88,6 +88,7 @@ lib_sources = [
 	'igt_store.c',
 	'uwildmat/uwildmat.c',
 	'igt_kmod.c',
+	'igt_ktap.c',
 	'igt_panfrost.c',
 	'igt_v3d.c',
 	'igt_vc4.c',
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 04/10] tests: DRM selftests: switch to KUnit
  2023-06-13  7:27 [igt-dev] [PATCH v7 i-g-t 00/10] Introduce KUnit Dominik Karol Piatkowski
                   ` (2 preceding siblings ...)
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 03/10] lib/igt_kmod: add compatibility for KUnit Dominik Karol Piatkowski
@ 2023-06-13  7:27 ` Dominik Karol Piatkowski
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 05/10] Change logic of ktap parser to run on a thread Dominik Karol Piatkowski
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Dominik Karol Piatkowski @ 2023-06-13  7:27 UTC (permalink / raw)
  To: igt-dev; +Cc: Isabella Basso

From: Isabella Basso <isabbasso@riseup.net>

As the DRM selftests are now using KUnit [1], update IGT tests as well.

[1] - https://lore.kernel.org/all/20220708203052.236290-1-maira.canal@usp.br/

Signed-off-by: Isabella Basso <isabbasso@riseup.net>

v1 -> v2:
- drm_buddy|drm_mm: fallback to igt_kselftests if igt_kunit failed
  with code other than IGT_EXIT_ABORT
- kms_selftest: move igt_kunit tests to separate subtests
- kms_selftest: fallback to igt_kselftests if all subtests failed

v2 -> v3:
- expose all subtests

Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
Acked-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Cc: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>
---
 tests/drm_buddy.c    | 4 +++-
 tests/drm_mm.c       | 4 +++-
 tests/kms_selftest.c | 8 ++++++++
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/tests/drm_buddy.c b/tests/drm_buddy.c
index 06876e0cd..3261f0d61 100644
--- a/tests/drm_buddy.c
+++ b/tests/drm_buddy.c
@@ -10,5 +10,7 @@ IGT_TEST_DESCRIPTION("Basic sanity check of DRM's buddy allocator (struct drm_bu
 
 igt_main
 {
-	igt_kselftests("test-drm_buddy", NULL, NULL, NULL);
+	int ret = igt_kunit("drm_buddy_test", NULL);
+	if (ret != 0 && ret != IGT_EXIT_ABORT)
+		igt_kselftests("test-drm_buddy", NULL, NULL, NULL);
 }
diff --git a/tests/drm_mm.c b/tests/drm_mm.c
index 0bce7139d..88f76a57c 100644
--- a/tests/drm_mm.c
+++ b/tests/drm_mm.c
@@ -156,5 +156,7 @@ IGT_TEST_DESCRIPTION("Basic sanity check of DRM's range manager (struct drm_mm)"
 
 igt_main
 {
-	igt_kselftests("test-drm_mm", NULL, NULL, NULL);
+	int ret = igt_kunit("drm_mm_test", NULL);
+	if (ret != 0 && ret != IGT_EXIT_ABORT)
+		igt_kselftests("test-drm_mm", NULL, NULL, NULL);
 }
diff --git a/tests/kms_selftest.c b/tests/kms_selftest.c
index abc4bfe98..b27f60fb3 100644
--- a/tests/kms_selftest.c
+++ b/tests/kms_selftest.c
@@ -28,5 +28,13 @@ IGT_TEST_DESCRIPTION("Basic sanity check of KMS selftests.");
 
 igt_main
 {
+	static const char *kunit_subtests[] = { "drm_cmdline_parser_test", "drm_damage_helper_test",
+						"drm_dp_mst_helper_test", "drm_format_helper_test",
+						"drm_format_test", "drm_framebuffer_test",
+						"drm_plane_helper_test", NULL };
+
+	for (int i = 0; kunit_subtests[i] != NULL; i++)
+		igt_kunit(kunit_subtests[i], NULL);
+
 	igt_kselftests("test-drm_modeset", NULL, NULL, NULL);
 }
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 05/10] Change logic of ktap parser to run on a thread
  2023-06-13  7:27 [igt-dev] [PATCH v7 i-g-t 00/10] Introduce KUnit Dominik Karol Piatkowski
                   ` (3 preceding siblings ...)
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 04/10] tests: DRM selftests: switch to KUnit Dominik Karol Piatkowski
@ 2023-06-13  7:27 ` Dominik Karol Piatkowski
  2023-06-13  8:44   ` Mauro Carvalho Chehab
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 06/10] kunit tests: add an optional name for the selftests Dominik Karol Piatkowski
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 22+ messages in thread
From: Dominik Karol Piatkowski @ 2023-06-13  7:27 UTC (permalink / raw)
  To: igt-dev

The ktap parser should be listening and parsing messages as the tests
are executed, and not after the end of module load.

v1 -> v2:
- fix coding style
- remove usleep
- add error check logic
- follow the structure of igt_kselftests more closely

v2 -> v3:
- fixed sublevel issues by rewriting tap parser flow

v3 -> v4:
- fixed delimiter

- squashed lib/igt_kmod: fix nesting igt_fixture in igt_subtest
Fix the following issue:
	$ ./build/tests/drm_buddy
	Starting subtest: all-tests
	nesting igt_fixture in igt_subtest is invalid
	please refer to lib/igt_core documentation

- squashed lib/igt_kmod: place KUnit tests on a subtest
There's a hidden bug at KUnit implementation: as it doesn't
place tests inside a subtest, trying to use it with igt_main
causes a crash:

	$ ./build/tests/drm_mm --list
	skipping is allowed only in fixtures, subtests or igt_simple_main
	please refer to lib/igt_core documentation
	drm_mm: ../lib/igt_core.c:437: internal_assert: Assertion `0' failed.
	Received signal SIGABRT.
	Stack trace:
	 #0 [fatal_sig_handler+0x17b]
	 #1 [__sigaction+0x50]
	 #2 [__pthread_kill_implementation+0x114]
	 #3 [gsignal+0x1e]
	 #4 [abort+0xdf]
	 #5 [__assert_fail_base.cold+0xe]
	 #6 [__assert_fail+0x47]
	 #7 [internal_assert+0xe5]
	 #8 [igt_skip+0x169]
	 #9 [__igt_skip_check+0x1bb]
	 #10 [igt_ktest_begin+0xa6]
	 #11 [igt_kunit+0x70]
	 #12 [main+0x2a]
	 #13 [__libc_start_call_main+0x7a]
	 #14 [__libc_start_main+0x8b]
	 #15 [_start+0x25]

Fix it by using igt_subtests() before actually implememnting
KUnit logic.

After the patch, it should now report subtests:

	$ ./build/tests/drm_mm --list
	all-tests

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Cc: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>
---
 lib/igt_kmod.c |  51 +++++-
 lib/igt_ktap.c | 431 ++++++++++++++++++++++++++++++++++++++++---------
 lib/igt_ktap.h |  21 ++-
 3 files changed, 421 insertions(+), 82 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 21e801bde..73478f75e 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -754,14 +754,15 @@ void igt_kselftest_get_tests(struct kmod_module *kmod,
  *
  * Returns: IGT default codes
  */
-int igt_kunit(const char *module_name, const char *opts)
+static int __igt_kunit(const char *module_name, const char *opts)
 {
 	struct igt_ktest tst;
 	struct kmod_module *kunit_kmod;
-	char record[BUF_LEN + 1];
 	FILE *f;
 	bool is_builtin;
 	int ret;
+	struct ktap_test_results *results;
+	struct ktap_test_results_element *temp;
 
 	ret = IGT_EXIT_INVALID;
 
@@ -804,25 +805,63 @@ int igt_kunit(const char *module_name, const char *opts)
 
 	is_builtin = kmod_module_get_initstate(kunit_kmod) == KMOD_MODULE_BUILTIN;
 
+	results = ktap_parser_start(f, is_builtin);
+
 	if (igt_kmod_load(module_name, opts) != 0) {
 		igt_warn("Unable to load %s module\n", module_name);
+		ret = ktap_parser_stop();
 		igt_fail(IGT_EXIT_FAILURE);
 	}
 
-	ret = igt_ktap_parser(f, record, is_builtin);
-	if (ret != 0)
-		ret = IGT_EXIT_ABORT;
+	while (READ_ONCE(results->still_running) || READ_ONCE(results->head) != NULL)
+	{
+		if (READ_ONCE(results->head) != NULL) {
+			pthread_mutex_lock(&results->mutex);
+
+			igt_dynamic(results->head->test_name) {
+				if (READ_ONCE(results->head->passed))
+					igt_success();
+				else
+					igt_fail(IGT_EXIT_FAILURE);
+			}
+
+			temp = results->head;
+			results->head = results->head->next;
+			free(temp);
+
+			pthread_mutex_unlock(&results->mutex);
+		}
+	}
+
 unload:
 	igt_ktest_end(&tst);
 
 	igt_ktest_fini(&tst);
 
+	ret = ktap_parser_stop();
+
+	if (ret != 0)
+		ret = IGT_EXIT_ABORT;
+
 	if (ret == 0)
 		igt_success();
-
 	return ret;
 }
 
+int igt_kunit(const char *module_name, 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
+	 * 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("all-tests")
+		return __igt_kunit(module_name, opts);
+	return 0;
+}
+
 static int open_parameters(const char *module_name)
 {
 	char path[256];
diff --git a/lib/igt_ktap.c b/lib/igt_ktap.c
index 117598faa..ecdcb8d83 100644
--- a/lib/igt_ktap.c
+++ b/lib/igt_ktap.c
@@ -5,11 +5,25 @@
 
 #include <ctype.h>
 #include <limits.h>
+#include <libkmod.h>
+#include <pthread.h>
+#include <errno.h>
 
 #include "igt_aux.h"
 #include "igt_core.h"
 #include "igt_ktap.h"
 
+#define DELIMITER "-"
+
+struct ktap_parser_args {
+	FILE *fp;
+	bool is_builtin;
+	volatile bool is_running;
+	int ret;
+} ktap_args;
+
+static struct ktap_test_results results;
+
 static int log_to_end(enum igt_log_level level, FILE *f,
 		      char *record, const char *format, ...) __attribute__((format(printf, 4, 5)));
 
@@ -30,6 +44,14 @@ static int log_to_end(enum igt_log_level level, FILE *f,
 {
 	va_list args;
 	const char *lend;
+	int f_fd = fileno(f);
+
+	/* Cutoff after newline character, in order to not display garbage */
+	char *cutoff = strchr(record, '\n');
+	if (cutoff) {
+		if (cutoff - record < BUF_LEN)
+			cutoff[1] = '\0';
+	}
 
 	va_start(args, format);
 	igt_vlog(IGT_LOG_DOMAIN, level, format, args);
@@ -38,10 +60,29 @@ static int log_to_end(enum igt_log_level level, FILE *f,
 	lend = strchrnul(record, '\n');
 	while (*lend == '\0') {
 		igt_log(IGT_LOG_DOMAIN, level, "%s", record);
-		if (fgets(record, BUF_LEN, f) == NULL) {
+
+		while (read(f_fd, record, BUF_LEN) < 0) {
+			if (!READ_ONCE(ktap_args.is_running)) {
+				igt_warn("ktap parser stopped\n");
+				return -2;
+			}
+
+			if (errno == EINTR)
+				continue;
+
+			if (errno == EPIPE) {
+				igt_warn("kmsg truncated: too many messages. You may want to increase log_buf_len in kmcdline\n");
+				return -2;
+			}
+
+			if (errno == EAGAIN)
+				/* No records available */
+				continue;
+
 			igt_warn("kmsg truncated: unknown error (%m)\n");
 			return -2;
 		}
+
 		lend = strchrnul(record, '\n');
 	}
 	return 0;
@@ -65,7 +106,7 @@ static long lookup_value(const char *haystack, const char *needle)
 	if (needle_rptr == NULL)
 		return -1;
 
-	/* skip search string and whitespaces after it */
+	/* Skip search string and whitespaces after it */
 	needle_rptr += strlen(needle);
 
 	num = strtol(needle_rptr, &needle_end, 10);
@@ -79,6 +120,41 @@ static long lookup_value(const char *haystack, const char *needle)
 	return num > 0 ? num : 0;
 }
 
+/**
+ * tap_version_present:
+ * @record: buffer with tap data
+ * @print_info: whether tap version should be printed or not
+ *
+ * Returns:
+ * 0 if not found
+ * 1 if found
+ */
+static int tap_version_present(char* record, bool print_info)
+{
+	/*
+	 * "(K)TAP version XX" should be the first line on all (sub)tests as per
+	 * https://kernel.org/doc/html/latest/dev-tools/ktap.html#version-lines
+	 *
+	 * but actually isn't, as it currently depends on the KUnit module
+	 * being built-in, so we can't rely on it every time
+	 */
+	const char *version_rptr = strcasestr(record, "TAP version ");
+	char *cutoff;
+
+	if (version_rptr == NULL)
+		return 0;
+
+	/* Cutoff after newline character, in order to not display garbage */
+	cutoff = strchr(version_rptr, '\n');
+	if (cutoff)
+		cutoff[0] = '\0';
+
+	if (print_info)
+		igt_info("%s\n", version_rptr);
+
+	return 1;
+}
+
 /**
  * find_next_tap_subtest:
  * @fp: FILE pointer
@@ -91,11 +167,12 @@ static long lookup_value(const char *haystack, const char *needle)
  * -2 if there are problems while reading the file.
  * any other value corresponds to the amount of cases of the next (sub)test
  */
-static int find_next_tap_subtest(FILE *fp, char *record, bool is_builtin)
+static int find_next_tap_subtest(FILE *fp, char *record, char *test_name, bool is_builtin)
 {
-	const char *test_lookup_str, *subtest_lookup_str, *name_rptr, *version_rptr;
-	char test_name[BUF_LEN + 1];
+	const char *test_lookup_str, *subtest_lookup_str, *name_rptr;
 	long test_count;
+	int fp_fd = fileno(fp);
+	char *cutoff;
 
 	test_name[0] = '\0';
 	test_name[BUF_LEN] = '\0';
@@ -103,21 +180,28 @@ static int find_next_tap_subtest(FILE *fp, char *record, bool is_builtin)
 	test_lookup_str = " subtest: ";
 	subtest_lookup_str = " test: ";
 
-	/*
-	 * "(K)TAP version XX" should be the first line on all (sub)tests as per
-	 * https://kernel.org/doc/html/latest/dev-tools/ktap.html#version-lines
-	 *
-	 * but actually isn't, as it currently depends on the KUnit module
-	 * being built-in, so we can't rely on it every time
-	 */
+	if (!tap_version_present(record, true))
+		return -1;
+
 	if (is_builtin) {
-		version_rptr = strcasestr(record, "TAP version ");
-		if (version_rptr == NULL)
-			return -1;
+		while (read(fp_fd, record, BUF_LEN) < 0) {
+			if (!READ_ONCE(ktap_args.is_running)) {
+				igt_warn("ktap parser stopped\n");
+				return -2;
+			}
+
+			if (errno == EINTR)
+				continue;
 
-		igt_info("%s", version_rptr);
+			if (errno == EPIPE) {
+				igt_warn("kmsg truncated: too many messages. You may want to increase log_buf_len in kmcdline\n");
+				return -2;
+			}
+
+			if (errno == EAGAIN)
+				/* No records available */
+				continue;
 
-		if (fgets(record, BUF_LEN, fp) == NULL) {
 			igt_warn("kmsg truncated: unknown error (%m)\n");
 			return -2;
 		}
@@ -134,22 +218,45 @@ static int find_next_tap_subtest(FILE *fp, char *record, bool is_builtin)
 
 	if (name_rptr == NULL) {
 		if (!is_builtin)
-			/* we've probably found nothing */
+			/* We've probably found nothing */
 			return -1;
 		igt_info("Missing test name\n");
 	} else {
 		strncpy(test_name, name_rptr, BUF_LEN);
-		if (fgets(record, BUF_LEN, fp) == NULL) {
+		/* Cutoff after newline character, in order to not display garbage */
+		cutoff = strchr(test_name, '\n');
+		if (cutoff)
+			cutoff[0] = '\0';
+
+		while (read(fp_fd, record, BUF_LEN) < 0) {
+			if (!READ_ONCE(ktap_args.is_running)) {
+				igt_warn("ktap parser stopped\n");
+				return -2;
+			}
+
+			if (errno == EINTR)
+				continue;
+
+			if (errno == EPIPE) {
+				igt_warn("kmsg truncated: too many messages. You may want to increase log_buf_len in kmcdline\n");
+				return -2;
+			}
+
+			if (errno == EAGAIN)
+				/* No records available */
+				continue;
+
 			igt_warn("kmsg truncated: unknown error (%m)\n");
 			return -2;
 		}
-		/* now we can be sure we found tests */
+
+		/* Now we can be sure we found tests */
 		if (!is_builtin)
 			igt_info("KUnit is not built-in, skipping version check...\n");
 	}
 
 	/*
-	 * total test count will almost always appear as 0..N at the beginning
+	 * Total test count will almost always appear as 0..N at the beginning
 	 * of a run, so we use it to reliably identify a new run
 	 */
 	test_count = lookup_value(record, "..");
@@ -159,7 +266,7 @@ static int find_next_tap_subtest(FILE *fp, char *record, bool is_builtin)
 		if (test_name[0] == '\0')
 			return 0;
 		if (log_to_end(IGT_LOG_INFO, fp, record,
-				"Running some tests in: %s",
+				"Running some tests in: %s\n",
 				test_name) < 0)
 			return -2;
 		return 0;
@@ -169,7 +276,7 @@ static int find_next_tap_subtest(FILE *fp, char *record, bool is_builtin)
 	}
 
 	if (log_to_end(IGT_LOG_INFO, fp, record,
-			"Executing %ld tests in: %s",
+			"Executing %ld tests in: %s\n",
 			test_count, test_name) < 0)
 		return -2;
 
@@ -177,7 +284,7 @@ static int find_next_tap_subtest(FILE *fp, char *record, bool is_builtin)
 }
 
 /**
- * find_next_tap_test:
+ * parse_kmsg_for_tap:
  * @fp: FILE pointer
  * @record: buffer used to read fp
  * @test_name: buffer to store the test name
@@ -225,7 +332,7 @@ static int parse_kmsg_for_tap(FILE *fp, char *record, char *test_name)
 
 	comment_start = strchrnul(lstart, '#');
 
-	/* check if we're still in a subtest */
+	/* Check if we're still in a subtest */
 	if (*comment_start != '\0') {
 		comment_start++;
 		value_parse_start = comment_start;
@@ -254,81 +361,255 @@ static int parse_kmsg_for_tap(FILE *fp, char *record, char *test_name)
 }
 
 /**
- * igt_ktap_parser:
+ * parse_tap_level:
  * @fp: FILE pointer
- * @record: buffer used to read fp
+ * @base_test_name: test_name from upper recursion level
+ * @test_count: test_count of this level
+ * @failed_tests: top level failed_tests pointer
+ * @found_tests: top level found_tests pointer
  * @is_builtin: whether the KUnit module is built-in or not
  *
- * This function parses the output of a ktap script and prints the test results,
- * as well as any other output to stdout.
- *
- * Returns: IGT default codes
+ * Returns:
+ * 0 if succeded
+ * -1 if error occurred
  */
-int igt_ktap_parser(FILE *fp, char *record, bool is_builtin)
+static int parse_tap_level(FILE *fp, char *base_test_name, int test_count, bool *failed_tests,
+			   bool *found_tests, bool is_builtin)
 {
+	int fp_fd = fileno(fp);
+	char record[BUF_LEN + 1];
+	struct ktap_test_results_element *r, *temp;
+	int internal_test_count;
 	char test_name[BUF_LEN + 1];
-	bool failed_tests, found_tests;
-	int sublevel = 0;
+	char base_test_name_for_next_level[BUF_LEN + 1];
 
-	test_name[0] = '\0';
-	test_name[BUF_LEN] = '\0';
+	for (int i = 0; i < test_count; i++) {
+		while (read(fp_fd, record, BUF_LEN) < 0) {
+			if (!READ_ONCE(ktap_args.is_running)) {
+				igt_warn("ktap parser stopped\n");
+				return -1;
+			}
 
-	failed_tests = false;
-	found_tests = false;
+			if (errno == EINTR)
+				continue;
 
-	while (sublevel >= 0) {
-		if (fgets(record, BUF_LEN, fp) == NULL) {
-			if (!found_tests)
-				igt_warn("kmsg truncated: unknown error (%m)\n");
-			break;
+			if (errno == EAGAIN)
+				/* No records available */
+				continue;
+
+			if (errno == EPIPE) {
+				igt_warn("kmsg truncated: too many messages. You may want to increase log_buf_len in kmcdline\n");
+				return -1;
+			}
+
+			igt_warn("kmsg truncated: unknown error (%m)\n");
+			return -1;
 		}
 
-		switch (find_next_tap_subtest(fp, record, is_builtin)) {
-		case -2:
-			/* no more data to read */
-			return IGT_EXIT_FAILURE;
-		case -1:
-			/* no test found, so we keep parsing */
-			break;
-		case 0:
-			/*
-			 * tests found, but they're missing info, so we might
-			 * have read into test output
-			 */
-			found_tests = true;
-			sublevel++;
-			break;
-		default:
-			if (fgets(record, BUF_LEN, fp) == NULL) {
-				igt_warn("kmsg truncated: unknown error (%m)\n");
-				return -2;
+		/* Sublevel found */
+		if (tap_version_present(record, false))
+		{
+			internal_test_count = find_next_tap_subtest(fp, record, test_name,
+								    is_builtin);
+			switch (internal_test_count) {
+			case -2:
+				/* No more data to read */
+				return -1;
+			case -1:
+				/* No test found */
+				return -1;
+			case 0:
+				/* Tests found, but they're missing info */
+				*found_tests = true;
+				return -1;
+			default:
+				*found_tests = true;
+
+				memcpy(base_test_name_for_next_level, base_test_name, BUF_LEN);
+				if (strlen(base_test_name_for_next_level) < BUF_LEN - 1 &&
+				    base_test_name_for_next_level[0])
+					strncat(base_test_name_for_next_level, DELIMITER,
+						BUF_LEN - strlen(base_test_name_for_next_level));
+				memcpy(base_test_name_for_next_level + strlen(base_test_name_for_next_level),
+				       test_name, BUF_LEN - strlen(base_test_name_for_next_level));
+
+				if (parse_tap_level(fp, base_test_name_for_next_level,
+						    internal_test_count, failed_tests, found_tests,
+						    is_builtin) == -1)
+					return -1;
+				break;
 			}
-			found_tests = true;
-			sublevel++;
-			break;
 		}
 
 		switch (parse_kmsg_for_tap(fp, record, test_name)) {
 		case -2:
-			return IGT_EXIT_FAILURE;
+			return -1;
 		case -1:
-			sublevel--;
-			failed_tests = true;
-			igt_subtest(test_name)
-				igt_fail(IGT_EXIT_FAILURE);
+			*failed_tests = true;
+
+			r = malloc(sizeof(*r));
+
+			memcpy(r->test_name, base_test_name, BUF_LEN);
+			if (strlen(r->test_name) < BUF_LEN - 1)
+				if (r->test_name[0])
+					strncat(r->test_name, DELIMITER,
+						BUF_LEN - strlen(r->test_name));
+			memcpy(r->test_name + strlen(r->test_name), test_name,
+			       BUF_LEN - strlen(r->test_name));
+			r->test_name[BUF_LEN] = '\0';
+
+			r->passed = false;
+			r->next = NULL;
+
+			pthread_mutex_lock(&results.mutex);
+			if (results.head == NULL) {
+				results.head = r;
+			} else {
+				temp = results.head;
+				while (temp->next != NULL)
+					temp = temp->next;
+				temp->next = r;
+			}
+			pthread_mutex_unlock(&results.mutex);
+
 			test_name[0] = '\0';
 			break;
-		case 0: /* fallthrough */
-			igt_subtest(test_name)
-				igt_success();
+		case 0:
+			r = malloc(sizeof(*r));
+
+			memcpy(r->test_name, base_test_name, BUF_LEN);
+			if (strlen(r->test_name) < BUF_LEN - 1)
+				if (r->test_name[0])
+					strncat(r->test_name, DELIMITER,
+						BUF_LEN - strlen(r->test_name));
+			memcpy(r->test_name + strlen(r->test_name), test_name,
+			       BUF_LEN - strlen(r->test_name));
+			r->test_name[BUF_LEN] = '\0';
+
+			r->passed = true;
+			r->next = NULL;
+
+			pthread_mutex_lock(&results.mutex);
+			if (results.head == NULL) {
+				results.head = r;
+			} else {
+				temp = results.head;
+				while (temp->next != NULL)
+					temp = temp->next;
+				temp->next = r;
+			}
+			pthread_mutex_unlock(&results.mutex);
+
 			test_name[0] = '\0';
+			break;
 		default:
 			break;
 		}
 	}
+	return 0;
+}
+
+/**
+ * igt_ktap_parser:
+ *
+ * This function parses the output of a ktap script and passes it to main thread.
+ */
+void *igt_ktap_parser(void *unused)
+{
+	FILE *fp = ktap_args.fp;
+	int fp_fd = fileno(fp);
+	char record[BUF_LEN + 1];
+	bool is_builtin = ktap_args.is_builtin;
+	char test_name[BUF_LEN + 1];
+	bool failed_tests, found_tests;
+	int test_count;
+
+	failed_tests = false;
+	found_tests = false;
+
+	if (!READ_ONCE(ktap_args.is_running))
+		goto igt_ktap_parser_end;
+
+igt_ktap_parser_start:
+	test_name[0] = '\0';
+	test_name[BUF_LEN] = '\0';
+
+	while (read(fp_fd, record, BUF_LEN) < 0) {
+		if (!READ_ONCE(ktap_args.is_running)) {
+			igt_warn("ktap parser stopped\n");
+			goto igt_ktap_parser_end;
+		}
+
+		if (errno == EAGAIN)
+			/* No records available */
+			continue;
+
+		if (errno == EINTR)
+			continue;
+
+		if (errno == EPIPE) {
+			igt_warn("kmsg truncated: too many messages. You may want to increase log_buf_len in kmcdline\n");
+			goto igt_ktap_parser_end;
+		}
+	}
+
+	test_count = find_next_tap_subtest(fp, record, test_name, is_builtin);
+
+	switch (test_count) {
+	case -2:
+		/* Problems while reading the file */
+		goto igt_ktap_parser_end;
+	case -1:
+		/* No test found */
+		goto igt_ktap_parser_start;
+	case 0:
+		/* Tests found, but they're missing info */
+		found_tests = true;
+		goto igt_ktap_parser_end;
+	default:
+		found_tests = true;
+
+		if (parse_tap_level(fp, test_name, test_count, &failed_tests, &found_tests,
+				    is_builtin) == -1)
+			goto igt_ktap_parser_end;
+
+		break;
+	}
+
+	/* Parse topmost level */
+	test_name[0] = '\0';
+	parse_tap_level(fp, test_name, test_count, &failed_tests, &found_tests, is_builtin);
+
+igt_ktap_parser_end:
+	results.still_running = false;
 
 	if (failed_tests || !found_tests)
-		return IGT_EXIT_FAILURE;
+		ktap_args.ret = IGT_EXIT_FAILURE;
+	else
+		ktap_args.ret = IGT_EXIT_SUCCESS;
+
+	return NULL;
+}
+
+static pthread_t ktap_parser_thread;
+
+struct ktap_test_results *ktap_parser_start(FILE *fp, bool is_builtin)
+{
+	results.head = NULL;
+	pthread_mutex_init(&results.mutex, NULL);
+	results.still_running = true;
 
-	return IGT_EXIT_SUCCESS;
+	ktap_args.fp = fp;
+	ktap_args.is_builtin = is_builtin;
+	ktap_args.is_running = true;
+	pthread_create(&ktap_parser_thread, NULL, igt_ktap_parser, NULL);
+
+	return &results;
+}
+
+int ktap_parser_stop(void)
+{
+	ktap_args.is_running = false;
+	pthread_join(ktap_parser_thread, NULL);
+	return ktap_args.ret;
 }
diff --git a/lib/igt_ktap.h b/lib/igt_ktap.h
index b2f69df2d..34fe09572 100644
--- a/lib/igt_ktap.h
+++ b/lib/igt_ktap.h
@@ -26,6 +26,25 @@
 
 #define BUF_LEN 4096
 
-int igt_ktap_parser(FILE *fp, char *record, bool is_builtin);
+#include <pthread.h>
+
+void *igt_ktap_parser(void *unused);
+
+typedef struct ktap_test_results_element {
+	char test_name[BUF_LEN + 1];
+	bool passed;
+	struct ktap_test_results_element *next;
+} ktap_test_results_element;
+
+struct ktap_test_results {
+	ktap_test_results_element *head;
+	pthread_mutex_t mutex;
+	bool still_running;
+};
+
+
+
+struct ktap_test_results *ktap_parser_start(FILE *fp, bool is_builtin);
+int ktap_parser_stop(void);
 
 #endif /* IGT_KTAP_H */
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 06/10] kunit tests: add an optional name for the selftests
  2023-06-13  7:27 [igt-dev] [PATCH v7 i-g-t 00/10] Introduce KUnit Dominik Karol Piatkowski
                   ` (4 preceding siblings ...)
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 05/10] Change logic of ktap parser to run on a thread Dominik Karol Piatkowski
@ 2023-06-13  7:27 ` Dominik Karol Piatkowski
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 07/10] KUnit: Remove igt_kselftest fallback Dominik Karol Piatkowski
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Dominik Karol Piatkowski @ 2023-06-13  7:27 UTC (permalink / raw)
  To: igt-dev

From: Mauro Carvalho Chehab <mchehab@kernel.org>

When multiple KUnit tests are called by the same program, it is
interesting to group them with a name. This would allow IGT
namespace to better refer to the KUnit tests and will give some
filtering capability to it.

After those changes, the IGT kUnit tests will be better named:

	$ for i in kms_selftest drm_buddy drm_mm; do echo $i:; build/tests/$i --list; echo; done
	kms_selftest:
	drm_cmdline
	drm_damage
	drm_dp_mst
	drm_format_helper
	drm_format
	framebuffer
	drm_plane
	all-tests

	drm_buddy:
	all-tests

	drm_mm:
	all-tests

Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
---
 lib/igt_kmod.c       |  7 +++++--
 lib/igt_kmod.h       |  2 +-
 tests/drm_buddy.c    |  2 +-
 tests/drm_mm.c       |  3 ++-
 tests/kms_selftest.c | 23 +++++++++++++++++------
 5 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 73478f75e..2c0cc026d 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -848,7 +848,7 @@ unload:
 	return ret;
 }
 
-int igt_kunit(const char *module_name, const char *opts)
+int igt_kunit(const char *module_name, const char *name, const char *opts)
 {
 	/*
 	 * We need to use igt_subtest here, as otherwise it may crash with:
@@ -857,7 +857,10 @@ int igt_kunit(const char *module_name, const char *opts)
 	 * proper namespace for dynamic subtests, with is required for CI
 	 * and for documentation.
 	 */
-	igt_subtest_with_dynamic("all-tests")
+	if (name == NULL)
+		name = "all-tests";
+
+	igt_subtest_with_dynamic(name)
 		return __igt_kunit(module_name, opts);
 	return 0;
 }
diff --git a/lib/igt_kmod.h b/lib/igt_kmod.h
index ce17c714e..248955475 100644
--- a/lib/igt_kmod.h
+++ b/lib/igt_kmod.h
@@ -71,7 +71,7 @@ static inline int igt_xe_driver_unload(void)
 int igt_amdgpu_driver_load(const char *opts);
 int igt_amdgpu_driver_unload(void);
 
-int igt_kunit(const char *module_name, const char *opts);
+int igt_kunit(const char *module_name, const char *name, const char *opts);
 
 void igt_kselftests(const char *module_name,
 		    const char *module_options,
diff --git a/tests/drm_buddy.c b/tests/drm_buddy.c
index 3261f0d61..09feaf635 100644
--- a/tests/drm_buddy.c
+++ b/tests/drm_buddy.c
@@ -10,7 +10,7 @@ IGT_TEST_DESCRIPTION("Basic sanity check of DRM's buddy allocator (struct drm_bu
 
 igt_main
 {
-	int ret = igt_kunit("drm_buddy_test", NULL);
+	int ret = igt_kunit("drm_buddy_test", NULL, NULL);
 	if (ret != 0 && ret != IGT_EXIT_ABORT)
 		igt_kselftests("test-drm_buddy", NULL, NULL, NULL);
 }
diff --git a/tests/drm_mm.c b/tests/drm_mm.c
index 88f76a57c..ada8cb936 100644
--- a/tests/drm_mm.c
+++ b/tests/drm_mm.c
@@ -156,7 +156,8 @@ IGT_TEST_DESCRIPTION("Basic sanity check of DRM's range manager (struct drm_mm)"
 
 igt_main
 {
-	int ret = igt_kunit("drm_mm_test", NULL);
+	int ret = igt_kunit("drm_mm_test", NULL, NULL);
+
 	if (ret != 0 && ret != IGT_EXIT_ABORT)
 		igt_kselftests("test-drm_mm", NULL, NULL, NULL);
 }
diff --git a/tests/kms_selftest.c b/tests/kms_selftest.c
index b27f60fb3..d83e5ff4b 100644
--- a/tests/kms_selftest.c
+++ b/tests/kms_selftest.c
@@ -26,15 +26,26 @@
 
 IGT_TEST_DESCRIPTION("Basic sanity check of KMS selftests.");
 
+struct kms_kunittests {
+	const char *kunit;
+	const char *name;
+};
+
 igt_main
 {
-	static const char *kunit_subtests[] = { "drm_cmdline_parser_test", "drm_damage_helper_test",
-						"drm_dp_mst_helper_test", "drm_format_helper_test",
-						"drm_format_test", "drm_framebuffer_test",
-						"drm_plane_helper_test", NULL };
+	static const struct kms_kunittests kunit_subtests[] = {
+		{ "drm_cmdline_parser_test",	"drm_cmdline" },
+		{ "drm_damage_helper_test",	"drm_damage" },
+		{ "drm_dp_mst_helper_test",	"drm_dp_mst" },
+		{ "drm_format_helper_test",	"drm_format_helper" },
+		{ "drm_format_test",		"drm_format" },
+		{ "drm_framebuffer_test",	"framebuffer" },
+		{ "drm_plane_helper_test",	"drm_plane" },
+		{ NULL, NULL}
+	};
 
-	for (int i = 0; kunit_subtests[i] != NULL; i++)
-		igt_kunit(kunit_subtests[i], NULL);
+	for (int i = 0; kunit_subtests[i].kunit != NULL; i++)
+		igt_kunit(kunit_subtests[i].kunit, kunit_subtests[i].name, NULL);
 
 	igt_kselftests("test-drm_modeset", NULL, NULL, NULL);
 }
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 07/10] KUnit: Remove igt_kselftest fallback
  2023-06-13  7:27 [igt-dev] [PATCH v7 i-g-t 00/10] Introduce KUnit Dominik Karol Piatkowski
                   ` (5 preceding siblings ...)
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 06/10] kunit tests: add an optional name for the selftests Dominik Karol Piatkowski
@ 2023-06-13  7:27 ` Dominik Karol Piatkowski
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 08/10] KUnit: Change subtest name from all-tests to module name Dominik Karol Piatkowski
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Dominik Karol Piatkowski @ 2023-06-13  7:27 UTC (permalink / raw)
  To: igt-dev

As igt_kselftest fallback is removed, igt_kunit does not need
to return a value.

Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
Reviewed-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Cc: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>
---
 lib/igt_kmod.c       | 16 ++++++----------
 lib/igt_kmod.h       |  2 +-
 tests/drm_buddy.c    |  4 +---
 tests/drm_mm.c       |  5 +----
 tests/kms_selftest.c |  2 --
 5 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 2c0cc026d..a6e751482 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -754,7 +754,7 @@ void igt_kselftest_get_tests(struct kmod_module *kmod,
  *
  * Returns: IGT default codes
  */
-static int __igt_kunit(const char *module_name, const char *opts)
+static void __igt_kunit(const char *module_name, const char *opts)
 {
 	struct igt_ktest tst;
 	struct kmod_module *kunit_kmod;
@@ -764,19 +764,17 @@ static int __igt_kunit(const char *module_name, const char *opts)
 	struct ktap_test_results *results;
 	struct ktap_test_results_element *temp;
 
-	ret = IGT_EXIT_INVALID;
-
 	/* get normalized module name */
 	if (igt_ktest_init(&tst, module_name) != 0) {
 		igt_warn("Unable to initialize ktest for %s\n", module_name);
-		return ret;
+		igt_fail(IGT_EXIT_SKIP);
 	}
 
 	if (igt_ktest_begin(&tst) != 0) {
 		igt_warn("Unable to begin ktest for %s\n", module_name);
 
 		igt_ktest_fini(&tst);
-		return ret;
+		igt_fail(IGT_EXIT_SKIP);
 	}
 
 	if (tst.kmsg < 0) {
@@ -841,14 +839,13 @@ unload:
 	ret = ktap_parser_stop();
 
 	if (ret != 0)
-		ret = IGT_EXIT_ABORT;
+		igt_fail(IGT_EXIT_ABORT);
 
 	if (ret == 0)
 		igt_success();
-	return ret;
 }
 
-int igt_kunit(const char *module_name, const char *name, const char *opts)
+void igt_kunit(const char *module_name, const char *name, const char *opts)
 {
 	/*
 	 * We need to use igt_subtest here, as otherwise it may crash with:
@@ -861,8 +858,7 @@ int igt_kunit(const char *module_name, const char *name, const char *opts)
 		name = "all-tests";
 
 	igt_subtest_with_dynamic(name)
-		return __igt_kunit(module_name, opts);
-	return 0;
+		__igt_kunit(module_name, opts);
 }
 
 static int open_parameters(const char *module_name)
diff --git a/lib/igt_kmod.h b/lib/igt_kmod.h
index 248955475..990e5309d 100644
--- a/lib/igt_kmod.h
+++ b/lib/igt_kmod.h
@@ -71,7 +71,7 @@ static inline int igt_xe_driver_unload(void)
 int igt_amdgpu_driver_load(const char *opts);
 int igt_amdgpu_driver_unload(void);
 
-int igt_kunit(const char *module_name, const char *name, const char *opts);
+void igt_kunit(const char *module_name, const char *name, const char *opts);
 
 void igt_kselftests(const char *module_name,
 		    const char *module_options,
diff --git a/tests/drm_buddy.c b/tests/drm_buddy.c
index 09feaf635..4f411464a 100644
--- a/tests/drm_buddy.c
+++ b/tests/drm_buddy.c
@@ -10,7 +10,5 @@ IGT_TEST_DESCRIPTION("Basic sanity check of DRM's buddy allocator (struct drm_bu
 
 igt_main
 {
-	int ret = igt_kunit("drm_buddy_test", NULL, NULL);
-	if (ret != 0 && ret != IGT_EXIT_ABORT)
-		igt_kselftests("test-drm_buddy", NULL, NULL, NULL);
+	igt_kunit("drm_buddy_test", NULL, NULL);
 }
diff --git a/tests/drm_mm.c b/tests/drm_mm.c
index ada8cb936..089eae2b9 100644
--- a/tests/drm_mm.c
+++ b/tests/drm_mm.c
@@ -156,8 +156,5 @@ IGT_TEST_DESCRIPTION("Basic sanity check of DRM's range manager (struct drm_mm)"
 
 igt_main
 {
-	int ret = igt_kunit("drm_mm_test", NULL, NULL);
-
-	if (ret != 0 && ret != IGT_EXIT_ABORT)
-		igt_kselftests("test-drm_mm", NULL, NULL, NULL);
+	igt_kunit("drm_mm_test", NULL, NULL);
 }
diff --git a/tests/kms_selftest.c b/tests/kms_selftest.c
index d83e5ff4b..5495c24f2 100644
--- a/tests/kms_selftest.c
+++ b/tests/kms_selftest.c
@@ -46,6 +46,4 @@ igt_main
 
 	for (int i = 0; kunit_subtests[i].kunit != NULL; i++)
 		igt_kunit(kunit_subtests[i].kunit, kunit_subtests[i].name, NULL);
-
-	igt_kselftests("test-drm_modeset", NULL, NULL, NULL);
 }
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 08/10] KUnit: Change subtest name from all-tests to module name
  2023-06-13  7:27 [igt-dev] [PATCH v7 i-g-t 00/10] Introduce KUnit Dominik Karol Piatkowski
                   ` (6 preceding siblings ...)
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 07/10] KUnit: Remove igt_kselftest fallback Dominik Karol Piatkowski
@ 2023-06-13  7:27 ` Dominik Karol Piatkowski
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 09/10] tests/xe: Add a test that launches the xe driver live kunit tests Dominik Karol Piatkowski
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Dominik Karol Piatkowski @ 2023-06-13  7:27 UTC (permalink / raw)
  To: igt-dev

Use KUnit module name instead of all-tests when no name is specified.

Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
Reviewed-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Cc: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>
---
 lib/igt_kmod.c |  2 +-
 tests/drm_mm.c | 42 +++++++++++++++++++++---------------------
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index a6e751482..1511bdef4 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -855,7 +855,7 @@ void igt_kunit(const char *module_name, const char *name, const char *opts)
 	 * and for documentation.
 	 */
 	if (name == NULL)
-		name = "all-tests";
+		name = module_name;
 
 	igt_subtest_with_dynamic(name)
 		__igt_kunit(module_name, opts);
diff --git a/tests/drm_mm.c b/tests/drm_mm.c
index 089eae2b9..9a8b3f3fc 100644
--- a/tests/drm_mm.c
+++ b/tests/drm_mm.c
@@ -29,123 +29,123 @@
  * Feature: mapping
  * Run type: FULL
  *
- * SUBTEST: all-tests
+ * SUBTEST: drm_mm_test
  *
- * SUBTEST: all-tests@align
+ * SUBTEST: drm_mm_test@align
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@align32
+ * SUBTEST: drm_mm_test@align32
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@align64
+ * SUBTEST: drm_mm_test@align64
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@bottomup
+ * SUBTEST: drm_mm_test@bottomup
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@color
+ * SUBTEST: drm_mm_test@color
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@color_evict
+ * SUBTEST: drm_mm_test@color_evict
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@color_evict_range
+ * SUBTEST: drm_mm_test@color_evict_range
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@debug
+ * SUBTEST: drm_mm_test@debug
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@evict
+ * SUBTEST: drm_mm_test@evict
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@evict_range
+ * SUBTEST: drm_mm_test@evict_range
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@frag
+ * SUBTEST: drm_mm_test@frag
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@highest
+ * SUBTEST: drm_mm_test@highest
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@init
+ * SUBTEST: drm_mm_test@init
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@insert
+ * SUBTEST: drm_mm_test@insert
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@insert_range
+ * SUBTEST: drm_mm_test@insert_range
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@lowest
+ * SUBTEST: drm_mm_test@lowest
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@replace
+ * SUBTEST: drm_mm_test@replace
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@reserve
+ * SUBTEST: drm_mm_test@reserve
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@sanitycheck
+ * SUBTEST: drm_mm_test@sanitycheck
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
  * Test category: GEM_Legacy
  *
- * SUBTEST: all-tests@topdown
+ * SUBTEST: drm_mm_test@topdown
  * Category: Infrastructure
  * Description: drm_mm range manager SW validation
  * Functionality: DRM memory mangemnt
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 09/10] tests/xe: Add a test that launches the xe driver live kunit tests
  2023-06-13  7:27 [igt-dev] [PATCH v7 i-g-t 00/10] Introduce KUnit Dominik Karol Piatkowski
                   ` (7 preceding siblings ...)
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 08/10] KUnit: Change subtest name from all-tests to module name Dominik Karol Piatkowski
@ 2023-06-13  7:27 ` Dominik Karol Piatkowski
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 10/10] KUnit: replace abort with graceful skip Dominik Karol Piatkowski
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Dominik Karol Piatkowski @ 2023-06-13  7:27 UTC (permalink / raw)
  To: igt-dev; +Cc: Thomas Hellström

From: Mauro Carvalho Chehab <mchehab@kernel.org>

The xe driver live kunit tests live in the xe driver tests/
subdirectory and are intended to be launched by this igt test.

To begin with we only have one test ("dma-buf").

[mchehab: updated to reflect current KUnit tests]

Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
---
 tests/meson.build        |  1 +
 tests/xe/xe_live_ktest.c | 52 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)
 create mode 100644 tests/xe/xe_live_ktest.c

diff --git a/tests/meson.build b/tests/meson.build
index f908ae885..61dcc0769 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -259,6 +259,7 @@ xe_progs = [
 	'xe_guc_pc',
 	'xe_huc_copy',
 	'xe_intel_bb',
+	'xe_live_ktest',
 	'xe_mmap',
 	'xe_mmio',
 	'xe_module_load',
diff --git a/tests/xe/xe_live_ktest.c b/tests/xe/xe_live_ktest.c
new file mode 100644
index 000000000..7dcf67906
--- /dev/null
+++ b/tests/xe/xe_live_ktest.c
@@ -0,0 +1,52 @@
+#include "igt.h"
+#include "igt_kmod.h"
+
+/**
+ * TEST: Xe driver live kunit tests
+ * Description: Xe driver live dmabuf unit tests
+ * Category: Software building block
+ * Sub-category: kunit
+ * Functionality: kunit
+ * Test category: functionality test
+ * Run type: BAT
+ *
+ * SUBTEST: bo
+ * Functionality: bo
+ *
+ * SUBTEST: dmabuf
+ * Functionality: dmabuf
+ *
+ * SUBTEST: migrate
+ * Functionality: migrate
+ *
+ * SUBTEST: pci
+ * Functionality: pci
+ *
+ * SUBTEST: rtp
+ * Functionality: rtp
+ *
+ * SUBTEST: wa
+ * Functionality: workarounds
+ */
+
+struct kunit_tests {
+	const char *kunit;
+	const char *name;
+};
+
+static const struct kunit_tests live_tests[] = {
+	{ "xe_bo_test",		"bo" },
+	{ "xe_dma_buf_test",	"dmabuf" },
+	{ "xe_migrate_test",	"migrate" },
+	{ "xe_pci_test",	"pci" },
+	{ "xe_rtp_test",	"rtp" },
+	{ "xe_wa_test",		"wa" },
+};
+
+igt_main
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(live_tests); i++)
+		igt_kunit(live_tests[i].kunit, live_tests[i].name, NULL);
+}
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] [PATCH i-g-t 10/10] KUnit: replace abort with graceful skip
  2023-06-13  7:27 [igt-dev] [PATCH v7 i-g-t 00/10] Introduce KUnit Dominik Karol Piatkowski
                   ` (8 preceding siblings ...)
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 09/10] tests/xe: Add a test that launches the xe driver live kunit tests Dominik Karol Piatkowski
@ 2023-06-13  7:27 ` Dominik Karol Piatkowski
  2023-06-13  8:41   ` Mauro Carvalho Chehab
  2023-06-13  8:20 ` [igt-dev] ✓ Fi.CI.BAT: success for Introduce KUnit (rev7) Patchwork
  2023-06-13 11:13 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  11 siblings, 1 reply; 22+ messages in thread
From: Dominik Karol Piatkowski @ 2023-06-13  7:27 UTC (permalink / raw)
  To: igt-dev

Sample drm_buddy output with missing requirements:
	Starting subtest: drm_buddy_test
	(drm_buddy:32218) igt_kmod-WARNING: Unable to load KUnit
	Subtest drm_buddy_test: SKIP (0.001s)

Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Cc: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>
---
 lib/igt_kmod.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 1511bdef4..04220f404 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -763,27 +763,27 @@ static void __igt_kunit(const char *module_name, const char *opts)
 	int ret;
 	struct ktap_test_results *results;
 	struct ktap_test_results_element *temp;
+	bool skip = false;
 
 	/* get normalized module name */
 	if (igt_ktest_init(&tst, module_name) != 0) {
-		igt_warn("Unable to initialize ktest for %s\n", module_name);
-		igt_fail(IGT_EXIT_SKIP);
+		igt_skip("Unable to initialize ktest for %s\n", module_name);
 	}
 
 	if (igt_ktest_begin(&tst) != 0) {
-		igt_warn("Unable to begin ktest for %s\n", module_name);
-
 		igt_ktest_fini(&tst);
-		igt_fail(IGT_EXIT_SKIP);
+		igt_skip("Unable to begin ktest for %s\n", module_name);
 	}
 
 	if (tst.kmsg < 0) {
 		igt_warn("Could not open /dev/kmsg\n");
+		skip = true;
 		goto unload;
 	}
 
 	if (lseek(tst.kmsg, 0, SEEK_END)) {
 		igt_warn("Could not seek the end of /dev/kmsg\n");
+		skip = true;
 		goto unload;
 	}
 
@@ -791,6 +791,7 @@ static void __igt_kunit(const char *module_name, const char *opts)
 
 	if (f == NULL) {
 		igt_warn("Could not turn /dev/kmsg file descriptor into a FILE pointer\n");
+		skip = true;
 		goto unload;
 	}
 
@@ -798,7 +799,8 @@ static void __igt_kunit(const char *module_name, const char *opts)
 	if (igt_kmod_load("kunit", NULL) != 0 ||
 	    kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) != 0) {
 		igt_warn("Unable to load KUnit\n");
-		igt_fail(IGT_EXIT_FAILURE);
+		skip = true;
+		goto unload;
 	}
 
 	is_builtin = kmod_module_get_initstate(kunit_kmod) == KMOD_MODULE_BUILTIN;
@@ -808,7 +810,8 @@ static void __igt_kunit(const char *module_name, const char *opts)
 	if (igt_kmod_load(module_name, opts) != 0) {
 		igt_warn("Unable to load %s module\n", module_name);
 		ret = ktap_parser_stop();
-		igt_fail(IGT_EXIT_FAILURE);
+		skip = true;
+		goto unload;
 	}
 
 	while (READ_ONCE(results->still_running) || READ_ONCE(results->head) != NULL)
@@ -836,6 +839,9 @@ unload:
 
 	igt_ktest_fini(&tst);
 
+	if (skip)
+		igt_skip("");
+
 	ret = ktap_parser_stop();
 
 	if (ret != 0)
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [igt-dev] ✓ Fi.CI.BAT: success for Introduce KUnit (rev7)
  2023-06-13  7:27 [igt-dev] [PATCH v7 i-g-t 00/10] Introduce KUnit Dominik Karol Piatkowski
                   ` (9 preceding siblings ...)
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 10/10] KUnit: replace abort with graceful skip Dominik Karol Piatkowski
@ 2023-06-13  8:20 ` Patchwork
  2023-06-13 11:13 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  11 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2023-06-13  8:20 UTC (permalink / raw)
  To: Dominik Karol Piatkowski; +Cc: igt-dev

[-- Attachment #1: Type: text/plain, Size: 7084 bytes --]

== Series Details ==

Series: Introduce KUnit (rev7)
URL   : https://patchwork.freedesktop.org/series/114612/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13264 -> IGTPW_9155
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/index.html

Participating hosts (41 -> 41)
------------------------------

  Additional (1): fi-kbl-soraka 
  Missing    (1): fi-snb-2520m 

Known issues
------------

  Here are the changes found in IGTPW_9155 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@core_auth@basic-auth:
    - bat-adlp-11:        NOTRUN -> [ABORT][1] ([i915#8011])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/bat-adlp-11/igt@core_auth@basic-auth.html

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#2190])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#4613]) +3 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@i915_selftest@live@gt_engines:
    - bat-atsm-1:         [PASS][4] -> [FAIL][5] ([i915#6268])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/bat-atsm-1/igt@i915_selftest@live@gt_engines.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/bat-atsm-1/igt@i915_selftest@live@gt_engines.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][6] ([i915#1886] / [i915#7913])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@requests:
    - bat-rpls-2:         [PASS][7] -> [ABORT][8] ([i915#7913])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/bat-rpls-2/igt@i915_selftest@live@requests.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/bat-rpls-2/igt@i915_selftest@live@requests.html

  * igt@i915_suspend@basic-s3-without-i915:
    - bat-rpls-1:         NOTRUN -> [ABORT][9] ([i915#6687] / [i915#7978])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/bat-rpls-1/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][10] ([fdo#109271]) +14 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/fi-kbl-soraka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1:
    - bat-dg2-8:          [PASS][11] -> [FAIL][12] ([i915#7932])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][13] ([fdo#109271] / [i915#4579])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/fi-kbl-soraka/igt@kms_setmode@basic-clone-single-crtc.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_mocs:
    - bat-mtlp-8:         [DMESG-FAIL][14] ([i915#7059]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/bat-mtlp-8/igt@i915_selftest@live@gt_mocs.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/bat-mtlp-8/igt@i915_selftest@live@gt_mocs.html

  * igt@i915_selftest@live@reset:
    - bat-rpls-1:         [ABORT][16] ([i915#4983] / [i915#7461] / [i915#8347] / [i915#8384]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/bat-rpls-1/igt@i915_selftest@live@reset.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/bat-rpls-1/igt@i915_selftest@live@reset.html

  * igt@i915_selftest@live@slpc:
    - bat-mtlp-6:         [DMESG-WARN][18] ([i915#6367]) -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/bat-mtlp-6/igt@i915_selftest@live@slpc.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/bat-mtlp-6/igt@i915_selftest@live@slpc.html

  
#### Warnings ####

  * igt@i915_module_load@load:
    - bat-adlp-11:        [ABORT][20] ([i915#4423]) -> [DMESG-WARN][21] ([i915#4423])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/bat-adlp-11/igt@i915_module_load@load.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/bat-adlp-11/igt@i915_module_load@load.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#4423]: https://gitlab.freedesktop.org/drm/intel/issues/4423
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
  [i915#7059]: https://gitlab.freedesktop.org/drm/intel/issues/7059
  [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
  [i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978
  [i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
  [i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347
  [i915#8384]: https://gitlab.freedesktop.org/drm/intel/issues/8384


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_7326 -> IGTPW_9155

  CI-20190529: 20190529
  CI_DRM_13264: 807d06e86e11466ef4fe74e2ddcb93de8e3a535b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_9155: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/index.html
  IGT_7326: 02c2cf17628b6203d6105d4a91dfe8a101d482ce @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git


Testlist changes
----------------

+igt@drm_buddy@drm_buddy_test
+igt@drm_mm@drm_mm_test
+igt@kms_selftest@drm_cmdline
+igt@kms_selftest@drm_damage
+igt@kms_selftest@drm_dp_mst
+igt@kms_selftest@drm_format
+igt@kms_selftest@drm_format_helper
+igt@kms_selftest@drm_plane
+igt@kms_selftest@framebuffer
+igt@xe_live_ktest@bo
+igt@xe_live_ktest@dmabuf
+igt@xe_live_ktest@migrate
+igt@xe_live_ktest@pci
+igt@xe_live_ktest@rtp
+igt@xe_live_ktest@wa
-igt@drm_buddy@all-tests
-igt@drm_mm@all-tests
-igt@kms_selftest@all-tests

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/index.html

[-- Attachment #2: Type: text/html, Size: 8451 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 10/10] KUnit: replace abort with graceful skip
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 10/10] KUnit: replace abort with graceful skip Dominik Karol Piatkowski
@ 2023-06-13  8:41   ` Mauro Carvalho Chehab
  2023-06-13  9:11     ` Janusz Krzysztofik
  0 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2023-06-13  8:41 UTC (permalink / raw)
  To: Dominik Karol Piatkowski; +Cc: igt-dev

On Tue, 13 Jun 2023 09:27:26 +0200
Dominik Karol Piatkowski <dominik.karol.piatkowski@intel.com> wrote:

> Sample drm_buddy output with missing requirements:
> 	Starting subtest: drm_buddy_test
> 	(drm_buddy:32218) igt_kmod-WARNING: Unable to load KUnit
> 	Subtest drm_buddy_test: SKIP (0.001s)
> 
> Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
> Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>
> ---
>  lib/igt_kmod.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index 1511bdef4..04220f404 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -763,27 +763,27 @@ static void __igt_kunit(const char *module_name, const char *opts)
>  	int ret;
>  	struct ktap_test_results *results;
>  	struct ktap_test_results_element *temp;
> +	bool skip = false;
>  
>  	/* get normalized module name */
>  	if (igt_ktest_init(&tst, module_name) != 0) {
> -		igt_warn("Unable to initialize ktest for %s\n", module_name);
> -		igt_fail(IGT_EXIT_SKIP);
> +		igt_skip("Unable to initialize ktest for %s\n", module_name);
>  	}
>  
>  	if (igt_ktest_begin(&tst) != 0) {
> -		igt_warn("Unable to begin ktest for %s\n", module_name);
> -
>  		igt_ktest_fini(&tst);
> -		igt_fail(IGT_EXIT_SKIP);
> +		igt_skip("Unable to begin ktest for %s\n", module_name);
>  	}
>  
>  	if (tst.kmsg < 0) {
>  		igt_warn("Could not open /dev/kmsg\n");
> +		skip = true;

Hmm.... not being able to read from /dev/kmsg doesn't seem a reason for
skipping the test.

IMO, the test should be skipped only if:

1. The Kernel is not compiled with KUnit support;
2. The KUnit test was not compiled.

Errors for all other conditions should be considered as failures.

Now, I'm not sure what would be the proper way to test for such
skip condition. I suspect you could check the error code at
igt_kmod_load(), calling igt_skip() if it returns -ENOENT,
e. g. on this part of __igt_kunit():

        /* The KUnit module is required for running any KUnit tests */
        if (igt_kmod_load("kunit", NULL) != 0 ||
            kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) != 0) {
                igt_warn("Unable to load KUnit\n");
                igt_fail(IGT_EXIT_FAILURE);
        }

        is_builtin = kmod_module_get_initstate(kunit_kmod) == KMOD_MODULE_BUILTIN;

        results = ktap_parser_start(f, is_builtin);

        if (igt_kmod_load(module_name, opts) != 0) {
                igt_warn("Unable to load %s module\n", module_name);
                ret = ktap_parser_stop();
                igt_fail(IGT_EXIT_FAILURE);
        }


This could be replaced by something similar to:

	err = igt_kmod_load("kunit", NULL);
	igt_skip_on(err == ENOENT, "KUnit not supported");
	igt_fail_on_f(err != 0, "Error %d when trying to load kunit", err);
        if (kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) != 0) {
                igt_warn("Unable to load KUnit\n");
                igt_fail(IGT_EXIT_FAILURE);
        }


        is_builtin = kmod_module_get_initstate(kunit_kmod) == KMOD_MODULE_BUILTIN;

        results = ktap_parser_start(f, is_builtin);

        err = igt_kmod_load(module_name, opts) 
	igt_skip_on(err == ENOENT, "Module %s not found", module_name);
	if (err) {
                igt_warn("Error %d when trying to load %s module\n", err, module_name);
                ret = ktap_parser_stop();
                igt_fail(IGT_EXIT_FAILURE);
        }


In order to explicitly check for (1) and (2).

Regards,
Mauro

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 05/10] Change logic of ktap parser to run on a thread
  2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 05/10] Change logic of ktap parser to run on a thread Dominik Karol Piatkowski
@ 2023-06-13  8:44   ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2023-06-13  8:44 UTC (permalink / raw)
  To: Dominik Karol Piatkowski; +Cc: igt-dev

On Tue, 13 Jun 2023 09:27:21 +0200
Dominik Karol Piatkowski <dominik.karol.piatkowski@intel.com> wrote:

> The ktap parser should be listening and parsing messages as the tests
> are executed, and not after the end of module load.
> 
> v1 -> v2:
> - fix coding style
> - remove usleep
> - add error check logic
> - follow the structure of igt_kselftests more closely
> 
> v2 -> v3:
> - fixed sublevel issues by rewriting tap parser flow
> 
> v3 -> v4:
> - fixed delimiter
> 
> - squashed lib/igt_kmod: fix nesting igt_fixture in igt_subtest
> Fix the following issue:
> 	$ ./build/tests/drm_buddy
> 	Starting subtest: all-tests
> 	nesting igt_fixture in igt_subtest is invalid
> 	please refer to lib/igt_core documentation
> 
> - squashed lib/igt_kmod: place KUnit tests on a subtest
> There's a hidden bug at KUnit implementation: as it doesn't
> place tests inside a subtest, trying to use it with igt_main
> causes a crash:
> 
> 	$ ./build/tests/drm_mm --list
> 	skipping is allowed only in fixtures, subtests or igt_simple_main
> 	please refer to lib/igt_core documentation
> 	drm_mm: ../lib/igt_core.c:437: internal_assert: Assertion `0' failed.
> 	Received signal SIGABRT.
> 	Stack trace:
> 	 #0 [fatal_sig_handler+0x17b]
> 	 #1 [__sigaction+0x50]
> 	 #2 [__pthread_kill_implementation+0x114]
> 	 #3 [gsignal+0x1e]
> 	 #4 [abort+0xdf]
> 	 #5 [__assert_fail_base.cold+0xe]
> 	 #6 [__assert_fail+0x47]
> 	 #7 [internal_assert+0xe5]
> 	 #8 [igt_skip+0x169]
> 	 #9 [__igt_skip_check+0x1bb]
> 	 #10 [igt_ktest_begin+0xa6]
> 	 #11 [igt_kunit+0x70]
> 	 #12 [main+0x2a]
> 	 #13 [__libc_start_call_main+0x7a]
> 	 #14 [__libc_start_main+0x8b]
> 	 #15 [_start+0x25]
> 
> Fix it by using igt_subtests() before actually implememnting
> KUnit logic.
> 
> After the patch, it should now report subtests:
> 
> 	$ ./build/tests/drm_mm --list
> 	all-tests
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> 
> Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
> Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>

LGTM.

Acked-by:  Mauro Carvalho Chehab <mchehab@kernel.org>

> ---
>  lib/igt_kmod.c |  51 +++++-
>  lib/igt_ktap.c | 431 ++++++++++++++++++++++++++++++++++++++++---------
>  lib/igt_ktap.h |  21 ++-
>  3 files changed, 421 insertions(+), 82 deletions(-)
> 
> diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> index 21e801bde..73478f75e 100644
> --- a/lib/igt_kmod.c
> +++ b/lib/igt_kmod.c
> @@ -754,14 +754,15 @@ void igt_kselftest_get_tests(struct kmod_module *kmod,
>   *
>   * Returns: IGT default codes
>   */
> -int igt_kunit(const char *module_name, const char *opts)
> +static int __igt_kunit(const char *module_name, const char *opts)
>  {
>  	struct igt_ktest tst;
>  	struct kmod_module *kunit_kmod;
> -	char record[BUF_LEN + 1];
>  	FILE *f;
>  	bool is_builtin;
>  	int ret;
> +	struct ktap_test_results *results;
> +	struct ktap_test_results_element *temp;
>  
>  	ret = IGT_EXIT_INVALID;
>  
> @@ -804,25 +805,63 @@ int igt_kunit(const char *module_name, const char *opts)
>  
>  	is_builtin = kmod_module_get_initstate(kunit_kmod) == KMOD_MODULE_BUILTIN;
>  
> +	results = ktap_parser_start(f, is_builtin);
> +
>  	if (igt_kmod_load(module_name, opts) != 0) {
>  		igt_warn("Unable to load %s module\n", module_name);
> +		ret = ktap_parser_stop();
>  		igt_fail(IGT_EXIT_FAILURE);
>  	}
>  
> -	ret = igt_ktap_parser(f, record, is_builtin);
> -	if (ret != 0)
> -		ret = IGT_EXIT_ABORT;
> +	while (READ_ONCE(results->still_running) || READ_ONCE(results->head) != NULL)
> +	{
> +		if (READ_ONCE(results->head) != NULL) {
> +			pthread_mutex_lock(&results->mutex);
> +
> +			igt_dynamic(results->head->test_name) {
> +				if (READ_ONCE(results->head->passed))
> +					igt_success();
> +				else
> +					igt_fail(IGT_EXIT_FAILURE);
> +			}
> +
> +			temp = results->head;
> +			results->head = results->head->next;
> +			free(temp);
> +
> +			pthread_mutex_unlock(&results->mutex);
> +		}
> +	}
> +
>  unload:
>  	igt_ktest_end(&tst);
>  
>  	igt_ktest_fini(&tst);
>  
> +	ret = ktap_parser_stop();
> +
> +	if (ret != 0)
> +		ret = IGT_EXIT_ABORT;
> +
>  	if (ret == 0)
>  		igt_success();
> -
>  	return ret;
>  }
>  
> +int igt_kunit(const char *module_name, 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
> +	 * 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("all-tests")
> +		return __igt_kunit(module_name, opts);
> +	return 0;
> +}
> +
>  static int open_parameters(const char *module_name)
>  {
>  	char path[256];
> diff --git a/lib/igt_ktap.c b/lib/igt_ktap.c
> index 117598faa..ecdcb8d83 100644
> --- a/lib/igt_ktap.c
> +++ b/lib/igt_ktap.c
> @@ -5,11 +5,25 @@
>  
>  #include <ctype.h>
>  #include <limits.h>
> +#include <libkmod.h>
> +#include <pthread.h>
> +#include <errno.h>
>  
>  #include "igt_aux.h"
>  #include "igt_core.h"
>  #include "igt_ktap.h"
>  
> +#define DELIMITER "-"
> +
> +struct ktap_parser_args {
> +	FILE *fp;
> +	bool is_builtin;
> +	volatile bool is_running;
> +	int ret;
> +} ktap_args;
> +
> +static struct ktap_test_results results;
> +
>  static int log_to_end(enum igt_log_level level, FILE *f,
>  		      char *record, const char *format, ...) __attribute__((format(printf, 4, 5)));
>  
> @@ -30,6 +44,14 @@ static int log_to_end(enum igt_log_level level, FILE *f,
>  {
>  	va_list args;
>  	const char *lend;
> +	int f_fd = fileno(f);
> +
> +	/* Cutoff after newline character, in order to not display garbage */
> +	char *cutoff = strchr(record, '\n');
> +	if (cutoff) {
> +		if (cutoff - record < BUF_LEN)
> +			cutoff[1] = '\0';
> +	}
>  
>  	va_start(args, format);
>  	igt_vlog(IGT_LOG_DOMAIN, level, format, args);
> @@ -38,10 +60,29 @@ static int log_to_end(enum igt_log_level level, FILE *f,
>  	lend = strchrnul(record, '\n');
>  	while (*lend == '\0') {
>  		igt_log(IGT_LOG_DOMAIN, level, "%s", record);
> -		if (fgets(record, BUF_LEN, f) == NULL) {
> +
> +		while (read(f_fd, record, BUF_LEN) < 0) {
> +			if (!READ_ONCE(ktap_args.is_running)) {
> +				igt_warn("ktap parser stopped\n");
> +				return -2;
> +			}
> +
> +			if (errno == EINTR)
> +				continue;
> +
> +			if (errno == EPIPE) {
> +				igt_warn("kmsg truncated: too many messages. You may want to increase log_buf_len in kmcdline\n");
> +				return -2;
> +			}
> +
> +			if (errno == EAGAIN)
> +				/* No records available */
> +				continue;
> +
>  			igt_warn("kmsg truncated: unknown error (%m)\n");
>  			return -2;
>  		}
> +
>  		lend = strchrnul(record, '\n');
>  	}
>  	return 0;
> @@ -65,7 +106,7 @@ static long lookup_value(const char *haystack, const char *needle)
>  	if (needle_rptr == NULL)
>  		return -1;
>  
> -	/* skip search string and whitespaces after it */
> +	/* Skip search string and whitespaces after it */
>  	needle_rptr += strlen(needle);
>  
>  	num = strtol(needle_rptr, &needle_end, 10);
> @@ -79,6 +120,41 @@ static long lookup_value(const char *haystack, const char *needle)
>  	return num > 0 ? num : 0;
>  }
>  
> +/**
> + * tap_version_present:
> + * @record: buffer with tap data
> + * @print_info: whether tap version should be printed or not
> + *
> + * Returns:
> + * 0 if not found
> + * 1 if found
> + */
> +static int tap_version_present(char* record, bool print_info)
> +{
> +	/*
> +	 * "(K)TAP version XX" should be the first line on all (sub)tests as per
> +	 * https://kernel.org/doc/html/latest/dev-tools/ktap.html#version-lines
> +	 *
> +	 * but actually isn't, as it currently depends on the KUnit module
> +	 * being built-in, so we can't rely on it every time
> +	 */
> +	const char *version_rptr = strcasestr(record, "TAP version ");
> +	char *cutoff;
> +
> +	if (version_rptr == NULL)
> +		return 0;
> +
> +	/* Cutoff after newline character, in order to not display garbage */
> +	cutoff = strchr(version_rptr, '\n');
> +	if (cutoff)
> +		cutoff[0] = '\0';
> +
> +	if (print_info)
> +		igt_info("%s\n", version_rptr);
> +
> +	return 1;
> +}
> +
>  /**
>   * find_next_tap_subtest:
>   * @fp: FILE pointer
> @@ -91,11 +167,12 @@ static long lookup_value(const char *haystack, const char *needle)
>   * -2 if there are problems while reading the file.
>   * any other value corresponds to the amount of cases of the next (sub)test
>   */
> -static int find_next_tap_subtest(FILE *fp, char *record, bool is_builtin)
> +static int find_next_tap_subtest(FILE *fp, char *record, char *test_name, bool is_builtin)
>  {
> -	const char *test_lookup_str, *subtest_lookup_str, *name_rptr, *version_rptr;
> -	char test_name[BUF_LEN + 1];
> +	const char *test_lookup_str, *subtest_lookup_str, *name_rptr;
>  	long test_count;
> +	int fp_fd = fileno(fp);
> +	char *cutoff;
>  
>  	test_name[0] = '\0';
>  	test_name[BUF_LEN] = '\0';
> @@ -103,21 +180,28 @@ static int find_next_tap_subtest(FILE *fp, char *record, bool is_builtin)
>  	test_lookup_str = " subtest: ";
>  	subtest_lookup_str = " test: ";
>  
> -	/*
> -	 * "(K)TAP version XX" should be the first line on all (sub)tests as per
> -	 * https://kernel.org/doc/html/latest/dev-tools/ktap.html#version-lines
> -	 *
> -	 * but actually isn't, as it currently depends on the KUnit module
> -	 * being built-in, so we can't rely on it every time
> -	 */
> +	if (!tap_version_present(record, true))
> +		return -1;
> +
>  	if (is_builtin) {
> -		version_rptr = strcasestr(record, "TAP version ");
> -		if (version_rptr == NULL)
> -			return -1;
> +		while (read(fp_fd, record, BUF_LEN) < 0) {
> +			if (!READ_ONCE(ktap_args.is_running)) {
> +				igt_warn("ktap parser stopped\n");
> +				return -2;
> +			}
> +
> +			if (errno == EINTR)
> +				continue;
>  
> -		igt_info("%s", version_rptr);
> +			if (errno == EPIPE) {
> +				igt_warn("kmsg truncated: too many messages. You may want to increase log_buf_len in kmcdline\n");
> +				return -2;
> +			}
> +
> +			if (errno == EAGAIN)
> +				/* No records available */
> +				continue;
>  
> -		if (fgets(record, BUF_LEN, fp) == NULL) {
>  			igt_warn("kmsg truncated: unknown error (%m)\n");
>  			return -2;
>  		}
> @@ -134,22 +218,45 @@ static int find_next_tap_subtest(FILE *fp, char *record, bool is_builtin)
>  
>  	if (name_rptr == NULL) {
>  		if (!is_builtin)
> -			/* we've probably found nothing */
> +			/* We've probably found nothing */
>  			return -1;
>  		igt_info("Missing test name\n");
>  	} else {
>  		strncpy(test_name, name_rptr, BUF_LEN);
> -		if (fgets(record, BUF_LEN, fp) == NULL) {
> +		/* Cutoff after newline character, in order to not display garbage */
> +		cutoff = strchr(test_name, '\n');
> +		if (cutoff)
> +			cutoff[0] = '\0';
> +
> +		while (read(fp_fd, record, BUF_LEN) < 0) {
> +			if (!READ_ONCE(ktap_args.is_running)) {
> +				igt_warn("ktap parser stopped\n");
> +				return -2;
> +			}
> +
> +			if (errno == EINTR)
> +				continue;
> +
> +			if (errno == EPIPE) {
> +				igt_warn("kmsg truncated: too many messages. You may want to increase log_buf_len in kmcdline\n");
> +				return -2;
> +			}
> +
> +			if (errno == EAGAIN)
> +				/* No records available */
> +				continue;
> +
>  			igt_warn("kmsg truncated: unknown error (%m)\n");
>  			return -2;
>  		}
> -		/* now we can be sure we found tests */
> +
> +		/* Now we can be sure we found tests */
>  		if (!is_builtin)
>  			igt_info("KUnit is not built-in, skipping version check...\n");
>  	}
>  
>  	/*
> -	 * total test count will almost always appear as 0..N at the beginning
> +	 * Total test count will almost always appear as 0..N at the beginning
>  	 * of a run, so we use it to reliably identify a new run
>  	 */
>  	test_count = lookup_value(record, "..");
> @@ -159,7 +266,7 @@ static int find_next_tap_subtest(FILE *fp, char *record, bool is_builtin)
>  		if (test_name[0] == '\0')
>  			return 0;
>  		if (log_to_end(IGT_LOG_INFO, fp, record,
> -				"Running some tests in: %s",
> +				"Running some tests in: %s\n",
>  				test_name) < 0)
>  			return -2;
>  		return 0;
> @@ -169,7 +276,7 @@ static int find_next_tap_subtest(FILE *fp, char *record, bool is_builtin)
>  	}
>  
>  	if (log_to_end(IGT_LOG_INFO, fp, record,
> -			"Executing %ld tests in: %s",
> +			"Executing %ld tests in: %s\n",
>  			test_count, test_name) < 0)
>  		return -2;
>  
> @@ -177,7 +284,7 @@ static int find_next_tap_subtest(FILE *fp, char *record, bool is_builtin)
>  }
>  
>  /**
> - * find_next_tap_test:
> + * parse_kmsg_for_tap:
>   * @fp: FILE pointer
>   * @record: buffer used to read fp
>   * @test_name: buffer to store the test name
> @@ -225,7 +332,7 @@ static int parse_kmsg_for_tap(FILE *fp, char *record, char *test_name)
>  
>  	comment_start = strchrnul(lstart, '#');
>  
> -	/* check if we're still in a subtest */
> +	/* Check if we're still in a subtest */
>  	if (*comment_start != '\0') {
>  		comment_start++;
>  		value_parse_start = comment_start;
> @@ -254,81 +361,255 @@ static int parse_kmsg_for_tap(FILE *fp, char *record, char *test_name)
>  }
>  
>  /**
> - * igt_ktap_parser:
> + * parse_tap_level:
>   * @fp: FILE pointer
> - * @record: buffer used to read fp
> + * @base_test_name: test_name from upper recursion level
> + * @test_count: test_count of this level
> + * @failed_tests: top level failed_tests pointer
> + * @found_tests: top level found_tests pointer
>   * @is_builtin: whether the KUnit module is built-in or not
>   *
> - * This function parses the output of a ktap script and prints the test results,
> - * as well as any other output to stdout.
> - *
> - * Returns: IGT default codes
> + * Returns:
> + * 0 if succeded
> + * -1 if error occurred
>   */
> -int igt_ktap_parser(FILE *fp, char *record, bool is_builtin)
> +static int parse_tap_level(FILE *fp, char *base_test_name, int test_count, bool *failed_tests,
> +			   bool *found_tests, bool is_builtin)
>  {
> +	int fp_fd = fileno(fp);
> +	char record[BUF_LEN + 1];
> +	struct ktap_test_results_element *r, *temp;
> +	int internal_test_count;
>  	char test_name[BUF_LEN + 1];
> -	bool failed_tests, found_tests;
> -	int sublevel = 0;
> +	char base_test_name_for_next_level[BUF_LEN + 1];
>  
> -	test_name[0] = '\0';
> -	test_name[BUF_LEN] = '\0';
> +	for (int i = 0; i < test_count; i++) {
> +		while (read(fp_fd, record, BUF_LEN) < 0) {
> +			if (!READ_ONCE(ktap_args.is_running)) {
> +				igt_warn("ktap parser stopped\n");
> +				return -1;
> +			}
>  
> -	failed_tests = false;
> -	found_tests = false;
> +			if (errno == EINTR)
> +				continue;
>  
> -	while (sublevel >= 0) {
> -		if (fgets(record, BUF_LEN, fp) == NULL) {
> -			if (!found_tests)
> -				igt_warn("kmsg truncated: unknown error (%m)\n");
> -			break;
> +			if (errno == EAGAIN)
> +				/* No records available */
> +				continue;
> +
> +			if (errno == EPIPE) {
> +				igt_warn("kmsg truncated: too many messages. You may want to increase log_buf_len in kmcdline\n");
> +				return -1;
> +			}
> +
> +			igt_warn("kmsg truncated: unknown error (%m)\n");
> +			return -1;
>  		}
>  
> -		switch (find_next_tap_subtest(fp, record, is_builtin)) {
> -		case -2:
> -			/* no more data to read */
> -			return IGT_EXIT_FAILURE;
> -		case -1:
> -			/* no test found, so we keep parsing */
> -			break;
> -		case 0:
> -			/*
> -			 * tests found, but they're missing info, so we might
> -			 * have read into test output
> -			 */
> -			found_tests = true;
> -			sublevel++;
> -			break;
> -		default:
> -			if (fgets(record, BUF_LEN, fp) == NULL) {
> -				igt_warn("kmsg truncated: unknown error (%m)\n");
> -				return -2;
> +		/* Sublevel found */
> +		if (tap_version_present(record, false))
> +		{
> +			internal_test_count = find_next_tap_subtest(fp, record, test_name,
> +								    is_builtin);
> +			switch (internal_test_count) {
> +			case -2:
> +				/* No more data to read */
> +				return -1;
> +			case -1:
> +				/* No test found */
> +				return -1;
> +			case 0:
> +				/* Tests found, but they're missing info */
> +				*found_tests = true;
> +				return -1;
> +			default:
> +				*found_tests = true;
> +
> +				memcpy(base_test_name_for_next_level, base_test_name, BUF_LEN);
> +				if (strlen(base_test_name_for_next_level) < BUF_LEN - 1 &&
> +				    base_test_name_for_next_level[0])
> +					strncat(base_test_name_for_next_level, DELIMITER,
> +						BUF_LEN - strlen(base_test_name_for_next_level));
> +				memcpy(base_test_name_for_next_level + strlen(base_test_name_for_next_level),
> +				       test_name, BUF_LEN - strlen(base_test_name_for_next_level));
> +
> +				if (parse_tap_level(fp, base_test_name_for_next_level,
> +						    internal_test_count, failed_tests, found_tests,
> +						    is_builtin) == -1)
> +					return -1;
> +				break;
>  			}
> -			found_tests = true;
> -			sublevel++;
> -			break;
>  		}
>  
>  		switch (parse_kmsg_for_tap(fp, record, test_name)) {
>  		case -2:
> -			return IGT_EXIT_FAILURE;
> +			return -1;
>  		case -1:
> -			sublevel--;
> -			failed_tests = true;
> -			igt_subtest(test_name)
> -				igt_fail(IGT_EXIT_FAILURE);
> +			*failed_tests = true;
> +
> +			r = malloc(sizeof(*r));
> +
> +			memcpy(r->test_name, base_test_name, BUF_LEN);
> +			if (strlen(r->test_name) < BUF_LEN - 1)
> +				if (r->test_name[0])
> +					strncat(r->test_name, DELIMITER,
> +						BUF_LEN - strlen(r->test_name));
> +			memcpy(r->test_name + strlen(r->test_name), test_name,
> +			       BUF_LEN - strlen(r->test_name));
> +			r->test_name[BUF_LEN] = '\0';
> +
> +			r->passed = false;
> +			r->next = NULL;
> +
> +			pthread_mutex_lock(&results.mutex);
> +			if (results.head == NULL) {
> +				results.head = r;
> +			} else {
> +				temp = results.head;
> +				while (temp->next != NULL)
> +					temp = temp->next;
> +				temp->next = r;
> +			}
> +			pthread_mutex_unlock(&results.mutex);
> +
>  			test_name[0] = '\0';
>  			break;
> -		case 0: /* fallthrough */
> -			igt_subtest(test_name)
> -				igt_success();
> +		case 0:
> +			r = malloc(sizeof(*r));
> +
> +			memcpy(r->test_name, base_test_name, BUF_LEN);
> +			if (strlen(r->test_name) < BUF_LEN - 1)
> +				if (r->test_name[0])
> +					strncat(r->test_name, DELIMITER,
> +						BUF_LEN - strlen(r->test_name));
> +			memcpy(r->test_name + strlen(r->test_name), test_name,
> +			       BUF_LEN - strlen(r->test_name));
> +			r->test_name[BUF_LEN] = '\0';
> +
> +			r->passed = true;
> +			r->next = NULL;
> +
> +			pthread_mutex_lock(&results.mutex);
> +			if (results.head == NULL) {
> +				results.head = r;
> +			} else {
> +				temp = results.head;
> +				while (temp->next != NULL)
> +					temp = temp->next;
> +				temp->next = r;
> +			}
> +			pthread_mutex_unlock(&results.mutex);
> +
>  			test_name[0] = '\0';
> +			break;
>  		default:
>  			break;
>  		}
>  	}
> +	return 0;
> +}
> +
> +/**
> + * igt_ktap_parser:
> + *
> + * This function parses the output of a ktap script and passes it to main thread.
> + */
> +void *igt_ktap_parser(void *unused)
> +{
> +	FILE *fp = ktap_args.fp;
> +	int fp_fd = fileno(fp);
> +	char record[BUF_LEN + 1];
> +	bool is_builtin = ktap_args.is_builtin;
> +	char test_name[BUF_LEN + 1];
> +	bool failed_tests, found_tests;
> +	int test_count;
> +
> +	failed_tests = false;
> +	found_tests = false;
> +
> +	if (!READ_ONCE(ktap_args.is_running))
> +		goto igt_ktap_parser_end;
> +
> +igt_ktap_parser_start:
> +	test_name[0] = '\0';
> +	test_name[BUF_LEN] = '\0';
> +
> +	while (read(fp_fd, record, BUF_LEN) < 0) {
> +		if (!READ_ONCE(ktap_args.is_running)) {
> +			igt_warn("ktap parser stopped\n");
> +			goto igt_ktap_parser_end;
> +		}
> +
> +		if (errno == EAGAIN)
> +			/* No records available */
> +			continue;
> +
> +		if (errno == EINTR)
> +			continue;
> +
> +		if (errno == EPIPE) {
> +			igt_warn("kmsg truncated: too many messages. You may want to increase log_buf_len in kmcdline\n");
> +			goto igt_ktap_parser_end;
> +		}
> +	}
> +
> +	test_count = find_next_tap_subtest(fp, record, test_name, is_builtin);
> +
> +	switch (test_count) {
> +	case -2:
> +		/* Problems while reading the file */
> +		goto igt_ktap_parser_end;
> +	case -1:
> +		/* No test found */
> +		goto igt_ktap_parser_start;
> +	case 0:
> +		/* Tests found, but they're missing info */
> +		found_tests = true;
> +		goto igt_ktap_parser_end;
> +	default:
> +		found_tests = true;
> +
> +		if (parse_tap_level(fp, test_name, test_count, &failed_tests, &found_tests,
> +				    is_builtin) == -1)
> +			goto igt_ktap_parser_end;
> +
> +		break;
> +	}
> +
> +	/* Parse topmost level */
> +	test_name[0] = '\0';
> +	parse_tap_level(fp, test_name, test_count, &failed_tests, &found_tests, is_builtin);
> +
> +igt_ktap_parser_end:
> +	results.still_running = false;
>  
>  	if (failed_tests || !found_tests)
> -		return IGT_EXIT_FAILURE;
> +		ktap_args.ret = IGT_EXIT_FAILURE;
> +	else
> +		ktap_args.ret = IGT_EXIT_SUCCESS;
> +
> +	return NULL;
> +}
> +
> +static pthread_t ktap_parser_thread;
> +
> +struct ktap_test_results *ktap_parser_start(FILE *fp, bool is_builtin)
> +{
> +	results.head = NULL;
> +	pthread_mutex_init(&results.mutex, NULL);
> +	results.still_running = true;
>  
> -	return IGT_EXIT_SUCCESS;
> +	ktap_args.fp = fp;
> +	ktap_args.is_builtin = is_builtin;
> +	ktap_args.is_running = true;
> +	pthread_create(&ktap_parser_thread, NULL, igt_ktap_parser, NULL);
> +
> +	return &results;
> +}
> +
> +int ktap_parser_stop(void)
> +{
> +	ktap_args.is_running = false;
> +	pthread_join(ktap_parser_thread, NULL);
> +	return ktap_args.ret;
>  }
> diff --git a/lib/igt_ktap.h b/lib/igt_ktap.h
> index b2f69df2d..34fe09572 100644
> --- a/lib/igt_ktap.h
> +++ b/lib/igt_ktap.h
> @@ -26,6 +26,25 @@
>  
>  #define BUF_LEN 4096
>  
> -int igt_ktap_parser(FILE *fp, char *record, bool is_builtin);
> +#include <pthread.h>
> +
> +void *igt_ktap_parser(void *unused);
> +
> +typedef struct ktap_test_results_element {
> +	char test_name[BUF_LEN + 1];
> +	bool passed;
> +	struct ktap_test_results_element *next;
> +} ktap_test_results_element;
> +
> +struct ktap_test_results {
> +	ktap_test_results_element *head;
> +	pthread_mutex_t mutex;
> +	bool still_running;
> +};
> +
> +
> +
> +struct ktap_test_results *ktap_parser_start(FILE *fp, bool is_builtin);
> +int ktap_parser_stop(void);
>  
>  #endif /* IGT_KTAP_H */

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 10/10] KUnit: replace abort with graceful skip
  2023-06-13  8:41   ` Mauro Carvalho Chehab
@ 2023-06-13  9:11     ` Janusz Krzysztofik
  2023-06-13  9:58       ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 22+ messages in thread
From: Janusz Krzysztofik @ 2023-06-13  9:11 UTC (permalink / raw)
  To: Dominik Karol Piatkowski, Mauro Carvalho Chehab; +Cc: igt-dev

On Tuesday, 13 June 2023 10:41:03 CEST Mauro Carvalho Chehab wrote:
> On Tue, 13 Jun 2023 09:27:26 +0200
> Dominik Karol Piatkowski <dominik.karol.piatkowski@intel.com> wrote:
> 
> > Sample drm_buddy output with missing requirements:
> > 	Starting subtest: drm_buddy_test
> > 	(drm_buddy:32218) igt_kmod-WARNING: Unable to load KUnit
> > 	Subtest drm_buddy_test: SKIP (0.001s)
> > 
> > Signed-off-by: Dominik Karol Piątkowski 
<dominik.karol.piatkowski@intel.com>
> > Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > Cc: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>
> > ---
> >  lib/igt_kmod.c | 20 +++++++++++++-------
> >  1 file changed, 13 insertions(+), 7 deletions(-)
> > 
> > diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> > index 1511bdef4..04220f404 100644
> > --- a/lib/igt_kmod.c
> > +++ b/lib/igt_kmod.c
> > @@ -763,27 +763,27 @@ static void __igt_kunit(const char *module_name, 
const char *opts)
> >  	int ret;
> >  	struct ktap_test_results *results;
> >  	struct ktap_test_results_element *temp;
> > +	bool skip = false;
> >  
> >  	/* get normalized module name */
> >  	if (igt_ktest_init(&tst, module_name) != 0) {
> > -		igt_warn("Unable to initialize ktest for %s\n", 
module_name);
> > -		igt_fail(IGT_EXIT_SKIP);
> > +		igt_skip("Unable to initialize ktest for %s\n", 
module_name);
> >  	}
> >  
> >  	if (igt_ktest_begin(&tst) != 0) {
> > -		igt_warn("Unable to begin ktest for %s\n", module_name);
> > -
> >  		igt_ktest_fini(&tst);
> > -		igt_fail(IGT_EXIT_SKIP);
> > +		igt_skip("Unable to begin ktest for %s\n", module_name);
> >  	}
> >  
> >  	if (tst.kmsg < 0) {
> >  		igt_warn("Could not open /dev/kmsg\n");
> > +		skip = true;
> 
> Hmm.... not being able to read from /dev/kmsg doesn't seem a reason for
> skipping the test.
> 
> IMO, the test should be skipped only if:
> 
> 1. The Kernel is not compiled with KUnit support;
> 2. The KUnit test was not compiled.
> 
> Errors for all other conditions should be considered as failures.
> 
> Now, I'm not sure what would be the proper way to test for such
> skip condition. I suspect you could check the error code at
> igt_kmod_load(), calling igt_skip() if it returns -ENOENT,
> e. g. on this part of __igt_kunit():
> 
>         /* The KUnit module is required for running any KUnit tests */
>         if (igt_kmod_load("kunit", NULL) != 0 ||
>             kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) != 
0) {
>                 igt_warn("Unable to load KUnit\n");
>                 igt_fail(IGT_EXIT_FAILURE);
>         }
> 
>         is_builtin = kmod_module_get_initstate(kunit_kmod) == 
KMOD_MODULE_BUILTIN;
> 
>         results = ktap_parser_start(f, is_builtin);
> 
>         if (igt_kmod_load(module_name, opts) != 0) {
>                 igt_warn("Unable to load %s module\n", module_name);
>                 ret = ktap_parser_stop();
>                 igt_fail(IGT_EXIT_FAILURE);
>         }
> 
> 
> This could be replaced by something similar to:
> 
> 	err = igt_kmod_load("kunit", NULL);
> 	igt_skip_on(err == ENOENT, "KUnit not supported");
> 	igt_fail_on_f(err != 0, "Error %d when trying to load kunit", err);
>         if (kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) != 
0) {
>                 igt_warn("Unable to load KUnit\n");
>                 igt_fail(IGT_EXIT_FAILURE);
>         }
> 
> 
>         is_builtin = kmod_module_get_initstate(kunit_kmod) == 
KMOD_MODULE_BUILTIN;
> 
>         results = ktap_parser_start(f, is_builtin);
> 
>         err = igt_kmod_load(module_name, opts) 
> 	igt_skip_on(err == ENOENT, "Module %s not found", module_name);
> 	if (err) {
>                 igt_warn("Error %d when trying to load %s module\n", err, 
module_name);
>                 ret = ktap_parser_stop();
>                 igt_fail(IGT_EXIT_FAILURE);
>         }
> 
> 
> In order to explicitly check for (1) and (2).

Why do we reinvent the wheel instead of following the pattern from 
igt_kselftest()?  We used to follow it, before the whole igt_kunit() body was 
put under igt_subtest_with_dynamic() (patch 6/8 of v5, then squashed with 5/5, 
IIRC).  That forced us to introduce a series of changes inside igt_kunit(), 
which we now contest.  Was that really necessary?

Thanks,
Janusz

> 
> Regards,
> Mauro
> 






^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 10/10] KUnit: replace abort with graceful skip
  2023-06-13  9:11     ` Janusz Krzysztofik
@ 2023-06-13  9:58       ` Mauro Carvalho Chehab
  2023-06-13 11:15         ` Janusz Krzysztofik
  0 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2023-06-13  9:58 UTC (permalink / raw)
  To: Janusz Krzysztofik; +Cc: igt-dev

On Tue, 13 Jun 2023 11:11:00 +0200
Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> wrote:

> On Tuesday, 13 June 2023 10:41:03 CEST Mauro Carvalho Chehab wrote:
> > On Tue, 13 Jun 2023 09:27:26 +0200
> > Dominik Karol Piatkowski <dominik.karol.piatkowski@intel.com> wrote:
> >   
> > > Sample drm_buddy output with missing requirements:
> > > 	Starting subtest: drm_buddy_test
> > > 	(drm_buddy:32218) igt_kmod-WARNING: Unable to load KUnit
> > > 	Subtest drm_buddy_test: SKIP (0.001s)
> > > 
> > > Signed-off-by: Dominik Karol Piątkowski   
> <dominik.karol.piatkowski@intel.com>
> > > Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > > Cc: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>
> > > ---
> > >  lib/igt_kmod.c | 20 +++++++++++++-------
> > >  1 file changed, 13 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> > > index 1511bdef4..04220f404 100644
> > > --- a/lib/igt_kmod.c
> > > +++ b/lib/igt_kmod.c
> > > @@ -763,27 +763,27 @@ static void __igt_kunit(const char *module_name,   
> const char *opts)
> > >  	int ret;
> > >  	struct ktap_test_results *results;
> > >  	struct ktap_test_results_element *temp;
> > > +	bool skip = false;
> > >  
> > >  	/* get normalized module name */
> > >  	if (igt_ktest_init(&tst, module_name) != 0) {
> > > -		igt_warn("Unable to initialize ktest for %s\n",   
> module_name);
> > > -		igt_fail(IGT_EXIT_SKIP);
> > > +		igt_skip("Unable to initialize ktest for %s\n",   
> module_name);
> > >  	}
> > >  
> > >  	if (igt_ktest_begin(&tst) != 0) {
> > > -		igt_warn("Unable to begin ktest for %s\n", module_name);
> > > -
> > >  		igt_ktest_fini(&tst);
> > > -		igt_fail(IGT_EXIT_SKIP);
> > > +		igt_skip("Unable to begin ktest for %s\n", module_name);
> > >  	}
> > >  
> > >  	if (tst.kmsg < 0) {
> > >  		igt_warn("Could not open /dev/kmsg\n");
> > > +		skip = true;  
> > 
> > Hmm.... not being able to read from /dev/kmsg doesn't seem a reason for
> > skipping the test.
> > 
> > IMO, the test should be skipped only if:
> > 
> > 1. The Kernel is not compiled with KUnit support;
> > 2. The KUnit test was not compiled.
> > 
> > Errors for all other conditions should be considered as failures.
> > 
> > Now, I'm not sure what would be the proper way to test for such
> > skip condition. I suspect you could check the error code at
> > igt_kmod_load(), calling igt_skip() if it returns -ENOENT,
> > e. g. on this part of __igt_kunit():
> > 
> >         /* The KUnit module is required for running any KUnit tests */
> >         if (igt_kmod_load("kunit", NULL) != 0 ||
> >             kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) !=   
> 0) {
> >                 igt_warn("Unable to load KUnit\n");
> >                 igt_fail(IGT_EXIT_FAILURE);
> >         }
> > 
> >         is_builtin = kmod_module_get_initstate(kunit_kmod) ==   
> KMOD_MODULE_BUILTIN;
> > 
> >         results = ktap_parser_start(f, is_builtin);
> > 
> >         if (igt_kmod_load(module_name, opts) != 0) {
> >                 igt_warn("Unable to load %s module\n", module_name);
> >                 ret = ktap_parser_stop();
> >                 igt_fail(IGT_EXIT_FAILURE);
> >         }
> > 
> > 
> > This could be replaced by something similar to:
> > 
> > 	err = igt_kmod_load("kunit", NULL);
> > 	igt_skip_on(err == ENOENT, "KUnit not supported");
> > 	igt_fail_on_f(err != 0, "Error %d when trying to load kunit", err);
> >         if (kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) !=   
> 0) {
> >                 igt_warn("Unable to load KUnit\n");
> >                 igt_fail(IGT_EXIT_FAILURE);
> >         }
> > 
> > 
> >         is_builtin = kmod_module_get_initstate(kunit_kmod) ==   
> KMOD_MODULE_BUILTIN;
> > 
> >         results = ktap_parser_start(f, is_builtin);
> > 
> >         err = igt_kmod_load(module_name, opts) 
> > 	igt_skip_on(err == ENOENT, "Module %s not found", module_name);
> > 	if (err) {
> >                 igt_warn("Error %d when trying to load %s module\n", err,   
> module_name);
> >                 ret = ktap_parser_stop();
> >                 igt_fail(IGT_EXIT_FAILURE);
> >         }
> > 
> > 
> > In order to explicitly check for (1) and (2).  
> 
> Why do we reinvent the wheel instead of following the pattern from 
> igt_kselftest()? 

Nobody is talking about reinventing the wheel. Selftests and KUnit tests
are executed on different ways.

For selftests, the same driver that was loaded before needs to be unloaded
and re-probed with a different argument.

For KUnit tests, a different module will be loaded, if the Kernel was
compiled with KUnit support for a given test suite.

Looking at KUnit modprobe time, if a module doesn't load because the file
doesn't exist (-ENOENT error), it means that:

- for KUnit: the KUnit module was not compiled.
- for selftests: this is really unlikely to happen, as all other
  previous IGT tests depend on having the driver module loaded.
  So, no need for any explicit check for this specific condition.

> We used to follow it, before the whole igt_kunit() body was 
> put under igt_subtest_with_dynamic() (patch 6/8 of v5, then squashed with 5/5, 
> IIRC).  That forced us to introduce a series of changes inside igt_kunit(), 
> which we now contest.  Was that really necessary?

This has nothing to do with using (or not) a dynamic IGT subtest.

It is related to how to report IGT errors while trying to run KUnit
tests.

Without this patch, every kind of errors will produce a failure;
after this patch, they'll all produce a skip.

IMO, what it is needed is to split KUnit errors on two different
categories:

1. "real" errors while running KUnit (like troubles reading dmesg);
2. the KUnit module to be modprobed doesn't exist.

for (1), igt_fail() is the proper error handling.

For (2), igt_skip() is the right error handling, as the Kernel under
tests doesn't have what it is needed to run KUnit, as the module was
not compiled.

Regards,
Mauro


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [igt-dev] ✓ Fi.CI.IGT: success for Introduce KUnit (rev7)
  2023-06-13  7:27 [igt-dev] [PATCH v7 i-g-t 00/10] Introduce KUnit Dominik Karol Piatkowski
                   ` (10 preceding siblings ...)
  2023-06-13  8:20 ` [igt-dev] ✓ Fi.CI.BAT: success for Introduce KUnit (rev7) Patchwork
@ 2023-06-13 11:13 ` Patchwork
  11 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2023-06-13 11:13 UTC (permalink / raw)
  To: Dominik Karol Piatkowski; +Cc: igt-dev

[-- Attachment #1: Type: text/plain, Size: 46784 bytes --]

== Series Details ==

Series: Introduce KUnit (rev7)
URL   : https://patchwork.freedesktop.org/series/114612/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_13264_full -> IGTPW_9155_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with IGTPW_9155_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_9155_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/index.html

Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_9155_full:

### IGT changes ###

#### Possible regressions ####

  * {igt@drm_mm@drm_mm_test} (NEW):
    - {shard-dg1}:        NOTRUN -> [SKIP][1] +8 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-dg1-19/igt@drm_mm@drm_mm_test.html

  * {igt@kms_selftest@drm_dp_mst} (NEW):
    - shard-tglu:         NOTRUN -> [SKIP][2] +7 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-9/igt@kms_selftest@drm_dp_mst.html

  * {igt@kms_selftest@framebuffer} (NEW):
    - shard-rkl:          NOTRUN -> [SKIP][3] +6 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-2/igt@kms_selftest@framebuffer.html

  
#### Warnings ####

  * igt@kms_plane_alpha_blend@alpha-opaque-fb:
    - shard-snb:          [SKIP][4] ([fdo#109271] / [i915#4579]) -> [INCOMPLETE][5]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-snb6/igt@kms_plane_alpha_blend@alpha-opaque-fb.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-snb1/igt@kms_plane_alpha_blend@alpha-opaque-fb.html

  
New tests
---------

  New tests have been introduced between CI_DRM_13264_full and IGTPW_9155_full:

### New IGT tests (55) ###

  * igt@drm_buddy@drm_buddy_test:
    - Statuses : 6 skip(s)
    - Exec time: [0.0] s

  * igt@drm_mm@drm_mm_test:
    - Statuses : 5 skip(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@alternate-sync-async-flip@pipe-a-hdmi-a-1:
    - Statuses : 2 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@alternate-sync-async-flip@pipe-a-vga-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@alternate-sync-async-flip@pipe-b-hdmi-a-1:
    - Statuses : 2 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@alternate-sync-async-flip@pipe-b-vga-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@alternate-sync-async-flip@pipe-c-hdmi-a-1:
    - Statuses : 2 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@alternate-sync-async-flip@pipe-d-hdmi-a-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@crc@pipe-a-hdmi-a-1:
    - Statuses : 2 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@crc@pipe-b-hdmi-a-1:
    - Statuses : 2 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@crc@pipe-c-hdmi-a-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@invalid-async-flip@pipe-a-dp-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@invalid-async-flip@pipe-a-hdmi-a-1:
    - Statuses : 2 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@invalid-async-flip@pipe-b-dp-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@invalid-async-flip@pipe-b-hdmi-a-1:
    - Statuses : 2 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@invalid-async-flip@pipe-c-dp-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@invalid-async-flip@pipe-c-hdmi-a-1:
    - Statuses : 2 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@invalid-async-flip@pipe-d-hdmi-a-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@test-cursor@pipe-a-dp-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@test-cursor@pipe-a-hdmi-a-1:
    - Statuses : 4 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@test-cursor@pipe-b-dp-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@test-cursor@pipe-b-hdmi-a-1:
    - Statuses : 4 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@test-cursor@pipe-c-dp-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@test-cursor@pipe-c-hdmi-a-1:
    - Statuses : 2 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@test-cursor@pipe-d-hdmi-a-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@test-time-stamp@pipe-a-hdmi-a-1:
    - Statuses : 2 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@test-time-stamp@pipe-a-vga-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@test-time-stamp@pipe-b-hdmi-a-1:
    - Statuses : 2 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@test-time-stamp@pipe-b-vga-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@test-time-stamp@pipe-c-hdmi-a-1:
    - Statuses : 2 pass(s)
    - Exec time: [0.0] s

  * igt@kms_async_flips@test-time-stamp@pipe-d-hdmi-a-1:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_flip@single-buffer-flip-vs-dpms-off-vs-modeset-interruptible@d-hdmi-a3:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@plane-downscale-with-modifiers-factor-0-5@pipe-a-hdmi-a-3:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@plane-downscale-with-modifiers-factor-0-5@pipe-b-hdmi-a-3:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@plane-downscale-with-modifiers-factor-0-5@pipe-c-hdmi-a-3:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@plane-downscale-with-modifiers-factor-0-5@pipe-d-hdmi-a-3:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-a-hdmi-a-3:
    - Statuses : 1 skip(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-b-hdmi-a-3:
    - Statuses : 1 skip(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-c-hdmi-a-3:
    - Statuses : 1 skip(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-d-hdmi-a-3:
    - Statuses : 1 skip(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@plane-upscale-with-modifiers-factor-0-25@pipe-a-hdmi-a-3:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@plane-upscale-with-modifiers-factor-0-25@pipe-b-hdmi-a-3:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@plane-upscale-with-modifiers-factor-0-25@pipe-c-hdmi-a-3:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@plane-upscale-with-modifiers-factor-0-25@pipe-d-hdmi-a-3:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@planes-downscale-factor-0-5@pipe-a-hdmi-a-3:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@planes-downscale-factor-0-5@pipe-b-hdmi-a-3:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@planes-downscale-factor-0-5@pipe-c-hdmi-a-3:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_plane_scaling@planes-downscale-factor-0-5@pipe-d-hdmi-a-3:
    - Statuses : 1 pass(s)
    - Exec time: [0.0] s

  * igt@kms_selftest@drm_cmdline:
    - Statuses : 6 skip(s)
    - Exec time: [0.0] s

  * igt@kms_selftest@drm_damage:
    - Statuses : 6 skip(s)
    - Exec time: [0.0] s

  * igt@kms_selftest@drm_dp_mst:
    - Statuses : 5 skip(s)
    - Exec time: [0.0] s

  * igt@kms_selftest@drm_format:
    - Statuses : 5 skip(s)
    - Exec time: [0.0] s

  * igt@kms_selftest@drm_format_helper:
    - Statuses : 6 skip(s)
    - Exec time: [0.0] s

  * igt@kms_selftest@drm_plane:
    - Statuses : 6 skip(s)
    - Exec time: [0.0] s

  * igt@kms_selftest@framebuffer:
    - Statuses : 5 skip(s)
    - Exec time: [0.0] s

  

Known issues
------------

  Here are the changes found in IGTPW_9155_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * {igt@drm_buddy@drm_buddy_test} (NEW):
    - shard-snb:          NOTRUN -> [SKIP][6] ([fdo#109271]) +26 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-snb4/igt@drm_buddy@drm_buddy_test.html

  * igt@drm_fdinfo@most-busy-check-all@rcs0:
    - shard-rkl:          [PASS][7] -> [FAIL][8] ([i915#7742])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-rkl-7/igt@drm_fdinfo@most-busy-check-all@rcs0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-7/igt@drm_fdinfo@most-busy-check-all@rcs0.html

  * {igt@drm_mm@drm_mm_test} (NEW):
    - shard-apl:          NOTRUN -> [SKIP][9] ([fdo#109271]) +34 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-apl4/igt@drm_mm@drm_mm_test.html

  * igt@feature_discovery@psr1:
    - shard-tglu:         NOTRUN -> [SKIP][10] ([i915#658]) +1 similar issue
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-2/igt@feature_discovery@psr1.html

  * igt@feature_discovery@psr2:
    - shard-rkl:          NOTRUN -> [SKIP][11] ([i915#658])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@feature_discovery@psr2.html

  * igt@gem_ccs@ctrl-surf-copy-new-ctx:
    - shard-rkl:          NOTRUN -> [SKIP][12] ([i915#4098] / [i915#4579] / [i915#5325])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@gem_ccs@ctrl-surf-copy-new-ctx.html

  * igt@gem_ccs@suspend-resume:
    - shard-tglu:         NOTRUN -> [SKIP][13] ([i915#4579] / [i915#5325])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-2/igt@gem_ccs@suspend-resume.html

  * igt@gem_ctx_exec@basic-nohangcheck:
    - shard-rkl:          NOTRUN -> [FAIL][14] ([i915#6268])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@gem_ctx_exec@basic-nohangcheck.html

  * igt@gem_exec_fair@basic-none-rrul@rcs0:
    - shard-rkl:          NOTRUN -> [FAIL][15] ([i915#2842])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@gem_exec_fair@basic-none-rrul@rcs0.html

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - shard-tglu:         NOTRUN -> [FAIL][16] ([i915#2842]) +1 similar issue
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-5/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_exec_fair@basic-none-solo@rcs0:
    - shard-apl:          [PASS][17] -> [FAIL][18] ([i915#2842])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-apl7/igt@gem_exec_fair@basic-none-solo@rcs0.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-apl7/igt@gem_exec_fair@basic-none-solo@rcs0.html

  * igt@gem_exec_fair@basic-pace@rcs0:
    - shard-rkl:          [PASS][19] -> [FAIL][20] ([i915#2842]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-rkl-2/igt@gem_exec_fair@basic-pace@rcs0.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@gem_exec_fair@basic-pace@rcs0.html

  * igt@gem_exec_flush@basic-batch-kernel-default-cmd:
    - shard-tglu:         NOTRUN -> [SKIP][21] ([fdo#109313])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-10/igt@gem_exec_flush@basic-batch-kernel-default-cmd.html

  * igt@gem_exec_params@secure-non-root:
    - shard-rkl:          NOTRUN -> [SKIP][22] ([fdo#112283])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-1/igt@gem_exec_params@secure-non-root.html

  * igt@gem_exec_reloc@basic-wc-cpu:
    - shard-rkl:          NOTRUN -> [SKIP][23] ([i915#3281]) +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-7/igt@gem_exec_reloc@basic-wc-cpu.html

  * igt@gem_lmem_swapping@smem-oom:
    - shard-rkl:          NOTRUN -> [SKIP][24] ([i915#4613])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@gem_lmem_swapping@smem-oom.html

  * igt@gem_pread@uncached:
    - shard-rkl:          NOTRUN -> [SKIP][25] ([i915#3282]) +2 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@gem_pread@uncached.html

  * igt@gem_pxp@create-valid-protected-context:
    - shard-tglu:         NOTRUN -> [SKIP][26] ([i915#4270]) +1 similar issue
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-8/igt@gem_pxp@create-valid-protected-context.html

  * igt@gem_pxp@verify-pxp-execution-after-suspend-resume:
    - shard-rkl:          NOTRUN -> [SKIP][27] ([i915#4270]) +1 similar issue
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-7/igt@gem_pxp@verify-pxp-execution-after-suspend-resume.html

  * igt@gem_softpin@evict-snoop:
    - shard-tglu:         NOTRUN -> [SKIP][28] ([fdo#109312])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-7/igt@gem_softpin@evict-snoop.html

  * igt@gem_userptr_blits@unsync-unmap-cycles:
    - shard-rkl:          NOTRUN -> [SKIP][29] ([i915#3297])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@gem_userptr_blits@unsync-unmap-cycles.html
    - shard-tglu:         NOTRUN -> [SKIP][30] ([i915#3297])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-8/igt@gem_userptr_blits@unsync-unmap-cycles.html

  * igt@gen9_exec_parse@basic-rejected-ctx-param:
    - shard-rkl:          NOTRUN -> [SKIP][31] ([i915#2527])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@gen9_exec_parse@basic-rejected-ctx-param.html

  * igt@gen9_exec_parse@bb-large:
    - shard-tglu:         NOTRUN -> [SKIP][32] ([i915#2527] / [i915#2856]) +1 similar issue
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-6/igt@gen9_exec_parse@bb-large.html

  * igt@i915_pm_backlight@fade-with-suspend:
    - shard-rkl:          NOTRUN -> [SKIP][33] ([i915#7561])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-7/igt@i915_pm_backlight@fade-with-suspend.html

  * igt@i915_pm_dc@dc9-dpms:
    - shard-apl:          [PASS][34] -> [SKIP][35] ([fdo#109271])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-apl6/igt@i915_pm_dc@dc9-dpms.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-apl6/igt@i915_pm_dc@dc9-dpms.html

  * igt@i915_pm_freq_api@freq-reset:
    - shard-tglu:         NOTRUN -> [SKIP][36] ([i915#8399])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-4/igt@i915_pm_freq_api@freq-reset.html

  * igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a:
    - shard-rkl:          [PASS][37] -> [SKIP][38] ([i915#1937] / [i915#4579])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-rkl-7/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-1/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-hdmi-a.html

  * igt@i915_pm_rpm@gem-execbuf-stress-pc8:
    - shard-glk:          NOTRUN -> [SKIP][39] ([fdo#109271]) +41 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-glk6/igt@i915_pm_rpm@gem-execbuf-stress-pc8.html
    - shard-rkl:          NOTRUN -> [SKIP][40] ([fdo#109506])
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-4/igt@i915_pm_rpm@gem-execbuf-stress-pc8.html
    - shard-tglu:         NOTRUN -> [SKIP][41] ([fdo#109506])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-10/igt@i915_pm_rpm@gem-execbuf-stress-pc8.html

  * igt@i915_pm_rps@reset:
    - shard-snb:          [PASS][42] -> [DMESG-FAIL][43] ([i915#8319])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-snb6/igt@i915_pm_rps@reset.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-snb2/igt@i915_pm_rps@reset.html

  * igt@kms_async_flips@async-flip-with-page-flip-events@pipe-b-hdmi-a-1-y-rc_ccs:
    - shard-rkl:          NOTRUN -> [SKIP][44] ([i915#8502]) +3 similar issues
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-7/igt@kms_async_flips@async-flip-with-page-flip-events@pipe-b-hdmi-a-1-y-rc_ccs.html

  * igt@kms_big_fb@4-tiled-16bpp-rotate-0:
    - shard-rkl:          NOTRUN -> [SKIP][45] ([i915#5286])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-6/igt@kms_big_fb@4-tiled-16bpp-rotate-0.html

  * igt@kms_big_fb@linear-64bpp-rotate-90:
    - shard-rkl:          NOTRUN -> [SKIP][46] ([fdo#111614] / [i915#3638])
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@kms_big_fb@linear-64bpp-rotate-90.html
    - shard-tglu:         NOTRUN -> [SKIP][47] ([fdo#111614])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-2/igt@kms_big_fb@linear-64bpp-rotate-90.html

  * igt@kms_big_fb@yf-tiled-addfb:
    - shard-tglu:         NOTRUN -> [SKIP][48] ([fdo#111615]) +3 similar issues
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-9/igt@kms_big_fb@yf-tiled-addfb.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-0-hflip:
    - shard-rkl:          NOTRUN -> [SKIP][49] ([fdo#110723]) +1 similar issue
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-0-hflip.html

  * igt@kms_ccs@pipe-b-crc-primary-basic-4_tiled_dg2_mc_ccs:
    - shard-tglu:         NOTRUN -> [SKIP][50] ([i915#5354] / [i915#6095]) +7 similar issues
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-2/igt@kms_ccs@pipe-b-crc-primary-basic-4_tiled_dg2_mc_ccs.html

  * igt@kms_ccs@pipe-b-crc-primary-basic-4_tiled_mtl_mc_ccs:
    - shard-rkl:          NOTRUN -> [SKIP][51] ([i915#5354] / [i915#6095]) +8 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-1/igt@kms_ccs@pipe-b-crc-primary-basic-4_tiled_mtl_mc_ccs.html

  * igt@kms_ccs@pipe-b-crc-primary-basic-y_tiled_gen12_mc_ccs:
    - shard-rkl:          NOTRUN -> [SKIP][52] ([i915#3886] / [i915#5354] / [i915#6095])
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-6/igt@kms_ccs@pipe-b-crc-primary-basic-y_tiled_gen12_mc_ccs.html
    - shard-apl:          NOTRUN -> [SKIP][53] ([fdo#109271] / [i915#3886])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-apl3/igt@kms_ccs@pipe-b-crc-primary-basic-y_tiled_gen12_mc_ccs.html
    - shard-tglu:         NOTRUN -> [SKIP][54] ([i915#3689] / [i915#3886] / [i915#5354] / [i915#6095])
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-7/igt@kms_ccs@pipe-b-crc-primary-basic-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-d-bad-pixel-format-y_tiled_gen12_mc_ccs:
    - shard-tglu:         NOTRUN -> [SKIP][55] ([i915#3689] / [i915#5354] / [i915#6095]) +1 similar issue
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-7/igt@kms_ccs@pipe-d-bad-pixel-format-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-d-ccs-on-another-bo-y_tiled_gen12_rc_ccs_cc:
    - shard-rkl:          NOTRUN -> [SKIP][56] ([i915#5354]) +8 similar issues
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-4/igt@kms_ccs@pipe-d-ccs-on-another-bo-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-d-missing-ccs-buffer-yf_tiled_ccs:
    - shard-tglu:         NOTRUN -> [SKIP][57] ([fdo#111615] / [i915#3689] / [i915#5354] / [i915#6095])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-10/igt@kms_ccs@pipe-d-missing-ccs-buffer-yf_tiled_ccs.html

  * igt@kms_cdclk@mode-transition-all-outputs:
    - shard-rkl:          NOTRUN -> [SKIP][58] ([i915#3742])
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-2/igt@kms_cdclk@mode-transition-all-outputs.html

  * igt@kms_chamelium_color@ctm-negative:
    - shard-rkl:          NOTRUN -> [SKIP][59] ([fdo#111827])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@kms_chamelium_color@ctm-negative.html

  * igt@kms_chamelium_frames@dp-crc-multiple:
    - shard-tglu:         NOTRUN -> [SKIP][60] ([i915#7828]) +1 similar issue
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-5/igt@kms_chamelium_frames@dp-crc-multiple.html

  * igt@kms_chamelium_hpd@vga-hpd-for-each-pipe:
    - shard-rkl:          NOTRUN -> [SKIP][61] ([i915#7828]) +1 similar issue
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@kms_chamelium_hpd@vga-hpd-for-each-pipe.html

  * igt@kms_content_protection@dp-mst-lic-type-1:
    - shard-rkl:          NOTRUN -> [SKIP][62] ([i915#3116])
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@kms_content_protection@dp-mst-lic-type-1.html

  * igt@kms_cursor_crc@cursor-suspend@pipe-a-dp-1:
    - shard-apl:          [PASS][63] -> [ABORT][64] ([i915#180])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-apl3/igt@kms_cursor_crc@cursor-suspend@pipe-a-dp-1.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-apl3/igt@kms_cursor_crc@cursor-suspend@pipe-a-dp-1.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
    - shard-tglu:         NOTRUN -> [SKIP][65] ([fdo#109274])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-7/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - shard-rkl:          NOTRUN -> [SKIP][66] ([i915#4103])
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-1/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-apl:          [PASS][67] -> [FAIL][68] ([i915#2346])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-apl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-apl2/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_cursor_legacy@single-move@pipe-b:
    - shard-rkl:          [PASS][69] -> [INCOMPLETE][70] ([i915#8011]) +1 similar issue
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-rkl-2/igt@kms_cursor_legacy@single-move@pipe-b.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-7/igt@kms_cursor_legacy@single-move@pipe-b.html

  * igt@kms_dither@fb-8bpc-vs-panel-6bpc@pipe-a-hdmi-a-2:
    - shard-rkl:          NOTRUN -> [SKIP][71] ([i915#3804] / [i915#4579])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-2/igt@kms_dither@fb-8bpc-vs-panel-6bpc@pipe-a-hdmi-a-2.html

  * igt@kms_flip@2x-flip-vs-dpms:
    - shard-rkl:          NOTRUN -> [SKIP][72] ([fdo#111825]) +7 similar issues
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-7/igt@kms_flip@2x-flip-vs-dpms.html
    - shard-tglu:         NOTRUN -> [SKIP][73] ([fdo#109274] / [i915#3637]) +2 similar issues
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-5/igt@kms_flip@2x-flip-vs-dpms.html

  * igt@kms_flip@flip-vs-suspend-interruptible@a-hdmi-a1:
    - shard-tglu:         [PASS][74] -> [ABORT][75] ([i915#5122])
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-tglu-6/igt@kms_flip@flip-vs-suspend-interruptible@a-hdmi-a1.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-6/igt@kms_flip@flip-vs-suspend-interruptible@a-hdmi-a1.html

  * igt@kms_flip@flip-vs-suspend@a-vga1:
    - shard-snb:          [PASS][76] -> [DMESG-WARN][77] ([i915#5090])
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-snb2/igt@kms_flip@flip-vs-suspend@a-vga1.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-snb6/igt@kms_flip@flip-vs-suspend@a-vga1.html

  * igt@kms_flip_scaled_crc@flip-64bpp-4tile-to-32bpp-4tiledg2rcccs-upscaling@pipe-a-valid-mode:
    - shard-rkl:          NOTRUN -> [SKIP][78] ([i915#2672] / [i915#4579])
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-6/igt@kms_flip_scaled_crc@flip-64bpp-4tile-to-32bpp-4tiledg2rcccs-upscaling@pipe-a-valid-mode.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-pwrite:
    - shard-tglu:         NOTRUN -> [SKIP][79] ([fdo#109280]) +13 similar issues
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-6/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-spr-indfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbc-tiling-4:
    - shard-rkl:          NOTRUN -> [SKIP][80] ([i915#5439])
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-7/igt@kms_frontbuffer_tracking@fbc-tiling-4.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-shrfb-pgflip-blt:
    - shard-rkl:          NOTRUN -> [SKIP][81] ([fdo#111825] / [i915#1825]) +13 similar issues
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-7/igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-shrfb-pgflip-blt.html

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-render:
    - shard-tglu:         NOTRUN -> [SKIP][82] ([fdo#110189]) +11 similar issues
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-9/igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-rkl:          NOTRUN -> [SKIP][83] ([i915#3023]) +7 similar issues
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-1/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-pwrite.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-rkl:          NOTRUN -> [SKIP][84] ([i915#3555] / [i915#4579])
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_hdr@invalid-hdr:
    - shard-rkl:          NOTRUN -> [SKIP][85] ([i915#4579] / [i915#6953] / [i915#8228])
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@kms_hdr@invalid-hdr.html
    - shard-tglu:         NOTRUN -> [SKIP][86] ([i915#4579] / [i915#6953] / [i915#8228])
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-2/igt@kms_hdr@invalid-hdr.html

  * igt@kms_panel_fitting@legacy:
    - shard-rkl:          NOTRUN -> [SKIP][87] ([i915#4579] / [i915#6301])
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-1/igt@kms_panel_fitting@legacy.html

  * igt@kms_pipe_b_c_ivb@enable-pipe-c-while-b-has-3-lanes:
    - shard-tglu:         NOTRUN -> [SKIP][88] ([fdo#109289])
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-5/igt@kms_pipe_b_c_ivb@enable-pipe-c-while-b-has-3-lanes.html
    - shard-rkl:          NOTRUN -> [SKIP][89] ([fdo#109289])
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-7/igt@kms_pipe_b_c_ivb@enable-pipe-c-while-b-has-3-lanes.html

  * igt@kms_plane_scaling@plane-downscale-with-modifiers-factor-0-25@pipe-a-hdmi-a-1:
    - shard-rkl:          NOTRUN -> [SKIP][90] ([i915#5176]) +2 similar issues
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-7/igt@kms_plane_scaling@plane-downscale-with-modifiers-factor-0-25@pipe-a-hdmi-a-1.html

  * igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-a-hdmi-a-3 (NEW):
    - {shard-dg1}:        NOTRUN -> [SKIP][91] ([i915#5176]) +2 similar issues
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-dg1-12/igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-a-hdmi-a-3.html

  * igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-b-hdmi-a-2:
    - shard-rkl:          NOTRUN -> [SKIP][92] ([i915#4579] / [i915#5176]) +2 similar issues
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-2/igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-b-hdmi-a-2.html

  * igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-c-dp-1:
    - shard-apl:          NOTRUN -> [SKIP][93] ([fdo#109271] / [i915#4579])
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-apl3/igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-c-dp-1.html

  * igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-c-hdmi-a-1:
    - shard-tglu:         NOTRUN -> [SKIP][94] ([i915#5176]) +2 similar issues
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-3/igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-c-hdmi-a-1.html

  * igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-d-hdmi-a-1:
    - shard-tglu:         NOTRUN -> [SKIP][95] ([i915#4579] / [i915#5176])
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-3/igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-d-hdmi-a-1.html

  * igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-d-hdmi-a-3 (NEW):
    - {shard-dg1}:        NOTRUN -> [SKIP][96] ([i915#4579] / [i915#5176])
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-dg1-12/igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-25@pipe-d-hdmi-a-3.html

  * igt@kms_plane_scaling@planes-downscale-factor-0-25-upscale-20x20@pipe-b-hdmi-a-2:
    - shard-rkl:          NOTRUN -> [SKIP][97] ([i915#4579] / [i915#5235]) +3 similar issues
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@kms_plane_scaling@planes-downscale-factor-0-25-upscale-20x20@pipe-b-hdmi-a-2.html

  * igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-25@pipe-a-hdmi-a-2:
    - shard-rkl:          NOTRUN -> [SKIP][98] ([i915#5235]) +3 similar issues
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-1/igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-25@pipe-a-hdmi-a-2.html

  * igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-5@pipe-b-hdmi-a-1:
    - shard-snb:          NOTRUN -> [SKIP][99] ([fdo#109271] / [i915#4579]) +9 similar issues
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-snb1/igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-5@pipe-b-hdmi-a-1.html

  * igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area:
    - shard-tglu:         NOTRUN -> [SKIP][100] ([fdo#111068] / [i915#658])
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-7/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area.html

  * igt@kms_psr@cursor_mmap_gtt:
    - shard-rkl:          NOTRUN -> [SKIP][101] ([i915#1072]) +2 similar issues
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-4/igt@kms_psr@cursor_mmap_gtt.html

  * igt@kms_setmode@invalid-clone-single-crtc:
    - shard-rkl:          NOTRUN -> [SKIP][102] ([i915#3555] / [i915#4098] / [i915#4579])
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-4/igt@kms_setmode@invalid-clone-single-crtc.html

  * igt@kms_vblank@pipe-c-wait-forked-busy-hang:
    - shard-rkl:          NOTRUN -> [SKIP][103] ([i915#4070] / [i915#6768]) +1 similar issue
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-2/igt@kms_vblank@pipe-c-wait-forked-busy-hang.html

  * igt@kms_vblank@pipe-d-ts-continuation-idle:
    - shard-rkl:          NOTRUN -> [SKIP][104] ([i915#4070] / [i915#533] / [i915#6768]) +2 similar issues
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@kms_vblank@pipe-d-ts-continuation-idle.html

  * igt@kms_vrr@negative-basic:
    - shard-glk:          NOTRUN -> [SKIP][105] ([fdo#109271] / [i915#4579]) +2 similar issues
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-glk6/igt@kms_vrr@negative-basic.html

  * igt@kms_writeback@writeback-pixel-formats:
    - shard-rkl:          NOTRUN -> [SKIP][106] ([i915#2437])
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-7/igt@kms_writeback@writeback-pixel-formats.html

  * igt@perf@stress-open-close@0-rcs0:
    - shard-glk:          [PASS][107] -> [ABORT][108] ([i915#5213] / [i915#7941])
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-glk9/igt@perf@stress-open-close@0-rcs0.html
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-glk4/igt@perf@stress-open-close@0-rcs0.html

  * igt@perf_pmu@rc6@other-idle-gt0:
    - shard-tglu:         NOTRUN -> [SKIP][109] ([i915#8516])
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-2/igt@perf_pmu@rc6@other-idle-gt0.html

  * igt@v3d/v3d_submit_cl@multisync-out-syncs:
    - shard-rkl:          NOTRUN -> [SKIP][110] ([fdo#109315]) +2 similar issues
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-4/igt@v3d/v3d_submit_cl@multisync-out-syncs.html

  * igt@v3d/v3d_submit_csd@multiple-job-submission:
    - shard-tglu:         NOTRUN -> [SKIP][111] ([fdo#109315] / [i915#2575]) +4 similar issues
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-2/igt@v3d/v3d_submit_csd@multiple-job-submission.html

  * igt@vc4/vc4_perfmon@get-values-valid-perfmon:
    - shard-rkl:          NOTRUN -> [SKIP][112] ([i915#7711]) +1 similar issue
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-4/igt@vc4/vc4_perfmon@get-values-valid-perfmon.html

  * igt@vc4/vc4_wait_bo@used-bo:
    - shard-tglu:         NOTRUN -> [SKIP][113] ([i915#2575])
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-4/igt@vc4/vc4_wait_bo@used-bo.html

  
#### Possible fixes ####

  * igt@device_reset@unbind-reset-rebind:
    - shard-rkl:          [ABORT][114] ([i915#5507]) -> [PASS][115]
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-rkl-7/igt@device_reset@unbind-reset-rebind.html
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-3/igt@device_reset@unbind-reset-rebind.html
    - shard-apl:          [ABORT][116] ([i915#5507]) -> [PASS][117]
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-apl6/igt@device_reset@unbind-reset-rebind.html
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-apl2/igt@device_reset@unbind-reset-rebind.html
    - shard-tglu:         [ABORT][118] ([i915#5507]) -> [PASS][119]
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-tglu-2/igt@device_reset@unbind-reset-rebind.html
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-4/igt@device_reset@unbind-reset-rebind.html
    - shard-glk:          [ABORT][120] ([i915#5507]) -> [PASS][121]
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-glk2/igt@device_reset@unbind-reset-rebind.html
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-glk9/igt@device_reset@unbind-reset-rebind.html

  * igt@gem_barrier_race@remote-request@rcs0:
    - shard-glk:          [ABORT][122] ([i915#7461] / [i915#8211]) -> [PASS][123]
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-glk5/igt@gem_barrier_race@remote-request@rcs0.html
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-glk8/igt@gem_barrier_race@remote-request@rcs0.html

  * igt@gem_eio@reset-stress:
    - {shard-dg1}:        [FAIL][124] ([i915#5784]) -> [PASS][125]
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-dg1-17/igt@gem_eio@reset-stress.html
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-dg1-16/igt@gem_eio@reset-stress.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-tglu:         [FAIL][126] ([i915#2842]) -> [PASS][127]
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-tglu-3/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-7/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_exec_whisper@basic-fds-priority-all:
    - shard-tglu:         [INCOMPLETE][128] ([i915#6755] / [i915#7392]) -> [PASS][129]
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-tglu-8/igt@gem_exec_whisper@basic-fds-priority-all.html
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-10/igt@gem_exec_whisper@basic-fds-priority-all.html

  * igt@gem_mmap_offset@clear@smem0:
    - {shard-dg1}:        [DMESG-WARN][130] ([i915#8304]) -> [PASS][131]
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-dg1-17/igt@gem_mmap_offset@clear@smem0.html
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-dg1-18/igt@gem_mmap_offset@clear@smem0.html

  * igt@gen9_exec_parse@allowed-single:
    - shard-glk:          [ABORT][132] ([i915#5566]) -> [PASS][133]
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-glk6/igt@gen9_exec_parse@allowed-single.html
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-glk5/igt@gen9_exec_parse@allowed-single.html

  * igt@i915_pm_dc@dc9-dpms:
    - shard-tglu:         [SKIP][134] ([i915#4281]) -> [PASS][135]
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-tglu-9/igt@i915_pm_dc@dc9-dpms.html
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-tglu-2/igt@i915_pm_dc@dc9-dpms.html

  * igt@i915_pm_rpm@modeset-lpsp-stress:
    - {shard-dg1}:        [SKIP][136] ([i915#1397]) -> [PASS][137]
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-dg1-15/igt@i915_pm_rpm@modeset-lpsp-stress.html
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-dg1-19/igt@i915_pm_rpm@modeset-lpsp-stress.html

  * igt@kms_async_flips@alternate-sync-async-flip@pipe-b-hdmi-a-1 (NEW):
    - shard-glk:          [FAIL][138] ([i915#2521]) -> [PASS][139]
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-glk9/igt@kms_async_flips@alternate-sync-async-flip@pipe-b-hdmi-a-1.html
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-glk5/igt@kms_async_flips@alternate-sync-async-flip@pipe-b-hdmi-a-1.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-apl:          [FAIL][140] ([i915#2346]) -> [PASS][141]
   [140]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-apl3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [141]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-apl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a2:
    - shard-glk:          [FAIL][142] ([i915#79]) -> [PASS][143] +1 similar issue
   [142]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-glk7/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a2.html
   [143]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-glk8/igt@kms_flip@flip-vs-expired-vblank@a-hdmi-a2.html

  
#### Warnings ####

  * igt@kms_fbcon_fbt@psr:
    - shard-rkl:          [SKIP][144] ([fdo#110189] / [i915#3955]) -> [SKIP][145] ([i915#3955])
   [144]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13264/shard-rkl-2/igt@kms_fbcon_fbt@psr.html
   [145]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/shard-rkl-4/igt@kms_fbcon_fbt@psr.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109300]: https://bugs.freedesktop.org/show_bug.cgi?id=109300
  [fdo#109312]: https://bugs.freedesktop.org/show_bug.cgi?id=109312
  [fdo#109313]: https://bugs.freedesktop.org/show_bug.cgi?id=109313
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#110723]: https://bugs.freedesktop.org/show_bug.cgi?id=110723
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111825]: https://bugs.freedesktop.org/show_bug.cgi?id=111825
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [fdo#112283]: https://bugs.freedesktop.org/show_bug.cgi?id=112283
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1397]: https://gitlab.freedesktop.org/drm/intel/issues/1397
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1825]: https://gitlab.freedesktop.org/drm/intel/issues/1825
  [i915#1937]: https://gitlab.freedesktop.org/drm/intel/issues/1937
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
  [i915#3023]: https://gitlab.freedesktop.org/drm/intel/issues/3023
  [i915#3116]: https://gitlab.freedesktop.org/drm/intel/issues/3116
  [i915#3281]: https://gitlab.freedesktop.org/drm/intel/issues/3281
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3458]: https://gitlab.freedesktop.org/drm/intel/issues/3458
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3591]: https://gitlab.freedesktop.org/drm/intel/issues/3591
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3638]: https://gitlab.freedesktop.org/drm/intel/issues/3638
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3742]: https://gitlab.freedesktop.org/drm/intel/issues/3742
  [i915#3804]: https://gitlab.freedesktop.org/drm/intel/issues/3804
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3955]: https://gitlab.freedesktop.org/drm/intel/issues/3955
  [i915#4070]: https://gitlab.freedesktop.org/drm/intel/issues/4070
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4270]: https://gitlab.freedesktop.org/drm/intel/issues/4270
  [i915#4281]: https://gitlab.freedesktop.org/drm/intel/issues/4281
  [i915#4538]: https://gitlab.freedesktop.org/drm/intel/issues/4538
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4833]: https://gitlab.freedesktop.org/drm/intel/issues/4833
  [i915#5090]: https://gitlab.freedesktop.org/drm/intel/issues/5090
  [i915#5122]: https://gitlab.freedesktop.org/drm/intel/issues/5122
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5213]: https://gitlab.freedesktop.org/drm/intel/issues/5213
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5325]: https://gitlab.freedesktop.org/drm/intel/issues/5325
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#5439]: https://gitlab.freedesktop.org/drm/intel/issues/5439
  [i915#5493]: https://gitlab.freedesktop.org/drm/intel/issues/5493
  [i915#5507]: https://gitlab.freedesktop.org/drm/intel/issues/5507
  [i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
  [i915#5784]: https://gitlab.freedesktop.org/drm/intel/issues/5784
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
  [i915#6301]: https://gitlab.freedesktop.org/drm/intel/issues/6301
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6755]: https://gitlab.freedesktop.org/drm/intel/issues/6755
  [i915#6768]: https://gitlab.freedesktop.org/drm/intel/issues/6768
  [i915#6953]: https://gitlab.freedesktop.org/drm/intel/issues/6953
  [i915#7116]: https://gitlab.freedesktop.org/drm/intel/issues/7116
  [i915#7392]: https://gitlab.freedesktop.org/drm/intel/issues/7392
  [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7711]: https://gitlab.freedesktop.org/drm/intel/issues/7711
  [i915#7742]: https://gitlab.freedesktop.org/drm/intel/issues/7742
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#7941]: https://gitlab.freedesktop.org/drm/intel/issues/7941
  [i915#8011]: https://gitlab.freedesktop.org/drm/intel/issues/8011
  [i915#8211]: https://gitlab.freedesktop.org/drm/intel/issues/8211
  [i915#8228]: https://gitlab.freedesktop.org/drm/intel/issues/8228
  [i915#8247]: https://gitlab.freedesktop.org/drm/intel/issues/8247
  [i915#8304]: https://gitlab.freedesktop.org/drm/intel/issues/8304
  [i915#8319]: https://gitlab.freedesktop.org/drm/intel/issues/8319
  [i915#8399]: https://gitlab.freedesktop.org/drm/intel/issues/8399
  [i915#8502]: https://gitlab.freedesktop.org/drm/intel/issues/8502
  [i915#8516]: https://gitlab.freedesktop.org/drm/intel/issues/8516


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_7326 -> IGTPW_9155
  * Piglit: piglit_4509 -> None

  CI-20190529: 20190529
  CI_DRM_13264: 807d06e86e11466ef4fe74e2ddcb93de8e3a535b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_9155: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/index.html
  IGT_7326: 02c2cf17628b6203d6105d4a91dfe8a101d482ce @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_9155/index.html

[-- Attachment #2: Type: text/html, Size: 56177 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 10/10] KUnit: replace abort with graceful skip
  2023-06-13  9:58       ` Mauro Carvalho Chehab
@ 2023-06-13 11:15         ` Janusz Krzysztofik
  2023-06-13 13:34           ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 22+ messages in thread
From: Janusz Krzysztofik @ 2023-06-13 11:15 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev

On Tuesday, 13 June 2023 11:58:47 CEST Mauro Carvalho Chehab wrote:
> On Tue, 13 Jun 2023 11:11:00 +0200
> Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> wrote:
> 
> > On Tuesday, 13 June 2023 10:41:03 CEST Mauro Carvalho Chehab wrote:
> > > On Tue, 13 Jun 2023 09:27:26 +0200
> > > Dominik Karol Piatkowski <dominik.karol.piatkowski@intel.com> wrote:
> > >   
> > > > Sample drm_buddy output with missing requirements:
> > > > 	Starting subtest: drm_buddy_test
> > > > 	(drm_buddy:32218) igt_kmod-WARNING: Unable to load KUnit
> > > > 	Subtest drm_buddy_test: SKIP (0.001s)
> > > > 
> > > > Signed-off-by: Dominik Karol Piątkowski   
> > <dominik.karol.piatkowski@intel.com>
> > > > Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > > > Cc: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>
> > > > ---
> > > >  lib/igt_kmod.c | 20 +++++++++++++-------
> > > >  1 file changed, 13 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> > > > index 1511bdef4..04220f404 100644
> > > > --- a/lib/igt_kmod.c
> > > > +++ b/lib/igt_kmod.c
> > > > @@ -763,27 +763,27 @@ static void __igt_kunit(const char *module_name,   
> > const char *opts)
> > > >  	int ret;
> > > >  	struct ktap_test_results *results;
> > > >  	struct ktap_test_results_element *temp;
> > > > +	bool skip = false;
> > > >  
> > > >  	/* get normalized module name */
> > > >  	if (igt_ktest_init(&tst, module_name) != 0) {
> > > > -		igt_warn("Unable to initialize ktest for %s\n",   
> > module_name);
> > > > -		igt_fail(IGT_EXIT_SKIP);
> > > > +		igt_skip("Unable to initialize ktest for %s\n",   
> > module_name);
> > > >  	}
> > > >  
> > > >  	if (igt_ktest_begin(&tst) != 0) {
> > > > -		igt_warn("Unable to begin ktest for %s\n", module_name);
> > > > -
> > > >  		igt_ktest_fini(&tst);
> > > > -		igt_fail(IGT_EXIT_SKIP);
> > > > +		igt_skip("Unable to begin ktest for %s\n", module_name);
> > > >  	}
> > > >  
> > > >  	if (tst.kmsg < 0) {
> > > >  		igt_warn("Could not open /dev/kmsg\n");
> > > > +		skip = true;  
> > > 
> > > Hmm.... not being able to read from /dev/kmsg doesn't seem a reason for
> > > skipping the test.
> > > 
> > > IMO, the test should be skipped only if:
> > > 
> > > 1. The Kernel is not compiled with KUnit support;
> > > 2. The KUnit test was not compiled.
> > > 
> > > Errors for all other conditions should be considered as failures.
> > > 
> > > Now, I'm not sure what would be the proper way to test for such
> > > skip condition. I suspect you could check the error code at
> > > igt_kmod_load(), calling igt_skip() if it returns -ENOENT,
> > > e. g. on this part of __igt_kunit():
> > > 
> > >         /* The KUnit module is required for running any KUnit tests */
> > >         if (igt_kmod_load("kunit", NULL) != 0 ||
> > >             kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) !=   
> > 0) {
> > >                 igt_warn("Unable to load KUnit\n");
> > >                 igt_fail(IGT_EXIT_FAILURE);
> > >         }
> > > 
> > >         is_builtin = kmod_module_get_initstate(kunit_kmod) ==   
> > KMOD_MODULE_BUILTIN;
> > > 
> > >         results = ktap_parser_start(f, is_builtin);
> > > 
> > >         if (igt_kmod_load(module_name, opts) != 0) {
> > >                 igt_warn("Unable to load %s module\n", module_name);
> > >                 ret = ktap_parser_stop();
> > >                 igt_fail(IGT_EXIT_FAILURE);
> > >         }
> > > 
> > > 
> > > This could be replaced by something similar to:
> > > 
> > > 	err = igt_kmod_load("kunit", NULL);
> > > 	igt_skip_on(err == ENOENT, "KUnit not supported");
> > > 	igt_fail_on_f(err != 0, "Error %d when trying to load kunit", err);
> > >         if (kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) !=   
> > 0) {
> > >                 igt_warn("Unable to load KUnit\n");
> > >                 igt_fail(IGT_EXIT_FAILURE);
> > >         }
> > > 
> > > 
> > >         is_builtin = kmod_module_get_initstate(kunit_kmod) ==   
> > KMOD_MODULE_BUILTIN;
> > > 
> > >         results = ktap_parser_start(f, is_builtin);
> > > 
> > >         err = igt_kmod_load(module_name, opts) 
> > > 	igt_skip_on(err == ENOENT, "Module %s not found", module_name);
> > > 	if (err) {
> > >                 igt_warn("Error %d when trying to load %s module\n", err,   
> > module_name);
> > >                 ret = ktap_parser_stop();
> > >                 igt_fail(IGT_EXIT_FAILURE);
> > >         }
> > > 
> > > 
> > > In order to explicitly check for (1) and (2).  
> > 
> > Why do we reinvent the wheel instead of following the pattern from 
> > igt_kselftest()? 
> 
> Nobody is talking about reinventing the wheel. Selftests and KUnit tests
> are executed on different ways.
> 
> For selftests, the same driver that was loaded before needs to be unloaded
> and re-probed with a different argument.
> 
> For KUnit tests, a different module will be loaded, if the Kernel was
> compiled with KUnit support for a given test suite.
> 
> Looking at KUnit modprobe time, if a module doesn't load because the file
> doesn't exist (-ENOENT error), it means that:
> 
> - for KUnit: the KUnit module was not compiled.
> - for selftests: this is really unlikely to happen, as all other
>   previous IGT tests depend on having the driver module loaded.
>   So, no need for any explicit check for this specific condition.
> 
> > We used to follow it, before the whole igt_kunit() body was 
> > put under igt_subtest_with_dynamic() (patch 6/8 of v5, then squashed with 5/5, 
> > IIRC).  That forced us to introduce a series of changes inside igt_kunit(), 
> > which we now contest.  Was that really necessary?
> 
> This has nothing to do with using (or not) a dynamic IGT subtest.

It has.  As soon as you enter an igt_subtest or igt_subtest_with_dynamic 
section, your inform users (CI) that the subtest execution started.  And, you 
can't call return, only igt_success(), or igt_skip()/igt_fail() and friends, 
form inside, while you can before.

The DRM selftests we are going to update are now using igt_kselftest(), which 
calls return on some initial conditions not met, before the 
igt_subtest_with_dynamic section is entered.  CI expects that behavior, users 
are used to it.  We shouldn't change that only because we switch from an old 
underlying kernel selftest format to a new shiny one, I believe, unless we are 
able to prove that there was something wrong with that former approach, or we 
can explain why it no longer fits into the kunit variant.

> 
> It is related to how to report IGT errors while trying to run KUnit
> tests.
> 
> Without this patch, every kind of errors will produce a failure;

Yes, that was wrong, and thank you Dominik for fixing this.  But plaese also 
note that the badness was mostly caused by ad-hoc corrections to igt_kunit() 
body, forced by putting it as a whole insinde igt_subtest_with_dynamic() 
section while it was not ready for that.  IOW, my objections don't apply to 
this patch specifically, but rather the the whole series that started from 
igt_kunit() trying to provide the same used experience as igt_ksleftest(), 
then that approach being destroyed with patches subsequently added to the 
series.

> after this patch, they'll all produce a skip.
> 
> IMO, what it is needed is to split KUnit errors on two different
> categories:
> 
> 1. "real" errors while running KUnit (like troubles reading dmesg);

I agree that troubles reading dmesg are critical to kunit variants, while they 
were negligible for old selftests, hence their handling specifically must now 
be different, but for me such a difference is not a sufficient justification 
for killing the whole igt_ksleftest() error handling pattern and inventing a 
new one from scratch.

Thanks,
Janusz

> 2. the KUnit module to be modprobed doesn't exist.
> 
> for (1), igt_fail() is the proper error handling.
> 
> For (2), igt_skip() is the right error handling, as the Kernel under
> tests doesn't have what it is needed to run KUnit, as the module was
> not compiled.
> 
> Regards,
> Mauro
> 
> 
> 






^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 10/10] KUnit: replace abort with graceful skip
  2023-06-13 11:15         ` Janusz Krzysztofik
@ 2023-06-13 13:34           ` Mauro Carvalho Chehab
  2023-06-13 14:25             ` Janusz Krzysztofik
  0 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2023-06-13 13:34 UTC (permalink / raw)
  To: Janusz Krzysztofik; +Cc: igt-dev

On Tue, 13 Jun 2023 13:15:50 +0200
Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> wrote:

> On Tuesday, 13 June 2023 11:58:47 CEST Mauro Carvalho Chehab wrote:
> > On Tue, 13 Jun 2023 11:11:00 +0200
> > Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> wrote:
> >   
> > > On Tuesday, 13 June 2023 10:41:03 CEST Mauro Carvalho Chehab wrote:  
> > > > On Tue, 13 Jun 2023 09:27:26 +0200
> > > > Dominik Karol Piatkowski <dominik.karol.piatkowski@intel.com> wrote:
> > > >     
> > > > > Sample drm_buddy output with missing requirements:
> > > > > 	Starting subtest: drm_buddy_test
> > > > > 	(drm_buddy:32218) igt_kmod-WARNING: Unable to load KUnit
> > > > > 	Subtest drm_buddy_test: SKIP (0.001s)
> > > > > 
> > > > > Signed-off-by: Dominik Karol Piątkowski     
> > > <dominik.karol.piatkowski@intel.com>  
> > > > > Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > > > > Cc: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>
> > > > > ---
> > > > >  lib/igt_kmod.c | 20 +++++++++++++-------
> > > > >  1 file changed, 13 insertions(+), 7 deletions(-)
> > > > > 
> > > > > diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> > > > > index 1511bdef4..04220f404 100644
> > > > > --- a/lib/igt_kmod.c
> > > > > +++ b/lib/igt_kmod.c
> > > > > @@ -763,27 +763,27 @@ static void __igt_kunit(const char *module_name,     
> > > const char *opts)  
> > > > >  	int ret;
> > > > >  	struct ktap_test_results *results;
> > > > >  	struct ktap_test_results_element *temp;
> > > > > +	bool skip = false;
> > > > >  
> > > > >  	/* get normalized module name */
> > > > >  	if (igt_ktest_init(&tst, module_name) != 0) {
> > > > > -		igt_warn("Unable to initialize ktest for %s\n",     
> > > module_name);  
> > > > > -		igt_fail(IGT_EXIT_SKIP);
> > > > > +		igt_skip("Unable to initialize ktest for %s\n",     
> > > module_name);  
> > > > >  	}
> > > > >  
> > > > >  	if (igt_ktest_begin(&tst) != 0) {
> > > > > -		igt_warn("Unable to begin ktest for %s\n", module_name);
> > > > > -
> > > > >  		igt_ktest_fini(&tst);
> > > > > -		igt_fail(IGT_EXIT_SKIP);
> > > > > +		igt_skip("Unable to begin ktest for %s\n", module_name);
> > > > >  	}
> > > > >  
> > > > >  	if (tst.kmsg < 0) {
> > > > >  		igt_warn("Could not open /dev/kmsg\n");
> > > > > +		skip = true;    
> > > > 
> > > > Hmm.... not being able to read from /dev/kmsg doesn't seem a reason for
> > > > skipping the test.
> > > > 
> > > > IMO, the test should be skipped only if:
> > > > 
> > > > 1. The Kernel is not compiled with KUnit support;
> > > > 2. The KUnit test was not compiled.
> > > > 
> > > > Errors for all other conditions should be considered as failures.
> > > > 
> > > > Now, I'm not sure what would be the proper way to test for such
> > > > skip condition. I suspect you could check the error code at
> > > > igt_kmod_load(), calling igt_skip() if it returns -ENOENT,
> > > > e. g. on this part of __igt_kunit():
> > > > 
> > > >         /* The KUnit module is required for running any KUnit tests */
> > > >         if (igt_kmod_load("kunit", NULL) != 0 ||
> > > >             kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) !=     
> > > 0) {  
> > > >                 igt_warn("Unable to load KUnit\n");
> > > >                 igt_fail(IGT_EXIT_FAILURE);
> > > >         }
> > > > 
> > > >         is_builtin = kmod_module_get_initstate(kunit_kmod) ==     
> > > KMOD_MODULE_BUILTIN;  
> > > > 
> > > >         results = ktap_parser_start(f, is_builtin);
> > > > 
> > > >         if (igt_kmod_load(module_name, opts) != 0) {
> > > >                 igt_warn("Unable to load %s module\n", module_name);
> > > >                 ret = ktap_parser_stop();
> > > >                 igt_fail(IGT_EXIT_FAILURE);
> > > >         }
> > > > 
> > > > 
> > > > This could be replaced by something similar to:
> > > > 
> > > > 	err = igt_kmod_load("kunit", NULL);
> > > > 	igt_skip_on(err == ENOENT, "KUnit not supported");
> > > > 	igt_fail_on_f(err != 0, "Error %d when trying to load kunit", err);
> > > >         if (kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) !=     
> > > 0) {  
> > > >                 igt_warn("Unable to load KUnit\n");
> > > >                 igt_fail(IGT_EXIT_FAILURE);
> > > >         }
> > > > 
> > > > 
> > > >         is_builtin = kmod_module_get_initstate(kunit_kmod) ==     
> > > KMOD_MODULE_BUILTIN;  
> > > > 
> > > >         results = ktap_parser_start(f, is_builtin);
> > > > 
> > > >         err = igt_kmod_load(module_name, opts) 
> > > > 	igt_skip_on(err == ENOENT, "Module %s not found", module_name);
> > > > 	if (err) {
> > > >                 igt_warn("Error %d when trying to load %s module\n", err,     
> > > module_name);  
> > > >                 ret = ktap_parser_stop();
> > > >                 igt_fail(IGT_EXIT_FAILURE);
> > > >         }
> > > > 
> > > > 
> > > > In order to explicitly check for (1) and (2).    
> > > 
> > > Why do we reinvent the wheel instead of following the pattern from 
> > > igt_kselftest()?   
> > 
> > Nobody is talking about reinventing the wheel. Selftests and KUnit tests
> > are executed on different ways.
> > 
> > For selftests, the same driver that was loaded before needs to be unloaded
> > and re-probed with a different argument.
> > 
> > For KUnit tests, a different module will be loaded, if the Kernel was
> > compiled with KUnit support for a given test suite.
> > 
> > Looking at KUnit modprobe time, if a module doesn't load because the file
> > doesn't exist (-ENOENT error), it means that:
> > 
> > - for KUnit: the KUnit module was not compiled.
> > - for selftests: this is really unlikely to happen, as all other
> >   previous IGT tests depend on having the driver module loaded.
> >   So, no need for any explicit check for this specific condition.
> >   
> > > We used to follow it, before the whole igt_kunit() body was 
> > > put under igt_subtest_with_dynamic() (patch 6/8 of v5, then squashed with 5/5, 
> > > IIRC).  That forced us to introduce a series of changes inside igt_kunit(), 
> > > which we now contest.  Was that really necessary?  
> > 
> > This has nothing to do with using (or not) a dynamic IGT subtest.  
> 
> It has.  As soon as you enter an igt_subtest or igt_subtest_with_dynamic 
> section, your inform users (CI) that the subtest execution started.  And, you 
> can't call return, only igt_success(), or igt_skip()/igt_fail() and friends, 
> form inside, while you can before.

Well, igt selftest excecution also calls igt_subtest_with_dynamic().
However, on kunit, the tests start running during modprobe time, so
igt_subtest_with_dynamic() needs to be called before that. Maybe some
validations could happen earlier, but the most important one (checking
that the module exists) happens when the module is probed, which is the
same call that starts the test.

> 
> The DRM selftests we are going to update are now using igt_kselftest(), which 
> calls return on some initial conditions not met, before the 
> igt_subtest_with_dynamic section is entered.

No. Since 2022 - specifically, since those commits:

	- 932da861956a ("drm: selftest: convert drm_buddy selftest to KUnit")
	- fc8d29e298cf ("drm: selftest: convert drm_mm selftest to KUnit")

those tests don't work anymore, as upstream removed support for selftest,
in favor of KUnit. So, those tests currently do nothing.

We are changing because those tests are currently broken on IGT. That
is the reason why we need KUnit support for such tests.

> CI expects that behavior, users 
> are used to it.  We shouldn't change that only because we switch from an old 
> underlying kernel selftest format to a new shiny one, I believe, unless we are 
> able to prove that there was something wrong with that former approach, or we 
> can explain why it no longer fits into the kunit variant.

Yes, there's something wrong with the former approach: the tests are
currently broken, as upstream moved on.

> 
> > 
> > It is related to how to report IGT errors while trying to run KUnit
> > tests.
> > 
> > Without this patch, every kind of errors will produce a failure;  
> 
> Yes, that was wrong, and thank you Dominik for fixing this.  But plaese also 
> note that the badness was mostly caused by ad-hoc corrections to igt_kunit() 
> body, forced by putting it as a whole insinde igt_subtest_with_dynamic() 
> section while it was not ready for that.  IOW, my objections don't apply to 
> this patch specifically, but rather the the whole series that started from 
> igt_kunit() trying to provide the same used experience as igt_ksleftest(), 
> then that approach being destroyed with patches subsequently added to the 
> series.
> 
> > after this patch, they'll all produce a skip.
> > 
> > IMO, what it is needed is to split KUnit errors on two different
> > categories:
> > 
> > 1. "real" errors while running KUnit (like troubles reading dmesg);  
> 
> I agree that troubles reading dmesg are critical to kunit variants, while they 
> were negligible for old selftests, hence their handling specifically must now 
> be different, but for me such a difference is not a sufficient justification 
> for killing the whole igt_ksleftest() error handling pattern and inventing a 
> new one from scratch.
> 
> Thanks,
> Janusz
> 
> > 2. the KUnit module to be modprobed doesn't exist.
> > 
> > for (1), igt_fail() is the proper error handling.
> > 
> > For (2), igt_skip() is the right error handling, as the Kernel under
> > tests doesn't have what it is needed to run KUnit, as the module was
> > not compiled.
> > 
> > Regards,
> > Mauro
> > 
> > 
> >   
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 10/10] KUnit: replace abort with graceful skip
  2023-06-13 13:34           ` Mauro Carvalho Chehab
@ 2023-06-13 14:25             ` Janusz Krzysztofik
  2023-06-14  7:58               ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 22+ messages in thread
From: Janusz Krzysztofik @ 2023-06-13 14:25 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev

On Tuesday, 13 June 2023 15:34:00 CEST Mauro Carvalho Chehab wrote:
> On Tue, 13 Jun 2023 13:15:50 +0200
> Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> wrote:
> 
> > On Tuesday, 13 June 2023 11:58:47 CEST Mauro Carvalho Chehab wrote:
> > > On Tue, 13 Jun 2023 11:11:00 +0200
> > > Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> wrote:
> > >   
> > > > On Tuesday, 13 June 2023 10:41:03 CEST Mauro Carvalho Chehab wrote:  
> > > > > On Tue, 13 Jun 2023 09:27:26 +0200
> > > > > Dominik Karol Piatkowski <dominik.karol.piatkowski@intel.com> wrote:
> > > > >     
> > > > > > Sample drm_buddy output with missing requirements:
> > > > > > 	Starting subtest: drm_buddy_test
> > > > > > 	(drm_buddy:32218) igt_kmod-WARNING: Unable to load KUnit
> > > > > > 	Subtest drm_buddy_test: SKIP (0.001s)
> > > > > > 
> > > > > > Signed-off-by: Dominik Karol Piątkowski     
> > > > <dominik.karol.piatkowski@intel.com>  
> > > > > > Cc: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > > > > > Cc: Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>
> > > > > > ---
> > > > > >  lib/igt_kmod.c | 20 +++++++++++++-------
> > > > > >  1 file changed, 13 insertions(+), 7 deletions(-)
> > > > > > 
> > > > > > diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
> > > > > > index 1511bdef4..04220f404 100644
> > > > > > --- a/lib/igt_kmod.c
> > > > > > +++ b/lib/igt_kmod.c
> > > > > > @@ -763,27 +763,27 @@ static void __igt_kunit(const char *module_name,     
> > > > const char *opts)  
> > > > > >  	int ret;
> > > > > >  	struct ktap_test_results *results;
> > > > > >  	struct ktap_test_results_element *temp;
> > > > > > +	bool skip = false;
> > > > > >  
> > > > > >  	/* get normalized module name */
> > > > > >  	if (igt_ktest_init(&tst, module_name) != 0) {
> > > > > > -		igt_warn("Unable to initialize ktest for %s\n",     
> > > > module_name);  
> > > > > > -		igt_fail(IGT_EXIT_SKIP);
> > > > > > +		igt_skip("Unable to initialize ktest for %s\n",     
> > > > module_name);  
> > > > > >  	}
> > > > > >  
> > > > > >  	if (igt_ktest_begin(&tst) != 0) {
> > > > > > -		igt_warn("Unable to begin ktest for %s\n", module_name);
> > > > > > -
> > > > > >  		igt_ktest_fini(&tst);
> > > > > > -		igt_fail(IGT_EXIT_SKIP);
> > > > > > +		igt_skip("Unable to begin ktest for %s\n", module_name);
> > > > > >  	}
> > > > > >  
> > > > > >  	if (tst.kmsg < 0) {
> > > > > >  		igt_warn("Could not open /dev/kmsg\n");
> > > > > > +		skip = true;    
> > > > > 
> > > > > Hmm.... not being able to read from /dev/kmsg doesn't seem a reason for
> > > > > skipping the test.
> > > > > 
> > > > > IMO, the test should be skipped only if:
> > > > > 
> > > > > 1. The Kernel is not compiled with KUnit support;
> > > > > 2. The KUnit test was not compiled.
> > > > > 
> > > > > Errors for all other conditions should be considered as failures.
> > > > > 
> > > > > Now, I'm not sure what would be the proper way to test for such
> > > > > skip condition. I suspect you could check the error code at
> > > > > igt_kmod_load(), calling igt_skip() if it returns -ENOENT,
> > > > > e. g. on this part of __igt_kunit():
> > > > > 
> > > > >         /* The KUnit module is required for running any KUnit tests */
> > > > >         if (igt_kmod_load("kunit", NULL) != 0 ||
> > > > >             kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) !=     
> > > > 0) {  
> > > > >                 igt_warn("Unable to load KUnit\n");
> > > > >                 igt_fail(IGT_EXIT_FAILURE);
> > > > >         }
> > > > > 
> > > > >         is_builtin = kmod_module_get_initstate(kunit_kmod) ==     
> > > > KMOD_MODULE_BUILTIN;  
> > > > > 
> > > > >         results = ktap_parser_start(f, is_builtin);
> > > > > 
> > > > >         if (igt_kmod_load(module_name, opts) != 0) {
> > > > >                 igt_warn("Unable to load %s module\n", module_name);
> > > > >                 ret = ktap_parser_stop();
> > > > >                 igt_fail(IGT_EXIT_FAILURE);
> > > > >         }
> > > > > 
> > > > > 
> > > > > This could be replaced by something similar to:
> > > > > 
> > > > > 	err = igt_kmod_load("kunit", NULL);
> > > > > 	igt_skip_on(err == ENOENT, "KUnit not supported");
> > > > > 	igt_fail_on_f(err != 0, "Error %d when trying to load kunit", err);
> > > > >         if (kmod_module_new_from_name(kmod_ctx(), "kunit", &kunit_kmod) !=     
> > > > 0) {  
> > > > >                 igt_warn("Unable to load KUnit\n");
> > > > >                 igt_fail(IGT_EXIT_FAILURE);
> > > > >         }
> > > > > 
> > > > > 
> > > > >         is_builtin = kmod_module_get_initstate(kunit_kmod) ==     
> > > > KMOD_MODULE_BUILTIN;  
> > > > > 
> > > > >         results = ktap_parser_start(f, is_builtin);
> > > > > 
> > > > >         err = igt_kmod_load(module_name, opts) 
> > > > > 	igt_skip_on(err == ENOENT, "Module %s not found", module_name);
> > > > > 	if (err) {
> > > > >                 igt_warn("Error %d when trying to load %s module\n", err,     
> > > > module_name);  
> > > > >                 ret = ktap_parser_stop();
> > > > >                 igt_fail(IGT_EXIT_FAILURE);
> > > > >         }
> > > > > 
> > > > > 
> > > > > In order to explicitly check for (1) and (2).    
> > > > 
> > > > Why do we reinvent the wheel instead of following the pattern from 
> > > > igt_kselftest()?   
> > > 
> > > Nobody is talking about reinventing the wheel. Selftests and KUnit tests
> > > are executed on different ways.
> > > 
> > > For selftests, the same driver that was loaded before needs to be unloaded
> > > and re-probed with a different argument.
> > > 
> > > For KUnit tests, a different module will be loaded, if the Kernel was
> > > compiled with KUnit support for a given test suite.
> > > 
> > > Looking at KUnit modprobe time, if a module doesn't load because the file
> > > doesn't exist (-ENOENT error), it means that:
> > > 
> > > - for KUnit: the KUnit module was not compiled.
> > > - for selftests: this is really unlikely to happen, as all other
> > >   previous IGT tests depend on having the driver module loaded.
> > >   So, no need for any explicit check for this specific condition.
> > >   
> > > > We used to follow it, before the whole igt_kunit() body was 
> > > > put under igt_subtest_with_dynamic() (patch 6/8 of v5, then squashed with 5/5, 
> > > > IIRC).  That forced us to introduce a series of changes inside igt_kunit(), 
> > > > which we now contest.  Was that really necessary?  
> > > 
> > > This has nothing to do with using (or not) a dynamic IGT subtest.  
> > 
> > It has.  As soon as you enter an igt_subtest or igt_subtest_with_dynamic 
> > section, your inform users (CI) that the subtest execution started.  And, you 
> > can't call return, only igt_success(), or igt_skip()/igt_fail() and friends, 
> > form inside, while you can before.
> 
> Well, igt selftest excecution also calls igt_subtest_with_dynamic().
> However, on kunit, the tests start running during modprobe time, 

The same as it was happening on modules that implemented the old DRM i915-
alike selftests, and still happens on i915 selftests.  AFAICT, kunit modules 
are not any different in that aspect.

> so
> igt_subtest_with_dynamic() needs to be called before that. Maybe some
> validations could happen earlier, but the most important one (checking
> that the module exists) happens when the module is probed, which is the
> same call that starts the test.
> 
> > 
> > The DRM selftests we are going to update are now using igt_kselftest(), which 
> > calls return on some initial conditions not met, before the 
> > igt_subtest_with_dynamic section is entered.
> 
> No. Since 2022 - specifically, since those commits:
> 
> 	- 932da861956a ("drm: selftest: convert drm_buddy selftest to KUnit")
> 	- fc8d29e298cf ("drm: selftest: convert drm_mm selftest to KUnit")
> 
> those tests don't work anymore, as upstream removed support for selftest,
> in favor of KUnit. So, those tests currently do nothing.

In my opinion -- no, those tests currently skip, which is different from doing 
nothing.

> 
> We are changing because those tests are currently broken on IGT. 

In my opinion -- no, they are not broken, they are outdated.

> That
> is the reason why we need KUnit support for such tests.
> 
> > CI expects that behavior, users 
> > are used to it.  We shouldn't change that only because we switch from an old 
> > underlying kernel selftest format to a new shiny one, I believe, unless we are 
> > able to prove that there was something wrong with that former approach, or we 
> > can explain why it no longer fits into the kunit variant.
> 
> Yes, there's something wrong with the former approach: 

Are you saying that error handling in igt_ksleftest() is broken?  In my 
opinion, i915 selftests successfully executing on CI now and again, as well as 
the outdated DRM selftests successfully skipping, prove something different.

I believe that igt_kunit() that follows as closely as possible the pattern of 
how igt_kselftest() is handling unmet conditions and errors, with full respect 
to real differences between kunit and i915-like selftests, would be the best 
solution.

Thanks,
Janusz


> the tests are
> currently broken, as upstream moved on.
> 
> > 
> > > 
> > > It is related to how to report IGT errors while trying to run KUnit
> > > tests.
> > > 
> > > Without this patch, every kind of errors will produce a failure;  
> > 
> > Yes, that was wrong, and thank you Dominik for fixing this.  But plaese also 
> > note that the badness was mostly caused by ad-hoc corrections to igt_kunit() 
> > body, forced by putting it as a whole insinde igt_subtest_with_dynamic() 
> > section while it was not ready for that.  IOW, my objections don't apply to 
> > this patch specifically, but rather the the whole series that started from 
> > igt_kunit() trying to provide the same used experience as igt_ksleftest(), 
> > then that approach being destroyed with patches subsequently added to the 
> > series.
> > 
> > > after this patch, they'll all produce a skip.
> > > 
> > > IMO, what it is needed is to split KUnit errors on two different
> > > categories:
> > > 
> > > 1. "real" errors while running KUnit (like troubles reading dmesg);  
> > 
> > I agree that troubles reading dmesg are critical to kunit variants, while they 
> > were negligible for old selftests, hence their handling specifically must now 
> > be different, but for me such a difference is not a sufficient justification 
> > for killing the whole igt_ksleftest() error handling pattern and inventing a 
> > new one from scratch.
> > 
> > Thanks,
> > Janusz
> > 
> > > 2. the KUnit module to be modprobed doesn't exist.
> > > 
> > > for (1), igt_fail() is the proper error handling.
> > > 
> > > For (2), igt_skip() is the right error handling, as the Kernel under
> > > tests doesn't have what it is needed to run KUnit, as the module was
> > > not compiled.
> > > 
> > > Regards,
> > > Mauro
> > > 
> > > 
> > >   
> > 
> > 
> > 
> > 
> 






^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 10/10] KUnit: replace abort with graceful skip
  2023-06-13 14:25             ` Janusz Krzysztofik
@ 2023-06-14  7:58               ` Mauro Carvalho Chehab
  2023-06-14  9:40                 ` Janusz Krzysztofik
  0 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2023-06-14  7:58 UTC (permalink / raw)
  To: Janusz Krzysztofik; +Cc: igt-dev

On Tue, 13 Jun 2023 16:25:01 +0200
Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> wrote:

> > No. Since 2022 - specifically, since those commits:
> > 
> > 	- 932da861956a ("drm: selftest: convert drm_buddy selftest to KUnit")
> > 	- fc8d29e298cf ("drm: selftest: convert drm_mm selftest to KUnit")
> > 
> > those tests don't work anymore, as upstream removed support for selftest,
> > in favor of KUnit. So, those tests currently do nothing.  
> 
> In my opinion -- no, those tests currently skip, which is different from doing 
> nothing.
> 
> > 
> > We are changing because those tests are currently broken on IGT.   
> 
> In my opinion -- no, they are not broken, they are outdated.

IGT tests are broken, as IGT is not able anymore to execute drm_buddy and
drm_mm unit tests. This series fix it.

> 
> > That
> > is the reason why we need KUnit support for such tests.
> >   
> > > CI expects that behavior, users 
> > > are used to it.  We shouldn't change that only because we switch from an old 
> > > underlying kernel selftest format to a new shiny one, I believe, unless we are 
> > > able to prove that there was something wrong with that former approach, or we 
> > > can explain why it no longer fits into the kunit variant.  
> > 
> > Yes, there's something wrong with the former approach:   
> 
> Are you saying that error handling in igt_ksleftest() is broken?  In my 
> opinion, i915 selftests successfully executing on CI now and again, as well as 
> the outdated DRM selftests successfully skipping, prove something different.

No, I'm saying that drm core tests are broken on IGT, because drm core doesn't
support the legacy selftest infrastructure since 2022, and IGT is currently
missing kUnit support.

On other words, IGT won't run such tests *even* if the Kernel is build with
support for DRM core tests.

> I believe that igt_kunit() that follows as closely as possible the pattern of 
> how igt_kselftest() is handling unmet conditions and errors, with full respect 
> to real differences between kunit and i915-like selftests, would be the best 
> solution.

I'm not saying anthing different, but KUnit and selftest executions are
different:

- with selftest, the module loads asynchronously. After the module is
  loaded with success, the probe() method will be called and the tests
  will run.

  It means that it is possible to first load the driver and then run
  igt dynamic subtest logic, parsing the results from dmesg.

- with KUnit, the unit tests will run synchronously at modprobe time.
  So, when modprobe() finishes execution, all KUnit tests were already
  executed - and if the Kernel crashes on a KUnit test - results will be
  lost.
  
  It means that all steps needed to execute KUnit should be handled
  before modprobing, including starting a thread to capture dmesg
  results during modprobe time. So, igt dynamic subtests should be created 
  before calling modprobe() function.

due to such difference, igt_subtest_with_dynamic() should be called
before modprobe(), and, if something goes wrong on that time - for instance
if the module doesn't exist and returns EENOENT - IGT needs to use
igt_skip() or igt_fail().

Regards,
Mauro

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [igt-dev] [PATCH i-g-t 10/10] KUnit: replace abort with graceful skip
  2023-06-14  7:58               ` Mauro Carvalho Chehab
@ 2023-06-14  9:40                 ` Janusz Krzysztofik
  0 siblings, 0 replies; 22+ messages in thread
From: Janusz Krzysztofik @ 2023-06-14  9:40 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: igt-dev

On Wednesday, 14 June 2023 09:58:34 CEST Mauro Carvalho Chehab wrote:
> On Tue, 13 Jun 2023 16:25:01 +0200
> Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> wrote:
> 
> > > No. Since 2022 - specifically, since those commits:
> > > 
> > > 	- 932da861956a ("drm: selftest: convert drm_buddy selftest to KUnit")
> > > 	- fc8d29e298cf ("drm: selftest: convert drm_mm selftest to KUnit")
> > > 
> > > those tests don't work anymore, as upstream removed support for selftest,
> > > in favor of KUnit. So, those tests currently do nothing.  
> > 
> > In my opinion -- no, those tests currently skip, which is different from doing 
> > nothing.
> > 
> > > 
> > > We are changing because those tests are currently broken on IGT.   
> > 
> > In my opinion -- no, they are not broken, they are outdated.
> 
> IGT tests are broken, as IGT is not able anymore to execute drm_buddy and
> drm_mm unit tests. This series fix it.

Semantics.

> 
> > 
> > > That
> > > is the reason why we need KUnit support for such tests.
> > >   
> > > > CI expects that behavior, users 
> > > > are used to it.  We shouldn't change that only because we switch from an old 
> > > > underlying kernel selftest format to a new shiny one, I believe, unless we are 
> > > > able to prove that there was something wrong with that former approach, or we 
> > > > can explain why it no longer fits into the kunit variant.  
> > > 
> > > Yes, there's something wrong with the former approach:   
> > 
> > Are you saying that error handling in igt_ksleftest() is broken?  In my 
> > opinion, i915 selftests successfully executing on CI now and again, as well as 
> > the outdated DRM selftests successfully skipping, prove something different.
> 
> No, I'm saying that drm core tests are broken on IGT, because drm core doesn't
> support the legacy selftest infrastructure since 2022, and IGT is currently
> missing kUnit support.
> 
> On other words, IGT won't run such tests *even* if the Kernel is build with
> support for DRM core tests.
> 
> > I believe that igt_kunit() that follows as closely as possible the pattern of 
> > how igt_kselftest() is handling unmet conditions and errors, with full respect 
> > to real differences between kunit and i915-like selftests, would be the best 
> > solution.
> 
> I'm not saying anthing different, but KUnit and selftest executions are
> different:
> 
> - with selftest, the module loads asynchronously. After the module is
>   loaded with success, the probe() method will be called and the tests
>   will run.
> 
>   It means that it is possible to first load the driver and then run
>   igt dynamic subtest logic, parsing the results from dmesg.

Please have a look at drivers/gpu/drm/i915/i915_module.c, see how 
i915_mock_selftests() is called, then reevaluate your opinion on kunit doing 
that in a different way.

Thanks,
Janusz


> 
> - with KUnit, the unit tests will run synchronously at modprobe time.
>   So, when modprobe() finishes execution, all KUnit tests were already
>   executed - and if the Kernel crashes on a KUnit test - results will be
>   lost.
>   
>   It means that all steps needed to execute KUnit should be handled
>   before modprobing, including starting a thread to capture dmesg
>   results during modprobe time. So, igt dynamic subtests should be created 
>   before calling modprobe() function.
> 
> due to such difference, igt_subtest_with_dynamic() should be called
> before modprobe(), and, if something goes wrong on that time - for instance
> if the module doesn't exist and returns EENOENT - IGT needs to use
> igt_skip() or igt_fail().
> 
> Regards,
> Mauro
> 






^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2023-06-14  9:40 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-13  7:27 [igt-dev] [PATCH v7 i-g-t 00/10] Introduce KUnit Dominik Karol Piatkowski
2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 01/10] lib/igt_kmod: rename kselftest functions to ktest Dominik Karol Piatkowski
2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 02/10] lib/igt_kmod.c: check if module is builtin before attempting to unload it Dominik Karol Piatkowski
2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 03/10] lib/igt_kmod: add compatibility for KUnit Dominik Karol Piatkowski
2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 04/10] tests: DRM selftests: switch to KUnit Dominik Karol Piatkowski
2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 05/10] Change logic of ktap parser to run on a thread Dominik Karol Piatkowski
2023-06-13  8:44   ` Mauro Carvalho Chehab
2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 06/10] kunit tests: add an optional name for the selftests Dominik Karol Piatkowski
2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 07/10] KUnit: Remove igt_kselftest fallback Dominik Karol Piatkowski
2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 08/10] KUnit: Change subtest name from all-tests to module name Dominik Karol Piatkowski
2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 09/10] tests/xe: Add a test that launches the xe driver live kunit tests Dominik Karol Piatkowski
2023-06-13  7:27 ` [igt-dev] [PATCH i-g-t 10/10] KUnit: replace abort with graceful skip Dominik Karol Piatkowski
2023-06-13  8:41   ` Mauro Carvalho Chehab
2023-06-13  9:11     ` Janusz Krzysztofik
2023-06-13  9:58       ` Mauro Carvalho Chehab
2023-06-13 11:15         ` Janusz Krzysztofik
2023-06-13 13:34           ` Mauro Carvalho Chehab
2023-06-13 14:25             ` Janusz Krzysztofik
2023-06-14  7:58               ` Mauro Carvalho Chehab
2023-06-14  9:40                 ` Janusz Krzysztofik
2023-06-13  8:20 ` [igt-dev] ✓ Fi.CI.BAT: success for Introduce KUnit (rev7) Patchwork
2023-06-13 11:13 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.