[PATCH v2 0/2] accel/rocket: fix shared-device lifecycle on probe failure and unbind

Igor Paunovic posted 2 patches 2 months ago
drivers/accel/rocket/rocket_device.c |  2 ++
drivers/accel/rocket/rocket_device.h |  3 +++
drivers/accel/rocket/rocket_drv.c    | 41 ++++++++++++++++++++++++++++++++----
drivers/accel/rocket/rocket_job.c    | 13 +++++++------
4 files changed, 53 insertions(+), 10 deletions(-)
[PATCH v2 0/2] accel/rocket: fix shared-device lifecycle on probe failure and unbind
Posted by Igor Paunovic 2 months ago
The rocket driver keeps a single shared DRM device on a driverless
"rknn" platform device: the first core to probe initializes it, the
last one to go away tears it down. This series fixes two independent
bugs in that lifecycle. Both were flagged by the Sashiko AI review on
my clks patch; I verified each by hand against the code and then on
hardware before writing the fixes.

Patch 1 releases the devres of the shared device on teardown. Today
every fini/re-init cycle leaks the previous rocket_device and pins its
accel minor - observable as /dev/accel/accel0 coming back as accel1,
then accel2, on unbind/rebind cycles of all cores.

Patch 2 makes the per-core slot bookkeeping stable across unbind and
rebind in any order. Today unbinding a lower-numbered core makes
higher-numbered ones unfindable (their runtime PM callbacks start
returning -ENODEV), a later unbind of such a core is silently skipped,
and a subsequent bind overwrites a slot whose IRQ handler and DRM
scheduler are still live.

Verified on RK3588 (Orange Pi 5 Plus, all three cores): the
unbind/rebind matrix keeps /dev/accel/accel0 stable and every core
findable; single-core operation works from the highest slot alone
(confirmed via the per-core IRQ counters moving to that core); the
forced-init-failure path releases its slot cleanly; and MobileNetV1
inference via the Teflon TFLite delegate stays bit-identical to the
stock driver throughout.

The series applies on top of Guangshuo Li's pending fix, on which
patch 1 depends textually (reviewed on-list):
https://lore.kernel.org/dri-devel/20260708062845.716487-1-lgs201920130244@gmail.com/

v2:
 - patch 2: clear the slot's .dev when rocket_core_init() fails -
   with .dev as the liveness marker a failed init left a
   half-initialised core visible to lookups and made
   rocket_job_open()'s live-slot walk overflow its allocation by one
   entry (Jiaxing Hu); make the never-initialised slot skip in
   sched_to_core() explicit; document the synchronous-probe assumption
 - patch 1: unchanged
v1: https://lore.kernel.org/dri-devel/20260730080355.177422-1-royalnet026@gmail.com/

Igor Paunovic (2):
  accel/rocket: release the shared device's devres on teardown
  accel/rocket: keep core slots stable across unbind and rebind

 drivers/accel/rocket/rocket_device.c |  2 ++
 drivers/accel/rocket/rocket_device.h |  3 +++
 drivers/accel/rocket/rocket_drv.c    | 41 ++++++++++++++++++++++++++++++++----
 drivers/accel/rocket/rocket_job.c    | 13 +++++++------
 4 files changed, 53 insertions(+), 10 deletions(-)

--
2.53.0
Re: [PATCH v2 0/2] accel/rocket: fix shared-device lifecycle on probe failure and unbind
Posted by Igor Paunovic 1 month, 2 weeks ago
Hi Tomeu,

A ping on this one. It has been on the list since 31 July and 2/2
carries Jiaxing's Reviewed-by.

I am pinging now rather than just waiting because the failure it
describes stopped being a code reading yesterday. I hit it on hardware
while testing Jiaxing's v7 RK3576 series on RK3588, on a tree that did
not have this series applied.

The sequence is the one 2/2 predicts: unbind core 0 while cores 1 and 2
stay bound, then bind it back. What happens without these patches, on
an Orange Pi 5 Plus with all three NPU cores:

  - the returning core takes the index of a core that is still live.
    The driver prints

      rocket fdab0000.npu: Rockchip NPU core 2 version: 1179210309

    for the device that is physically core 0;

  - inference stops being correct. The same MobileNet V1 input that
    classified as "military uniform" before the rebind classifies as
    "toilet tissue" after it, and the oracle hashes change with it;

  - throughput falls from 88.6 to 1.9 inferences per second, and core 0
    stops taking interrupts entirely - 0.00 per inference where it had
    been taking 41.74 - while core 1 absorbs everything;

  - the next unbind then dies. Ten "NPU job timed out" in a row,
    followed by

      Unable to handle kernel paging request at virtual address
      dead000000000122
      pc : destroy_workqueue+0x1b8/0x3e0
      Call trace:
       destroy_workqueue+0x1b8/0x3e0
       drm_sched_fini+0x178/0x1a8 [gpu_sched]
       rocket_job_fini+0x28/0x60 [rocket]
       rocket_core_fini+0x4c/0x78 [rocket]
       rocket_remove+0x6c/0x150 [rocket]
       ... unbind_store

    That is LIST_POISON2 being dereferenced. The sysfs write never
    returns, the task is in uninterruptible sleep and cannot be killed,
    and the machine needs a reboot.

That is the second bullet of 2/2's commit message happening: the bind
reuses the index of a still-live core and overwrites its slot while its
IRQ handler and its DRM scheduler are still active.

With the two patches applied and nothing else changed, the same
sequence - core 2 out and back, core 0 out and back, all three out and
all three back - runs clean. Twelve inference runs across two modules,
one oracle hash for all of them, correct classification every time, and
nothing in dmesg beyond the probe messages.

I am happy to resend with the tag collected if that is easier. There
is also a practical reason to have it in: the three-core RK3588 test
that Jiaxing asked for on his v7 cannot run to completion on a tree
without this, because the core-0 rebind step is what trips it.

Thanks,
Igor
Re: [PATCH v2 0/2] accel/rocket: fix shared-device lifecycle on probe failure and unbind
Posted by Igor Paunovic 1 week, 3 days ago
Hi Tomeu,

A status update on this series.

2/2 now travels as patch 4 of the DVFS v2 series, as v3, rebased onto
the slot fixes it builds on:
https://lore.kernel.org/r/20260922080114.44662-5-royalnet026@gmail.com
Its commit message shows the crash it fixes, and the notes under ---
list what changed since v2. Jiaxing, v3 changes the code you reviewed
here: the scheduler list is checked for allocation failure and freed
when only one core is live, and the patch sits on the slot fixes before
it. So I did not carry your Reviewed-by; a look at the new version
would be welcome.

1/2 is not in that series. It builds on Guangshuo Li's "accel/rocket:
clear rdev on device init failure", which is not in drm-misc-next yet:
https://lore.kernel.org/r/20260708062845.716487-1-lgs201920130244@gmail.com
I will resend 1/2 on top of it once that one is in.

Thanks,
Igor