diff mbox series

[v9,28/70] drm/i915: Take obj lock around set_domain ioctl

Message ID 20210323155059.628690-29-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Remove obj->mm.lock! | expand

Commit Message

Maarten Lankhorst March 23, 2021, 3:50 p.m. UTC 
We need to lock the object to move it to the correct domain,
add the missing lock.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_domain.c | 41 ++++++++++------------
 1 file changed, 19 insertions(+), 22 deletions(-)

Comments

Daniel Vetter March 24, 2021, 2:12 p.m. UTC | #1 
On Tue, Mar 23, 2021 at 04:50:17PM +0100, Maarten Lankhorst wrote:
> We need to lock the object to move it to the correct domain,
> add the missing lock.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>

This conflicted real bad with the in-flight -gt-next stuff that wasn't
reset yet, so I picked up the old version here:

https://lore.kernel.org/intel-gfx/20210128162612.927917-29-maarten.lankhorst@linux.intel.com/

That one looks a lot more reasonable.
-Daniel

> ---
>  drivers/gpu/drm/i915/gem/i915_gem_domain.c | 41 ++++++++++------------
>  1 file changed, 19 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> index 41dae0d83dbb..e3537922183b 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
> @@ -456,13 +456,7 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
>  		 * userptr validity
>  		 */
>  		err = i915_gem_object_userptr_validate(obj);
> -		if (!err)
> -			err = i915_gem_object_wait(obj,
> -						   I915_WAIT_INTERRUPTIBLE |
> -						   I915_WAIT_PRIORITY |
> -						   (write_domain ? I915_WAIT_ALL : 0),
> -						   MAX_SCHEDULE_TIMEOUT);
> -		goto out;
> +		goto out_wait;
>  	}
>  
>  	/*
> @@ -476,6 +470,10 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
>  		goto out;
>  	}
>  
> +	err = i915_gem_object_lock_interruptible(obj, NULL);
> +	if (err)
> +		goto out;
> +
>  	/*
>  	 * Flush and acquire obj->pages so that we are coherent through
>  	 * direct access in memory with previous cached writes through
> @@ -487,7 +485,7 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
>  	 */
>  	err = i915_gem_object_pin_pages(obj);
>  	if (err)
> -		goto out;
> +		goto out_unlock;
>  
>  	/*
>  	 * Already in the desired write domain? Nothing for us to do!
> @@ -500,10 +498,6 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
>  	 * without having to further check the requested write_domain.
>  	 */
>  	if (READ_ONCE(obj->write_domain) == read_domains)
> -		goto out_wait;
> -
> -	err = i915_gem_object_lock_interruptible(obj, NULL);
> -	if (err)
>  		goto out_unpin;
>  
>  	if (read_domains & I915_GEM_DOMAIN_WC)
> @@ -513,19 +507,22 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
>  	else
>  		i915_gem_object_set_to_cpu_domain(obj, write_domain);
>  
> -	i915_gem_object_unlock(obj);
> +out_unpin:
> +	i915_gem_object_unpin_pages(obj);
>  
> +out_unlock:
> +	i915_gem_object_unlock(obj);
>  out_wait:
> -	err = i915_gem_object_wait(obj,
> -				   I915_WAIT_INTERRUPTIBLE |
> -				   I915_WAIT_PRIORITY |
> -				   (write_domain ? I915_WAIT_ALL : 0),
> -				   MAX_SCHEDULE_TIMEOUT);
> -	if (write_domain)
> -		i915_gem_object_invalidate_frontbuffer(obj, ORIGIN_CPU);
> +	if (!err) {
> +		err = i915_gem_object_wait(obj,
> +					  I915_WAIT_INTERRUPTIBLE |
> +					  I915_WAIT_PRIORITY |
> +					  (write_domain ? I915_WAIT_ALL : 0),
> +					  MAX_SCHEDULE_TIMEOUT);
> +		if (write_domain)
> +			i915_gem_object_invalidate_frontbuffer(obj, ORIGIN_CPU);
> +	}
>  
> -out_unpin:
> -	i915_gem_object_unpin_pages(obj);
>  out:
>  	i915_gem_object_put(obj);
>  	return err;
> -- 
> 2.31.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 41dae0d83dbb..e3537922183b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -456,13 +456,7 @@  i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 		 * userptr validity
 		 */
 		err = i915_gem_object_userptr_validate(obj);
-		if (!err)
-			err = i915_gem_object_wait(obj,
-						   I915_WAIT_INTERRUPTIBLE |
-						   I915_WAIT_PRIORITY |
-						   (write_domain ? I915_WAIT_ALL : 0),
-						   MAX_SCHEDULE_TIMEOUT);
-		goto out;
+		goto out_wait;
 	}
 
 	/*
@@ -476,6 +470,10 @@  i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 		goto out;
 	}
 
+	err = i915_gem_object_lock_interruptible(obj, NULL);
+	if (err)
+		goto out;
+
 	/*
 	 * Flush and acquire obj->pages so that we are coherent through
 	 * direct access in memory with previous cached writes through
@@ -487,7 +485,7 @@  i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	 */
 	err = i915_gem_object_pin_pages(obj);
 	if (err)
-		goto out;
+		goto out_unlock;
 
 	/*
 	 * Already in the desired write domain? Nothing for us to do!
@@ -500,10 +498,6 @@  i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	 * without having to further check the requested write_domain.
 	 */
 	if (READ_ONCE(obj->write_domain) == read_domains)
-		goto out_wait;
-
-	err = i915_gem_object_lock_interruptible(obj, NULL);
-	if (err)
 		goto out_unpin;
 
 	if (read_domains & I915_GEM_DOMAIN_WC)
@@ -513,19 +507,22 @@  i915_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	else
 		i915_gem_object_set_to_cpu_domain(obj, write_domain);
 
-	i915_gem_object_unlock(obj);
+out_unpin:
+	i915_gem_object_unpin_pages(obj);
 
+out_unlock:
+	i915_gem_object_unlock(obj);
 out_wait:
-	err = i915_gem_object_wait(obj,
-				   I915_WAIT_INTERRUPTIBLE |
-				   I915_WAIT_PRIORITY |
-				   (write_domain ? I915_WAIT_ALL : 0),
-				   MAX_SCHEDULE_TIMEOUT);
-	if (write_domain)
-		i915_gem_object_invalidate_frontbuffer(obj, ORIGIN_CPU);
+	if (!err) {
+		err = i915_gem_object_wait(obj,
+					  I915_WAIT_INTERRUPTIBLE |
+					  I915_WAIT_PRIORITY |
+					  (write_domain ? I915_WAIT_ALL : 0),
+					  MAX_SCHEDULE_TIMEOUT);
+		if (write_domain)
+			i915_gem_object_invalidate_frontbuffer(obj, ORIGIN_CPU);
+	}
 
-out_unpin:
-	i915_gem_object_unpin_pages(obj);
 out:
 	i915_gem_object_put(obj);
 	return err;