On Thu, May 18, 2017 at 02:52:41PM +0100, Tvrtko Ursulin wrote:
>
> On 18/05/2017 10:46, Chris Wilson wrote:
> >Keep the recently freed context objects for reuse. This allows us to use
> >the current GGTT bindings and dma bound pages, avoiding any clflushes as
> >required. We mark the objects as purgeable under memory pressure, and
> >reap the list of freed objects as soon as the device is idle.
>
> Some description on when is this interesting and by how much?
No interesting workload (a few synthetic benchmarks including the GL).
Best argument would be a minor improvement in application startup.
> Since you drop everything on idle it must be for some workload which
> likes to go through context while keeping busy?
Yup, it's just the rule of thumb I have (from experience with the
cmdparser batch caching).
> >Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >---
> > drivers/gpu/drm/i915/i915_drv.h | 2 +
> > drivers/gpu/drm/i915/i915_gem.c | 1 +
> > drivers/gpu/drm/i915/i915_gem_context.c | 59 ++++++++++++++++++++++--
> > drivers/gpu/drm/i915/i915_gem_context.h | 5 ++
> > drivers/gpu/drm/i915/intel_lrc.c | 2 +-
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
> > drivers/gpu/drm/i915/selftests/mock_context.c | 1 +
> > drivers/gpu/drm/i915/selftests/mock_gem_device.c | 1 +
> > 8 files changed, 68 insertions(+), 5 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >index 1d55bbde68df..1fa1e7d48f02 100644
> >--- a/drivers/gpu/drm/i915/i915_drv.h
> >+++ b/drivers/gpu/drm/i915/i915_drv.h
> >@@ -2316,6 +2316,8 @@ struct drm_i915_private {
> > struct llist_head free_list;
> > struct work_struct free_work;
> >
> >+ struct list_head freed_objects;
>
> free_ctx_objects maybe?
This is i915->contexts.freed_objects (new contexts substruct). There is
already a precedent for using freed_ (mainly because I think its a
better description for that phase, free is too similar to avail,
although you may persuade me that free is better).
> Knowing you I am surprised this is not bucketed! :)
I didn't bother since we only really have two sizes - also reason why I
didn't make it per-engine so that we could share the xcs context
objects. My expectation is that we won't be caught up in slow list
searches, so went for simplicity for a change.
> >+struct drm_i915_gem_object *
> >+i915_gem_context_create_object(struct drm_i915_private *i915,
> >+ unsigned long size)
> >+{
> >+ struct drm_i915_gem_object *obj, *on;
> >+
> >+ lockdep_assert_held(&i915->drm.struct_mutex);
> >+
> >+ list_for_each_entry_safe(obj, on,
> >+ &i915->contexts.freed_objects,
> >+ batch_pool_link) {
> >+ if (obj->mm.madv != I915_MADV_DONTNEED) {
> >+ /* Purge the heretic! */
>
> I don't see this can happen in the current version?
The power of the shrinker. Remember it can and will stab you in the back
at any time.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
