Hi Janusz,
> The smem-oom subtest can expectedly result in oom-killer being triggered,
> which then dumps a call trace from a process that triggered it. If that
> happens to be a process that executes drm or i915 functions then the call
> trace dump contains lines recognized by igt_runner running in piglit mode
> as potential warnings. If severity of the call trace dump messages is
> NOTICE or higher, which isn't unlikely, then a dmesg-warn result is
> reported despite successful completion of the subtest.
>
> Fortunately, severity of those call trace dump messages depends on kernel
> default log level which can be controlled from user space over sysctl.
>
> To avoid false failure reports, relax kernel default log level to INFO so
> those log lines are ignored by igt_runner in piglit mode at an expense of
> call traces from real issues potentially detected by the subtest not
> contributing to the igt_runner reported result. Since those call traces
> are still available to developers, only submitted with reduced severity,
> that shouldn't hurt as long as the igt_runner still abandons further
> execution and reports an abort result on a kernel taint.
>
> v6: Expect fprintf() to return 3, not a mistyped 6 (Kamil),
> - fclose() the stream if not null, leaking it not only doesn't look good
> but also results in fprintf() output potentially not flushed.
> v5: Fix abort skipped on memory allocation failure or other error.
> v4: In the exit handler, restore the default log level with a simple
> write() from a formerly prepared buffer, and abort verbosely if that
> doesn't succeed (Kamil).
> v3: Move cleanup to an exit handler in case we are killed (Kamil).
> v2: Move default log level setup inside subtest smem-oom (Kamil),
> - move cleanup there as well.
>
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5493
> Cc: Kamil Konieczny <[email protected]>
> Signed-off-by: Janusz Krzysztofik <[email protected]>
[...]
> static void smem_oom_exit_handler(int sig)
> @@ -861,8 +891,63 @@ int igt_main_args("", long_options, help_str,
> opt_handler, NULL)
> }
>
> igt_describe("Exercise local memory swapping during exhausting system
> memory");
> - dynamic_lmem_subtest(region, regions, "smem-oom")
> - test_smem_oom(i915, ctx, region);
> + igt_subtest_with_dynamic("smem-oom") {
> + int console_log_level, default_log_level;
> + unsigned int fd, i = 0;
> + FILE *stream = NULL;
> +
> + /*
> + * This subtest can result in oom-killer being triggered, which
> + * then dumps a call trace from a process that triggered it.
> + * If that happens to be a process that executes drm or i915
> + * functions then the call trace dump contains lines recognized
> + * by igt_runner as warnings and a dmesg-warn result is
> + * reported. To avoid false failure reports, relax kernel
> + * default log level to INFO for those lines to be ignored by
You could also indicate here that the desired log level is
represented by "6" to relieve users'/developers' memory:
"...default log level to INFO (6) for those lines to be
ignored by..."
or, since this value is important:
#define LOG_LEVEL_INFO 6
and use that in the code below
> + * igt_runner in piglit mode, at an expense of call traces from
> + * potential real issues not contributing to the igt_runner
> + * reported result. Since those call traces are still available
> + * to developers, only displayed with relaxed severity, that
> + * shouldn't hurt as long as igt_runner still abandons further
> + * execution and reports an abort result on a kernel taint.
> + */
> + fd = open("/proc/sys/kernel/printk", O_RDWR);
> + if (!igt_debug_on(fd < 0))
> + stream = fdopen(fd, "r+");
> +
> + if (igt_debug_on(!stream))
> + close(fd);
> + else
> + i = fscanf(stream, "%d %d", &console_log_level,
> &default_log_level);
> +
> + if (igt_debug_on(i != 2) || default_log_level >= 6)
...here...
> + i = 0;
> + else
> + i = snprintf(log_levels, sizeof(log_levels), "%d %d",
> + console_log_level, default_log_level);
> +
> + if (!igt_debug_on(i != 3))
> + printk = dup(fd);
> +
> + if (!igt_debug_on(printk < 0)) {
> + igt_install_exit_handler(printk_exit_handler);
> +
> + rewind(stream);
> + igt_debug_on(fprintf(stream, "%d 6", console_log_level)
> != 3);
...and here.
> + }
> +
> + if (stream)
> + igt_debug_on(fclose(stream) == EOF);
> +
> + for (i = 0; i < regions->num_regions; i++) {
> + region = ®ions->regions[i];
> + if (region->region.memory_class ==
> I915_MEMORY_CLASS_DEVICE)
> + igt_dynamic_f("lmem%u",
> region->region.memory_instance)
> + test_smem_oom(i915, ctx, region);
> + }
> +
> + printk_exit_handler(0);
> + }
>
> igt_fixture() {
> intel_allocator_multiprocess_stop();
> --
> 2.52.0
>
After you address above:
Reviewed-by: Krzysztof Karas <[email protected]>
--
Best Regards,
Krzysztof
