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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
