[Intel-gfx] [PATCH v2 3/6] drm/i915: Only track live rings for retiring

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-04-24 10:37:03)
> 
> On 23/04/2018 19:08, Chris Wilson wrote:
> > @@ -1403,14 +1406,17 @@ static void ring_retire_requests(struct intel_ring *ring)
> >   
> >   void i915_retire_requests(struct drm_i915_private *i915)
> >   {
> > -     struct intel_ring *ring, *next;
> > +     struct intel_ring *ring, *tmp;
> >   
> >       lockdep_assert_held(&i915->drm.struct_mutex);
> >   
> >       if (!i915->gt.active_requests)
> >               return;
> >   
> > -     list_for_each_entry_safe(ring, next, &i915->gt.rings, link)
> > +     /* An outstanding request must be on a still active ring somewhere */
> > +     GEM_BUG_ON(list_empty(&i915->gt.active_rings));
> 
> Okay, but I still think my assert would be stronger. As long as we allow 
> calling this function with no active requests, why not check both 
> internal states are consistent every time?

I agree that checking both simultaneously would be useful, but we do
check both for cross-consistency already, just not in the same place.
It's just doing so in a readable fashion.

After this we aren't using active_requests as more than a boolean; so
I'm wondering if not just replacing it with list_empty(&active_rings)
for the infrequent uses.
-Chris