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

Christian König deathsimple at vodafone.de
Thu Oct 15 01:27:32 PDT 2015


On 15.10.2015 09:36, 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 at intel.com>

I'm not so deep into the pre R600 stuff but this looks completely sane.

Patch is Reviewed-by: Christian König <christian.koenig at amd.com>

Regards,
Christian.

> ---
>   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 efa45e502975..893cf1079552 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);
>   }
>   
>   /*
> @@ -727,10 +729,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);
>   }



More information about the dri-devel mailing list