[Intel-gfx] [PATCH] drm/i915: Add a warning on shutdown if signal threads still active

[Tvrtko Ursulin]

On 21/11/2016 10:48, Chris Wilson wrote:
> On Mon, Nov 21, 2016 at 10:42:37AM +0000, Tvrtko Ursulin wrote:
>>
>> On 21/11/2016 09:40, Chris Wilson wrote:
>>> When unloading the module, it is expected that we have finished
>>> executing all requests and so the signal threads should be idle. Add a
>>> warning in case there are any residual requests in the signaler rbtrees
>>> at that point.
>>>
>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>> ---
>>> drivers/gpu/drm/i915/intel_breadcrumbs.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
>>> index c9c46a538edb..b7006e90a167 100644
>>> --- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
>>> +++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
>>> @@ -623,6 +623,8 @@ void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine)
>>> {
>>> 	struct intel_breadcrumbs *b = &engine->breadcrumbs;
>>>
>>> +	WARN_ON(READ_ONCE(b->first_signal));
>>> +	WARN_ON(!RB_EMPTY_ROOT(&b->signals));
>>> 	if (!IS_ERR_OR_NULL(b->signaler))
>>> 		kthread_stop(b->signaler);
>>>
>>>
>>
>> Not sure if you are just testing out theories on the CI, but in any
>> case looks to me it wouldn't harm to have this in.
>
> Only staring at the impossible, ruling out the unlikely. A leak here
> would be caught by the request kmem_cache, but being explicit might help
> in future.
>
>> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> Imagine if I added a
>
> WARN_ON(READ_ONCE(b->first_waiter));
> WARN_ON(!RB_EMPTY_ROOT(&b->waiters));
>
> as well? Still r-b or go with a second patch for the afterthought?

It's the same thing conceptually so can expand and keep the r-b for what 
I am concerned.

Regards,

Tvrtko