[Intel-gfx] [PATCH 07/38] drm/i915: Stop tracking MRU activity on VMA

[Chris Wilson]

Quoting Chris Wilson (2019-01-25 13:38:09)
> Quoting Tvrtko Ursulin (2019-01-25 10:46:19)
> > 
> > On 22/01/2019 14:19, Chris Wilson wrote:
> > > Quoting Tvrtko Ursulin (2019-01-18 16:03:27)
> > >>
> > >> On 18/01/2019 14:00, Chris Wilson wrote:
> > >>> Our goal is to remove struct_mutex and replace it with fine grained
> > >>> locking. One of the thorny issues is our eviction logic for reclaiming
> > >>> space for an execbuffer (or GTT mmaping, among a few other examples).
> > >>> While eviction itself is easy to move under a per-VM mutex, performing
> > >>> the activity tracking is less agreeable. One solution is not to do any
> > >>> MRU tracking and do a simple coarse evaluation during eviction of
> > >>> active/inactive, with a loose temporal ordering of last
> > >>> insertion/evaluation. That keeps all the locking constrained to when we
> > >>> are manipulating the VM itself, neatly avoiding the tricky handling of
> > >>> possible recursive locking during execbuf and elsewhere.
> > >>>
> > >>> Note that discarding the MRU is unlikely to impact upon our efficiency
> > >>> to reclaim VM space (where we think a LRU model is best) as our
> > >>> current strategy is to use random idle replacement first before doing
> > >>> a search, and over time the use of softpinned 48b per-ppGTT is growing
> > >>> (thereby eliminating any need to perform any eviction searches, in
> > >>> theory at least).
> > >>
> > >> There is still no mention of list consolidation.
> > > 
> > > "Note that discarding the MRU (currently implemented as a pair of lists,
> > > to avoid scanning the active list for a NONBLOCKING search)"
> > > 
> > > Is that enough to make it clear?
> > 
> > How about:
> > 
> > "Note that discarding the MRU, which is implemented both by not doing 
> > list operations while tracking activity, and by replacing 
> > active/inactive list with a single bound list, is unlikely..."
> > 
> > ?
> > 
> > Plus I really want a changelog and that stale comment which talks about 
> > active/inactive *lists* updated.
> 
> But there's active/inactive lists within the list...

@@ -136,17 +136,15 @@ i915_gem_evict_something(struct i915_address_space *vm,
        trace_i915_gem_evict(vm, min_size, alignment, flags);

        /*
-        * The goal is to evict objects and amalgamate space in LRU order.
-        * The oldest idle objects reside on the inactive list, which is in
-        * retirement order. The next objects to retire are those in flight,
-        * on the active list, again in retirement order.
+        * The goal is to evict objects and amalgamate space in rough LRU order.
+        * Since both active and inactive objects reside on the same list,
+        * in a mix of creation and last scanned order, as we process the list
+        * we sort it into inactive/active, which keeps the active portion
+        * in a rough MRU order.
         *
         * The retirement sequence is thus:
         *   1. Inactive objects (already retired, random order)
         *   2. Active objects (will stall on unbinding, oldest scanned first)
-        *
-        * On each list, the oldest objects lie at the HEAD with the freshest
-        * object on the TAIL.
         */
-Chris