[PATCH 2/2] drm/amdgpu: Auto-validate DMABuf imports in compute VMs
Felix Kuehling
felix.kuehling at amd.com
Wed Dec 6 00:21:14 UTC 2023
On 2023-12-04 03:40, Christian König wrote:
>> @@ -416,6 +423,28 @@ int amdgpu_vm_validate_pt_bos(struct
>> amdgpu_device *adev, struct amdgpu_vm *vm,
>> }
>> spin_lock(&vm->status_lock);
>> }
>> + while (ticket && !list_empty(&vm->evicted_user)) {
>> + bo_base = list_first_entry(&vm->evicted_user,
>> + struct amdgpu_vm_bo_base,
>> + vm_status);
>> + spin_unlock(&vm->status_lock);
>> +
>> + bo = bo_base->bo;
>> + resv = bo->tbo.base.resv;
>> +
>> + if (dma_resv_locking_ctx(resv) != ticket) {
>> + pr_warn_ratelimited("Evicted user BO is not reserved in
>> pid %d\n",
>> + vm->task_info.pid);
>> + return -EINVAL;
>> + }
>> +
>> + r = validate(param, bo);
>> + if (r)
>> + return r;
>> +
>> + amdgpu_vm_bo_invalidated(bo_base);
>> + spin_lock(&vm->status_lock);
>> + }
>
> I see two main differences to the evicted state here:
>
> 1. We now handle BOs which don't use the shared reservation object and
> needs to be locked manually with the ticket before calling this function.
>
> 2. The BOs are then moved into a different state after validating them.
>
> That could be handled with just an additional if if I'm not completely
> mistaken which would make the extra state superfluous.
I tried this, and I think it's working. But if I don't have a ticket for
validating user BOs, I have to move the BO back to the vm->invalidated
list anyway, to avoid an infinite while(!list_empty()) loop. So the BO
may ping-pong between invalidated and evicted, and lead to some
redundant page table clearing in amdgpu_vm_handle_moved in cases where
amdgpu_vm_validate was called without a ticket and there happen to be
invalidated DMABuf imports.
I'm not sure if this will be a problem in practice. If it is, I think
keeping a separate state for evicted user BOs may still make sense.
Regards,
Felix
More information about the amd-gfx
mailing list