Re: [PATCH 02/16] drm/i915: Convert non-blocking waits for requests over to using RCU

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On ma, 2016-08-01 at 19:22 +0100, Chris Wilson wrote:
> -	if (!obj) {
> -		ret = -ENOENT;
> -		goto unlock;
> -	}
> +	if (!obj)
> +		return -ENOENT;
>  
>  	/* Try to flush the object off the GPU without holding the lock.
>  	 * We will repeat the flush holding the lock in the normal manner
>  	 * to catch cases where we are gazumped.
>  	 */
> -	ret = i915_gem_object_wait_rendering__nonblocking(obj,
> -							  to_rps_client(file),
> -							  !write_domain);
> +	ret = __unsafe_wait_rendering(obj, to_rps_client(file), !write_domain);
>  	if (ret)
> -		goto unref;
> +		goto out_unlocked;
> +
> +	ret = i915_mutex_lock_interruptible(dev);
> +	if (ret)
> +		goto out_unlocked;
>  
>  	if (read_domains & I915_GEM_DOMAIN_GTT)
>  		ret = i915_gem_object_set_to_gtt_domain(obj, write_domain != 0);
> @@ -1501,11 +1481,13 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
>  	if (write_domain != 0)
>  		intel_fb_obj_invalidate(obj, write_origin(obj, write_domain));
>  
> -unref:
>  	i915_gem_object_put(obj);
> -unlock:
>  	mutex_unlock(&dev->struct_mutex);
>  	return ret;
> +
> +out_unlocked:

This is the sole label, you could call 'err' too.

> +	i915_gem_object_put_unlocked(obj);
> +	return ret;
>  }
>  
>  /**
> @@ -1647,6 +1629,15 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  	int ret = 0;
>  	bool write = !!(vmf->flags & FAULT_FLAG_WRITE);
>  
> +	/* Try to flush the object off the GPU first without holding the lock.
> +	 * Upon acquiring the lock, we will perform our sanity checks and then
> +	 * repeat the flush holding the lock in the normal manner to catch cases
> +	 * where we are gazumped.
> +	 */
> +	ret = __unsafe_wait_rendering(obj, NULL, !write);
> +	if (ret)
> +		goto err;
> +

Why do you lift this call super early, tracing will be affected at
least.

>  	intel_runtime_pm_get(dev_priv);
>  
>  	/* We don't use vmf->pgoff since that has the fake offset */
> @@ -1655,23 +1646,14 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  
>  	ret = i915_mutex_lock_interruptible(dev);
>  	if (ret)
> -		goto out;
> +		goto err_rpm;
>  
>  	trace_i915_gem_object_fault(obj, page_offset, true, write);
>  
> -	/* Try to flush the object off the GPU first without holding the lock.
> -	 * Upon reacquiring the lock, we will perform our sanity checks and then
> -	 * repeat the flush holding the lock in the normal manner to catch cases
> -	 * where we are gazumped.
> -	 */
> -	ret = i915_gem_object_wait_rendering__nonblocking(obj, NULL, !write);
> -	if (ret)
> -		goto unlock;
> -
>  	/* Access to snoopable pages through the GTT is incoherent. */
>  	if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev)) {
>  		ret = -EFAULT;
> -		goto unlock;
> +		goto err_unlock;
>  	}
>  
>  	/* Use a partial view if the object is bigger than the aperture. */
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx