Re: [PATCH] drm/amdkfd: Terminate queues on surprise unplug with running processes

["Chen, Xiaogang" <xiaogang.chen@amd.com>]

On 4/22/2026 4:00 PM, Felix Kuehling wrote:
>
> On 2026-04-22 11:53, Chen, Xiaogang wrote:
>>
>> On 4/21/2026 8:56 PM, Kuehling, Felix wrote:
>>>
>>> On 2026-04-21 11:00, Chen, Xiaogang wrote:
>>>>
>>>> On 4/20/2026 4:25 PM, Mario Limonciello wrote:
>>>>>
>>>>>
>>>>> On 3/7/26 06:49, Mario Limonciello wrote:
>>>>>>
>>>>>>
>>>>>> On 1/12/26 12:29 PM, Mario Limonciello wrote:
>>>>>>> When a surprise unplug occurs while a process has active KFD 
>>>>>>> queues,
>>>>>>> userspace never gets a chance to call kfd_ioctl_destroy_queue() to
>>>>>>> properly clean them up. This leads to a WARN_ON in uninitialize()
>>>>>>> complaining about active_queue_count or processes_count being 
>>>>>>> non-zero.
>>>>>>>
>>>> During hot-unplug driver sends SIGBUS signal to all processes who 
>>>> are using the unplugged device. It is expected that affected 
>>>> processes will clean their workloads when get this signal.
>>>>
>>>> When a device got removed physically all sources from it will be 
>>>> removed. It is unnecessary(in theory) to clean them up. I am not 
>>>> surprised to see some software warnings due to hardware got 
>>>> physically removed since it is unexpected behavior at run time.
>>>>
>>>> I think what we need worry about is if there is memory leak. Driver 
>>>> also waits when an affected device is idle(by 
>>>> kgd2kfd_check_device_idle(adev)) by checking/waiting if there is 
>>>> process still using it. If there is no process using the being 
>>>> removed device the processes should have been terminated by same 
>>>> process termination logic from driver.
>>>
>>> The problem is, that a lot of the process termination stuff happens 
>>> in a worker thread. It can happen after the hot-unplug is already 
>>> done. That would lead to the cleanup worker accessing pointers to 
>>> device structures that are no longer there (or used by something else).
>>>
>>> We'd need to ensure proper synchronization so that the process 
>>> cleanup completes before the device unplug frees the device structures.
>>
>> How about at kgd2kfd_device_exit before doing any device clean up 
>> check/waiting there is no any kfd process run on this 
>> device(kgd2kfd_check_device_idle)?
>
> Looks like this should already be happening in this call chain: 
> amdgpu_device_ip_fini_early -> amdgpu_amdkfd_teardown_processes -> 
> kgd2kfd_teardown_processes -> kgd2kfd_check_device_idle
>
> Maybe whats missing at the end of kgd2kfd_teardown_processes is a 
> flush_workqueue(kfd_process_wq) to make sure that all the cleanup work 
> is done. After that, there should be no more process data structures 
> referencing the device.
>
We send signal SIGBUS to affected kfd processes(who are using the being 
removed device). The app signal handler will be executed asynchronously. 
There is a delay for signal handler got run. If call flush_workqueue 
immediately after sent the signal the kfd_process_ref_release(from 
kfd_release->kfd_unref_process) will not be got ran immediately after 
sent signal. Then flush_workqueue will not take effect since 
kfd_process_wq is empty or no filled work item yet.

Regards

Xiaogang


