On Friday, March 6, 2026 at 7:03 PM, Frank Li wrote:
- if (appstatus & APPSTATUS_FAULTCAUSE_MASK) {
dev_err(ndev->dev, "Neutron halted due to fault: 0x%lx\n",FIELD_GET(APPSTATUS_FAULTCAUSE_MASK,appstatus));
return neutron_job_err_handler(ndev);AI: neutron_job_err_handler() returns void, not int. Remove 'return'.
Ok, will fix.
- ret = drm_sched_job_init(&job->base, &npriv->sched_entity, 1, NULL,
filp->client_id);- if (ret)
goto out_put_syncobj;- ret = neutron_push_job(job, syncobj);
- if (ret)
goto out_sched_cleanup;- neutron_put_job(job);
- drm_syncobj_put(syncobj);
- return 0;
+out_sched_cleanup:
- drm_sched_job_cleanup(&job->base);
+out_put_syncobj:
- drm_syncobj_put(syncobj);
+out_put_gem:
- drm_gem_object_put(job->bo);
AI: In the success path, neutron_put_job(job) is called which decrements refcnt. But if neutron_push_job() fails and we hit out_sched_cleanup, the job refcnt is never decremented. This leaks the job structure. Consider: if neutron_push_job() succeeds, it calls kref_get() inside sched_lock. If it fails, no kref_get() happens, so don't call
(Need owner do judgment. Not sure if AI said correctly.)
I don't see an issue here, kref_get() is called at a point where neutron_push_job() can't fail anymore. And if neutron_push_job() fails earlier, error path looks clean, it frees everything in reverse order, including the job struct.
Btw, what agent did you use for review?
Thanks, Ioana
Frank