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/xe/drm_ras: Add per node cleanup action Date: Thu, 04 Jun 2026 13:22:00 +1000 Message-ID: In-Reply-To: <20260602044919.702209-4-raag.jadav@intel.com> References: <20260602044919.702209-1-raag.jadav@intel.com> <20260602044919.702209-4-raag.jadav@intel.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 **Verdict: Correct, with one observation.** This is the most substantial change. It replaces the monolithic `xe_drm_ras= _unregister_nodes()` cleanup (registered once for all nodes) with per-node = `drmm_add_action_or_reset()` calls, guaranteeing that each successfully-ini= tialized node is cleaned up individually during unwind. The error paths in `register_nodes()` are carefully structured: ```c ret =3D drmm_add_action_or_reset(&xe->drm, cleanup_node, node); if (ret) goto null_info; ``` Jumping to `null_info` (not `free_param`) on `drmm_add_action_or_reset` fai= lure is critical. Since the `_or_reset` variant calls `cleanup_node()` itse= lf on failure (which frees `device_name` and unregisters the node), jumping= to `free_param` would double-free `device_name`. The v3 changelog confirms= this was caught by a reviewer =E2=80=94 good catch. **Observation:** After patch 3, `cleanup_node_param()` no longer NULLs out = `ras->info[severity]`. During normal device teardown, the drmm actions run = `cleanup_node()` =E2=86=92 `cleanup_node_param()`, which frees `device_name= ` but leaves `ras->info[severity]` as a dangling pointer (the counter memor= y is freed separately by its own drmm tracking). This is safe because the x= e_device is being torn down, but it's a departure from the defensive NULL-a= fter-free pattern. Worth noting but not blocking =E2=80=94 the init-time er= ror path in `register_nodes()` does NULL it out via `null_info`, which is t= he path where it matters. **Teardown ordering is correct:** drmm releases in reverse registration ord= er, so per-node `cleanup_node` actions run before the node array itself is = freed (`drmm_kcalloc` in `xe_drm_ras_init`), ensuring the nodes are still v= alid when `drm_ras_node_unregister` and `cleanup_node_param` access them. --- --- Generated by Claude Code Patch Reviewer