On 22/02/2017 11:46, Chris Wilson wrote:
The plan in the near-future is to allow requests to be removed from the
signaler. We can no longer then rely on holding a reference to the
request for the duration it is in the signaling tree, and instead must
obtain a reference to the request for the current operation using RCU.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/intel_breadcrumbs.c | 24 ++++++++++++++++--------
drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +-
2 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index d5bf4c0b2deb..62e6b8181200 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -496,7 +496,11 @@ static int intel_breadcrumbs_signaler(void *arg)
* need to wait for a new interrupt from the GPU or for
* a new client.
*/
- request = READ_ONCE(b->first_signal);
+ rcu_read_lock();
+ request = rcu_dereference(b->first_signal);
+ if (request)
+ request = i915_gem_request_get_rcu(request);
+ rcu_read_unlock();
if (signal_complete(request)) {
local_bh_disable();
dma_fence_signal(&request->fence);
@@ -515,24 +519,28 @@ static int intel_breadcrumbs_signaler(void *arg)
* the oldest before picking the next one.
*/
spin_lock_irq(&b->lock);
- if (request == b->first_signal) {
+ if (request == rcu_access_pointer(b->first_signal)) {
struct rb_node *rb =
rb_next(&request->signaling.node);
- b->first_signal = rb ? to_signaler(rb) : NULL;
+ rcu_assign_pointer(b->first_signal,
+ rb ? to_signaler(rb) : NULL);
}
rb_erase(&request->signaling.node, &b->signals);
spin_unlock_irq(&b->lock);
i915_gem_request_put(request);
} else {
- if (kthread_should_stop())
+ if (kthread_should_stop()) {
+ GEM_BUG_ON(request);
break;
+ }
schedule();
if (kthread_should_park())
kthread_parkme();
}
+ i915_gem_request_put(request);
} while (1);
__set_current_state(TASK_RUNNING);
@@ -597,7 +605,7 @@ void intel_engine_enable_signaling(struct drm_i915_gem_request *request)
rb_link_node(&request->signaling.node, parent, p);
rb_insert_color(&request->signaling.node, &b->signals);
if (first)
- smp_store_mb(b->first_signal, request);
+ rcu_assign_pointer(b->first_signal, request);
spin_unlock(&b->lock);
@@ -670,7 +678,7 @@ void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine)
/* The engines should be idle and all requests accounted for! */
WARN_ON(READ_ONCE(b->first_wait));
WARN_ON(!RB_EMPTY_ROOT(&b->waiters));
- WARN_ON(READ_ONCE(b->first_signal));
+ WARN_ON(rcu_access_pointer(b->first_signal));
WARN_ON(!RB_EMPTY_ROOT(&b->signals));
if (!IS_ERR_OR_NULL(b->signaler))
@@ -686,8 +694,8 @@ bool intel_breadcrumbs_busy(struct intel_engine_cs *engine)
spin_lock_irq(&b->lock);
- if (b->first_wait) {
- wake_up_process(b->first_wait->tsk);
+ if (rcu_access_pointer(b->first_signal)) {
+ wake_up_process(b->signaler);
busy |= intel_engine_flag(engine);
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index b91c2c7ef5a6..3fcb9dd19b07 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -242,7 +242,7 @@ struct intel_engine_cs {
struct rb_root signals; /* sorted by retirement */
struct intel_wait *first_wait; /* oldest waiter by retirement */
struct task_struct *signaler; /* used for fence signalling */
- struct drm_i915_gem_request *first_signal;
+ struct drm_i915_gem_request __rcu *first_signal;
struct timer_list fake_irq; /* used after a missed interrupt */
struct timer_list hangcheck; /* detect missed interrupts */
Looks OK.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
