Re: [PATCH] drm/v3d: Using spin_lock_irqsave instead of spin_lock in the RT kernel

[Maíra Canal <mcanal@igalia.com>]

Hi Xiaolei,


On 04/03/26 00:17, Xiaolei Wang wrote:
> Disabling interrupts before calling 'spin_lock()' is unnecessary;
> its implementation is only for satisfying lockdep's requirements.

Yeah, the idea is really to satisfy lockdep's requirements. See comment
in `v3d_job_start_stats()`.

However, I sent a series recently to address this workaround [1]. I
appreciate reviews and comments about the series.

[1] 
https://lore.kernel.org/dri-devel/20260217-v3d-reset-locking-improv-v1-0-0db848016869@igalia.com/T/

Best regards,
- Maíra

> Since preemption is disabled by default when calling 'spin_lock()'
> in the standard kernel, both 'local_irq_save()' and 'preempt_disable()'
> become redundant. Therefore, we can replace 'spin_lock()' with
> 'spin_lock_irqsave()' to ensure compatibility between the standard
> kernel and the rt kernel. Avoid the following warning:
> 
> Hardware name: Raspberry Pi 5 Model B Rev 1.0 (DT)
> Call trace:
> dump_backtrace.part.0+0xe0/0x100
> show_stack+0x20/0x40
> dump_stack_lvl+0x60/0x80 dump_stack+0x18/0x28
> __might_resched+0x134/0x168
> rt_spin_lock+0x5c/0xe0
> v3d_job_update_stats+0x64/0x1d0 [v3d]
> v3d_irq+0xf0/0x410 [v3d]
> irq_forced_thread_fn+0x44/0xd8
> irq_thread+0x1ac/0x2d8
> kthread+0x124/0x138
> ret_from_fork+0x10/0x20
> 
> Fixes: fa6a20c87470 ("drm/v3d: Address race-condition between per-fd GPU stats and fd release")
> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> ---
>   drivers/gpu/drm/v3d/v3d_sched.c | 15 ++-------------
>   1 file changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
> index 6dc871fc9a62..5eaacb1d1480 100644
> --- a/drivers/gpu/drm/v3d/v3d_sched.c
> +++ b/drivers/gpu/drm/v3d/v3d_sched.c
> @@ -203,24 +203,13 @@ v3d_job_update_stats(struct v3d_job *job, enum v3d_queue q)
>   	u64 now = local_clock();
>   	unsigned long flags;
>   
> -	/* See comment in v3d_job_start_stats() */
> -	if (IS_ENABLED(CONFIG_LOCKDEP))
> -		local_irq_save(flags);
> -	else
> -		preempt_disable();
> -
>   	/* Don't update the local stats if the file context has already closed */
> -	spin_lock(&queue->queue_lock);
> +	spin_lock_irqsave(&queue->queue_lock, flags);
>   	if (job->file_priv)
>   		v3d_stats_update(&job->file_priv->stats[q], now);
> -	spin_unlock(&queue->queue_lock);
> +	spin_unlock_irqrestore(&queue->queue_lock, flags);
>   
>   	v3d_stats_update(global_stats, now);
> -
> -	if (IS_ENABLED(CONFIG_LOCKDEP))
> -		local_irq_restore(flags);
> -	else
> -		preempt_enable();
>   }
>   
>   static struct dma_fence *v3d_bin_job_run(struct drm_sched_job *sched_job)