[igt-dev] [Intel-gfx] [PATCH i-g-t v2] i915/pm_rps: install SIGTERM handler for load_helper process

[Chris Wilson]

Quoting Chuansheng Liu (2019-11-20 06:29:12)
> Reference:
> https://bugs.freedesktop.org/show_bug.cgi?id=112126
> 
> The issue we hit is the GPU keeps very high load after running
> the subtest min-max-config-loaded.
> 
> Some background of the issue:
> Currently the rps is not fully enabled yet on TGL, and running
> the subtest min-max-config-loaded will hit below assertion:
> ==
> (i915_pm_rps:1261) CRITICAL: Test assertion failure function loaded_check, file ../tests/i915/i915_pm_rps.c:505:
> (i915_pm_rps:1261) CRITICAL: Failed assertion: freqs[MAX] <= freqs[CUR]
> (i915_pm_rps:1261) CRITICAL: Last errno: 2, No such file or directory
> ==
> 
> with igt stress test, we find the GT keeps busy after running
> this subtest, it is due to the igt_spin_end() is not called
> randomly.
> 
> The root cause analysis is:
> When the main process i915_pm_rps for running the subtest
> min-max-config-loaded hits the assertion, the main process will
> try to send signal SIGTERM to the child process loader_helper
> which is created by main process for starting GT load, then the
> main process itself will exit.
> 
> The SIGTERM handler for loader_helper is the default one, which
> will cause the loader_helper exits directly. That is unsafe, we
> always expect the igt_spin_end() is called before loader_helper
> process exits, which is used to stop the load of GT.
> 
> Furthermore, in normal scenario, before main process exits,
> it will send SIGUSR1 to child process for stopping GT loading
> in safe way.
> 
> So here we install the proper handler for signal SIGTERM in the
> similar way. Without this patch, the GT may keep busy after
> running this subtest. Enabling rps should be tracked on the
> other side.
> 
> Signed-off-by: Chuansheng Liu <chuansheng.liu at intel.com>
> ---
>  tests/i915/i915_pm_rps.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/i915/i915_pm_rps.c b/tests/i915/i915_pm_rps.c
> index ef627c0b..8c71c1a1 100644
> --- a/tests/i915/i915_pm_rps.c
> +++ b/tests/i915/i915_pm_rps.c
> @@ -252,6 +252,7 @@ static void load_helper_run(enum load load)
>  
>                 signal(SIGUSR1, load_helper_signal_handler);
>                 signal(SIGUSR2, load_helper_signal_handler);
> +               signal(SIGTERM, load_helper_signal_handler);

I don't see any behaviour changes to igt to cause it to send SIGTERM on
exit_subtest.

But you might as well just s/SIGUSR2/SIGTERM/ for clearer and common
intentions.
-Chris