Re: [PATCH] drm/amdgpu: Place new CPU-accessbile BOs in GTT if visible VRAM is full

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

On 2017年05月19日 11:04, John Brooks wrote:
> Set GTT as the busy placement for newly created BOs that have the
> AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED flag, so that they don't cause
> established BOs to be evicted from visible VRAM.
You this patch is consistent with my opinion I mentioned in Marek thread 
(Plan: BO move throttling for visible VRAM evictions).
"I agree on this opinion, from our performance tuning experiment, this 
case indeed often happen especially under vram memory pressure. 
redirecting to GTT is better than heavy eviction between VRAM and GTT.
But we should get a condition for redirecting (eviction counter?), 
otherwise, BO have no change back to prefer domain."
You're using busy placement to solve it, then BO is trying its prefer 
domain first, so it has chance back to prefer domain when memory is free.
Although it cannot prevent evil program occupying memory, it still looks 
reasonable, since we can gain more from heavy eviction under vram 
pressure in most cases.

Above all, I vote 'Yes' to your idea, but I also want to listen what 
others' voice is.

Thanks,
David Zhou
>
> Signed-off-by: John Brooks <john@fastquake.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 365883d..655718a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -392,6 +392,17 @@ int amdgpu_bo_create_restricted(struct amdgpu_device *adev,
>   #endif
>   
>   	amdgpu_fill_placement_to_bo(bo, placement);
> +
> +	/* This is a new BO that wants to be CPU-visible; set GTT as the busy
> +	 * placement so that it does not evict established BOs from visible VRAM.
> +	 */
> +	if (domain & (AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT) &&
> +	    flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
> +		bo->placement.num_placement = 1;
> +		bo->placement.num_busy_placement = 1;
> +		bo->placement.busy_placement = &bo->placement.placement[1];
> +	}
> +
>   	/* Kernel allocation are uninterruptible */
>   
>   	initial_bytes_moved = atomic64_read(&adev->num_bytes_moved);
> @@ -484,6 +495,13 @@ int amdgpu_bo_create(struct amdgpu_device *adev,
>   	memset(&placements, 0,
>   	       (AMDGPU_GEM_DOMAIN_MAX + 1) * sizeof(struct ttm_place));
>   
> +
> +	/* New CPU-visible BOs will have GTT set as their busy placement */
> +	if (domain & AMDGPU_GEM_DOMAIN_VRAM &&
> +	    flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) {
> +		domain |= AMDGPU_GEM_DOMAIN_GTT;
> +	}
> +
>   	amdgpu_ttm_placement_init(adev, &placement,
>   				  placements, domain, flags);
>   

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