Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] accel/habanalabs: disabling reset_work in hl_device_fini() strands in_reset --
commit eb768a82770d27a90572b30cd01bc34db4fd01ba Author: Chengyu Liu <[email protected]> accel/habanalabs: disable pending-reset work in hl_device_fini() This commit replaces flush_delayed_work() with disable_delayed_work_sync() in hl_device_fini() to prevent device_hard_reset_pending() from re-arming hdev->device_reset_work.reset_work during device teardown. > diff --git a/drivers/accel/habanalabs/common/device.c > b/drivers/accel/habanalabs/common/device.c > index 09b27bac3a31d..4635dc8572f03 100644 > --- a/drivers/accel/habanalabs/common/device.c > +++ b/drivers/accel/habanalabs/common/device.c > @@ -2487,7 +2487,7 @@ void hl_device_fini(struct hl_device *hdev) > dev_info(hdev->dev, "Removing device %s\n", > dev_name(&(hdev)->pdev->dev)); > > hdev->device_fini_pending = 1; > - flush_delayed_work(&hdev->device_reset_work.reset_work); > + disable_delayed_work_sync(&hdev->device_reset_work.reset_work); [Severity: Critical] Does cancelling and disabling hdev->device_reset_work.reset_work here leave hdev->reset_info.in_reset stuck at 1 and cause hl_device_fini() to time out and skip teardown? When a hard reset is initiated outside the reset thread, hl_device_reset() sets hdev->reset_info.in_reset = 1 and hands off clearing it to hdev->device_reset_work.reset_work: drivers/accel/habanalabs/common/device.c:hl_device_reset() { ... hdev->reset_info.in_compute_reset = !hard_reset; hdev->reset_info.in_reset = 1; spin_unlock(&hdev->reset_info.lock); ... if ((hard_reset) && (!from_hard_reset_thread)) { hdev->reset_info.hard_reset_pending = true; ... queue_delayed_work(hdev->reset_wq, &hdev->device_reset_work.reset_work, 0); return 0; } } When device_hard_reset_pending() runs hl_device_reset() and device_kill_open_processes() returns -EBUSY while device_fini_pending is 0, hl_device_reset() returns -EBUSY directly without jumping to out_err, keeping hdev->reset_info.in_reset set to 1 across the retry delay: drivers/accel/habanalabs/common/device.c:hl_device_reset() { ... rc = device_kill_open_processes(hdev, 0, false); if (rc == -EBUSY) { if (hdev->device_fini_pending) { dev_crit(hdev->dev, "%s Failed to kill all open processes, stopping hard reset\n", dev_name(&(hdev)->pdev->dev)); goto out_err; } /* signal reset thread to reschedule */ return rc; } ... out_err: ... hdev->reset_info.in_reset = 0; spin_unlock(&hdev->reset_info.lock); return rc; } Previously, setting hdev->device_fini_pending = 1 and calling flush_delayed_work() forced any pending reset_work to execute immediately with device_fini_pending == 1, which entered the goto out_err path and cleared hdev->reset_info.in_reset = 0. With disable_delayed_work_sync(), if reset_work is waiting on its retry timer (or queued on reset_wq before starting), or if a running callback already read hdev->device_fini_pending == 0 in hl_device_reset() before returning -EBUSY, or if a concurrent hl_device_reset() sets in_reset = 1 and queues reset_work before hl_device_fini() takes hdev->reset_info.lock, the work item never runs again to clear hdev->reset_info.in_reset. Won't hl_device_fini() then spin in while (device_in_reset) for the full HL_HARD_RESET_MAX_TIMEOUT (or HL_PLDM_HARD_RESET_MAX_TIMEOUT) and return early without cleaning up the device before hl_pci_remove() calls destroy_hdev()? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
