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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@xxxxxxxxxxxxxxxxxx>
---
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@xxxxxxxxx>

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


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux