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: Don't emit UTS_RELEASE as part of device coredump Date: Tue, 28 Apr 2026 14:25:21 +1000 Message-ID: In-Reply-To: <20260427160902.1126027-2-u.kleine-koenig@baylibre.com> References: <20260427160902.1126027-2-u.kleine-koenig@baylibre.com> <20260427160902.1126027-2-u.kleine-koenig@baylibre.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 **Correctness:** The patch is mechanically correct. It removes one `drm_put= s` call and the surrounding code compiles fine without it. **Concern =E2=80=94 Loss of diagnostic information:** ```c - drm_puts(&p, "kernel: " UTS_RELEASE "\n"); ``` This line provides the kernel version in coredumps. When users submit bug r= eports with coredump data attached, the kernel version is one of the first = things developers check. Removing it means developers will need to ask user= s for this information separately or rely on other metadata (e.g., dmesg, u= name output) that may not always be included in the report. **Concern =E2=80=94 Inconsistency with other drivers:** The author's own grep shows that amdgpu, msm, and xe all emit `UTS_RELEASE`= in their coredumps. Removing it from xe alone creates an inconsistency. If= this change is desirable, it should be a series covering all DRM drivers t= hat use this pattern. **Concern =E2=80=94 Weak justification:** The commit message says *"It's little helpful to emit the kernel version in= the device coredump"* but doesn't provide evidence for this claim. The aut= hor even hedges in the cover letter. Build reproducibility is a valid conce= rn but is a niche developer workflow issue, not a reason to remove diagnost= ic data from production coredumps. **Alternative approach:** If the goal is to avoid compile-time `UTS_RELEASE= ` dependencies, the line could be changed to use the runtime equivalent: ```c drm_printf(&p, "kernel: %s\n", utsname()->release); ``` This would emit the same information at dump time without causing `xe_devco= redump.o` to be rebuilt when `UTS_RELEASE` changes. This addresses the buil= d reproducibility concern while preserving the diagnostic value. --- Generated by Claude Code Patch Reviewer