From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: memcg: dma-buf per-cgroup accounting via pid_fd Date: Sat, 16 May 2026 13:56:21 +1000 Message-ID: In-Reply-To: <20260512-v2_20230123_tjmercier_google_com-v1-0-6326701c3691@redhat.com> References: <20260512-v2_20230123_tjmercier_google_com-v1-0-6326701c3691@redhat.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Overall Series Review Subject: memcg: dma-buf per-cgroup accounting via pid_fd Author: Albert Esteve Patches: 16 Reviewed: 2026-05-16T13:56:21.141878 --- This RFC adds per-cgroup dma-buf memory accounting via a new `MEMCG_DMABUF` stat counter, with a `charge_pid_fd` mechanism in `DMA_HEAP_IOCTL_ALLOC` that lets a central allocator charge dma-buf memory to a client's cgroup at allocation time using a pidfd. The series also adds an LSM hook + SELinux implementation to control cross-cgroup charging, and selftests. **Strengths:** - The problem is real and well-motivated: on embedded platforms with central allocators, per-cgroup memory limits are meaningless without the ability to attribute dma-buf charges to the requesting client. - The pidfd-based mechanism is cleaner than the original binder-based transfer approach. - The ioctl handler's size-negotiation logic means extending the struct is backward-compatible (old userspace gets zero-filled new fields). - The LSM/SELinux access control for cross-cgroup charging is the right approach. **Key concerns:** - **Unsafe `task->mm` access** is a real bug (details below). - **Bisectability issue** between patches 1 and 2: patch 1 charges all dma-bufs at export time, then patch 2 removes that. Between these patches the behavior is neither the old nor the intended new behavior. - The series only covers the `dma-heap` path; non-heap dma-buf exporters (e.g., DRM drivers using `dma_buf_export()` directly) are not accounted for at all after patch 2. The cover letter acknowledges the single-owner limitation but this architectural gap should be discussed more explicitly. --- --- Generated by Claude Code Patch Reviewer