On 10/3/26 08:38, Olivier Kaloudoff wrote:
> On PCIe surprise removal (e.g. Thunderbolt eGPU link drop),
> amdgpu_device_fini_hw() calls amdgpu_amdkfd_device_fini_sw() when
> pci_dev_is_disconnected() is true, which frees adev->kfd.dev and sets
> it to NULL. amdgpu_device_ip_fini_early() then runs a few calls later
> in the same function and reaches kgd2kfd_teardown_processes(), which
> unconditionally dereferences adev->kfd.dev->kfd_processes_count.
>
> Observed on real hardware (MI50 eGPU behind TB3): NULL deref at +0x1b4
> (kfd_processes_count) inside kgd2kfd_teardown_processes, called from
> the pciehp removal thread; panic via panic_on_oops (pstore capture).
>
> Additionally, the wait-for-termination loop is unbounded: on a
> physically gone device, processes can be unable to make forward
> progress, wedging the pciehp removal thread in cond_resched() forever
> and cascading into hung-task/softlockup panics.
>
> Guard the wait on adev->kfd.dev and bound it to 30s with a dev_warn so
> removal can complete and the kernel survives.
That is clearly a really bad idea. Waiting for the termination *must* be
unbounded.
The real question is why those KFD processes doesn't terminate?
Regards,
Christian.
>
> Fixes: d12d05c4bc4c ("drm/amdkfd: Check if there are kfd porcesses using adev
> by kfd_processes_count")
> Cc: Xiaogang Chen <[email protected]>
> Signed-off-by: Olivier Kaloudoff <[email protected]>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_device.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> index 586e640..9326447 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
> @@ -1780,6 +1780,7 @@ bool kgd2kfd_vmfault_fast_path(struct amdgpu_device
> *adev, struct amdgpu_iv_entr
> void kgd2kfd_teardown_processes(struct amdgpu_device *adev)
> {
> struct hlist_node *p_temp;
> + unsigned long timeout;
> struct kfd_process *p;
> struct kfd_node *dev;
> unsigned int temp;
> @@ -1801,9 +1802,26 @@ void kgd2kfd_teardown_processes(struct amdgpu_device
> *adev)
>
> mutex_unlock(&kfd_processes_mutex);
>
> - /* wait all kfd processes use adev terminate */
> - while (!!atomic_read(&adev->kfd.dev->kfd_processes_count))
> + /* On surprise removal amdgpu_amdkfd_device_fini_sw() has already
> + * freed kfd.dev before we get here.
> + */
> + if (!adev->kfd.dev)
> + return;
> +
> + /* Wait for all kfd processes using adev to terminate. Bound the
> + * wait: on a physically gone device processes may never finish,
> + * and spinning here forever wedges the pciehp removal thread.
> + */
> + timeout = jiffies + msecs_to_jiffies(30000);
> + while (atomic_read(&adev->kfd.dev->kfd_processes_count)) {
> + if (time_after(jiffies, timeout)) {
> + dev_warn(adev->dev,
> + "kfd: %d processes still attached to removed
> device\n",
> +
> atomic_read(&adev->kfd.dev->kfd_processes_count));
> + break;
> + }
> cond_resched();
> + }
> }
>
> int kgd2kfd_reset_mes_queue(struct kfd_dev *kfd, uint32_t node_id,
