Hi Felix, > I'd avoid arbitrary open-coded timeout values. Would > AMDGPU_FENCE_JIFFIES_TIMEOUT work? It's currently half a second.
Yes, that's better — will switch to AMDGPU_FENCE_JIFFIES_TIMEOUT. > This seems racy. Should we use drm_dev_enter/exit instead? Agreed — that matches the drm_dev_is_unplugged() doc recommendation. New shape: drm_dev_enter() -> svm_migrate_copy_memory() submit -> drm_dev_exit() before the fence wait in svm_migrate_copy_done() (can't hold it across the wait — drm_dev_unplug() waits for exits, and a fence on a dying device may never signal). The bounded wait in copy_done then covers the submit->wait window. I'd keep pci_dev_is_disconnected() as an additional early-out since the PCI flag can be set well before drm_dev_unplug() runs. > I'm not convinced this is needed. cancel_delayed_work_sync in > svm_range_list_fini should take care of terminating the worker, even > if it rescheduled itself. It's more of an optimization to avoid > rescheduling the worker unnecessarily when it's known to be useless. > Either way, this would make more sense in patch 2, since it's > similar to the restore_userptr_worker change. Same situation as the userptr worker: svm_range_list_fini only runs at process teardown, which on unplug can be delayed indefinitely (the deadlocked exit path above). But yes, it's an optimization rather than a correctness fix — happy to fold it into patch 2 with the userptr worker change. Will respin as v4 with these changes. Olivier
