[Intel-gfx] [PATCH] drm/i915/perf: Refactor oa object to better manage resources

[Umesh Nerlige Ramappa]

On Wed, May 15, 2019 at 10:11:34AM +0100, Lionel Landwerlin wrote:
>On 14/05/2019 19:14, Umesh Nerlige Ramappa wrote:
>>On Tue, May 14, 2019 at 10:34:49AM +0100, Lionel Landwerlin wrote:
>>>Hi Umesh,
>>>
>>>I just noticed this different between v1 & v2.
>>>My understanding is that if destroy() is called, stream should be 
>>>the same as dev_priv->perf.exclusive_stream.
>>>If it's not it sounds like a bug. So why change this?
>>>
>>v2 fixes only checkpatch warnings. it warned on use of BUG_ON. 
>>BUG_ON is intended to crash the system in severe cases where the 
>>driver/kernel is unusable. In this case, the mismatch between user 
>>passed information and exclusive_stream may not require a crash.
>
>
>This is called from i915_perf_release() which is attached to the 
>i915-perf file descriptor only in i915_perf.c.
>
>If we managed to reach that function it must be because the file 
>descriptor given by userspace is associated to the i915-perf stream.
>
>Having stream != dev_priv->perf.exclusive_stream means that we 
>probably screwed up the locking somewhere in this file.
>
>So I would argue this is a kernel issue, not a user issue and that 
>using BUG_ON() is justified.
>
I see. Will change it back.

Thanks,
Umesh
>
>Thanks,
>
>
>-Lionel
>
>
>>>-Lionel
>>>
>>>On 03/05/2019 00:13, Umesh Nerlige Ramappa wrote:
>>>> static void i915_oa_stream_destroy(struct i915_perf_stream *stream)
>>>> {
>>>>     struct drm_i915_private *dev_priv = stream->dev_priv;
>>>>-    BUG_ON(stream != dev_priv->perf.oa.exclusive_stream);
>>>>+    if (stream != dev_priv->perf.exclusive_stream) {
>>>>+        WARN_ON_ONCE(stream != dev_priv->perf.exclusive_stream);
>>>>+        return;
>>>>+    }
>>>>     /*
>>>
>>>
>>
>