Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Acquiring `b->signaler_active_sync` with `spin_lock()` in `intel_context_remove_breadcrumbs()` without disabling interrupts creates a deadlock vulnerability and Lockdep splat on non-RT kernels. --
commit b2f8f390d7421e899486c8f32c2c91c29a783338 Author: Sebastian Andrzej Siewior <[email protected]> drm/i915/gt: Add a spinlock to prevent starvation of irq_work. This commit introduces a new spinlock, signaler_active_sync, to prevent starvation of irq-work. It acquires this lock in both signal_irq_work() and intel_context_remove_breadcrumbs() to force completion and synchronize signaler state. > diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c > b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c > --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c > +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c > @@ -209,6 +209,7 @@ static void signal_irq_work(struct irq_work *work) > intel_breadcrumbs_disarm_irq(b); > > rcu_read_lock(); > + spin_lock(&b->signaler_active_sync); > atomic_inc(&b->signaler_active); > list_for_each_entry_rcu(ce, &b->signalers, signal_link) { [ ... ] > @@ -487,8 +490,11 @@ void intel_context_remove_breadcrumbs(struct > intel_context *ce, > if (release) > intel_context_put(ce); > > - while (atomic_read(&b->signaler_active)) > + while (atomic_read(&b->signaler_active)) { > + spin_lock(&b->signaler_active_sync); > + spin_unlock(&b->signaler_active_sync); [Severity: High] Can this cause a deadlock on non-RT kernels? Because signal_irq_work() runs in hardirq context as an irq_work callback, acquiring signaler_active_sync here in intel_context_remove_breadcrumbs() without disabling interrupts creates a window for a self-deadlock. If a hardware interrupt occurs on the same CPU while this spinlock is momentarily held, the local irq_work will trigger and signal_irq_work() will try to acquire the already-held lock. Lockdep will also flag this as an inconsistent lock state (HARDIRQ-ON-W vs IN-HARDIRQ-W) on its first execution. Does this need to use spin_lock_irq() or spin_lock_irqsave() to safely synchronize against the irq_work? > cpu_relax(); > + } > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
