Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Silent failure when submitting multiple jobs due to ignored return value. - [High] Violation of DRM scheduler contract by calling drm_sched_job_cleanup() on an armed job. -- commit e8a7bda4ea0e8bc90689dd01aad3165b7c5004b2 Author: Triet Hoang <[email protected]> accel/rocket: Check allocations before use Check the result of kmalloc_objs() in rocket_job_open() before using the allocated buffers. It also updates the error handling when drm_sched_entity_init() fails by replacing WARN_ON(ret) with a plain if. > diff --git a/drivers/accel/rocket/rocket_job.c > b/drivers/accel/rocket/rocket_job.c > index f404355058185..ffa0f8db5da65 100644 > --- a/drivers/accel/rocket/rocket_job.c > +++ b/drivers/accel/rocket/rocket_job.c > @@ -516,6 +516,9 @@ int rocket_job_open(struct rocket_file_priv *rocket_priv) > unsigned int core; > int ret; > > + if (!scheds) > + return -ENOMEM; > + [ ... ] While reviewing this code, I noticed a couple of pre-existing issues in this file. [Severity: High] This is a pre-existing issue, but does calling drm_sched_job_arm() before rocket_acquire_object_fences() violate the DRM scheduler contract if resource acquisition fails? In rocket_job_push(): 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; If rocket_acquire_object_fences() returns an error (e.g., due to -ENOMEM in dma_resv_reserve_fences), the error path aborts the armed job by calling drm_sched_job_cleanup() back in rocket_ioctl_submit_job(). Since drm_sched_job_arm() initializes fences and assigns a sequence number, aborting it afterward leaks the assigned sequence number and can corrupt the scheduler's fence ordering, leading to hangs or crashes in the DRM scheduler core. [Severity: High] This is a pre-existing issue, but is it intentional that the return value of rocket_ioctl_submit_job() is ignored? In rocket_ioctl_submit(): for (i = 0; i < args->job_count; i++) rocket_ioctl_submit_job(dev, file, &jobs[i]); exit: kvfree(jobs); return ret; If rocket_ioctl_submit_job() fails (e.g., due to an invalid BO handle or -ENOMEM), the failure is silently swallowed and not propagated. The ioctl will return success even when jobs were not queued, which can cause userspace clients waiting on implicit fences for these jobs to hang indefinitely. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
