[PATCH 1/2] drm/amdgpu: drop multiple bo_unreserve() calls in amdgpu_gem_op_ioctl()

[Samuel Pitoiset]

On 02/10/2017 11:02 AM, Christian König wrote:
> NAK, we need to drop the reservation before calling copy_to_user().
>
> Otherwise you run into a double locking error when copy_to_user() is
> trying to copy to the BO in question.

Okay, I trust you but this seems tricky. copy_to_user() doesn't even 
take the BO as parameter, how can this happen?

>
> Regards,
> Christian.
>
> Am 09.02.2017 um 23:28 schrieb Samuel Pitoiset:
>> Move amdgpu_bo_unreserve() outside of the switch. While we are
>> at it, add a missing break in the default case.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 7 ++-----
>>   1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index 1dc59aafec71..ae4658a10e2c 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -660,7 +660,6 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
>> void *data,
>>           info.alignment = robj->tbo.mem.page_alignment << PAGE_SHIFT;
>>           info.domains = robj->prefered_domains;
>>           info.domain_flags = robj->flags;
>> -        amdgpu_bo_unreserve(robj);
>>           if (copy_to_user(out, &info, sizeof(info)))
>>               r = -EFAULT;
>>           break;
>> @@ -668,7 +667,6 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
>> void *data,
>>       case AMDGPU_GEM_OP_SET_PLACEMENT:
>>           if (amdgpu_ttm_tt_get_usermm(robj->tbo.ttm)) {
>>               r = -EPERM;
>> -            amdgpu_bo_unreserve(robj);
>>               break;
>>           }
>>           robj->prefered_domains = args->value &
>> (AMDGPU_GEM_DOMAIN_VRAM |
>> @@ -677,14 +675,13 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev,
>> void *data,
>>           robj->allowed_domains = robj->prefered_domains;
>>           if (robj->allowed_domains == AMDGPU_GEM_DOMAIN_VRAM)
>>               robj->allowed_domains |= AMDGPU_GEM_DOMAIN_GTT;
>> -
>> -        amdgpu_bo_unreserve(robj);
>>           break;
>>       default:
>> -        amdgpu_bo_unreserve(robj);
>>           r = -EINVAL;
>> +        break;
>>       }
>>   +    amdgpu_bo_unreserve(robj);
>>   out:
>>       drm_gem_object_unreference_unlocked(gobj);
>>       return r;
>
>