> 2020年3月13日 16:52，Koenig, Christian <Christian.Koenig@xxxxxxx> 写道：
>
> 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@xxxxxxx>
>> Cc: Alex Deucher <alexander.deucher@xxxxxxx>
>> Cc: Felix Kuehling <Felix.Kuehling@xxxxxxx>
>> Signed-off-by: xinhui pan <xinhui.pan@xxxxxxx>
>> ---
>> 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(¶ms, 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(¶ms, 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;
>
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
