Re: [PATCH 7/7] drm/i915/gt: Add a spinlock to prevent starvation of irq_work.

[sashiko-bot@kernel.org]

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 <bigeasy@linutronix.de>

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/20260702080913.434121-1-dev@lankhorst.se?part=7