On 03/05/2019 14:37, Chris Wilson wrote:
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.
I don't feel strongly either way. Was just curious why you decided to
put it where it was.
Helper I suppose is better since it is more self-documenting and it's
easy to call it from all strategic places.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
