Re: [PATCH 1/5] drm/amdgpu: rework moved handling in the VM v2

[zhoucm1 <david1.zhou-5C7GfCeVMHo@public.gmane.org>]

Hi Christian,

I asked you how you keep the access of base.moved is safely in last 
thread, I check it just now, it depends on the shared resv lock.

For patch itself, Reviewed-by: Chunming Zhou <david1.zhou@amd.com>

another question comes up to me, I find most of all vm lists are 
protected by the shared resv lock, whether the status lock can be 
removed as well? seems everything of vm is protected by that big resv lock.

Regards,
David Zhou

On 2017年08月29日 02:50, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> Instead of using the vm_state use a separate flag to note
> that the BO was moved.
>
> v2: reorder patches to avoid temporary lockless access
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 13 ++++++++++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  3 +++
>   2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index f621dba..ee53293 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1787,10 +1787,16 @@ 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))
> +	if (!clear && bo_va->base.moved) {
> +		bo_va->base.moved = false;
>   		list_splice_init(&bo_va->valids, &bo_va->invalids);
> -	spin_unlock(&vm->status_lock);
> +
> +	} else {
> +		spin_lock(&vm->status_lock);
> +		if (!list_empty(&bo_va->base.vm_status))
> +			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,
> @@ -2418,6 +2424,7 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
>   	struct amdgpu_vm_bo_base *bo_base;
>   
>   	list_for_each_entry(bo_base, &bo->va, bo_list) {
> +		bo_base->moved = true;
>   		spin_lock(&bo_base->vm->status_lock);
>   		if (list_empty(&bo_base->vm_status))
>   			list_add(&bo_base->vm_status,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 9347d28..1b478e6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -105,6 +105,9 @@ struct amdgpu_vm_bo_base {
>   
>   	/* protected by spinlock */
>   	struct list_head		vm_status;
> +
> +	/* protected by the BO being reserved */
> +	bool				moved;
>   };
>   
>   struct amdgpu_vm_pt {

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx