[PATCH 1/2] drm//amdgpu: Always sync fence before unlock eviction_lock

[Pan, Xinhui]

> 2020年3月13日 16:52，Koenig, Christian <Christian.Koenig at amd.com> 写道：
> 
> Am 13.03.20 um 08:43 schrieb xinhui pan:
>> The fence generated in ->commit is a shared one, so add it to resv.
>> And we need do that with eviction lock hold.
>> 
>> Currently we only sync last_direct/last_delayed before ->prepare. But we
>> fail to sync the last fence generated by ->commit. That cuases problems
>> if eviction happenes later, but it does not sync the last fence.
> 
> NAK, that won't work.
> 
> We can only add fences when the dma_resv object is locked and that is only the case when validating.
> 
well, tha tis true.
but considering this is a PT BO, and only eviction has race on it AFAIK.
as for the individualized resv in bo release, we unref PT BO just after that.
I am still thinking of other races in the real world.

thanks
xinhui

> I'm considering to just partially revert the patch originally stopping to add fences and instead only not add them when invalidating in a direct submit.
> 
> Christian.
> 
>> 
>> Cc: Christian König <christian.koenig at amd.com>
>> Cc: Alex Deucher <alexander.deucher at amd.com>
>> Cc: Felix Kuehling <Felix.Kuehling at amd.com>
>> Signed-off-by: xinhui pan <xinhui.pan at amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 73398831196f..f424b5969930 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1582,6 +1582,7 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>>  	struct amdgpu_vm_update_params params;
>>  	enum amdgpu_sync_mode sync_mode;
>>  	int r;
>> +	struct amdgpu_bo *root = vm->root.base.bo;
>>    	memset(&params, 0, sizeof(params));
>>  	params.adev = adev;
>> @@ -1604,8 +1605,6 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>>  	}
>>    	if (flags & AMDGPU_PTE_VALID) {
>> -		struct amdgpu_bo *root = vm->root.base.bo;
>> -
>>  		if (!dma_fence_is_signaled(vm->last_direct))
>>  			amdgpu_bo_fence(root, vm->last_direct, true);
>>  @@ -1623,6 +1622,12 @@ static int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>>    	r = vm->update_funcs->commit(&params, fence);
>>  +	if (!dma_fence_is_signaled(vm->last_direct))
>> +		amdgpu_bo_fence(root, vm->last_direct, true);
>> +
>> +	if (!dma_fence_is_signaled(vm->last_delayed))
>> +		amdgpu_bo_fence(root, vm->last_delayed, true);
>> +
>>  error_unlock:
>>  	amdgpu_vm_eviction_unlock(vm);
>>  	return r;
>