Re: [Intel-gfx] [PATCH] drm/i915: Lock timeline mutex directly in error path of eb_pin_timeline

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 04/01/2022 23:30, Matthew Brost wrote:
Don't use the interruptable version of the timeline mutex lock in the

interruptible

error path of eb_pin_timeline as the cleanup must always happen.

v2:
  (John Harrison)
   - Don't check for interrupt during mutex lock

Fixes: 544460c33821 ("drm/i915: Multi-BB execbuf")
Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx>
---
  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index e9541244027a..e96e133cbb1f 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -2516,9 +2516,9 @@ static int eb_pin_timeline(struct i915_execbuffer *eb, struct intel_context *ce,
  				      timeout) < 0) {
  			i915_request_put(rq);
- tl = intel_context_timeline_lock(ce);
+			mutex_lock(&ce->timeline->mutex);

On the other hand it is more user friendly to handle signals (which maybe does not matter in this case, not sure any longer how long hold time it can have) but there is also a question of consistency within the very function you are changing.

Apart from consistency, what about the parent-child magic intel_context_timeline_lock does and you wouldn't have here?

And what about the very existence of intel_context_timeline_lock as a component boundary separation API, if it is used inconsistently throughout i915_gem_execbuffer.c?

Regards,

Tvrtko

  			intel_context_exit(ce);
-			intel_context_timeline_unlock(tl);
+			mutex_unlock(&ce->timeline->mutex);
if (nonblock)
  				return -EWOULDBLOCK;




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux