Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Job completion fence is not attached to input BOs, breaking write-after-read implicit synchronization. -- commit cebaafa7df2deb9058de28738100dc37ff2d7c70 Author: MoGGuU <[email protected]> accel/rocket: Collect job dependencies before arming This commit modifies rocket_job_push() to collect implicit dependencies before taking the scheduler lock and arming the job. This prevents an armed job from being incorrectly aborted if dependency collection fails with -ENOMEM. Cc: [email protected] > diff --git a/drivers/accel/rocket/rocket_job.c > b/drivers/accel/rocket/rocket_job.c > index 13f4915c1b07c..1634c30ede456 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); > } > > rocket_attach_object_fences(job->out_bos, job->out_bo_count, > job->inference_done_fence); [Severity: High] This is a pre-existing issue, but does this code miss attaching the completion fence to the input buffers? If a userspace application submits a job with input buffers, then immediately submits another job (or performs a CPU write) to modify those same buffers, the hardware might read corrupted or new data because it is still reading from them. Because no read fence is explicitly attached to the input buffers (in_bos) after the job is pushed, the DRM scheduler and CPU wait paths will not see a dependency and will not wait for the inference job to complete its read operations. Should the driver also explicitly attach the completion fence to job->in_bos (perhaps with DMA_RESV_USAGE_READ) so that subsequent writers wait for this job to finish reading? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
