Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The panthor scheduler acquires `queue->fence_ctx.lock` using `spin_lock()` instead of `spin_lock_irqsave()`, violating the `dma_fence` locking rules and risking a deadlock against hardware interrupts. -- commit 3ecce8ed069220d684864af5f10bd10ad0ae008b Author: Boris Brezillon <[email protected]> drm/panthor: Don't update might_have_idle_groups in process_idle_event_locked() By scheduling an immediate tick, we already force idleness re-evaluation, which gives the scheduler the opportunity to evict idle groups and schedule onces that have jobs pending. > and schedule onces that have jobs pending. [Severity: Low] This isn't a bug, but there appears to be a typo (onces instead of ones) in the commit message. > diff --git a/drivers/gpu/drm/panthor/panthor_sched.c > b/drivers/gpu/drm/panthor/panthor_sched.c > index 237f6a75e624c..a5dfb1beafff8 100644 > --- a/drivers/gpu/drm/panthor/panthor_sched.c > +++ b/drivers/gpu/drm/panthor/panthor_sched.c > @@ -1734,8 +1734,6 @@ static void csg_slot_process_idle_event_locked(struct > panthor_device *ptdev, u32 > > lockdep_assert_held(&sched->lock); > > - sched->might_have_idle_groups = true; > - [Severity: High] This is a pre-existing issue, but looking at panthor_sched.c, queue->fence_ctx.lock is acquired using plain spin_lock() in multiple functions: queue_suspend_timeout() queue_resume_timeout() cs_slot_process_fault_event_locked() queue_check_job_completion() group_term_post_processing() queue_run_job() For example, in queue_check_job_completion(): spin_lock(&queue->fence_ctx.lock); ... This lock is used as the dma_fence lock for the job's done_fence. Since dma_fence objects can be shared with other subsystems (like KMS display drivers), they may be accessed from hardirq context (such as a VBLANK interrupt calling dma_fence_is_signaled() or dma_fence_add_callback(), which attempt to acquire the fence lock). If a hardware interrupt occurs on the same CPU while the panthor driver holds this lock with interrupts enabled, could the interrupt handler spin forever attempting to acquire the lock, causing a deadlock? Should these acquisitions be updated to use spin_lock_irqsave() to comply with dma_fence locking rules? > /* Schedule a tick so we can evict idle groups and schedule non-idle > * ones. This will also update runtime PM and devfreq busy/idle states, > * so the device can lower its frequency or get suspended. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=11
