Re: [PATCH 2/4] drm/amdkfd: evict the queues only once for CRIU PROCESS_INFO

[Felix Kuehling]

On 2026-10-13 05:20, Jesse Zhang wrote:
criu_process_info() evicts the queues of the process on every call, and
each kfd_process_evict_queues() increments qpd->evicted, but CRIU UNPAUSE
restores them only once. After PROCESS_INFO twice and UNPAUSE the queues
stay evicted for good. On navi31, a barrier-AND packet on an HSA queue
completes after PROCESS_INFO + UNPAUSE, but times out after
PROCESS_INFO, PROCESS_INFO, UNPAUSE.

I'm not disagreeing with the patch. It makes the API more robust. But is 
this a valid use case? I.e., is CRIU actually calling PROCESS_INFO 
multiple times? Why?



The error path has a related problem: when kfd_process_evict_queues()
itself fails, it has already rolled back its partial eviction, and the
kfd_process_restore_queues() in the error path restores once more.

I agree. Your patch addresses this as well.



Evict only when the queues are not paused yet, and on errors only undo
an eviction done by this call. The barrier now completes after any
number of PROCESS_INFO calls followed by UNPAUSE.

Fixes: cd9f79103003 ("drm/amdkfd: CRIU Implement KFD unpause operation")
Signed-off-by: Jesse Zhang <jesse.zh...@amd.com>

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


---
  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 15 ++++++++++-----
  1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 1d86ddfd6bde..2e1d412a0e6b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -2869,6 +2869,7 @@ static int criu_process_info(struct file *filep,
                                struct kfd_process *p,
                                struct kfd_ioctl_criu_args *args)
  {
+       bool paused_here = false;
        int ret = 0;
  
  	mutex_lock(&p->mutex);
@@ -2879,11 +2880,15 @@ static int criu_process_info(struct file *filep,
                goto err_unlock;
        }
  
-	ret = kfd_process_evict_queues(p, KFD_QUEUE_EVICTION_CRIU_CHECKPOINT);
-       if (ret)
-               goto err_unlock;
+       /* UNPAUSE restores the queues once, so evict them only once */
+       if (!p->queues_paused) {
+               ret = kfd_process_evict_queues(p, 
KFD_QUEUE_EVICTION_CRIU_CHECKPOINT);
+               if (ret)
+                       goto err_unlock;
  
-	p->queues_paused = true;
+               p->queues_paused = true;
+               paused_here = true;
+       }
  
  	args->pid = task_pid_nr_ns(p->lead_thread,
                                        task_active_pid_ns(p->lead_thread));
@@ -2898,7 +2903,7 @@ static int criu_process_info(struct file *filep,
                                args->priv_data_size);
  
  err_unlock:
-       if (ret) {
+       if (ret && paused_here) {
                kfd_process_restore_queues(p);
                p->queues_paused = false;
        }