Re: [PATCH] accel/habanalabs: disable pending-reset work in hl_device_fini()

[sashiko-bot@kernel.org]

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 <lui2012qwq@gmail.com>

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/20261103163354.1599176-1-lui2012qwq@gmail.com?part=1