Re: [PATCH v2 2/3] accel/rocket: Collect job dependencies before arming

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing read-fence attachment on input BOs leaves buffers unprotected from subsequent writers.
--

commit ac74f22db3a0d06711502b6864c7b006136b26f3
Author: MoGGuU <Naixumogu@whut.edu.cn>

accel/rocket: Collect job dependencies before arming

rocket_job_push() arms the scheduler job before collecting its implicit
dependencies. Dependency collection can fail with -ENOMEM, but an armed
job must be pushed and must not be aborted with drm_sched_job_cleanup().

> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index 7e3d123afc5ad..51e8b43c05131 100644
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
[ ... ]
> @@ -206,19 +206,21 @@ static int rocket_job_push(struct rocket_job *job)
[ ... ]
>  	scoped_guard(mutex, &rdev->sched_lock) {
>  		drm_sched_job_arm(&job->base);
>  
>  		job->inference_done_fence = dma_fence_get(&job->base.s_fence->finished);
>  
> -		ret = rocket_acquire_object_fences(job->in_bos, job->in_bo_count, &job->base, false);
> -		if (ret)
> -			goto err_unlock;
> -
> -		ret = rocket_acquire_object_fences(job->out_bos, job->out_bo_count, &job->base, true);
> -		if (ret)
> -			goto err_unlock;
> -
>  		kref_get(&job->refcount); /* put by scheduler job completion */
>  
>  		drm_sched_entity_push_job(&job->base);

[Severity: High]
This is a pre-existing issue, but does this leave the input buffers
unprotected from subsequent writers?

Looking at rocket_job_push() in drivers/accel/rocket/rocket_job.c, the
completion fence (job->inference_done_fence) is only attached to job->out_bos
just after this scoped_guard block:

    rocket_attach_object_fences(job->out_bos, job->out_bo_count,
                                job->inference_done_fence);

The job->in_bos receive no completion fence.

Furthermore, rocket_attach_object_fences() hardcodes DMA_RESV_USAGE_WRITE:

    for (i = 0; i < bo_count; i++)
        dma_resv_add_fence(bos[i]->resv, fence, DMA_RESV_USAGE_WRITE);

This hardcoding prevents it from correctly attaching a read-fence for in_bos
even if it were called for them.

If a subsequent job writes to these input BOs, could it begin execution
without waiting for the current job to finish reading them? Since it will
lock the reservations and find no implicit read dependency from this job,
this might lead to hardware data races if unprivileged users submit sequential
jobs where one reads and the next writes to the same BO.

Should we also attach a read-fence to the input BOs and allow
rocket_attach_object_fences() to specify the usage type?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827170608.39511-1-Naixumogu@whut.edu.cn?part=2