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/bridge: add drm_bridge_clear_and_put() Date: Wed, 11 Mar 2026 13:16:32 +1000 Message-ID: In-Reply-To: <20260310-drm-bridge-atomic-vs-remove-clear_and_put-v2-1-51fe222f3cf0@bootlin.com> References: <20260310-drm-bridge-atomic-vs-remove-clear_and_put-v2-0-51fe222f3cf0@bootlin.com> <20260310-drm-bridge-atomic-vs-remove-clear_and_put-v2-1-51fe222f3cf0@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 **Code review**: The implementation at `drivers/gpu/drm/drm_bridge.c:340-346`: ```c void drm_bridge_clear_and_put(struct drm_bridge **bridge_pp) { struct drm_bridge *bridge =3D *bridge_pp; *bridge_pp =3D NULL; drm_bridge_put(bridge); } ``` - **Concurrency concern**: As noted above, both the read of `*bridge_pp` an= d the write of `NULL` use plain accesses. If this is meant to be safe again= st concurrent readers (as the commit message strongly implies), the write s= hould be `WRITE_ONCE(*bridge_pp, NULL)` and the initial read should be a co= rresponding `READ_ONCE`. The kernel has established patterns for this =E2= =80=94 e.g., `rcu_assign_pointer()` or at minimum `WRITE_ONCE()` / `smp_sto= re_release()`. Without these, the compiler is free to reorder the NULL stor= e after the `drm_bridge_put()` call or even optimize it in unexpected ways. - **Documentation quality**: The kerneldoc is well-written, with clear exam= ples showing the equivalent manual pattern. The cross-reference added to `d= rm_bridge_put()` is a nice touch. - **Naming**: `clear_and_put` follows the existing kernel convention (simil= ar to `fput_and_clear`, though that one is actually named `fput` not `put_a= nd_clear`). The kernel has `kfree_and_null` patterns too. The name is fine. - **Minor**: The function doesn't explicitly document thread-safety guarant= ees. If it's not intended to be called concurrently with readers, that shou= ld be stated. If it is, it needs the memory barriers. --- Generated by Claude Code Patch Reviewer