From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: dma-buf/udmabuf: skip redundant cpu sync to fix cacheline EEXIST warning Date: Tue, 31 Mar 2026 16:42:34 +1000 Message-ID: In-Reply-To: <20260331061657.79983-1-mikhail.v.gavrilov@gmail.com> References: <20260331061657.79983-1-mikhail.v.gavrilov@gmail.com> <20260331061657.79983-1-mikhail.v.gavrilov@gmail.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 **Analysis of the change:** The patch changes two lines in `get_sg_table()` and `put_sg_table()`: ```c - ret =3D dma_map_sgtable(dev, sg, direction, 0); + ret =3D dma_map_sgtable(dev, sg, direction, DMA_ATTR_SKIP_CPU_SYNC); ``` ```c - dma_unmap_sgtable(dev, sg, direction, 0); + dma_unmap_sgtable(dev, sg, direction, DMA_ATTR_SKIP_CPU_SYNC); ``` **Correctness:** The change is correct. `get_sg_table()` is called from two= places: 1. `map_udmabuf()` (line 186) =E2=80=94 the dma-buf `map_dma_buf` op, calle= d by importers. CPU sync at map time is redundant; importers manage their o= wn cache coherency. 2. `begin_cpu_udmabuf()` (line 252) =E2=80=94 on the first call, creates th= e sg table. Subsequent calls use `dma_sync_sgtable_for_cpu()` explicitly (l= ine 258). The initial map doesn't need a sync either, as the pages are fres= hly mapped. Similarly, `put_sg_table()` is called from `unmap_udmabuf()` and `release_u= dmabuf()`, where the CPU sync at unmap time is unnecessary. **Precedent:** I confirmed that both `drm_gem_map_dma_buf()` (`drm_prime.c:= 684`) and `amdgpu_dma_buf_map()` (`amdgpu_dma_buf.c:213`) already pass `DMA= _ATTR_SKIP_CPU_SYNC`, matching the commit message's claim. **One minor observation:** The commit message says the flag "suppresses thi= s check in `add_dma_entry()`" =E2=80=94 that's slightly imprecise. `DMA_ATT= R_SKIP_CPU_SYNC` doesn't directly affect `add_dma_entry()`. Rather, by skip= ping the CPU cache maintenance, the overlapping mappings become harmless, a= nd the debug infrastructure's concern about cacheline sharing is moot. The = warning is still technically possible if both mappings exist simultaneously= , but in practice the flag prevents the cache ops that make the overlap dan= gerous. The actual suppression mechanism may be more nuanced, but the end r= esult is correct =E2=80=94 the warning goes away and the behavior is safe. **Verdict:** Reviewed-by worthy. Clean two-line fix, well-motivated, consis= tent with how other dma-buf exporters handle this. --- Generated by Claude Code Patch Reviewer