+
+ if (q_data->type == KFD_QUEUE_TYPE_COMPUTE) {
+ if (q_data->ctl_stack_size)
+ qp->ctl_stack_size = q_data->ctl_stack_size / num_xcc;
+ else if (topo_dev)
+ qp->ctl_stack_size = topo_dev->node_props.ctl_stack_size;
I'm curious what this case is needed for. In what circumstances would a
CRIU checkpoint contain 0 ctl_stack_size? And is that's the case,
wouldn't that break restore anyway?
A lot of mqd managers don't set mqd->get_checkpoint_info,
so the ctl_stack_size ends up as 0.
kfd_queue_acquire_buffers requires the ctl_stack_size to be equal to
topo_dev->node_props.ctl_stack_size anyways, so I think it
makes sense to default to that value if the input is 0.
David Francis
________________________________________
From: Kuehling, Felix <[email protected]>
Sent: Friday, October 30, 2026 5:15 PM
To: Francis, David; [email protected]
Cc: Koenig, Christian
Subject: Re: [PATCH v2 1/3] drm/amdkfd: Acquire buffers in CRIU queue restore
(again)
On 2026-10-30 10:05, David Francis wrote:
CRIU queue restore doesn't acquire the bo buffers in the same way
normal queue creation does. In normal queue creation, the driver
gets an additional reference on the queue's underlying buffers
that lasts until the queue is destroyed; this prevents the
queue's buffers from being removed from underneath it.
Acquire them, but don't get the svm bo. SVM isn't yet fully
restored by this point in CRIU and so there's no svm range to get.
This will be addressed in the next patch.
Signed-off-by: David Francis <[email protected]>
---
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 2 +-
drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 2 +-
.../amd/amdkfd/kfd_process_queue_manager.c | 34 +++++++++++++++++--
drivers/gpu/drm/amd/amdkfd/kfd_queue.c | 8 ++++-
4 files changed, 41 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 6c710e4a5220c..d8233337b8d5e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -433,7 +433,7 @@ static int kfd_ioctl_create_queue(struct file *filep,
struct kfd_process *p,
goto err_bind_process;
}
- err = kfd_queue_acquire_buffers(pdd, &q_properties);
+ err = kfd_queue_acquire_buffers(pdd, &q_properties, false);
if (err) {
pr_debug("failed to acquire user queue buffers\n");
goto err_acquire_queue_buf;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index f0ce5fc4a138f..eb4d35a42e917 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1381,7 +1381,7 @@ void print_queue(struct queue *q);
int kfd_queue_buffer_get(struct amdgpu_vm *vm, void __user *addr, struct
amdgpu_bo **pbo,
u64 expected_size);
void kfd_queue_buffer_put(struct amdgpu_bo **bo);
-int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct
queue_properties *properties);
+int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct
queue_properties *properties, bool is_criu);
int kfd_queue_release_buffers(struct kfd_process_device *pdd, struct
queue_properties *properties);
void kfd_queue_unref_bo_va(struct amdgpu_vm *vm, struct amdgpu_bo **bo);
int kfd_queue_unref_bo_vas(struct kfd_process_device *pdd,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index 4c4a3a8587f46..509529af2dc3c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -985,8 +985,14 @@ int kfd_criu_checkpoint_queues(struct kfd_process *p,
}
static void set_queue_properties_from_criu(struct queue_properties *qp,
- struct kfd_criu_queue_priv_data *q_data)
+ struct kfd_criu_queue_priv_data
*q_data,
+ struct kfd_process_device *pdd)
{
+ struct kfd_topology_device *topo_dev;
+ uint32_t num_xcc = NUM_XCC(pdd->dev->xcc_mask);
+
+ topo_dev = kfd_topology_device_by_id(pdd->dev->id);
+
qp->is_interop = false;
qp->queue_percent = q_data->q_percent;
qp->priority = q_data->priority;
@@ -998,6 +1004,16 @@ static void set_queue_properties_from_criu(struct
queue_properties *qp,
qp->eop_ring_buffer_size = q_data->eop_ring_buffer_size;
qp->ctx_save_restore_area_address =
q_data->ctx_save_restore_area_address;
qp->ctx_save_restore_area_size = q_data->ctx_save_restore_area_size;
+
+ if (q_data->type == KFD_QUEUE_TYPE_COMPUTE) {
+ if (q_data->ctl_stack_size)
+ qp->ctl_stack_size = q_data->ctl_stack_size / num_xcc;
+ else if (topo_dev)
+ qp->ctl_stack_size = topo_dev->node_props.ctl_stack_size;
I'm curious what this case is needed for. In what circumstances would a
CRIU checkpoint contain 0 ctl_stack_size? And is that's the case,
wouldn't that break restore anyway?
Other than that, this patch looks good to me.
Regards,
Felix
+ } else {
+ qp->ctl_stack_size = q_data->ctl_stack_size;
+ }
+
qp->type = q_data->type;
qp->format = q_data->format;
}
@@ -1047,7 +1063,13 @@ int kfd_criu_restore_queue(struct kfd_process *p,
}
memset(&qp, 0, sizeof(qp));
- set_queue_properties_from_criu(&qp, q_data);
+ set_queue_properties_from_criu(&qp, q_data, pdd);
+
+ if (q_data->type == KFD_QUEUE_TYPE_COMPUTE && !qp.ctl_stack_size) {
+ pr_err("Failed to determine ctl_stack_size for compute queue\n");
+ ret = -EINVAL;
+ goto exit;
+ }
/* Per-XCC control stack size from topology, as create-queue requires */
if (qp.type == KFD_QUEUE_TYPE_COMPUTE && pdd->dev->kfd->cwsr_enabled) {
@@ -1095,9 +1117,17 @@ int kfd_criu_restore_queue(struct kfd_process *p,
print_queue_properties(&qp);
+ ret = kfd_queue_acquire_buffers(pdd, &qp, true);
+ if (ret) {
+ pr_err("Failed to acquire user queue buffers err:%d\n", ret);
+ goto exit;
+ }
+
ret = pqm_create_queue(&p->pqm, pdd->dev, &qp, &queue_id, q_data, mqd,
ctl_stack, NULL);
if (ret) {
pr_err("Failed to create new queue err:%d\n", ret);
+ kfd_queue_unref_bo_vas(pdd, &qp);
+ kfd_queue_release_buffers(pdd, &qp);
goto exit;
}
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
index 6010a5d7a7eb9..18d24ddb455eb 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_queue.c
@@ -239,7 +239,7 @@ static void kfd_queue_put_bos(struct queue_properties
*properties)
kfd_queue_buffer_put(&properties->cwsr_bo);
}
-int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct
queue_properties *properties)
+int kfd_queue_acquire_buffers(struct kfd_process_device *pdd, struct
queue_properties *properties, bool is_criu)
{
struct kfd_topology_device *topo_dev;
u64 expected_queue_size;
@@ -334,6 +334,12 @@ int kfd_queue_acquire_buffers(struct kfd_process_device
*pdd, struct queue_prope
amdgpu_bo_unreserve(vm->root.bo);
+ /* CRIU svm buffers are restored at a later stage than queues. They'll be
in
+ * before the queue is actually used, so no risk here
+ */
+ if (is_criu)
+ return 0;
+
err = kfd_queue_buffer_svm_get(pdd,
properties->ctx_save_restore_area_address,
total_cwsr_size);
if (err)
