Re: [PATCH 01/11] drm/panthor: Always use the IRQ-safe variant when acquiring the fence lock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The function queue_run_job mixes goto-based error handling with scope-based cleanup helpers.
--

commit a4621d48f775641c2c35dd7c0b40ff8a402f0f4e
Author: Boris Brezillon <boris.brezillon@collabora.com>

drm/panthor: Always use the IRQ-safe variant when acquiring the fence lock

This commit updates the panthor scheduler to use the IRQ-safe variant of
spinlock when acquiring the fence context lock, as dma_fence objects can be
shared and accessed from hardirq context. It also transitions the code to
use the scope-based guard model.

Closes: https://sashiko.dev/#/patchset/20260625-panthor-signal-from-irq-v5-0-8836a74e0ef9@collabora.com?part=11

> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 5b34032deff81..e97f29469d28a 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c

[ ... ]

> @@ -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.

[Severity: Low]
Does this introduce a mix of goto-based error handling and scope-based
cleanup helpers in the same function?

Looking at queue_run_job(), it retains a goto out_unlock error handling path
earlier in the function while now introducing scoped_guard here. The kernel
cleanup guidelines generally state that for a given routine, we should convert
all resources that need a goto cleanup to scope-based cleanup, or convert none
of them, to avoid confusing ownership semantics.

Could this function be fully converted to use guards for sched->lock as well,
or should this specific lock retain the traditional spin_lock/spin_unlock
pattern to avoid mixing the two styles?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625-panthor-misc-fixes-v1-0-b67ed973fea6@collabora.com?part=1