On 11.01.2017 12:56, Christian König wrote:
> Am 11.01.2017 um 08:31 schrieb Nicolai Hähnle:
>> From: Nicolai Hähnle <nicolai.haehnle at amd.com>
>>
>> ttm_bo_init checks that the reservation object is locked. This is
>> the caller's responsibility when resv != NULL. Otherwise, the inline
>> reservation object of the newly allocated buffer is used and must
>> explicitly be locked. Using a trylock is fine, since nobody else
>> has a reference to the reservation lock.
>
> Good, catch. But while using trylock is fine, why do you need to use it
> in the first place?
(*bo_ptr)->tbo.resv is passed as the reservation object to ttm_bo_init
via amdgpu_bo_create_shadow, and ttm_bo_init has a lockdep check for it.
I haven't dug deeper whether that's _really_ necessary, but since among
other things ttm_bo_validate is called by ttm_bo_init, I'd assume that
it is (ttm_bo_validate also has a lockdep check). If it isn't, then the
internal locking of bo->resv->lock done by ttm_bo_init in the !resv case
would also be unnecessary...
Cheers,
Nicolai
>
> Regards,
> Christian.
>
>>
>> Signed-off-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> index 11c12c4d..357eed9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>> @@ -465,21 +465,32 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>> amdgpu_ttm_placement_init(adev, &placement,
>> placements, domain, flags);
>> r = amdgpu_bo_create_restricted(adev, size, byte_align, kernel,
>> domain, flags, sg, &placement,
>> resv, bo_ptr);
>> if (r)
>> return r;
>> if (amdgpu_need_backup(adev) && (flags &
>> AMDGPU_GEM_CREATE_SHADOW)) {
>> + if (!resv) {
>> + bool locked;
>> +
>> + locked = ww_mutex_trylock(&(*bo_ptr)->tbo.resv->lock);
>> + WARN_ON(!locked);
>> + }
>> +
>> r = amdgpu_bo_create_shadow(adev, size, byte_align, (*bo_ptr));
>> +
>> + if (!resv)
>> + ww_mutex_unlock(&(*bo_ptr)->tbo.resv->lock);
>> +
>> if (r)
>> amdgpu_bo_unref(bo_ptr);
>> }
>> return r;
>> }
>> int amdgpu_bo_backup_to_shadow(struct amdgpu_device *adev,
>> struct amdgpu_ring *ring,
>> struct amdgpu_bo *bo,
>
>
