On Sat, 2017-08-12 at 12:51 +0100, Chris Wilson wrote:
> 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.
s/pin_display/pin_permanent/g
>
> Signed-off-by: Chris Wilson
> 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
Oh, it sounds so much better here when the variable is 'pin_permanent'.
> + * 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++;
This should be closer to intel_ring_pin (my preference).
>
> 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--;
Or this should be closer to i915_gem_context_put. Just make it
symmetrict.
> @@ -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;
This is rather symmetric, the above can have mm.dirty after it, too.
As 'pin_permanent', this is;
Reviewed-by: Joonas Lahtinen
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
