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/exec, drm/xe: Avoid abusing the drm_exec retry pointer Date: Wed, 01 Apr 2026 07:52:57 +1000 Message-ID: In-Reply-To: <20260331092023.81616-5-thomas.hellstrom@linux.intel.com> References: <20260331092023.81616-1-thomas.hellstrom@linux.intel.com> <20260331092023.81616-5-thomas.hellstrom@linux.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 **Functionally correct but the WARN_ON condition may be wrong for the "newl= y initialized" case described in the doc comment.** The new `drm_exec_retry()` macro: ```c #define drm_exec_retry(_exec) do { WARN_ON(!drm_exec_is_contended(_exec)); goto *__drm_exec_retry_ptr; } while (0) ``` The doc comment says: > Unconditionally retry the loop to lock all objects. For consistency, > the exec object needs to be **newly initialized or contended**. But `WARN_ON(!drm_exec_is_contended(_exec))` will fire when the exec is new= ly initialized (since `contended` will be NULL after init). The WARN_ON onl= y checks for the contended case, not the "newly initialized" case. Either t= he comment or the assertion needs updating. If the intent is that only the = contended case is valid, the comment should be fixed. If newly-initialized = is also valid (as needed for the OOM retry path in xe), then the assertion = is too strict. Looking at the xe usage: `xe_validation_should_retry()` is called on `-ENOM= EM`, which could happen before any contention. If `exec->contended` is NULL= at that point, this will produce a spurious WARN. The commit message also says "This is relying on documented behaviour" =E2= =80=94 perhaps this should say "undocumented" given the context of the seri= es (cleaning up abuses)? --- --- Generated by Claude Code Patch Reviewer