On ma, 2016-08-01 at 19:22 +0100, Chris Wilson wrote:
> If we try and read or write to an active request, we first must wait
> upon the GPU completing that request. Let's do that without holding the
> mutex (and so allow someone else to access the GPU whilst we wait). Upn
Upon --^
> completion, we will reacquire the mutex and only then start the
> operation (i.e. we do not rely on state from before dropping the mutex).
>
<SNIP>
> @@ -953,25 +953,26 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data,
> args->size))
> return -EFAULT;
>
> - ret = i915_mutex_lock_interruptible(dev);
> - if (ret)
> - return ret;
> -
> obj = i915_gem_object_lookup(file, args->handle);
> - if (!obj) {
> - ret = -ENOENT;
> - goto unlock;
> - }
> + if (!obj)
> + return -ENOENT;
>
> /* Bounds check source. */
> if (args->offset > obj->base.size ||
> args->size > obj->base.size - args->offset) {
> ret = -EINVAL;
> - goto out;
> + goto out_unlocked;
Again the sole exit path, could be just 'err'
> @@ -1368,27 +1372,28 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data,
> return -EFAULT;
> }
>
> - intel_runtime_pm_get(dev_priv);
> -
> - ret = i915_mutex_lock_interruptible(dev);
> - if (ret)
> - goto put_rpm;
> -
> obj = i915_gem_object_lookup(file, args->handle);
> - if (!obj) {
> - ret = -ENOENT;
> - goto unlock;
> - }
> + if (!obj)
> + return -ENOENT;
>
> /* Bounds check destination. */
> if (args->offset > obj->base.size ||
> args->size > obj->base.size - args->offset) {
> ret = -EINVAL;
> - goto out;
> + goto out_unlocked;
Ditto in this func, just 'err'
> }
>
> - trace_i915_gem_object_pwrite(obj, args->offset, args->size);
> + ret = __unsafe_wait_rendering(obj, to_rps_client(file), false);
> + if (ret)
> + goto out_unlocked;
> +
> + intel_runtime_pm_get(dev_priv);
>
As discussed in IRC, pread_ioctl does not take RPM for the fallback
path, it should.
> + ret = i915_mutex_lock_interruptible(dev);
> + if (ret)
> + goto out_rpm;
> +
> + trace_i915_gem_object_pwrite(obj, args->offset, args->size);
For tracing, I'm not quite sure if we should emit failed attempts too,
I guess it's a policy decision?
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
