Re: [PATCH 1/3] drm/amdgpu: move GTT to SHM after eviction for hibernation

From: Samuel Zhang
Date: Wed Jul 02 2025 - 03:29:27 EST



On 2025/7/1 16:22, Christian König wrote:
On 01.07.25 10:18, Zhang, GuoQing (Sam) wrote:
[AMD Official Use Only - AMD Internal Distribution Only]


Hi Christian,


Thank you for the feedback.


For “return ret < 0 ? ret : 0;”, it is equivalent to “return ret;” since ret is always <= 0 after the loop.
No it isn't.

ttm_global_swapout() returns the number of pages swapped out and only a negative error code if something went wrong.


/**
 * move GTT BOs to shmem for hibernation.
 *
 * returns 0 on success, negative on failure.
 */
int ttm_device_prepare_hibernation(void)
{
    struct ttm_operation_ctx ctx = {
        .interruptible = false,
        .no_wait_gpu = false,
        .force_alloc = true
    };
    int ret;

    do {
        ret = ttm_global_swapout(&ctx, GFP_KERNEL);
    } while (ret > 0);
    return ret;
}

This is the new code version.
If ttm_global_swapout() return positive number, the while loop will continue to the next iteration.
The while loop stops only when ttm_global_swapout() returns 0 or negative number. In both case, the new function can just return the ret.

The ret values printed in the do while loop:
[   53.745892] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 512
[   53.950975] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 35840
[   53.951713] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 9
[   67.712196] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 2187264
[   67.713726] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 512
[   67.759212] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 32768
[   67.761946] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 1024
[   67.762685] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 85
[   67.763518] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 175
[   67.767318] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 2367
[   67.767942] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 1
[   67.768499] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 1
[   67.769054] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 1
...
[   67.783554] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 1
[   67.785755] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 1
[   67.788607] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 1
[   67.789906] [TTM DEVICE] ttm_device_prepare_hibernation:164 ret 0


Regards
Sam




And it's probably not a good idea to return that from the new function.

Regards,
Christian.


For all other comments, I will revise the patch accordingly in v2.


Regards

Sam



*From: *Koenig, Christian <Christian.Koenig@xxxxxxx>
*Date: *Monday, June 30, 2025 at 19:54
*To: *Zhang, GuoQing (Sam) <GuoQing.Zhang@xxxxxxx>, rafael@xxxxxxxxxx <rafael@xxxxxxxxxx>, len.brown@xxxxxxxxx <len.brown@xxxxxxxxx>, pavel@xxxxxxxxxx <pavel@xxxxxxxxxx>, Deucher, Alexander <Alexander.Deucher@xxxxxxx>, Limonciello, Mario <Mario.Limonciello@xxxxxxx>, Lazar, Lijo <Lijo.Lazar@xxxxxxx>
*Cc: *Zhao, Victor <Victor.Zhao@xxxxxxx>, Chang, HaiJun <HaiJun.Chang@xxxxxxx>, Ma, Qing (Mark) <Qing.Ma@xxxxxxx>, amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>, dri-devel@xxxxxxxxxxxxxxxxxxxxx <dri-devel@xxxxxxxxxxxxxxxxxxxxx>, linux-pm@xxxxxxxxxxxxxxx <linux-pm@xxxxxxxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>
*Subject: *Re: [PATCH 1/3] drm/amdgpu: move GTT to SHM after eviction for hibernation

On 30.06.25 12:41, Samuel Zhang wrote:
When hibernate with data center dGPUs, huge number of VRAM BOs evicted
to GTT and takes too much system memory. This will cause hibernation
fail due to insufficient memory for creating the hibernation image.

Move GTT BOs to shmem in KMD, then shmem to swap disk in kernel
hibernation code to make room for hibernation image.
This should probably be two patches, one for TTM and then an amdgpu patch to forward the event.

Signed-off-by: Samuel Zhang <guoqing.zhang@xxxxxxx>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 13 ++++++++++++-
  drivers/gpu/drm/ttm/ttm_resource.c      | 18 ++++++++++++++++++
  include/drm/ttm/ttm_resource.h          |  1 +
  3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 4d57269c9ca8..5aede907a591 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2889,6 +2889,7 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
  int amdgpu_ttm_evict_resources(struct amdgpu_device *adev, int mem_type)
  {
        struct ttm_resource_manager *man;
+     int r;
        switch (mem_type) {
        case TTM_PL_VRAM:
@@ -2903,7 +2904,17 @@ int amdgpu_ttm_evict_resources(struct amdgpu_device *adev, int mem_type)
                return -EINVAL;
        }
-     return ttm_resource_manager_evict_all(&adev->mman.bdev, man);
+     r = ttm_resource_manager_evict_all(&adev->mman.bdev, man);
+     if (r) {
+             DRM_ERROR("Failed to evict memory type %d\n", mem_type);
+             return r;
+     }
+     if (adev->in_s4 && mem_type == TTM_PL_VRAM) {
+             r = ttm_resource_manager_swapout();
+             if (r)
+                     DRM_ERROR("Failed to swap out, %d\n", r);
+     }
+     return r;
  }
  #if defined(CONFIG_DEBUG_FS)
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index fd41b56e2c66..07b1f5a5afc2 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -534,6 +534,24 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
  }
  EXPORT_SYMBOL(ttm_resource_manager_init);
+int ttm_resource_manager_swapout(void)
This needs documentation, better placement and a better name.

First of all put it into ttm_device.c instead of the resource manager.

Then call it something like ttm_device_prepare_hibernation or similar.


+{
+     struct ttm_operation_ctx ctx = {
+             .interruptible = false,
+             .no_wait_gpu = false,
+             .force_alloc = true
+     };
+     int ret;
+
+     while (true) {
Make that:

do {
        ret = ...
} while (ret > 0);

+             ret = ttm_global_swapout(&ctx, GFP_KERNEL);
+             if (ret <= 0)
+                     break;
+     }
+     return ret;
It's rather pointless to return the number of swapped out pages.

Make that "return ret < 0 ? ret : 0;

Regards,
Christian.

+}
+EXPORT_SYMBOL(ttm_resource_manager_swapout);
+
  /*
   * ttm_resource_manager_evict_all
   *
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index b873be9597e2..46181758068e 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -463,6 +463,7 @@ void ttm_resource_manager_init(struct ttm_resource_manager *man,
  int ttm_resource_manager_evict_all(struct ttm_device *bdev,
                                   struct ttm_resource_manager *man);
+int ttm_resource_manager_swapout(void);
  uint64_t ttm_resource_manager_usage(struct ttm_resource_manager *man);
  void ttm_resource_manager_debug(struct ttm_resource_manager *man,