[PATCH] drm/radeon: Use rdev->gem.mutex to protect hyperz/cmask owners

[PATCH] drm/radeon: Use rdev->gem.mutex to protect hyperz/cmask owners

[Daniel Vetter]

This removes the last depency of radeon for dev->struct_mutex!

Also the locking scheme for hyperz/cmask owners seems a bit unsound,
there's no protection in the preclose handler (and that never did hold
dev->struct_mutex while being called). So grab the same lock there,
too.

There's also all the checks in the cs checker, but since the overall
design seems to never stall for the previous owner I figured it's ok
if I leave this racy. It was racy even before I touched it after all
too.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/radeon/radeon_kms.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 4e2780f8c417..6e92fee22bda 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -181,7 +181,9 @@ static void radeon_set_filp_rights(struct drm_device *dev,
 				   struct drm_file *applier,
 				   uint32_t *value)
 {
-	mutex_lock(&dev->struct_mutex);
+	struct radeon_device *rdev = dev->dev_private;
+
+	mutex_lock(&rdev->gem.mutex);
 	if (*value == 1) {
 		/* wants rights */
 		if (!*owner)
@@ -192,7 +194,7 @@ static void radeon_set_filp_rights(struct drm_device *dev,
 			*owner = NULL;
 	}
 	*value = *owner == applier ? 1 : 0;
-	mutex_unlock(&dev->struct_mutex);
+	mutex_unlock(&rdev->gem.mutex);
 }
 
 /*
@@ -724,10 +726,14 @@ void radeon_driver_preclose_kms(struct drm_device *dev,
 				struct drm_file *file_priv)
 {
 	struct radeon_device *rdev = dev->dev_private;
+
+	mutex_lock(&rdev->gem.mutex);
 	if (rdev->hyperz_filp == file_priv)
 		rdev->hyperz_filp = NULL;
 	if (rdev->cmask_filp == file_priv)
 		rdev->cmask_filp = NULL;
+	mutex_unlock(&rdev->gem.mutex);
+
 	radeon_uvd_free_handles(rdev, file_priv);
 	radeon_vce_free_handles(rdev, file_priv);
 }
-- 
2.5.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/radeon: Use rdev->gem.mutex to protect hyperz/cmask owners

[Daniel Vetter]

On Tue, Sep 29, 2015 at 09:30:49AM +0200, Daniel Vetter wrote:
> This removes the last depency of radeon for dev->struct_mutex!
> 
> Also the locking scheme for hyperz/cmask owners seems a bit unsound,
> there's no protection in the preclose handler (and that never did hold
> dev->struct_mutex while being called). So grab the same lock there,
> too.
> 
> There's also all the checks in the cs checker, but since the overall
> design seems to never stall for the previous owner I figured it's ok
> if I leave this racy. It was racy even before I touched it after all
> too.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Please disregard this patch, misdirected git send-email ...
-Daniel

> ---
>  drivers/gpu/drm/radeon/radeon_kms.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
> index 4e2780f8c417..6e92fee22bda 100644
> --- a/drivers/gpu/drm/radeon/radeon_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_kms.c
> @@ -181,7 +181,9 @@ static void radeon_set_filp_rights(struct drm_device *dev,
>  				   struct drm_file *applier,
>  				   uint32_t *value)
>  {
> -	mutex_lock(&dev->struct_mutex);
> +	struct radeon_device *rdev = dev->dev_private;
> +
> +	mutex_lock(&rdev->gem.mutex);
>  	if (*value == 1) {
>  		/* wants rights */
>  		if (!*owner)
> @@ -192,7 +194,7 @@ static void radeon_set_filp_rights(struct drm_device *dev,
>  			*owner = NULL;
>  	}
>  	*value = *owner == applier ? 1 : 0;
> -	mutex_unlock(&dev->struct_mutex);
> +	mutex_unlock(&rdev->gem.mutex);
>  }
>  
>  /*
> @@ -724,10 +726,14 @@ void radeon_driver_preclose_kms(struct drm_device *dev,
>  				struct drm_file *file_priv)
>  {
>  	struct radeon_device *rdev = dev->dev_private;
> +
> +	mutex_lock(&rdev->gem.mutex);
>  	if (rdev->hyperz_filp == file_priv)
>  		rdev->hyperz_filp = NULL;
>  	if (rdev->cmask_filp == file_priv)
>  		rdev->cmask_filp = NULL;
> +	mutex_unlock(&rdev->gem.mutex);
> +
>  	radeon_uvd_free_handles(rdev, file_priv);
>  	radeon_vce_free_handles(rdev, file_priv);
>  }
> -- 
> 2.5.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx