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/encoder: drm_encoder_cleanup: lock the encoder chain mutex during removal Date: Wed, 25 Mar 2026 06:59:30 +1000 Message-ID: In-Reply-To: <20260324-drm-bridge-alloc-encoder-chain-mutex-v5-2-8bf786c5c7e6@bootlin.com> References: <20260324-drm-bridge-alloc-encoder-chain-mutex-v5-0-8bf786c5c7e6@bootlin.com> <20260324-drm-bridge-alloc-encoder-chain-mutex-v5-2-8bf786c5c7e6@bootlin.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 The ABBA deadlock analysis in the commit message is excellent. The solution= using `list_cut_before()` is correct: ```c list_cut_before(&tmplist, &encoder->bridge_chain, &encoder->bridge_chain); ``` When `entry =3D=3D head` (third arg =3D=3D second arg), `list_cut_before` m= oves all entries to `tmplist` and leaves `bridge_chain` empty, which is the= desired behavior. The empty-list case (`head->next =3D=3D entry` when the = list is empty, i.e., `head->next =3D=3D head`) correctly initializes `tmpli= st` as empty and returns early. **Minor note:** The comment says `drm_modeset_lock_fini()` but the code pat= h described mentions `drm_atomic_private_obj_fini -> DRM_MODESET_LOCK_ALL_B= EGIN()`. Worth verifying that `drm_modeset_lock_fini` is really what's mean= t here, but this is just a comment clarity issue. **Concern:** After `list_cut_before`, bridges are detached without the mute= x. During this window, another thread could see an empty `bridge_chain` and= proceed with operations on a partially-torn-down encoder. However, since `= drm_encoder_cleanup` is a teardown path, this is acceptable =E2=80=94 no ne= w operations should be starting on an encoder being cleaned up. --- Generated by Claude Code Patch Reviewer