In the next patch, we want to reduce the lock coverage within the shrinker, and one of the dangerous walks we have is over obj->vma_list. We are only walking the obj->vma_list in order to check whether it has been permanently pinned by HW access, typically via use on the scanout. But we have a couple of other long term pins, the context objects for which we currently have to check the individual vma pin_count. If we instead mark these using obj->pin_display, we can forgo the dangerous and sometimes slow list iteration. Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> --- drivers/gpu/drm/i915/i915_gem_shrinker.c | 27 +++++++-------------------- drivers/gpu/drm/i915/intel_lrc.c | 2 ++ drivers/gpu/drm/i915/intel_ringbuffer.c | 7 ++++++- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c index 80caeadb9d34..89b62b8045a5 100644 --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c @@ -71,25 +71,6 @@ static void shrinker_unlock(struct drm_i915_private *dev_priv, bool unlock) mutex_unlock(&dev_priv->drm.struct_mutex); } -static bool any_vma_pinned(struct drm_i915_gem_object *obj) -{ - struct i915_vma *vma; - - list_for_each_entry(vma, &obj->vma_list, obj_link) { - /* Only GGTT vma may be permanently pinned, and are always - * at the start of the list. We can stop hunting as soon - * as we see a ppGTT vma. - */ - if (!i915_vma_is_ggtt(vma)) - break; - - if (i915_vma_is_pinned(vma)) - return true; - } - - return false; -} - static bool swap_available(void) { return get_nr_swap_pages() > 0; @@ -115,7 +96,13 @@ static bool can_release_pages(struct drm_i915_gem_object *obj) if (atomic_read(&obj->mm.pages_pin_count) > obj->bind_count) return false; - if (any_vma_pinned(obj)) + /* If any vma are permanently pinned, it will prevent us from reclaiming + * the obj->mm.pages. We only allow scanout objects to claim a permanent + * pin, along with a few others like the reserved context object. To + * simplify the scan, and to avoid walking the list of vma under the + * object, we just check the count of its permanently pinned. + */ + if (obj->pin_display) return false; /* We can only return physical pages to the system if we can either diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index b0738d2b2a7f..874562bd59ae 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -866,6 +866,7 @@ execlists_context_pin(struct intel_engine_cs *engine, i915_ggtt_offset(ce->ring->vma); ce->state->obj->mm.dirty = true; + ce->state->obj->pin_display++; i915_gem_context_get(ctx); out: @@ -892,6 +893,7 @@ static void execlists_context_unpin(struct intel_engine_cs *engine, return; intel_ring_unpin(ce->ring); + ce->state->obj->pin_display--; i915_gem_object_unpin_map(ce->state->obj); i915_vma_unpin(ce->state); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index cdf084ef5aae..c90d3ea951e8 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1321,6 +1321,7 @@ int intel_ring_pin(struct intel_ring *ring, if (IS_ERR(addr)) goto err; + vma->obj->pin_display++; ring->vaddr = addr; return 0; @@ -1352,6 +1353,7 @@ void intel_ring_unpin(struct intel_ring *ring) i915_gem_object_unpin_map(ring->vma->obj); ring->vaddr = NULL; + ring->vma->obj->pin_display--; i915_vma_unpin(ring->vma); } @@ -1515,6 +1517,7 @@ intel_ring_context_pin(struct intel_engine_cs *engine, if (ret) goto err; + ce->state->obj->pin_display++; ce->state->obj->mm.dirty = true; } @@ -1550,8 +1553,10 @@ static void intel_ring_context_unpin(struct intel_engine_cs *engine, if (--ce->pin_count) return; - if (ce->state) + if (ce->state) { + ce->state->obj->pin_display--; i915_vma_unpin(ce->state); + } i915_gem_context_put(ctx); } -- 2.13.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx