All of lore.kernel.org
 help / color / mirror / Atom feed
From: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
To: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	Jani Nikula <jani.nikula@linux.intel.com>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	michal.wajdeczko@intel.com, intel-gfx@lists.freedesktop.org,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	Janusz Krzysztofik <janusz.krzysztofik@intel.com>
Subject: [PATCH] drm/i915: Fix context IDs not released on driver hot unbind
Date: Thu,  4 Apr 2019 12:24:45 +0200	[thread overview]
Message-ID: <20190404102445.12303-1-janusz.krzysztofik@linux.intel.com> (raw)

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

In case the driver gets unbound while a device is open, kernel panic
may be forced if a list of allocated context IDs is not empty.

When a device is open, the list may happen to be not empty because a
context ID, once allocated by a context ID allocator to a context
assosiated with that open file descriptor, is released as late as
on device close.

On the other hand, there is a need to release all allocated context IDs
and destroy the context ID allocator on driver unbind, even if a device
is open, in order to free memory resources consumed and prevent from
memory leaks.  The purpose of the forced kernel panic was to protect
the context ID allocator from being silently destroyed if not all
allocated IDs had been released.

Before forcing the kernel panic on non-empty list of allocated context
IDs, do that unlikely on non-empty list of contexts that should be
freed by preceding drain of work queue (there must be another bug if
that list happens to be not empty).  If empty, we may assume that
remaining contexts are idle (not pinned) and their IDs can be safely
released.

Once done, release context IDs of each of those remaining contexts
unless it happens a context is unlikely pinned.  Force kernel panic in
that case, there must be still another bug in the driver code.

Now the kernel panic protecting the allocator should not pop up as the
list it checks should be empty.  If it unlikely happens to be not
empty, there must be still another bug.

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

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 280813a4bf82..18d004d94e43 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -611,6 +611,8 @@ void i915_gem_contexts_lost(struct drm_i915_private *dev_priv)
 
 void i915_gem_contexts_fini(struct drm_i915_private *i915)
 {
+	struct i915_gem_context *ctx, *cn;
+
 	lockdep_assert_held(&i915->drm.struct_mutex);
 
 	if (i915->preempt_context)
@@ -618,6 +620,14 @@ void i915_gem_contexts_fini(struct drm_i915_private *i915)
 	destroy_kernel_context(&i915->kernel_context);
 
 	/* Must free all deferred contexts (via flush_workqueue) first */
+	GEM_BUG_ON(!llist_empty(&i915->contexts.free_list));
+
+	/* Release all remaining HW IDs before ID allocator is destroyed */
+	list_for_each_entry_safe(ctx, cn, &i915->contexts.hw_id_list,
+				 hw_id_link) {
+		GEM_BUG_ON(atomic_read(&ctx->hw_id_pin_count));
+		release_hw_id(ctx);
+	}
 	GEM_BUG_ON(!list_empty(&i915->contexts.hw_id_list));
 	ida_destroy(&i915->contexts.hw_ida);
 }
-- 
2.20.1


             reply	other threads:[~2019-04-04 10:24 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-04 10:24 Janusz Krzysztofik [this message]
2019-04-04 10:28 ` [Intel-gfx] [PATCH] drm/i915: Fix context IDs not released on driver hot unbind Chris Wilson
2019-04-04 10:28   ` Chris Wilson
2019-04-04 10:40   ` [Intel-gfx] " Janusz Krzysztofik
2019-04-04 10:40     ` Janusz Krzysztofik
2019-04-04 10:43     ` Chris Wilson
2019-04-04 10:43       ` Chris Wilson
2019-04-04 10:50       ` Janusz Krzysztofik
2019-04-04 10:53         ` Chris Wilson
2019-04-04 10:53           ` Chris Wilson
2019-04-04 13:47           ` Jani Nikula
2019-04-04 16:57 ` ✗ Fi.CI.BAT: failure for " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190404102445.12303-1-janusz.krzysztofik@linux.intel.com \
    --to=janusz.krzysztofik@linux.intel.com \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=janusz.krzysztofik@intel.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.wajdeczko@intel.com \
    --cc=rodrigo.vivi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.