On 10/12/2021 01:31, dong.yang@xxxxxxxxx wrote:
From: "Yang, Dong" <dong.yang@xxxxxxxxx>
With unknow race condition, the i915_request will be added
What do you mean with unknown here?
to intel_context list twice, and result in system panic.
If node alreay exist then do not add it again.
Note the call chains are under ce->signal_lock and protecting from double add AFAICT:
static void insert_breadcrumb(struct i915_request *rq)
{
...
if (test_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags))
return;
...
set_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags);
bool i915_request_enable_breadcrumb(struct i915_request *rq)
{
...
spin_lock(&ce->signal_lock);
if (test_bit(I915_FENCE_FLAG_ACTIVE, &rq->fence.flags))
insert_breadcrumb(rq);
spin_unlock(&ce->signal_lock);
void i915_request_cancel_breadcrumb(struct i915_request *rq)
{
...
spin_lock(&ce->signal_lock);
if (!test_and_clear_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags)) {
spin_unlock(&ce->signal_lock);
return;
}
void intel_context_remove_breadcrumbs(struct intel_context *ce,
struct intel_breadcrumbs *b)
{
...
spin_lock_irqsave(&ce->signal_lock, flags);
if (list_empty(&ce->signals))
goto unlock;
list_for_each_entry_safe(rq, rn, &ce->signals, signal_link) {
GEM_BUG_ON(!__i915_request_is_complete(rq));
if (!test_and_clear_bit(I915_FENCE_FLAG_SIGNAL,
&rq->fence.flags))
continue;
The last one in signal_irq_work is guarded by the __i915_request_is_complete check.
So I think more context is needed on how you found this may be an issue.
Regards,
Tvrtko
Signed-off-by: Yang, Dong <dong.yang@xxxxxxxxx>
---
drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
index 209cf265bf74..9c7bc060d2ae 100644
--- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
@@ -387,6 +387,9 @@ static void insert_breadcrumb(struct i915_request *rq)
}
}
+ if (&rq->signal_link == pos)
+ return;
+
i915_request_get(rq);
list_add_rcu(&rq->signal_link, pos);
GEM_BUG_ON(!check_signal_order(ce, rq));
