Claude review: dma-heap: charge dma-buf memory via explicit memcg

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

This is the core patch. It moves charging from `dma_buf_export()` to `dma_heap_buffer_alloc()`, adds the `charge_pid_fd` field to the UAPI struct, and uses pidfd to resolve the target memcg.

**Issue 1 (Bug) - Unsafe `task->mm` access:**
```c
+		task = pidfd_get_task(heap_allocation->charge_pid_fd, &pidfd_flags);
+		if (IS_ERR(task))
+			return PTR_ERR(task);
+		...
+		memcg = get_mem_cgroup_from_mm(task->mm);
+		put_task_struct(task);
```
`task->mm` can be NULL if the pidfd points to a kernel thread or a process that has already called `exit_mm()`. While `get_mem_cgroup_from_mm(NULL)` won't crash (it falls back to root/current memcg), it would silently charge the wrong cgroup. The correct pattern is:
```c
struct mm_struct *mm = get_task_mm(task);
if (!mm) {
    put_task_struct(task);
    return -ESRCH;
}
memcg = get_mem_cgroup_from_mm(mm);
mmput(mm);
put_task_struct(task);
```
This also closes a potential TOCTOU race where the task exits between `pidfd_get_task()` and the mm access.

**Issue 2 - Charge/release ordering:** The charge happens after `heap->ops->allocate()` succeeds:
```c
+	dmabuf = heap->ops->allocate(heap, len, fd_flags, heap_flags);
+	if (IS_ERR(dmabuf))
+		return PTR_ERR(dmabuf);
+
+	nr_pages = len / PAGE_SIZE;
+	...
+	if (memcg) {
+		if (!mem_cgroup_charge_dmabuf(memcg, nr_pages, GFP_KERNEL)) {
```
The pages are already allocated before the charge check. If the cgroup is over its limit, we've allocated pages we immediately need to free. Ideally the charge should happen before the allocation to serve as a gate, or the allocation should use `__GFP_ACCOUNT` to let the allocator handle cgroup limits during allocation itself.

**Issue 3 - `__GFP_ACCOUNT` removal:**
```c
-		if (mem_accounting)
-			flags |= __GFP_ACCOUNT;
```
Removing `__GFP_ACCOUNT` means system heap pages are no longer tracked under kmem in the cgroup. The cover letter says this avoids double-counting, but the two mechanisms track different things: `__GFP_ACCOUNT` charges to the cgroup's kmem counter, while `MEMCG_DMABUF` is a separate stat. They're not truly double-counted in a harmful way — they track different accounting dimensions. This design decision should be justified more carefully: are there real systems where both paths being active causes problems (e.g., triggering OOM too early)?

**Issue 4 - Refcount correctness:** The refcounting is actually correct, but subtle enough to deserve a comment. When `charge_to` is non-NULL:
- `css_get()` inside `dma_heap_buffer_alloc()` takes a new reference
- The caller's reference from `get_mem_cgroup_from_mm()` is dropped by `mem_cgroup_put(memcg)` after `dma_heap_buffer_alloc()` returns
- The buffer_alloc reference transfers to `dmabuf->memcg` and is dropped on release

When `charge_to` is NULL and `mem_accounting` is true, the reference from the internal `get_mem_cgroup_from_mm(current->mm)` transfers directly to `dmabuf->memcg`. This all works but is non-obvious — consider adding a brief comment.

---
Generated by Claude Code Patch Reviewer