[PATCH] drm/amdgpu: Adjust logic around GTT size (v2)

[PATCH] drm/amdgpu: Adjust logic around GTT size (v2)

[Alex Deucher]

Certain GL unit tests for large textures can cause problems
with the OOM killer since there is no way to link this memory
to a process.  This was originally mitigated (but not necessarily
eliminated) by limiting the GTT size.  The problem is this limit
is often too low for many modern games so just make the limit 3/4
of system memory. The OOM accounting needs to be addressed, but
we shouldn't prevent common 3D applications from being usable
just to potentially mitigate that corner case.

Set default GTT size to 3/4 of system ram by default.

v2: drop previous logic and default to 3/4 of ram

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 4b9ee6e27f74..4966412024a3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1804,12 +1804,19 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
 		struct sysinfo si;
 
 		si_meminfo(&si);
-		gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
-			       adev->gmc.mc_vram_size),
-			       ((uint64_t)si.totalram * si.mem_unit * 3/4));
-	}
-	else
+		/* Certain GL unit tests for large textures can cause problems
+		 * with the OOM killer since there is no way to link this memory
+		 * to a process.  This was originally mitigated (but not necessarily
+		 * eliminated) by limiting the GTT size.  The problem is this limit
+		 * is often too low for many modern games so just make the limit 3/4
+		 * of system memory. The OOM accounting needs to be addressed, but
+		 * we shouldn't prevent common 3D applications from being usable
+		 * just to potentially mitigate that corner case.
+		 */
+		gtt_size = (u64)si.totalram * si.mem_unit * 3 / 4;
+	} else {
 		gtt_size = (uint64_t)amdgpu_gtt_size << 20;
+	}
 
 	/* Initialize GTT memory pool */
 	r = amdgpu_gtt_mgr_init(adev, gtt_size);
-- 
2.35.3

Re: [PATCH] drm/amdgpu: Adjust logic around GTT size (v2)

[Christian König]

Am 19.05.22 um 17:57 schrieb Alex Deucher:
> Certain GL unit tests for large textures can cause problems
> with the OOM killer since there is no way to link this memory
> to a process.  This was originally mitigated (but not necessarily
> eliminated) by limiting the GTT size.  The problem is this limit
> is often too low for many modern games so just make the limit 3/4
> of system memory. The OOM accounting needs to be addressed, but
> we shouldn't prevent common 3D applications from being usable
> just to potentially mitigate that corner case.
>
> Set default GTT size to 3/4 of system ram by default.
>
> v2: drop previous logic and default to 3/4 of ram

Well that is a pretty clear NAK.

This will cause massive problems for some test cases.

Christian.

>
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 17 ++++++++++++-----
>   1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 4b9ee6e27f74..4966412024a3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1804,12 +1804,19 @@ int amdgpu_ttm_init(struct amdgpu_device *adev)
>   		struct sysinfo si;
>   
>   		si_meminfo(&si);
> -		gtt_size = min(max((AMDGPU_DEFAULT_GTT_SIZE_MB << 20),
> -			       adev->gmc.mc_vram_size),
> -			       ((uint64_t)si.totalram * si.mem_unit * 3/4));
> -	}
> -	else
> +		/* Certain GL unit tests for large textures can cause problems
> +		 * with the OOM killer since there is no way to link this memory
> +		 * to a process.  This was originally mitigated (but not necessarily
> +		 * eliminated) by limiting the GTT size.  The problem is this limit
> +		 * is often too low for many modern games so just make the limit 3/4
> +		 * of system memory. The OOM accounting needs to be addressed, but
> +		 * we shouldn't prevent common 3D applications from being usable
> +		 * just to potentially mitigate that corner case.
> +		 */
> +		gtt_size = (u64)si.totalram * si.mem_unit * 3 / 4;
> +	} else {
>   		gtt_size = (uint64_t)amdgpu_gtt_size << 20;
> +	}
>   
>   	/* Initialize GTT memory pool */
>   	r = amdgpu_gtt_mgr_init(adev, gtt_size);