I'm not sure this one, since you remove status lock for vm_status list,
I think we need to test this carefully.
Regards,
David Zhou
On 2017å¹´08æ??25æ?¥ 17:38, Christian König wrote:
> From: Christian König <christian.koenig at amd.com>
>
> We changed this to use an extra list a while back, but for the next
> series I need a separate flag again.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 3 +++
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 37 ++++++++++++++----------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 ---
> 3 files changed, 20 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index a288fa6..e613ba4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -55,6 +55,9 @@ struct amdgpu_bo_va {
> /* mappings for this bo_va */
> struct list_head invalids;
> struct list_head valids;
> +
> + /* If the mappings are cleared or filled */
> + bool cleared;
> };
>
> struct amdgpu_bo {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index f621dba..16148ef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1787,10 +1787,13 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
> else
> flags = 0x0;
>
> - spin_lock(&vm->status_lock);
> - if (!list_empty(&bo_va->base.vm_status))
> + /* We access vm_status without the status lock here, but that is ok
> + * because when we don't clear the BO is locked and so the status can't
> + * change
> + */
> + if ((!clear && !list_empty(&bo_va->base.vm_status)) ||
> + bo_va->cleared != clear)
> list_splice_init(&bo_va->valids, &bo_va->invalids);
> - spin_unlock(&vm->status_lock);
>
> list_for_each_entry(mapping, &bo_va->invalids, list) {
> r = amdgpu_vm_bo_split_mapping(adev, exclusive, pages_addr, vm,
> @@ -1800,25 +1803,22 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
> return r;
> }
>
> - if (trace_amdgpu_vm_bo_mapping_enabled()) {
> - list_for_each_entry(mapping, &bo_va->valids, list)
> - trace_amdgpu_vm_bo_mapping(mapping);
> -
> - list_for_each_entry(mapping, &bo_va->invalids, list)
> - trace_amdgpu_vm_bo_mapping(mapping);
> + if (vm->use_cpu_for_update) {
> + /* Flush HDP */
> + mb();
> + amdgpu_gart_flush_gpu_tlb(adev, 0);
> }
>
> spin_lock(&vm->status_lock);
> - list_splice_init(&bo_va->invalids, &bo_va->valids);
> list_del_init(&bo_va->base.vm_status);
> - if (clear)
> - list_add(&bo_va->base.vm_status, &vm->cleared);
> spin_unlock(&vm->status_lock);
>
> - if (vm->use_cpu_for_update) {
> - /* Flush HDP */
> - mb();
> - amdgpu_gart_flush_gpu_tlb(adev, 0);
> + list_splice_init(&bo_va->invalids, &bo_va->valids);
> + bo_va->cleared = clear;
> +
> + if (trace_amdgpu_vm_bo_mapping_enabled()) {
> + list_for_each_entry(mapping, &bo_va->valids, list)
> + trace_amdgpu_vm_bo_mapping(mapping);
> }
>
> return 0;
> @@ -2419,9 +2419,7 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
>
> list_for_each_entry(bo_base, &bo->va, bo_list) {
> spin_lock(&bo_base->vm->status_lock);
> - if (list_empty(&bo_base->vm_status))
> - list_add(&bo_base->vm_status,
> - &bo_base->vm->moved);
> + list_move(&bo_base->vm_status, &bo_base->vm->moved);
> spin_unlock(&bo_base->vm->status_lock);
> }
> }
> @@ -2508,7 +2506,6 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> vm->reserved_vmid[i] = NULL;
> spin_lock_init(&vm->status_lock);
> INIT_LIST_HEAD(&vm->moved);
> - INIT_LIST_HEAD(&vm->cleared);
> INIT_LIST_HEAD(&vm->freed);
>
> /* create scheduler entity for page table updates */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 9347d28..e705f0f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -126,9 +126,6 @@ struct amdgpu_vm {
> /* BOs moved, but not yet updated in the PT */
> struct list_head moved;
>
> - /* BOs cleared in the PT because of a move */
> - struct list_head cleared;
> -
> /* BO mappings freed, but not yet updated in the PT */
> struct list_head freed;
>
