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 <[email protected]> <SNIP> > 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 <[email protected]> Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
