From: Chris Wilson <chris@chris-wilson.co.uk>
Date: Mon, 15 Jan 2018 20:57:59 +0000
Subject: drm/i915: Rewrite some comments around RCU-deferred object free
Git-commit: 2ef1e729c7f0fbad9706848f5fe5cff84f5b6d24
Patch-mainline: v4.17-rc1
References: FATE#326289 FATE#326079 FATE#326049 FATE#322398 FATE#326166
Tvrtko noticed that the comments describing the interaction of RCU and
the deferred worker for freeing drm_i915_gem_object were a little
confusing, so attempt to bring some sense to them.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180115205759.13884-1-chris@chris-wilson.co.uk
Acked-by: Petr Tesarik <ptesarik@suse.com>
---
drivers/gpu/drm/i915/i915_gem.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4710,7 +4710,8 @@ static void __i915_gem_free_work(struct
container_of(work, struct drm_i915_private, mm.free_work);
struct llist_node *freed;
- /* All file-owned VMA should have been released by this point through
+ /*
+ * All file-owned VMA should have been released by this point through
* i915_gem_close_object(), or earlier by i915_gem_context_close().
* However, the object may also be bound into the global GTT (e.g.
* older GPUs without per-process support, or for direct access through
@@ -4737,10 +4738,15 @@ static void __i915_gem_free_object_rcu(s
container_of(head, typeof(*obj), rcu);
struct drm_i915_private *i915 = to_i915(obj->base.dev);
- /* We can't simply use call_rcu() from i915_gem_free_object()
- * as we need to block whilst unbinding, and the call_rcu
- * task may be called from softirq context. So we take a
- * detour through a worker.
+ /*
+ * Since we require blocking on struct_mutex to unbind the freed
+ * object from the GPU before releasing resources back to the
+ * system, we can not do that directly from the RCU callback (which may
+ * be a softirq context), but must instead then defer that work onto a
+ * kthread. We use the RCU callback rather than move the freed object
+ * directly onto the work queue so that we can mix between using the
+ * worker and performing frees directly from subsequent allocations for
+ * crude but effective memory throttling.
*/
if (llist_add(&obj->freed, &i915->mm.free_list))
queue_work(i915->wq, &i915->mm.free_work);
@@ -4756,7 +4762,8 @@ void i915_gem_free_object(struct drm_gem
if (discard_backing_storage(obj))
obj->mm.madv = I915_MADV_DONTNEED;
- /* Before we free the object, make sure any pure RCU-only
+ /*
+ * Before we free the object, make sure any pure RCU-only
* read-side critical sections are complete, e.g.
* i915_gem_busy_ioctl(). For the corresponding synchronized
* lookup see i915_gem_object_lookup_rcu().