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.
```
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);
```
--
Best regards,
Sebastian
