Am Sonntag, 1. November 2026, 09:22:55 Mitteleuropäische Normalzeit schrieb
Jiaxing Hu:
> drm_sched_stop() does not wait for a threaded handler that is already
> running. Call synchronize_irq() after it, outside job_lock, which the
> handler takes.
>
> Before the sync, mask the block's interrupt and clear its raw status, so
> that an active core cannot signal a completion after it. Do that under
> job_lock, since rocket_job_hw_submit() arms the same mask under that lock,
> and only when pm_runtime_get_if_active() returns a positive count: the
> reset holds no runtime PM reference, and with the domain down a register
> access takes an async SError.
>
> Igor Paunovic's induced-reset runs on RK3588, including a two-task job that
> puts hw_submit() on the IRQ thread, found no fault; as he put it, "this
> does not show the race is closed".
>
> Link: https://lore.kernel.org/all/[email protected]/
> Link:
> https://lore.kernel.org/all/caewpsh5mxtbuknouxm6yecmzyvdowquhvyvhaxq8homthd5...@mail.gmail.com/
> Link: https://lore.kernel.org/all/[email protected]/
> Link:
> https://lore.kernel.org/all/[email protected]/
> Link:
> https://lore.kernel.org/all/[email protected]/
> Suggested-by: Igor Paunovic <[email protected]>
> Signed-off-by: Jiaxing Hu <[email protected]>
> Tested-by: Igor Paunovic <[email protected]> # RK3588, three cores,
> induced reset, JOB_TIMEOUT_MS=2
> ---
> drivers/accel/rocket/rocket_job.c | 71 +++++++++++++++++++++++++++++--
> 1 file changed, 68 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/accel/rocket/rocket_job.c
> b/drivers/accel/rocket/rocket_job.c
> index 03e5f094f60b..5b2abf40b618 100644
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -401,9 +401,74 @@ rocket_reset(struct rocket_core *core, struct
> drm_sched_job *bad)
> drm_sched_stop(&core->sched, bad);
>
> /*
> - * Remaining interrupts have been handled, but we might still have
> - * stuck jobs. Let's make sure the PM counters stay balanced by
> - * manually calling pm_runtime_put_noidle().
> + * Mask the block before waiting. hw_submit() arms INTERRUPT_MASK on
> + * every submit and only the hardirq clears it, so on an ordinary
> + * timeout it is still live and a completion can arrive after the sync
> + * returns. The next submit re-arms it, so nothing is lost here.
> + *
> + * Only when the device is already awake, though. This function holds no
> + * runtime PM reference of its own: the only one in the window belongs
> to
> + * in_flight_job, and the completion path may have put it and cleared
> the
> + * pointer before the timeout worker got here. drm_sched_stop() above
> can
> + * block for a long time, and it drops every pending job's credits, so
> + * rocket_job_is_idle() is true and nothing keeps the core resumed. On
> + * this hardware a register access with the domain down takes an async
> + * SError, so a reset must not be the thing that causes one.
> + *
> + * Only a positive answer will do. pm_runtime_get_if_active() tests
> + * power.disable_depth before power.runtime_status, so -EINVAL MASKS a
> + * suspended device rather than excluding one:
> pm_runtime_force_suspend(),
> + * which is this driver's own system suspend callback, disables runtime
> PM
> + * first and turns the clocks off second, and rocket_core_fini()
> suspends
> + * the core and disables before it cancels the timeout worker. Both
> leave
> + * the domain down with -EINVAL on offer.
> + *
> + * The cost is the other half of that ambiguity. A core that is still up
> + * with runtime PM disabled (pm_runtime_force_suspend() before its
> + * callback has run, or CONFIG_PM=n under COMPILE_TEST) is left
> unmasked,
> + * because writing to it would mean writing to the half that is down as
> + * well.
> + *
> + * Clear the raw status along with the mask, the way the completion path
> + * does. Masking alone leaves the DPU bit latched until
> + * rocket_core_reset(), and the hardirq decides on raw status alone, so
> a
> + * fault from the IOMMU that shares this line would wake the thread
> again
> + * and what the comment below asserts would stop being true.
> + *
> + * UNDER job_lock, because rocket_job_hw_submit() arms this same
> + * register and always runs under that lock. reset.pending is set here
> + * without the lock and read there with it, so a submit that has already
> + * passed its check can re-arm the mask after this clears it, and then
> + * the synchronize_irq() below fences a handler that is no longer the
> + * one that matters: the block is left running a task with its
> + * interrupt live. rocket_job_handle_irq() avoids the same race on
> + * OPERATION_ENABLE by making its completion writes under this lock.
> + *
> + * pm_runtime_get_if_active() does not invoke a callback -- it only
> + * takes a reference on an already-active device -- and
> + * pm_runtime_put_autosuspend() is asynchronous, so neither can re-enter
> + * this driver's runtime PM callbacks while the lock is held.
This can be way short please, the wall of text is very hard to read.
> + */
> + scoped_guard(mutex, &core->job_lock) {
> + if (pm_runtime_get_if_active(core->dev) > 0) {
> + rocket_pc_writel(core, INTERRUPT_MASK, 0x0);
> + rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
> + pm_runtime_put_autosuspend(core->dev);
> + }
> + }
> +
> + /*
> + * drm_sched_stop() returns without waiting for a threaded handler that
> + * is already running, so wait for one here. This has to stay outside
> + * job_lock: the handler takes that lock, so waiting for it while
> + * holding it would deadlock instead of fencing anything.
> + */
> + synchronize_irq(core->irq);
> +
> + /*
> + * No handler is running now, but we might still have stuck jobs. Let's
> + * make sure the PM counters stay balanced by manually calling
> + * pm_runtime_put_noidle().
> */
> scoped_guard(mutex, &core->job_lock) {
> if (core->in_flight_job)
>
