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/ras: Introduce get-error-threshold Date: Sat, 16 May 2026 12:54:49 +1000 Message-ID: In-Reply-To: <20260512191610.1817578-3-raag.jadav@intel.com> References: <20260512191610.1817578-1-raag.jadav@intel.com> <20260512191610.1817578-3-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 Adds the `GET_ERROR_THRESHOLD` netlink command to drm_ras. The implementati= on follows the existing `GET_ERROR_COUNTER` pattern closely. **Issue: `get-error-threshold` requires `admin-perm` for a read-only query.= ** ```yaml flags: [admin-perm] ``` The existing `get-error-counter` is a dump operation without `admin-perm`. = It's worth questioning whether a threshold query truly requires admin privi= leges. If the intent is to keep parity with `set-error-threshold` that's on= e thing, but for a read-only query it seems unnecessarily restrictive compa= red to the existing counter read path. **Minor: No `dumpit` handler for thresholds.** Unlike `get-error-counter` which has both `doit` and `dumpit`, `get-error-t= hreshold` only has `doit`. If a user wants to query thresholds for all erro= rs across all nodes, they need to iterate manually. This may be intentional= but is worth noting for completeness. The core implementation is solid =E2=80=94 proper cleanup paths with `genlm= sg_cancel`/`nlmsg_free` on errors, correct attribute checking, and the `EOP= NOTSUPP` return when the callback is absent. --- --- Generated by Claude Code Patch Reviewer