On Thu, Jun 25, 2026 at 02:40:27PM +0200, Boris Brezillon wrote:
> Since dma_fence objects can be shared with other subsystems, they may be
> accessed from hardirq context in those drivers, and we have to take
> that into account by also using the IRQ-safe variant when acquiring
> the lock.
>
> While at it, switch to the guard model.
>
> Fixes: de8548813824 ("drm/panthor: Add the scheduler logical block")
> Reported-by: [email protected]
> Closes:
> https://sashiko.dev/#/patchset/[email protected]?part=11
> Signed-off-by: Boris Brezillon <[email protected]>
Reviewed-by: Liviu Dudau <[email protected]>
Best regards,
Liviu
> ---
> drivers/gpu/drm/panthor/panthor_sched.c | 83
> ++++++++++++++++-----------------
> 1 file changed, 39 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c
> b/drivers/gpu/drm/panthor/panthor_sched.c
> index 5b34032deff8..e97f29469d28 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -1151,15 +1151,14 @@ queue_suspend_timeout_locked(struct panthor_queue
> *queue)
> static void
> queue_suspend_timeout(struct panthor_queue *queue)
> {
> - spin_lock(&queue->fence_ctx.lock);
> + guard(spinlock_irqsave)(&queue->fence_ctx.lock);
> queue_suspend_timeout_locked(queue);
> - spin_unlock(&queue->fence_ctx.lock);
> }
>
> static void
> queue_resume_timeout(struct panthor_queue *queue)
> {
> - spin_lock(&queue->fence_ctx.lock);
> + guard(spinlock_irqsave)(&queue->fence_ctx.lock);
>
> if (queue_timeout_is_suspended(queue)) {
> mod_delayed_work(queue->scheduler.timeout_wq,
> @@ -1168,8 +1167,6 @@ queue_resume_timeout(struct panthor_queue *queue)
>
> queue->timeout.remaining = MAX_SCHEDULE_TIMEOUT;
> }
> -
> - spin_unlock(&queue->fence_ctx.lock);
> }
>
> /**
> @@ -1542,7 +1539,7 @@ cs_slot_process_fault_event_locked(struct
> panthor_device *ptdev,
> u64 cs_extract = queue->iface.output->extract;
> struct panthor_job *job;
>
> - spin_lock(&queue->fence_ctx.lock);
> + guard(spinlock_irqsave)(&queue->fence_ctx.lock);
> list_for_each_entry(job, &queue->fence_ctx.in_flight_jobs,
> node) {
> if (cs_extract >= job->ringbuf.end)
> continue;
> @@ -1552,7 +1549,6 @@ cs_slot_process_fault_event_locked(struct
> panthor_device *ptdev,
>
> dma_fence_set_error(job->done_fence, -EINVAL);
> }
> - spin_unlock(&queue->fence_ctx.lock);
> }
>
> if (group) {
> @@ -2183,13 +2179,13 @@ group_term_post_processing(struct panthor_group
> *group)
> if (!queue)
> continue;
>
> - spin_lock(&queue->fence_ctx.lock);
> - list_for_each_entry_safe(job, tmp,
> &queue->fence_ctx.in_flight_jobs, node) {
> - list_move_tail(&job->node, &faulty_jobs);
> - dma_fence_set_error(job->done_fence, err);
> - dma_fence_signal_locked(job->done_fence);
> + scoped_guard(spinlock_irqsave, &queue->fence_ctx.lock) {
> + list_for_each_entry_safe(job, tmp,
> &queue->fence_ctx.in_flight_jobs, node) {
> + list_move_tail(&job->node, &faulty_jobs);
> + dma_fence_set_error(job->done_fence, err);
> + dma_fence_signal_locked(job->done_fence);
> + }
> }
> - spin_unlock(&queue->fence_ctx.lock);
>
> /* Manually update the syncobj seqno to unblock waiters. */
> syncobj = group->syncobjs->kmap + (i * sizeof(*syncobj));
> @@ -3049,39 +3045,39 @@ static bool queue_check_job_completion(struct
> panthor_queue *queue)
> LIST_HEAD(done_jobs);
>
> cookie = dma_fence_begin_signalling();
> - spin_lock(&queue->fence_ctx.lock);
> - list_for_each_entry_safe(job, job_tmp,
> &queue->fence_ctx.in_flight_jobs, node) {
> - if (!syncobj) {
> - struct panthor_group *group = job->group;
> + scoped_guard(spinlock_irqsave, &queue->fence_ctx.lock) {
> + list_for_each_entry_safe(job, job_tmp,
> &queue->fence_ctx.in_flight_jobs, node) {
> + if (!syncobj) {
> + struct panthor_group *group = job->group;
>
> - syncobj = group->syncobjs->kmap +
> - (job->queue_idx * sizeof(*syncobj));
> + syncobj = group->syncobjs->kmap +
> + (job->queue_idx * sizeof(*syncobj));
> + }
> +
> + if (syncobj->seqno < job->done_fence->seqno)
> + break;
> +
> + list_move_tail(&job->node, &done_jobs);
> + dma_fence_signal_locked(job->done_fence);
> }
>
> - if (syncobj->seqno < job->done_fence->seqno)
> - break;
> + if (list_empty(&queue->fence_ctx.in_flight_jobs)) {
> + /* If we have no job left, we cancel the timer, and
> reset remaining
> + * time to its default so it can be restarted next time
> + * queue_resume_timeout() is called.
> + */
> + queue_suspend_timeout_locked(queue);
>
> - list_move_tail(&job->node, &done_jobs);
> - dma_fence_signal_locked(job->done_fence);
> + /* If there's no job pending, we consider it progress
> to avoid a
> + * spurious timeout if the timeout handler and the sync
> update
> + * handler raced.
> + */
> + progress = true;
> + } else if (!list_empty(&done_jobs)) {
> + queue_reset_timeout_locked(queue);
> + progress = true;
> + }
> }
> -
> - if (list_empty(&queue->fence_ctx.in_flight_jobs)) {
> - /* If we have no job left, we cancel the timer, and reset
> remaining
> - * time to its default so it can be restarted next time
> - * queue_resume_timeout() is called.
> - */
> - queue_suspend_timeout_locked(queue);
> -
> - /* If there's no job pending, we consider it progress to avoid a
> - * spurious timeout if the timeout handler and the sync update
> - * handler raced.
> - */
> - progress = true;
> - } else if (!list_empty(&done_jobs)) {
> - queue_reset_timeout_locked(queue);
> - progress = true;
> - }
> - spin_unlock(&queue->fence_ctx.lock);
> dma_fence_end_signalling(cookie);
>
> list_for_each_entry_safe(job, job_tmp, &done_jobs, node) {
> @@ -3346,9 +3342,8 @@ queue_run_job(struct drm_sched_job *sched_job)
> job->ringbuf.end = job->ringbuf.start + (instrs.count * sizeof(u64));
>
> panthor_job_get(&job->base);
> - spin_lock(&queue->fence_ctx.lock);
> - list_add_tail(&job->node, &queue->fence_ctx.in_flight_jobs);
> - spin_unlock(&queue->fence_ctx.lock);
> + scoped_guard(spinlock_irqsave, &queue->fence_ctx.lock)
> + list_add_tail(&job->node, &queue->fence_ctx.in_flight_jobs);
>
> /* Make sure the ring buffer is updated before the INSERT
> * register.
>
> --
> 2.54.0
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
