[Intel-gfx] [PATCH 01/13] drm/i915: Assert breadcrumbs are correctly ordered in the signal handler
Chris Wilson
chris at chris-wilson.co.uk
Fri May 3 13:37:13 UTC 2019
Quoting Tvrtko Ursulin (2019-05-03 14:32:59)
>
> On 03/05/2019 12:52, Chris Wilson wrote:
> > Inside the signal handler, we expect the requests to be ordered by their
> > breadcrumb such that no later request may be complete if we find an
> > earlier incomplete. Add an assert to check that the next breadcrumb
> > should not be logically before the current.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> > index 3cbffd400b1b..a6ffb25f72a2 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> > @@ -99,6 +99,12 @@ void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine)
> > struct i915_request *rq =
> > list_entry(pos, typeof(*rq), signal_link);
> >
> > + GEM_BUG_ON(next != &ce->signals &&
> > + i915_seqno_passed(rq->fence.seqno,
> > + list_entry(next,
> > + typeof(*rq),
> > + signal_link)->fence.seqno));
>
> I know its only GEM_BUG_ON, but why check for this in the irq handler?
> You don't trust the insertion, or group deletion? Or just becuase it is
> the smallest amount of code to piggy-back on existing iteration?
At this point, it's documenting the required sorting of ce->signals. The
vulnerable part is list insertion. Would you like something like
check_signal_order(ce, rq)
and use it after insertion as well?
We can do prev/next checking, just to be sure.
-Chris
More information about the Intel-gfx
mailing list