Hi Sebastian, Thanks for review.
On Tuesday, 4 November 2025 13:54:30 CET Sebastian Brzezinka wrote: > Hi Janusz > On Tue Nov 4, 2025 at 11:20 AM UTC, Janusz Krzysztofik wrote: > > Certain selftests, while basically correct, may fail on certain platforms. > > E.g., igt@dmabuf@all-test@dma_fence_chain used to complete successfully, > > but on slow machines it triggers soft lockup warnings which taint the > > kernel. > > > > Sometimes, like in the above mentioned case, it's not possible to fix a > > root cause of the issue since it is not recognized as a bug. To avoid > > ever returning CI bug reports in such cases, allow selftests to be called > > via user provided wrappers that take care of not triggering unavoidable > > failures, e.g. by skipping specific selftests if some conditions are not > > met, or watching their execution and acting upon certain conditions or > > events. > > > > With that in place, update the dmabuf test so it, as the first user of the > > new feature, skips the dma_fence_chain selftest if a machine looks too > > slow. Since that's a hardware agnostic selftest, running it on a limited > > subset of machines seems acceptable, especially when the soft lockups it > > can trigger aren't recognized as bugs on the kernel side. > > > > Signed-off-by: Janusz Krzysztofik <[email protected]> > > --- > > lib/igt_kmod.c | 13 +++++++--- > > lib/igt_kmod.h | 10 ++++++- > > tests/dmabuf.c | 52 ++++++++++++++++++++++++++++++++++++- > > tests/intel/i915_selftest.c | 6 ++--- > > 4 files changed, 73 insertions(+), 8 deletions(-) > > > > diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c > > index a10626eedf..68ab4dbd57 100644 > > --- a/lib/igt_kmod.c > > +++ b/lib/igt_kmod.c > > @@ -1355,7 +1355,8 @@ static const char *unfilter(const char *filter, const > > char *name) > > void igt_kselftests(const char *module_name, > > const char *options, > > const char *result, > > - const char *filter) > > + const char *filter, > > + igt_kselftest_wrap_t wrapper) > > { > > struct igt_ktest tst; > > IGT_LIST_HEAD(tests); > > @@ -1370,10 +1371,16 @@ void igt_kselftests(const char *module_name, > > igt_kselftest_get_tests(tst.kmod, filter, &tests); > > igt_subtest_with_dynamic(filter ?: "all-tests") { > > igt_list_for_each_entry_safe(tl, tn, &tests, link) { > > + const char *dynamic_name = unfilter(filter, tl->name); > > unsigned long taints; > > > > - igt_dynamic_f("%s", unfilter(filter, tl->name)) > > - igt_kselftest_execute(&tst, tl, options, > > result); > > + igt_dynamic_f("%s", dynamic_name) { > > + if (wrapper) > > + wrapper(dynamic_name, &tst, tl); > > + else > > + igt_kselftest_execute(&tst, tl, > > + options, result); > > + } > > free(tl); > > > > if (igt_kernel_tainted(&taints)) { > > diff --git a/lib/igt_kmod.h b/lib/igt_kmod.h > > index 9050708974..c9700240c9 100644 > > --- a/lib/igt_kmod.h > > +++ b/lib/igt_kmod.h > > @@ -28,6 +28,13 @@ > > > > #include "igt_list.h" > > > > +struct igt_ktest; > > +struct igt_kselftest_list; > I would avoid using this declaration. I’d rather place the function > pointer declaration lower in the code. The above declarations are needed for typedef of the wrapper function that follows. Whether those lines are placed at the top of the header file or in front of the declaration of a function that uses the latter is a matter of personal preferences, I believe. Anyway, I'd like to hear from Kamil what's his preference here before I change the order. Thanks, Janusz > > Overall, it’s a neat idea. I tried to think of what could go wrong, > but especially for a hardware agnostic test, it seems worth trying. > > Reviewed-by: Sebastian Brzezinka <[email protected]> >
