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: Mon, 25 May 2026 17:57:40 +1000 Message-ID: In-Reply-To: <20260523050212.557292-4-raag.jadav@intel.com> References: <20260523050212.557292-1-raag.jadav@intel.com> <20260523050212.557292-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 fix for a real leak, two minor observations.** This is the most substantial patch. The old code registered a single `xe_dr= m_ras_unregister_nodes` action in `xe_drm_ras_init()` *after* `register_nod= es()` succeeded. If `register_nodes()` failed partway through (e.g., node 0= succeeds but node 1 fails), previously registered nodes were never cleaned= up because the batch cleanup action was never registered. The new design r= egisters a `drmm_add_action_or_reset` per node immediately after successful= registration, guaranteeing cleanup on unwind. The error path after the loop is correct: ```c if (ret) { cleanup_node_param(node); ras->info[i] =3D NULL; } ``` This handles the current (failed) iteration's resources. Previously complet= ed iterations are covered by their already-registered drmm actions. **Minor observation 1:** If `drmm_add_action_or_reset()` fails, it calls `c= leanup_node()` itself (the `_or_reset` semantics), which already runs `clea= nup_node_param(node)`. The post-loop `cleanup_node_param(node)` is then a r= edundant second call. This is **safe** because `cleanup_node_param` sets `d= evice_name =3D NULL` after freeing, so `kfree(NULL)` on the second call is = a no-op. Just worth noting =E2=80=94 not a bug. **Minor observation 2:** `ret` is declared uninitialized: ```c int i, ret; ``` If `DRM_XE_RAS_ERR_SEV_MAX` were ever 0, `ret` would be used uninitialized = in `if (ret)`. In practice this can't happen (there are always correctable = and uncorrectable severities), but initializing `ret =3D 0` would silence p= otential compiler warnings and be more defensive. Very minor. --- Generated by Claude Code Patch Reviewer