Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing implicit synchronization for input BOs causes data corruption. -- commit 00caeb1bd76ecd4176b74ead738756a7e287a306 Author: Haoming Gao <[email protected]> accel/rocket: Collect job dependencies before arming This commit fixes a job submission bug by collecting implicit dependencies for input and output buffer objects before taking the scheduler lock and arming the job. This ensures that dependency collection failures do not cause an armed job to be aborted. > 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); [Severity: High] This isn't a bug introduced by this patch, but looking at rocket_job_push() immediately following this hunk: scoped_guard(mutex, &rdev->sched_lock) { ... drm_sched_entity_push_job(&job->base); } rocket_attach_object_fences(job->out_bos, job->out_bo_count, job->inference_done_fence); err_unlock: Does this code also need to attach the completion fence to the input BOs? While prior dependencies on in_bos are correctly acquired (ensuring we don't read before prior writers finish), the job's completion fence (inference_done_fence) is only attached to out_bos. Because there is no read fence added to the input buffers, could future writers (from userspace, CPU, or other GPU engines) bypass synchronization and overwrite these BOs while the NPU is still actively reading them, causing data corruption? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
