| Message ID | 1486161930-11764-1-git-send-email-daniele.ceraolospurio@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 03, 2017 at 02:45:29PM -0800, Daniele Ceraolo Spurio wrote: > Fences are creted/checked before the pm ref is taken, so if we jump to > pre_mutex_err we will uncorrectly call intel_runtime_pm_put. > > Fixes: fec0445caa27 (drm/i915: Support explicit fencing for execbuf) > Testcase: igt/gem_exec_params > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Sigh. The tree I was using has this: if (args->flags & I915_EXEC_FENCE_IN) { in_fence = sync_file_get_fence(lower_32_bits(args->rsvd2)); if (!in_fence) return -EINVAL; } if (args->flags & I915_EXEC_FENCE_OUT) { out_fence_fd = get_unused_fd_flags(O_CLOEXEC); if (out_fence_fd < 0) { ret = out_fence_fd; goto err_in_fence; } } ... err_unlock: mutex_unlock(&dev->struct_mutex); err_rpm: intel_runtime_pm_put(eb.i915); eb_destroy(&eb); if (out_fence_fd != -1) put_unused_fd(out_fence_fd); err_in_fence: dma_fence_put(in_fence); return ret; } Transforming the unwind sequence to match would be appreciated. -Chris
On Fri, Feb 03, 2017 at 10:55:32PM +0000, Chris Wilson wrote: > On Fri, Feb 03, 2017 at 02:45:29PM -0800, Daniele Ceraolo Spurio wrote: > > Fences are creted/checked before the pm ref is taken, so if we jump to > > pre_mutex_err we will uncorrectly call intel_runtime_pm_put. > > > > Fixes: fec0445caa27 (drm/i915: Support explicit fencing for execbuf) > > Testcase: igt/gem_exec_params > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > > Sigh. The tree I was using has this: > > if (args->flags & I915_EXEC_FENCE_IN) { > in_fence = sync_file_get_fence(lower_32_bits(args->rsvd2)); > if (!in_fence) > return -EINVAL; > } > > if (args->flags & I915_EXEC_FENCE_OUT) { > out_fence_fd = get_unused_fd_flags(O_CLOEXEC); > if (out_fence_fd < 0) { > ret = out_fence_fd; > goto err_in_fence; > } > } > > ... > > err_unlock: > mutex_unlock(&dev->struct_mutex); > err_rpm: > intel_runtime_pm_put(eb.i915); > eb_destroy(&eb); > if (out_fence_fd != -1) > put_unused_fd(out_fence_fd); > err_in_fence: > dma_fence_put(in_fence); > return ret; > } > > Transforming the unwind sequence to match would be appreciated. Just in case I wasn't clear, just do the unwind gotos for the out_fence, i.e add err_in_fence: -Chris
On Fri, Feb 03, 2017 at 02:45:29PM -0800, Daniele Ceraolo Spurio wrote: > Fences are creted/checked before the pm ref is taken, so if we jump to > pre_mutex_err we will uncorrectly call intel_runtime_pm_put. > > Fixes: fec0445caa27 (drm/i915: Support explicit fencing for execbuf) > Testcase: igt/gem_exec_params > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Pushed this patch and the update to gem_exec_params, thanks. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index a40ade6..7fb8bad 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1644,17 +1644,15 @@ static void eb_export_fence(struct drm_i915_gem_object *obj, if (args->flags & I915_EXEC_FENCE_IN) { in_fence = sync_file_get_fence(lower_32_bits(args->rsvd2)); if (!in_fence) { - ret = -EINVAL; - goto pre_mutex_err; + return -EINVAL; } } if (args->flags & I915_EXEC_FENCE_OUT) { out_fence_fd = get_unused_fd_flags(O_CLOEXEC); if (out_fence_fd < 0) { - ret = out_fence_fd; - out_fence_fd = -1; - goto pre_mutex_err; + dma_fence_put(in_fence); + return out_fence_fd; } }
Fences are creted/checked before the pm ref is taken, so if we jump to pre_mutex_err we will uncorrectly call intel_runtime_pm_put. Fixes: fec0445caa27 (drm/i915: Support explicit fencing for execbuf) Testcase: igt/gem_exec_params Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)