From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: misc: fastrpc: Remove buffer from list prior to unmap operation Date: Sat, 16 May 2026 09:09:54 +1000 Message-ID: In-Reply-To: <20260515124217.20723-3-jianping.li@oss.qualcomm.com> References: <20260515124217.20723-1-jianping.li@oss.qualcomm.com> <20260515124217.20723-3-jianping.li@oss.qualcomm.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Verdict: Looks good, minor style nit.** Moving the `list_del` from `fastrpc_req_munmap_impl()` into the caller `fastrpc_req_munmap()` before the unmap call is the correct fix for the race. The rollback on failure is also correct: ```c + err = fastrpc_req_munmap_impl(fl, buf); + if (err) { + spin_lock(&fl->lock); + list_add_tail(&buf->node, &fl->mmaps); + spin_unlock(&fl->lock); + } ``` The `fastrpc_req_mmap()` error path changes are also correct: the new `err_copy` label removes the buffer from the list (it was added in the success path above) before falling through to `err_assign` which calls `fastrpc_req_munmap_impl()`: ```c +err_copy: + spin_lock(&fl->lock); + list_del(&buf->node); + spin_unlock(&fl->lock); err_assign: fastrpc_req_munmap_impl(fl, buf); ``` This is consistent since `fastrpc_req_munmap_impl()` no longer touches the list. --- --- Generated by Claude Code Patch Reviewer