Hi Kamil,

Thank you for looking at this.

On Thursday, 18 December 2025 15:21:25 CET Kamil Konieczny wrote:
> Hi Janusz,
> On 2025-12-17 at 15:50:30 +0100, Janusz Krzysztofik wrote:
> > 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.
> > 
> > Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5493
> > Signed-off-by: Janusz Krzysztofik <[email protected]>
> > ---
> >  tests/intel/gem_lmem_swapping.c | 40 ++++++++++++++++++++++++++++++++-
> >  1 file changed, 39 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/intel/gem_lmem_swapping.c 
> > b/tests/intel/gem_lmem_swapping.c
> > index adae26716c..ab951a7414 100644
> > --- a/tests/intel/gem_lmem_swapping.c
> > +++ b/tests/intel/gem_lmem_swapping.c
> > @@ -804,8 +804,9 @@ int igt_main_args("", long_options, help_str, 
> > opt_handler, NULL)
> >             { "parallel-random-verify-ccs", TEST_PARALLEL | TEST_RANDOM | 
> > TEST_CCS },
> >             { }
> >     };
> > +   int i915 = -1, console_log_level, default_log_level;
> >     const intel_ctx_t *ctx;
> > -   int i915 = -1;
> > +   FILE *printk;
> 
>       FILE *printk = NULL;
> 
> For a reason see below.
> 
> >  
> >     igt_fixture() {
> >             struct intel_execution_engine2 *e;
> > @@ -860,11 +861,48 @@ int igt_main_args("", long_options, help_str, 
> > opt_handler, NULL)
> >                     test_evict(i915, ctx, region, test->flags);
> >     }
> >  
> > +   /*
> > +    * The smem-oom 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 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.
> > +    */
> > +   igt_fixture() {
> > +           printk = fopen("/proc/sys/kernel/printk", "r+");
> > +           if (igt_debug_on(!printk))
> > +                   break;
> > +
> > +           if (!igt_debug_on(fscanf(printk, "%d %d",
> > +                                    &console_log_level, 
> > &default_log_level) != 2) &&
> > +               default_log_level < 6) {
> > +                   rewind(printk);
> > +                   igt_debug_on(fprintf(printk, "%d 6", console_log_level) 
> > != 3);
> > +           } else {
> > +                   fclose(printk);
> > +                   printk = NULL;
> > +           }
> > +   }
> > +
> 
> Looks good but please move it inside subtest smem-oom,
> so it will affect only it. Cleanup should be done in final fixup
> so that code is ok.

The only way I can imagine other subtests affected is if someone adds a 
new subtest below without taking care of splitting the loglevel cleanup 
out of the final igt_fixture() into a separate one, placed right after 
the "smem_oom" subtest and before any other future subtests.  But we can 
take care now, either by already splitting it into two consecutive 
igt_fixture sections, or by adding a reminder to do that before adding 
new subtests in between.  I think that would be more clear then hiding 
the loglevel setup steps inside the test_smem_oom().  Moreover, your 
approach doesn't address potential cases of running more than one 
subtest in a single invocation of the test, which is perfectly possible 
when igt_runner is used with --multiple-mode option or in manual runs.

As additional benefits, we neither have to pass extra arguments to the 
test_smem_oom(), nor have to take care for the loglevel setup being 
processed only once in case more than one LMEM region exists and 
test_smmem_oom() is called several times.

Thanks,
Janusz


> 
> Regards,
> Kamil
> 
> >     igt_describe("Exercise local memory swapping during exhausting system 
> > memory");
> >     dynamic_lmem_subtest(region, regions, "smem-oom")
> >             test_smem_oom(i915, ctx, region);
> >  
> >     igt_fixture() {
> > +           if (printk) {
> > +                   rewind(printk);
> > +                   igt_debug_on(fprintf(printk, "%d %d",
> > +                                        console_log_level, 
> > default_log_level) != 3);
> > +                   fclose(printk);
> > +           }
> > +
> >             intel_allocator_multiprocess_stop();
> >             intel_ctx_destroy(i915, ctx);
> >             free(regions);
> 




Reply via email to