Hi Janusz,
On 2025-11-04 at 12:20:24 +0100, 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]>
> ---
>
[...]
> +static unsigned int bogomips(void)
> +{
> + unsigned int bogomips, ret = 0;
> + char *line = NULL;
> + size_t size = 0;
> + FILE *cpuinfo;
> +
> + cpuinfo = fopen("/proc/cpuinfo", "r");
> + if (igt_debug_on(!cpuinfo))
> + return UINT_MAX;
Could this result in running the test on a slower machine
unintentionally? fopen() could fail, the bogomips() would return
a large value and wrapper function would behave as if the test
is running on a faster platform.
> +
> + while (getline(&line, &size, cpuinfo) != -1) {
It is unlikely, but getline() may fail with EINVAL or ENOMEM, so
maybe while(getline(&line, &size, cpuinfo) > 0)?
> + char *colon;
> +
> + if (strncmp(line, "bogomips", 8))
> + continue;
> +
> + colon = strchr(line, ':');
> + if (igt_debug_on(!colon))
> + bogomips = 0;
> + else
> + bogomips = atoi(colon + 1);
> +
> + if (igt_debug_on(!bogomips))
> + break;
> +
> + ret += bogomips;
> + }
> + free(line);
> + fclose(cpuinfo);
> +
> + return igt_debug_on(!bogomips) ? UINT_MAX : ret;
Same as my first comment in this function: will this not cause
sporadic runs on machines that are supposed to skip the test?
> +}
> +
> +static int wrapper(const char *dynamic_name,
The name could be more descriptive about what function does/is
used for, so maybe "check_skip_wrapper" would better achieve
that?
[...]
--
Best Regards,
Krzysztof
