diff mbox series

[RFC] drm/i915: Don't panic on non-empty list of free cachelines

Message ID 20190405121331.6524-1-janusz.krzysztofik@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [RFC] drm/i915: Don't panic on non-empty list of free cachelines | expand

Commit Message

Janusz Krzysztofik April 5, 2019, 12:13 p.m. UTC 
From: Janusz Krzysztofik <janusz.krzysztofik@intel.com>

If there are active users of a device during driver unbind, the driver
now panics on non-empty list of free cachelines.

By design, chachelines which are not in use are kept on a list of free
chachelines associated with a timeline and rmoved from that list either
when in use or when the timeline is destroyed.  Timelines in turn are
assigned to open file descriptors.

As long as a device file is open, its associated timeline with its list
of free cachelines will be hopefully destroyed on device close, either
while outstanding execlists are destroyed or on i915_timeline_put()
called directly, so as long as device file descriptors are protected
from unwanted user activities by the device being marked unplugged,
there should be no reason to panic.

Moreover, timeline mutex which is destroyed right after the check for
emptyness of a free cacheline list succeeds is never used to protect
that list, only a list of active cachelines, so it can be freely
destroyed even if the former is not empty.

Simply remove the GEM_BUG_ON(!list_empty(&gt->hwsp_free_list)); line
from i915_timelines_fini().

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
---
 drivers/gpu/drm/i915/i915_timeline.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Chris Wilson April 5, 2019, 12:20 p.m. UTC | #1 
Quoting Janusz Krzysztofik (2019-04-05 13:13:31)
> From: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
> 
> If there are active users of a device during driver unbind, the driver
> now panics on non-empty list of free cachelines.

This panic is there to say that fini is being called with active
contexts, that it is being called too early. Those requests should be
cleaned up first, unpinning the contexts and resources, and so letting
the timeline be freed.
-Chris
Janusz Krzysztofik April 5, 2019, 12:42 p.m. UTC | #2 
On Fri, 2019-04-05 at 13:20 +0100, Chris Wilson wrote:
> Quoting Janusz Krzysztofik (2019-04-05 13:13:31)
> > From: Janusz Krzysztofik <janusz.krzysztofik@intel.com>
> > 
> > If there are active users of a device during driver unbind, the
> > driver
> > now panics on non-empty list of free cachelines.
> 
> This panic is there to say that fini is being called with active
> contexts, that it is being called too early. Those requests should be
> cleaned up first, unpinning the contexts and resources, and so
> letting
> the timeline be freed.

OK, I see.  But why panic?  Maybe a WARN() would be enough.

Thanks,
Janusz

> -Chris
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_timeline.c b/drivers/gpu/drm/i915/i915_timeline.c
index b2202d2e58a2..1f23c2dcc0da 100644
--- a/drivers/gpu/drm/i915/i915_timeline.c
+++ b/drivers/gpu/drm/i915/i915_timeline.c
@@ -325,7 +325,6 @@  void i915_timelines_fini(struct drm_i915_private *i915)
 	struct i915_gt_timelines *gt = &i915->gt.timelines;
 
 	GEM_BUG_ON(!list_empty(&gt->active_list));
-	GEM_BUG_ON(!list_empty(&gt->hwsp_free_list));
 
 	mutex_destroy(&gt->mutex);
 }