On 30/04/2020 17:55, Chris Wilson wrote:
Quoting Lionel Landwerlin (2020-04-30 14:55:35)
@@ -1382,6 +1446,12 @@ static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
BUG_ON(stream != perf->exclusive_stream);
+ err = intel_context_pin(stream->config_context);
+ if (err) {
+ drm_err(&perf->i915->drm,
+ "Error pinning i915-perf context\n");
+ }
Reading through, this is the last thing that leapt out at me.
Taking an error here would be nasty, and we do allow the pin to be
interrupted (which would be a surprise for most people who call
close()).
The only suggestion I can make, is that the stream always keeps it
pinned.
>From the robustness point of view, it's better if we break
stream->config_context than if we break stream->engine->kernel_context,
but equally pinning another context image [about 80k in total] just for
safety? Probably worth it. [If we break the kernel_context, we cannot do
hang detection or pm/idle, the driver comes grinding to a halt.]
-Chris
Yeah, I didn't like writing this error path there either.
-Lionel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
