Recent improvements to the kernel kunit framework allow us to obtain a
list of test cases provided by a kunit test module without actually
running them.  Use that feature to get a list of expected test cases
before we enter a loop around igt_dynamic().  Once done, enter the
igt_dynamic() section for each consecutive test case immediately, even
before first line of a related KTAP report appears, then look for a result
from that test case.  That should make our IGT results output still better
synchronized with related kernel messages.

The list of test cases provided by a kunit test module can be obtained by
loading the kunit base module with specific options, then loading the test
module.  For that to be possible, take care of unloading the kunit base
module before each kunit subtest (I was wrong when in one of my previous
commit messages I suggested that on final unload of a kunit test module
the kunit base module is unloaded automatically as its dependency,
however, that didn't matter before, then no separate fix was required).
Since that module can then be left loaded with non-default options if an
error occurs, unload it explicitly before returning from igt_kunit().

There are two possible ways of getting a list of test cases: by loading
the base kunit module with action=list module option, or by filtering
out all test cases from being executed while asking for SKIP results from
those filtered out.  Since the latter provides regular KTAP report that we
can alredy parse perfectly, use it instead of trying to identify an
unstructured list of test cases of unknown length submitted by the former.

If an IGT test that calls igt_kunit() provides a subtest name then use
that name to filter out potential test cases that don't belong to the
named test suite from the list.

To avoid loading any modules if no subtest is going to be executed (e.g.,
if a nonexistent subtest has been requested), load the kunit modules in
list mode from inside the igt_subtest_with_dynamic() section.  In order to
be free to skip the whole subtest on unmet requirements that need to be
verified after that list has been already populated, clean it up from a
follow up igt_fixture section.

Since we start reading the list of test cases from /dev/kmsg only after
the kunit test module is loaded successfully in list only mode, don't
synchronize reads with potential modprobe breakage in that case, unlike we
still do later when parsing KTAP results in parallel to loading the test
module in normal (execute) mode.

Since we neither fetch KTAP results before entering igt_dynamic section
nor even return an error from KTAP result fetch attempts immediately on
modprobe error or kernel taint, break the loop of dynamic sub-subtests
explicitly as soon as one of those conditions is detected.  Also, don't
force IGT SKIP result from the subtest if KTAP parsing hasn't completed.
That's perfectly legitimate since we no longer iterate over KTAP results,
only over a list of test cases obtained in advance, then we stop parsing
KTAP report as soon as we get a result from the last test case from the
list.

Signed-off-by: Janusz Krzysztofik <janusz.krzyszto...@linux.intel.com>
---
 lib/igt_kmod.c | 217 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 157 insertions(+), 60 deletions(-)

diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
index 387efbb59f..4fba77ead4 100644
--- a/lib/igt_kmod.c
+++ b/lib/igt_kmod.c
@@ -802,34 +802,36 @@ static int kunit_kmsg_result_get(struct igt_list_head 
*results,
                if (igt_debug_on(igt_kernel_tainted(&taints)))
                        return -ENOTRECOVERABLE;
 
-               err = igt_debug_on(sigaction(SIGCHLD, &sigchld, saved));
-               if (err == -1)
-                       return -errno;
-               else if (unlikely(err))
-                       return err;
-
-               err = pthread_mutex_lock(&modprobe->lock);
-               switch (err) {
-               case EOWNERDEAD:
-                       /* leave the mutex unrecoverable */
-                       igt_debug_on(pthread_mutex_unlock(&modprobe->lock));
-                       __attribute__ ((fallthrough));
-               case ENOTRECOVERABLE:
-                       igt_debug_on(sigaction(SIGCHLD, saved, NULL));
-                       if (igt_debug_on(modprobe->err))
-                               return modprobe->err;
-                       break;
-               case 0:
-                       break;
-               default:
-                       igt_debug("pthread_mutex_lock() error: %d\n", err);
-                       igt_debug_on(sigaction(SIGCHLD, saved, NULL));
-                       return -err;
+               if (modprobe) {
+                       err = igt_debug_on(sigaction(SIGCHLD, &sigchld, saved));
+                       if (err == -1)
+                               return -errno;
+                       else if (unlikely(err))
+                               return err;
+
+                       err = pthread_mutex_lock(&modprobe->lock);
+                       switch (err) {
+                       case EOWNERDEAD:
+                               /* leave the mutex unrecoverable */
+                               
igt_debug_on(pthread_mutex_unlock(&modprobe->lock));
+                               __attribute__ ((fallthrough));
+                       case ENOTRECOVERABLE:
+                               igt_debug_on(sigaction(SIGCHLD, saved, NULL));
+                               if (igt_debug_on(modprobe->err))
+                                       return modprobe->err;
+                               break;
+                       case 0:
+                               break;
+                       default:
+                               igt_debug("pthread_mutex_lock() error: %d\n", 
err);
+                               igt_debug_on(sigaction(SIGCHLD, saved, NULL));
+                               return -err;
+                       }
                }
 
                ret = read(fd, record, BUF_LEN);
 
-               if (!err) {
+               if (modprobe && !err) {
                        igt_debug_on(pthread_mutex_unlock(&modprobe->lock));
                        igt_debug_on(sigaction(SIGCHLD, saved, NULL));
                }
@@ -885,17 +887,15 @@ static void kunit_result_free(struct igt_ktap_result **r,
        *r = NULL;
 }
 
-static void
-__igt_kunit(struct igt_ktest *tst, const char *name, const char *opts)
+static void kunit_get_tests(struct igt_list_head *tests,
+                           struct igt_ktest *tst,
+                           const char *filter,
+                           const char *opts)
 {
-       struct modprobe_data modprobe = { pthread_self(), tst->kmod, opts, 0, };
        char *suite_name = NULL, *case_name = NULL;
        struct igt_ktap_result *r, *rn;
        struct igt_ktap_results *ktap;
-       pthread_mutexattr_t attr;
-       IGT_LIST_HEAD(results);
-       unsigned long taints;
-       int flags, ret;
+       int flags, err;
 
        igt_skip_on_f(tst->kmsg < 0, "Could not open /dev/kmsg\n");
 
@@ -905,6 +905,63 @@ __igt_kunit(struct igt_ktest *tst, const char *name, const 
char *opts)
 
        igt_skip_on(lseek(tst->kmsg, 0, SEEK_END) < 0);
 
+       /*
+        * To get a list of test cases provided by a kunit test module, ask the
+        * generic kunit module to respond with SKIP result for each test found.
+        * We could also use action=list kunit parameter to get the listing,
+        * however, parsing a KTAP report -- something that we alredy can do
+        * perfectly -- seems to be more safe than extracting a test case list
+        * of unknown length from /dev/kmsg.
+        */
+       igt_skip_on(igt_kmod_load("kunit", "filter=module=none 
filter_action=skip"));
+       igt_skip_on(modprobe(tst->kmod, opts));
+
+       ktap = igt_ktap_alloc(tests);
+       igt_require(ktap);
+
+       do {
+               err = kunit_kmsg_result_get(tests, NULL, tst->kmsg, ktap);
+               if (err && !igt_debug_on(err != -EINPROGRESS)) {
+                       r = igt_list_last_entry(tests, r, link);
+
+                       if (igt_debug_on(r->code != IGT_EXIT_SKIP))
+                               err = r->code ?: -EPROTO;
+               }
+
+       } while (err == -EINPROGRESS);
+
+       igt_ktap_free(ktap);
+
+       if (err || filter) {
+               igt_list_for_each_entry_safe(r, rn, tests, link) {
+                       if (err || strcmp(r->suite_name, filter))
+                               kunit_result_free(&r, &case_name, &suite_name);
+               }
+       }
+
+       igt_skip_on(kmod_module_remove_module(tst->kmod, KMOD_REMOVE_FORCE));
+       igt_skip_on(igt_kmod_unload("kunit", KMOD_REMOVE_FORCE));
+
+       igt_skip_on_f(err,
+                     "KTAP parser failed while getting a list of test 
cases\n");
+}
+
+static void __igt_kunit(struct igt_ktest *tst,
+                       const char *name,
+                       const char *opts,
+                       struct igt_list_head *tests)
+{
+       struct modprobe_data modprobe = { pthread_self(), tst->kmod, opts, 0, };
+       char *suite_name = NULL, *case_name = NULL;
+       struct igt_ktap_result *t, *r = NULL, *rn;
+       struct igt_ktap_results *ktap;
+       pthread_mutexattr_t attr;
+       IGT_LIST_HEAD(results);
+       int ret = -EINPROGRESS;
+       unsigned long taints;
+
+       igt_skip_on(lseek(tst->kmsg, 0, SEEK_END) < 0);
+
        igt_skip_on(pthread_mutexattr_init(&attr));
        igt_skip_on(pthread_mutexattr_setrobust(&attr, PTHREAD_MUTEX_ROBUST));
        igt_skip_on(pthread_mutex_init(&modprobe.lock, &attr));
@@ -918,37 +975,50 @@ __igt_kunit(struct igt_ktest *tst, const char *name, 
const char *opts)
                igt_skip("Failed to create a modprobe thread\n");
        }
 
-       do {
-               ret = kunit_kmsg_result_get(&results, &modprobe,
-                                           tst->kmsg, ktap);
-               if (igt_debug_on(ret && ret != -EINPROGRESS))
-                       break;
-
-               if (igt_debug_on(igt_list_empty(&results)))
-                       break;
-
-               r = igt_list_first_entry(&results, r, link);
-
+       igt_list_for_each_entry(t, tests, link) {
                igt_dynamic_f("%s%s%s",
-                             strcmp(r->suite_name, name) ?  r->suite_name : "",
-                             strcmp(r->suite_name, name) ? "-" : "",
-                             r->case_name) {
+                             strcmp(t->suite_name, name) ?  t->suite_name : "",
+                             strcmp(t->suite_name, name) ? "-" : "",
+                             t->case_name) {
+                       if (!r) {
+                               if (igt_list_empty(&results)) {
+                                       igt_assert_eq(ret, -EINPROGRESS);
+                                       ret = kunit_kmsg_result_get(&results,
+                                                                   &modprobe,
+                                                                   tst->kmsg,
+                                                                   ktap);
+                                       igt_fail_on(igt_list_empty(&results));
+                               }
 
-                       if (r->code == IGT_EXIT_INVALID) {
-                               /* parametrized test case, get actual result */
-                               kunit_result_free(&r, &suite_name, &case_name);
+                               r = igt_list_first_entry(&results, r, link);
+                       }
 
-                               igt_assert(igt_list_empty(&results));
+                       while (igt_debug_on_f(strcmp(r->suite_name, 
t->suite_name),
+                                             "suite_name expected: %s, got: 
%s\n",
+                                             t->suite_name, r->suite_name) ||
+                              igt_debug_on_f(strcmp(r->case_name, 
t->case_name),
+                                             "case_name expected: %s, got: 
%s\n",
+                                             t->case_name, r->case_name) ||
+                              r->code == IGT_EXIT_INVALID) {
 
-                               ret = kunit_kmsg_result_get(&results, &modprobe,
-                                                           tst->kmsg, ktap);
-                               if (ret != -EINPROGRESS)
-                                       igt_fail_on(ret);
+                               int code = r->code;
 
-                               igt_fail_on(igt_list_empty(&results));
+                               kunit_result_free(&r, &suite_name, &case_name);
+                               if (igt_list_empty(&results)) {
+                                       igt_assert_eq(ret, -EINPROGRESS);
+                                       ret = kunit_kmsg_result_get(&results,
+                                                                   &modprobe,
+                                                                   tst->kmsg,
+                                                                   ktap);
+                                       igt_fail_on(igt_list_empty(&results));
+                               }
 
                                r = igt_list_first_entry(&results, r, link);
 
+                               if (code != IGT_EXIT_INVALID)
+                                       continue;
+
+                               /* result from parametrized test case */
                                igt_fail_on_f(strcmp(r->suite_name, suite_name),
                                              "suite_name expected: %s, got: 
%s\n",
                                              suite_name, r->suite_name);
@@ -995,7 +1065,9 @@ __igt_kunit(struct igt_ktest *tst, const char *name, const 
char *opts)
 
                kunit_result_free(&r, &suite_name, &case_name);
 
-       } while (ret == -EINPROGRESS);
+               if (modprobe.err || igt_kernel_tainted(&taints))
+                       break;
+       }
 
        igt_list_for_each_entry_safe(r, rn, &results, link)
                kunit_result_free(&r, &suite_name, &case_name);
@@ -1025,7 +1097,8 @@ __igt_kunit(struct igt_ktest *tst, const char *name, 
const char *opts)
 
        igt_skip_on(modprobe.err);
        igt_skip_on(igt_kernel_tainted(&taints));
-       igt_skip_on_f(ret, "KTAP parser failed\n");
+       if (ret != -EINPROGRESS)
+               igt_skip_on_f(ret, "KTAP parser failed\n");
 }
 
 /**
@@ -1039,7 +1112,8 @@ __igt_kunit(struct igt_ktest *tst, const char *name, 
const char *opts)
 void igt_kunit(const char *module_name, const char *name, const char *opts)
 {
        struct igt_ktest tst = { .kmsg = -1, };
-
+       const char *filter = name;
+       IGT_LIST_HEAD(tests);
 
        /*
         * If the caller (an IGT test) provides no subtest name then we
@@ -1064,6 +1138,15 @@ void igt_kunit(const char *module_name, const char 
*name, const char *opts)
 
                igt_skip_on(igt_ktest_init(&tst, module_name));
                igt_skip_on(igt_ktest_begin(&tst));
+
+               /*
+                * Since we need to load kunit base module with specific
+                * options in order to get a list of test cases, make
+                * sure that the module is not loaded.  However, since
+                * unload may fail if kunit base module is not loaded,
+                * ignore any failures, we'll fail later if still loaded.
+                */
+               igt_ignore_warn(igt_kmod_unload("kunit", KMOD_REMOVE_FORCE));
        }
 
        /*
@@ -1073,11 +1156,25 @@ void igt_kunit(const char *module_name, const char 
*name, const char *opts)
         * proper namespace for dynamic subtests, with is required for CI
         * and for documentation.
         */
-       igt_subtest_with_dynamic(name)
-               __igt_kunit(&tst, name, opts);
+       igt_subtest_with_dynamic(name) {
+               kunit_get_tests(&tests, &tst, filter, opts);
+               igt_skip_on(igt_list_empty(&tests));
+
+               __igt_kunit(&tst, name, opts, &tests);
+       }
+
+       igt_fixture {
+               char *suite_name = NULL, *case_name = NULL;
+               struct igt_ktap_result *t, *tn;
+
+               igt_list_for_each_entry_safe(t, tn, &tests, link)
+                       kunit_result_free(&t, &suite_name, &case_name);
+               free(case_name);
+               free(suite_name);
 
-       igt_fixture
                igt_ktest_end(&tst);
+               igt_debug_on(igt_kmod_unload("kunit", KMOD_REMOVE_FORCE));
+       }
 
        igt_ktest_fini(&tst);
 }
-- 
2.42.0

Reply via email to