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
