[PATCH] drm/amdkfd: remove unnecessary cpu domain validation

[PATCH] drm/amdkfd: remove unnecessary cpu domain validation

[James Zhu]

before move to GTT domain.

Signed-off-by: James Zhu <James.Zhu@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 62ca12e94581..2ac6d4fa0601 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -595,12 +595,6 @@ kfd_mem_dmamap_dmabuf(struct kfd_mem_attachment *attachment)
 {
 	struct ttm_operation_ctx ctx = {.interruptible = true};
 	struct amdgpu_bo *bo = attachment->bo_va->base.bo;
-	int ret;
-
-	amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU);
-	ret = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
-	if (ret)
-		return ret;
 
 	amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT);
 	return ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
-- 
2.34.1

Re: [PATCH] drm/amdkfd: remove unnecessary cpu domain validation

[Christian König]

Am 03.03.25 um 19:45 schrieb James Zhu:
> before move to GTT domain.

That might not be unnecessary. We sometimes intentionally move BOs to the CPU domain to invalidate all VM mappings.

Christian.

>
> Signed-off-by: James Zhu <James.Zhu@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 62ca12e94581..2ac6d4fa0601 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -595,12 +595,6 @@ kfd_mem_dmamap_dmabuf(struct kfd_mem_attachment *attachment)
>  {
>  	struct ttm_operation_ctx ctx = {.interruptible = true};
>  	struct amdgpu_bo *bo = attachment->bo_va->base.bo;
> -	int ret;
> -
> -	amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU);
> -	ret = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
> -	if (ret)
> -		return ret;
>  
>  	amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT);
>  	return ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);

Re: [PATCH] drm/amdkfd: remove unnecessary cpu domain validation

[Felix Kuehling]

On 2025-03-03 13:48, Christian König wrote:
> Am 03.03.25 um 19:45 schrieb James Zhu:
>> before move to GTT domain.
> That might not be unnecessary. We sometimes intentionally move BOs to the CPU domain to invalidate all VM mappings.

We discussed this in our VM sync meeting this morning, and I wasn't 100% sure either. In this case it's causing a nearly live-lock of mutual evictions when two processes share the same BO and have both their eviction fences on the shared reservation object.

I was thinking the VM invalidation should be taken care of by the DMABuf move notifier, so this explicit invalidation seemed redundant. Is there a way we can test or otherwise verify that we're not missing anything?

Thanks,
  Felix


>
> Christian.
>
>> Signed-off-by: James Zhu <James.Zhu@amd.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 6 ------
>>  1 file changed, 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> index 62ca12e94581..2ac6d4fa0601 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>> @@ -595,12 +595,6 @@ kfd_mem_dmamap_dmabuf(struct kfd_mem_attachment *attachment)
>>  {
>>  	struct ttm_operation_ctx ctx = {.interruptible = true};
>>  	struct amdgpu_bo *bo = attachment->bo_va->base.bo;
>> -	int ret;
>> -
>> -	amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU);
>> -	ret = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
>> -	if (ret)
>> -		return ret;
>>  
>>  	amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT);
>>  	return ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);

Re: [PATCH] drm/amdkfd: remove unnecessary cpu domain validation

[Christian König]

Am 03.03.25 um 22:01 schrieb Felix Kuehling:
> On 2025-03-03 13:48, Christian König wrote:
>> Am 03.03.25 um 19:45 schrieb James Zhu:
>>> before move to GTT domain.
>> That might not be unnecessary. We sometimes intentionally move BOs to the CPU domain to invalidate all VM mappings.
> We discussed this in our VM sync meeting this morning, and I wasn't 100% sure either. In this case it's causing a nearly live-lock of mutual evictions when two processes share the same BO and have both their eviction fences on the shared reservation object.
>
> I was thinking the VM invalidation should be taken care of by the DMABuf move notifier, so this explicit invalidation seemed redundant. Is there a way we can test or otherwise verify that we're not missing anything?

We used this hack to validate into system domain and then GTT for userptrs, but I can't think of any reason to do this for DMA-buf imports.

Regards,
Christian.

>
> Thanks,
>   Felix
>
>
>> Christian.
>>
>>> Signed-off-by: James Zhu <James.Zhu@amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 6 ------
>>>  1 file changed, 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>> index 62ca12e94581..2ac6d4fa0601 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>> @@ -595,12 +595,6 @@ kfd_mem_dmamap_dmabuf(struct kfd_mem_attachment *attachment)
>>>  {
>>>  	struct ttm_operation_ctx ctx = {.interruptible = true};
>>>  	struct amdgpu_bo *bo = attachment->bo_va->base.bo;
>>> -	int ret;
>>> -
>>> -	amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU);
>>> -	ret = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
>>> -	if (ret)
>>> -		return ret;
>>>  
>>>  	amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT);
>>>  	return ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);

Re: [PATCH] drm/amdkfd: remove unnecessary cpu domain validation

[Felix Kuehling]

On 2025-03-04 5:38, Christian König wrote:
>
> Am 03.03.25 um 22:01 schrieb Felix Kuehling:
>> On 2025-03-03 13:48, Christian König wrote:
>>> Am 03.03.25 um 19:45 schrieb James Zhu:
>>>> before move to GTT domain.
>>> That might not be unnecessary. We sometimes intentionally move BOs to the CPU domain to invalidate all VM mappings.
>> We discussed this in our VM sync meeting this morning, and I wasn't 100% sure either. In this case it's causing a nearly live-lock of mutual evictions when two processes share the same BO and have both their eviction fences on the shared reservation object.
>>
>> I was thinking the VM invalidation should be taken care of by the DMABuf move notifier, so this explicit invalidation seemed redundant. Is there a way we can test or otherwise verify that we're not missing anything?
> We used this hack to validate into system domain and then GTT for userptrs, but I can't think of any reason to do this for DMA-buf imports.

OK, I had the same intuition. In that case the patch is

Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>


>
> Regards,
> Christian.
>
>> Thanks,
>>   Felix
>>
>>
>>> Christian.
>>>
>>>> Signed-off-by: James Zhu <James.Zhu@amd.com>
>>>> ---
>>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 6 ------
>>>>  1 file changed, 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>>> index 62ca12e94581..2ac6d4fa0601 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>>> @@ -595,12 +595,6 @@ kfd_mem_dmamap_dmabuf(struct kfd_mem_attachment *attachment)
>>>>  {
>>>>  	struct ttm_operation_ctx ctx = {.interruptible = true};
>>>>  	struct amdgpu_bo *bo = attachment->bo_va->base.bo;
>>>> -	int ret;
>>>> -
>>>> -	amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_CPU);
>>>> -	ret = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
>>>> -	if (ret)
>>>> -		return ret;
>>>>  
>>>>  	amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT);
>>>>  	return ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);