| Message ID | 20190503135229.32039-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Check the target has not already completed before waiting on it | expand |
Quoting Chris Wilson (2019-05-03 14:52:29) > When we want to wait for a request to be executed, we first ask if it is > not on the GPU as if it's on the gpu, there's no need to wait. However, > we have to take into account that a request may not be on the GPU > because it has already completed! > > The window is small due to the numerous preceding checks that our target > has not yet completed, yet there is still a very small window across the > kmalloc. Ok, there's a second part to this problem as this only happens under preempt-to-busy as it requires the request running in the background and completing after unsubmission. > Fixes: e88619646971 ("drm/i915: Use HW semaphores for inter-engine synchronisation on gen8+") So not required. -Chris
On 03/05/2019 14:52, Chris Wilson wrote: > When we want to wait for a request to be executed, we first ask if it is > not on the GPU as if it's on the gpu, there's no need to wait. However, > we have to take into account that a request may not be on the GPU > because it has already completed! > > The window is small due to the numerous preceding checks that our target > has not yet completed, yet there is still a very small window across the > kmalloc. > > Fixes: e88619646971 ("drm/i915: Use HW semaphores for inter-engine synchronisation on gen8+") > Testcase: igt/gem_concurrent_blit > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/i915_request.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c > index d06c45305b03..6dbf8dc5cd6a 100644 > --- a/drivers/gpu/drm/i915/i915_request.c > +++ b/drivers/gpu/drm/i915/i915_request.c > @@ -373,7 +373,7 @@ i915_request_await_execution(struct i915_request *rq, > init_irq_work(&cb->work, irq_execute_cb); > > spin_lock_irq(&signal->lock); > - if (i915_request_is_active(signal)) { > + if (i915_request_is_active(signal) || i915_request_completed(signal)) { > i915_sw_fence_complete(cb->fence); > kmem_cache_free(global.slab_execute_cbs, cb); > } else { > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index d06c45305b03..6dbf8dc5cd6a 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -373,7 +373,7 @@ i915_request_await_execution(struct i915_request *rq, init_irq_work(&cb->work, irq_execute_cb); spin_lock_irq(&signal->lock); - if (i915_request_is_active(signal)) { + if (i915_request_is_active(signal) || i915_request_completed(signal)) { i915_sw_fence_complete(cb->fence); kmem_cache_free(global.slab_execute_cbs, cb); } else {
When we want to wait for a request to be executed, we first ask if it is not on the GPU as if it's on the gpu, there's no need to wait. However, we have to take into account that a request may not be on the GPU because it has already completed! The window is small due to the numerous preceding checks that our target has not yet completed, yet there is still a very small window across the kmalloc. Fixes: e88619646971 ("drm/i915: Use HW semaphores for inter-engine synchronisation on gen8+") Testcase: igt/gem_concurrent_blit Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/i915_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)