On Wednesday, 5 November 2025 14:10:48 CET Sebastian Brzezinka wrote:
> On Wed Nov 5, 2025 at 12:52 PM UTC, Janusz Krzysztofik wrote:
> > 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.
> I meant that you can slightly rearrange the code,
> and the forward declarations will no longer be needed.
Neither struct igt_ktest nor struct igt_kselftest_list internals are used
outside of lib/igt_kmod.c, then those structure definitions belong to that
source, not to the header, I believe. That's why I didn't consider making use
of them, only added forward declarations as needed for those structures,
opaque as they should be.
Thanks,
Janusz
>
> ```
> diff --git a/lib/igt_kmod.h b/lib/igt_kmod.h
> index c9700240c..5a2ee1576 100644
> --- a/lib/igt_kmod.h
> +++ b/lib/igt_kmod.h
> @@ -28,13 +28,6 @@
>
> #include "igt_list.h"
>
> -struct igt_ktest;
> -struct igt_kselftest_list;
> -
> -typedef int (*igt_kselftest_wrap_t)(const char *dynamic_name,
> - struct igt_ktest *tst,
> - struct igt_kselftest_list *tl);
> -
> bool igt_kmod_is_loaded(const char *mod_name);
> void igt_kmod_list_loaded(void);
>
> @@ -83,12 +76,6 @@ int igt_amdgpu_driver_unload(void);
>
> void igt_kunit(const char *module_name, const char *name, const char *opts);
>
> -void igt_kselftests(const char *module_name,
> - const char *module_options,
> - const char *result_option,
> - const char *filter,
> - igt_kselftest_wrap_t wrapper);
> -
> struct igt_ktest {
> struct kmod_module *kmod;
> char *module_name;
> @@ -102,6 +89,16 @@ struct igt_kselftest_list {
> char param[];
> };
> +typedef int (*igt_kselftest_wrap_t)(const char *dynamic_name,
> + struct igt_ktest *tst,
> + struct igt_kselftest_list *tl);
> +
> +void igt_kselftests(const char *module_name,
> + const char *module_options,
> + const char *result_option,
> + const char *filter,
> + igt_kselftest_wrap_t wrapper);
> +
> int igt_ktest_init(struct igt_ktest *tst,
> const char *module_name);
> int igt_ktest_begin(struct igt_ktest *tst);
> ```
>
>
