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