>> +
>> + 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)