> Regards,
>   Felix
>
>
>>
>> Regards
>>
>> Xiaogang
>>
>>>
>>> Regards,
>>>   Felix
>>>
>>>
>>>
>>>>
>>>> Regards
>>>>
>>>> Xiaogang
>>>>
>>>>>>> The issue is that during surprise unplug:
>>>>>>> 1. amdgpu_device_fini_hw() checks drm_dev_is_unplugged()
>>>>>>> 2. It calls amdgpu_amdkfd_device_fini_sw()
>>>>>>> 3. This leads to kfd_cleanup_nodes() -> 
>>>>>>> device_queue_manager_uninit()
>>>>>>> 4. uninitialize() has: WARN_ON(dqm->active_queue_count > 0 ||
>>>>>>>     dqm->processes_count > 0)
>>>>>>>
>>>>>>> The warning triggers because the queues were never destroyed - 
>>>>>>> userspace
>>>>>>> had no opportunity to clean them up before the device disappeared.
>>>>>>>
>>>>>>> Fix this by checking for device unplug in kfd_cleanup_nodes() and
>>>>>>> calling process_termination for each affected process before
>>>>>>> uninitializing the DQM. This mirrors what happens during normal 
>>>>>>> process
>>>>>>> shutdown (kfd_process_notifier_release_internal), ensuring 
>>>>>>> queues are
>>>>>>> properly cleaned up even during surprise removal.
>>>>>>>
>>>>>>> Cc: Felix Kuehling <Felix.Kuehling@amd.com>
>>>>>>> Cc: Kent Russell <kent.russell@amd.com>
>>>>>>> Cc: Xiaogang.chen@amd.com
>>>>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>>>>
>>>>>> Ping?
>>>>> Ping?
>>>>>>
>>>>>>> ---
>>>>>>>   drivers/gpu/drm/amd/amdkfd/kfd_device.c | 32 
>>>>>>> ++++++++++++++++++++++++
>>>>>>>   1 file changed, 32 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
>>>>>>> b/drivers/gpu/ drm/amd/amdkfd/kfd_device.c
>>>>>>> index e9cfb80bd436..7727b66e6afb 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>>>>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
>>>>>>> @@ -664,6 +664,38 @@ static void kfd_cleanup_nodes(struct 
>>>>>>> kfd_dev *kfd, unsigned int num_nodes)
>>>>>>>       flush_workqueue(kfd->ih_wq);
>>>>>>>       destroy_workqueue(kfd->ih_wq);
>>>>>>> +    /*
>>>>>>> +     * For surprise unplugs with running processes, we need to 
>>>>>>> clean up
>>>>>>> +     * queues before uninitializing the DQM to avoid WARN in 
>>>>>>> uninitialize.
>>>>>>> +     * This handles the case where userspace can't destroy 
>>>>>>> queues normally.
>>>>>>> +     */
>>>>>>> +    if (drm_dev_is_unplugged(adev_to_drm(kfd->adev))) {
>>>>>>> +        struct kfd_process *p;
>>>>>>> +        unsigned int temp;
>>>>>>> +        int idx;
>>>>>>> +
>>>>>>> +        idx = srcu_read_lock(&kfd_processes_srcu);
>>>>>>> +        hash_for_each_rcu(kfd_processes_table, temp, p, 
>>>>>>> kfd_processes) {
>>>>>>> +            int j;
>>>>>>> +
>>>>>>> +            for (j = 0; j < p->n_pdds; j++) {
>>>>>>> +                struct kfd_process_device *pdd = p->pdds[j];
>>>>>>> +
>>>>>>> +                if (pdd->dev->kfd != kfd)
>>>>>>> +                    continue;
>>>>>>> +
>>>>>>> +                dev_info(kfd_device,
>>>>>>> +                     "Terminating queues for process %d on 
>>>>>>> unplugged device\n",
>>>>>>> +                     p->lead_thread->pid);
>>>>>>> +
>>>>>>> + pdd->dev->dqm->ops.process_termination(pdd->dev->dqm,
>>>>>>> + &pdd->qpd);
>>>>>>> +                pdd->already_dequeued = true;
>>>>>>> +            }
>>>>>>> +        }
>>>>>>> +        srcu_read_unlock(&kfd_processes_srcu, idx);
>>>>>>> +    }
>>>>>>> +
>>>>>>>       for (i = 0; i < num_nodes; i++) {
>>>>>>>           knode = kfd->nodes[i];
>>>>>>>           device_queue_manager_uninit(knode->dqm);
>>>>>>
>>>>>>
>>>>>