On Wed, Jan 13, 2016 at 05:57:32PM +0000, John.C.Harrison@xxxxxxxxx wrote:
static int
i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct drm_file *file,
@@ -1428,6 +1465,17 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
u32 dispatch_flags;
int ret, i;
bool need_relocs;
+ int fd_fence_complete = -1;
+ int fd_fence_wait = lower_32_bits(args->rsvd2);
+ struct sync_fence *sync_fence;
+
+ /*
+ * Make sure an broken fence handle is not returned no matter
+ * how early an error might be hit. Note that rsvd2 has been
+ * saved away above because it is also an input parameter!
+ */
+ if (args->flags & I915_EXEC_CREATE_FENCE)
+ args->rsvd2 = (__u64) -1;
But you are not restoring the user input parameter upon an error path.
Very simple example is the user trying to do a wait on a fence but is
woken up by a signal and then tries to restart the syscall, the standard
do {
ret = ioctl(fd, request, arg);
} while (ret == -1 && (errno == EINTR || errno == EAGAIN));
loop errors out with EINVAL on the second pass.
if (!i915_gem_check_execbuffer(args))
return -EINVAL;
@@ -1511,6 +1559,17 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
dispatch_flags |= I915_DISPATCH_RS;
}
+ /*
+ * Without a GPU scheduler, any fence waits must be done up front.
+ */
+ if (args->flags & I915_EXEC_WAIT_FENCE) {
+ ret = i915_early_fence_wait(ring, fd_fence_wait);
+ if (ret < 0)
+ return ret;
+
+ args->flags &= ~I915_EXEC_WAIT_FENCE;
+ }
+
ret = i915_mutex_lock_interruptible(dev);
if (ret)
goto pre_mutex_err;
@@ -1695,13 +1754,41 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
i915_gem_context_reference(ctx);
params->ctx = ctx;
+ if (args->flags & I915_EXEC_CREATE_FENCE) {
+ /*
+ * Caller has requested a sync fence.
+ * User interrupts will be enabled to make sure that
+ * the timeline is signalled on completion.
+ */
+ ret = i915_create_sync_fence(params->request, &sync_fence,
+ &fd_fence_complete);
+ if (ret) {
+ DRM_ERROR("Fence creation failed for ring %d, ctx %p\n",
+ ring->id, ctx);
+ goto err_batch_unpin;
+ }
+ }
+
ret = dev_priv->gt.execbuf_submit(params, args, &eb->vmas);
if (ret)
- goto err_batch_unpin;
+ goto err_fence;
/* the request owns the ref now */
i915_gem_context_unreference(ctx);
+ if (fd_fence_complete != -1) {
+ /*
+ * Install the fence into the pre-allocated file
+ * descriptor to the fence object so that user land
+ * can wait on it...
+ */
+ i915_install_sync_fence_fd(params->request,
+ sync_fence, fd_fence_complete);
+
+ /* Return the fence through the rsvd2 field */
+ args->rsvd2 = (__u64) fd_fence_complete;
Use the upper s32 for the output, so again you are not overwriting user
state without good reason.
-Chris
