Re: [PATCH 01/14] drm/amdgpu: fix wrong release of vmid owner

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, May 04, 2016 at 02:26:42PM -0400, Alex Deucher wrote:
> From: Chunming Zhou <David1.Zhou@amd.com>
> 
> The release of the vmid owner was not handled
> correctly.  We need to take the lock and walk
> the lru list.
> 
> Signed-off-by: Chunming Zhou <David1.Zhou@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> Reviewed-by: Monk Liu <monk.liu@amd.com>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

I know that it's super hard to get former proprietary driver teams to
stick their heads out on a public mailing lists. But imo being steward for
them is totally the worst case option you can pick long term. It means you
keep all the frustration of them not being fully in control (because
sometimes other people from outside the company jump in), never learning
how to driver the process themselves. And from the community pov it just
looks like code-drop over the wall. In my experience (I've been trying to
pull this off in public for almost 4 years now) trying to make exceptions
to get folks started just doesn't help anyone.

Imo contributors need to fence for their patches themselves (with you
helping behind the scenes ofc) from the start.

Cheers, Daniel

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 856116a..e06d066 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1454,6 +1454,7 @@ error_free_sched_entity:
>  void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>  {
>  	struct amdgpu_bo_va_mapping *mapping, *tmp;
> +	struct amdgpu_vm_id *id, *id_tmp;
>  	int i;
>  
>  	amd_sched_entity_fini(vm->entity.sched, &vm->entity);
> @@ -1478,14 +1479,17 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
>  	amdgpu_bo_unref(&vm->page_directory);
>  	fence_put(vm->page_directory_fence);
>  
> -	for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
> -		struct amdgpu_vm_id *id = vm->ids[i];
> -
> +	mutex_lock(&adev->vm_manager.lock);
> +	list_for_each_entry_safe(id, id_tmp, &adev->vm_manager.ids_lru,
> +				 list) {
>  		if (!id)
>  			continue;
> -
> -		atomic_long_cmpxchg(&id->owner, (long)vm, 0);
> +		if (atomic_long_read(&id->owner) == (long)vm) {
> +			atomic_long_set(&id->owner, 0);
> +			id->pd_gpu_addr = 0;
> +		}
>  	}
> +	mutex_unlock(&adev->vm_manager.lock);
>  }
>  
>  /**
> -- 
> 2.5.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel