From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/sched: Remove redundant entity->rq initialization and checks Date: Thu, 04 Jun 2026 12:41:20 +1000 Message-ID: In-Reply-To: <20260602153339.43453-1-tvrtko.ursulin@igalia.com> References: <20260602153339.43453-1-tvrtko.ursulin@igalia.com> <20260602153339.43453-1-tvrtko.ursulin@igalia.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **sched_entity.c =E2=80=94 removal of `entity->rq =3D NULL` in init:** Correct. The function does `memset(entity, 0, sizeof(struct drm_sched_entit= y))` at line 125 (which already zeroes `rq`), and then immediately sets `en= tity->rq =3D &sched_list[0]->rq` at line 138. The explicit NULL assignment = is doubly redundant. **sched_entity.c =E2=80=94 removal of `!entity->rq` check in `drm_sched_ent= ity_kill()`:** Correct. After successful init, `entity->rq` is always non-NULL, and no cod= e path sets it to NULL afterwards (confirmed by grep =E2=80=94 the only `en= tity->rq =3D NULL` in the scheduler was the one being removed in init). **sched_entity.c =E2=80=94 removal of `!entity->rq` check in `drm_sched_ent= ity_flush()`:** Correct, and the refactoring to move the `sched` variable initialization to= the declaration is clean: ```c - struct drm_gpu_scheduler *sched; + struct drm_gpu_scheduler *sched =3D + container_of(entity->rq, typeof(*sched), rq); ``` This is safe since the early-return that guarded it is no longer needed. **sched_main.c =E2=80=94 removal of `!entity->rq` check in `drm_sched_job_i= nit()`:** Correct. As noted in the commit message, the error path's `dev_err(job->sch= ed->dev, ...)` was buggy =E2=80=94 `job->sched` is uninitialized at that po= int (only set in `drm_sched_job_arm()`), and even the `memset(job, 0, sizeo= f(*job))` runs *after* this check. If this dead code ever executed, it woul= d dereference garbage memory rather than printing a useful diagnostic. **Minor issue =E2=80=94 stale kdoc:** The function comment for `drm_sched_j= ob_init()` (lines 579=E2=80=93582 in the current tree) contains: ```c * WARNING: amdgpu abuses &drm_sched.ready to signal when the hardware * has died, which can mean that there's no valid runqueue for a @entity. * This function returns -ENOENT in this case (which probably should be -EI= O as * a more meanigful return value). ``` After this patch, `drm_sched_job_init()` no longer returns `-ENOENT` at all= . This WARNING should be removed or updated in the same patch to avoid a mi= sleading kdoc comment. **Verdict:** Patch is correct. The one piece of feedback is to also clean u= p the now-stale WARNING in the `drm_sched_job_init()` kdoc. --- Generated by Claude Code Patch Reviewer