On 18/05/2017 15:19, Chris Wilson wrote:
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.
In this case I would be reluctant to complicate the code for no benefit.
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).
My bad, I thought it is drm_i915_private.
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.
Ok I forgot about the purged state, I just was surprised that anyone
would be able to move the state away from don't need.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
