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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 15, 2015 at 4:27 AM, Christian König
<deathsimple@xxxxxxxxxxx> wrote:
> 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@xxxxxxxxx>
>
>
> I'm not so deep into the pre R600 stuff but this looks completely sane.
>
> Patch is Reviewed-by: Christian König <christian.koenig@xxxxxxx>

Applied to my -next branch.

Thanks,

Alex

>
> 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);
>>   }
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux