On 2017-08-10 10:50 AM, Christian König wrote:
> @@ -2384,11 +2360,24 @@ void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
> * Mark @bo as invalid.
> */
> void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
> - struct amdgpu_bo *bo)
> + struct amdgpu_bo *bo, bool evicted)
> {
> struct amdgpu_vm_bo_base *bo_base;
>
> list_for_each_entry(bo_base, &bo->va, bo_list) {
> + struct amdgpu_vm *vm = bo_base->vm;
> +
> + if (evicted && bo->tbo.resv == vm->root.base.bo->tbo.resv) {
> + spin_lock(&bo_base->vm->status_lock);
> + list_move(&bo_base->vm_status, &vm->evicted);
> + spin_unlock(&bo_base->vm->status_lock);
> + continue;
> + }
> +
> + /* Don't add page tables to the moved state */
> + if (bo->tbo.type == ttm_bo_type_kernel)
> + continue;
> +
It looks like you're using two different conditions to detect page
tables. When evicted is true, you use (bo->tbo.resv ==
vm->root.base.bo->tbo.resv). Otherwise you use (bo->tbo.type ==
ttm_bo_type_kernel). Are these really the same or is there a subtle
difference I'm missing? I'd find the following logic easier to read, but
I'm not sure if that's really your intention:
if (bo->tbo.resv == vm->root.base.bo->tbo.resv) {
spin_lock(&bo_base->vm->status_lock);
if (evicted)
list_move(&bo_base->vm_status, &vm->evicted);
else if (list_empty(&bo_base->vm_status))
/* this is from the next commit */
list_add(&bo_base->vm_status, &vm->relocated);
spin_unlock(&bo_base->vm->status_lock);
continue;
}
Regards,
Felix
> spin_lock(&bo_base->vm->status_lock);
> if (list_empty(&bo_base->vm_status))
> list_add(&bo_base->vm_status,
