[PATCH 1/1] drm/i915: Drop unnecessary WARN on driver unbind

[Janusz Krzysztofik]

From: Janusz Krzysztofik <janusz.krzysztofik at intel.com>

During i915_driver_unload(), GEM contexts are verified restrictively
inside i915_gem_fini() if they don't consume shared resources which
should be cleaned up before the driver is released.  If those checks
don't result in kernel panic, one more check is performed at the end of
i915_gem_fini() which issues a WARN_ON() if GEM contexts still exist.

Some GEM contexts are allocated unconditionally on device file open,
one per each file descriptor, and are kept open until those file
descriptors are closed.  Since open file descriptors prevent the driver
module from being unloaded, that protects the driver from being
released while contexts are still open.  However, that's not the case
on driver unbind or device unplug sysfs operations which are executed
regardless of open file descriptors.

To protect kernel resources from being accessed by those open file
decriptors while the driver unbind or the device unplug operation is in
progress, the driver now calls drm_device_unplug() at the beginning of
that process and relies on the drm layer to provide such protection.

Taking all above information into account, as soon as shared resources
not associated with specific file descriptors are cleaned up, it should
be safe to wait with completion of driver release until users of those
open file decriptors give up on errors and close them.

Use WARN_ON() conditionally so the warning is only displayed if a
context not accociated with a file descriptor is still allocated.
For debugging porposes, add a message which pops up while contexts
associated with open file descriptors are released on file close.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         | 7 ++++++-
 drivers/gpu/drm/i915/i915_gem_context.c | 2 ++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 54f27cabae2a..e6a060e8e4ba 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4644,6 +4644,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
 
 void i915_gem_fini(struct drm_i915_private *dev_priv)
 {
+	struct i915_gem_context *ctx;
+
 	GEM_BUG_ON(dev_priv->gt.awake);
 
 	i915_gem_suspend_late(dev_priv);
@@ -4670,7 +4672,10 @@ void i915_gem_fini(struct drm_i915_private *dev_priv)
 
 	i915_gem_drain_freed_objects(dev_priv);
 
-	WARN_ON(!list_empty(&dev_priv->contexts.list));
+	list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
+		WARN_ON(!ctx->file_priv);
+		break;
+	}
 }
 
 void i915_gem_init_mmio(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 76ed74e75d82..be21b62c07e2 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -247,6 +247,8 @@ static void i915_gem_context_free(struct i915_gem_context *ctx)
 	if (ctx->timeline)
 		i915_timeline_put(ctx->timeline);
 
+	if (ctx->name)
+		DRM_DEBUG_DRIVER("Dropping GEM context %s\n", ctx->name);
 	kfree(ctx->name);
 	put_pid(ctx->pid);
 
-- 
2.21.0