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 <[email protected]>

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/[email protected]?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/[email protected]?part=1

Reply via email to