From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: iov_iter: add iterator type for dmabuf maps Date: Tue, 05 May 2026 11:26:00 +1000 Message-ID: In-Reply-To: <20a233d2f35274817aa643cc0fe113707eb47e72.1777475843.git.asml.silence@gmail.com> References: <20a233d2f35274817aa643cc0fe113707eb47e72.1777475843.git.asml.silence@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 Introduces `ITER_DMABUF_MAP` iterator type. The approach of treating it as = an opaque handle that only the target driver can interpret is sound. **Issue: Pre-existing parenthesization bug perpetuated in `iov_iter_restore= `** The patch modifies the `WARN_ON_ONCE` in `iov_iter_restore` but does not fi= x the existing mis-parenthesization: ```c if (WARN_ON_ONCE(!iov_iter_is_bvec(i) && !iter_is_iovec(i) && !iter_is_ubuf(i) && !iov_iter_is_kvec(i) && !iov_iter_is_dmabuf_map(i))) ``` The original code has: ```c if (WARN_ON_ONCE(!iov_iter_is_bvec(i) && !iter_is_iovec(i) && !iter_is_ubuf(i)) && !iov_iter_is_kvec(i)) ``` The closing paren of `WARN_ON_ONCE` comes after `!iter_is_ubuf(i)`, meaning= `!iov_iter_is_kvec(i)` is checked *after* the WARN fires, not as part of t= he condition. The patched version fixes this by moving the closing paren to= include all the checks, but this is done silently =E2=80=94 it should be c= alled out since it changes the existing behavior (kvec iterators would prev= iously pass through without warning, now they'd warn if somehow removed fro= m the list). This is actually a bugfix that should be noted in the commit m= essage or split out. **Minor: `iov_iter_alignment` shares ubuf path** ```c if (likely(iter_is_ubuf(i)) || iov_iter_is_dmabuf_map(i)) { ... return ((unsigned long)i->ubuf + i->iov_offset) | size; ``` This accesses `i->ubuf` for a dmabuf_map iterator. Since `ubuf` and `dmabuf= _map` are in a union, this is reading the `dmabuf_map` pointer as a `void _= _user *`, which works for alignment checking purposes but is semantically o= dd. The comment or code should clarify that alignment of dmabuf_map iterato= rs is always satisfied, or this path should return 0 for dmabuf_map iterato= rs explicitly. --- --- Generated by Claude Code Patch Reviewer