On Tue, May 19, 2026 at 12:43 AM Barry Song <[email protected]> wrote: > > On Mon, May 18, 2026 at 8:16 PM Albert Esteve <[email protected]> wrote: > > > > On Sat, May 16, 2026 at 9:37 AM Barry Song <[email protected]> wrote: > > > > > > On Tue, May 12, 2026 at 5:18 PM Albert Esteve <[email protected]> wrote: > > > > > > > > On embedded platforms a central process often allocates dma-buf > > > > memory on behalf of client applications. Without a way to > > > > attribute the charge to the requesting client's cgroup, the > > > > cost lands on the allocator, making per-cgroup memory limits > > > > ineffective for the actual consumers. > > > > > > > > Add charge_pid_fd to struct dma_heap_allocation_data. When set to > > > > a valid pidfd, DMA_HEAP_IOCTL_ALLOC resolves the target task's > > > > memcg and charges the buffer there via mem_cgroup_charge_dmabuf() > > > > inside dma_heap_buffer_alloc(). Without charge_pid_fd, and with > > > > the mem_accounting module parameter enabled, the buffer is charged > > > > to the allocator's own cgroup. > > > > > > > > Additionally, commit 3c227be90659 ("dma-buf: system_heap: account for > > > > system heap allocation in memcg") adds __GFP_ACCOUNT to system-heap > > > > page allocations. Keeping __GFP_ACCOUNT would charge the same pages > > > > twice (once to kmem, once to MEMCG_DMABUF), thus remove it and route > > > > all accounting through a single MEMCG_DMABUF path. > > > > > > > [...] > > > > > > > - if (mem_accounting) > > > > - flags |= __GFP_ACCOUNT; > > > > > > Hi Albert, > > > > > > would it be better to move this and its description to patch 1? It > > > looks like patch 1 already introduces the double accounting changes, > > > and patch 2 is mainly just supporting remote charging. > > > > Hi Barry, > > > > Thanks for looking into this series! Yes, in my head I was trying to > > keep patch 1, which was taken from a previous, different series, and > > then diverge from it starting with patch 2. This would clarify the > > difference between the two. But I can see it just added some confusion > > (for example, patch 1 charges on dma_buf_export() and then it is moved > > to dma_heap_buffer_alloc() in patch 2). I will reorganize it better > > for the next version, including your suggestion. > > Yep, I understand the situation now. I also understand > that you were referring to T.J.'s patch, which caused > some back-and-forth confusion for readers when reading > patches 1 and 2. > > > > > > > > > Also, mem_accounting is only used by system_heap.c; has this patchset > > > also eliminated its need? > > > > No, mem_accounting is still handled in this patch for the general case > > where no `charge_pid_fd` is used. See dma_heap_buffer_alloc() code: > > > > + if (memcg) > > + css_get(&memcg->css); > > + else if (mem_accounting) > > + memcg = get_mem_cgroup_from_mm(current->mm); > > I see. What feels a bit odd to me is that mem_accounting > could either be dropped (with unconditional charging), or > it should cover both remote and local charge cases.
Good point. If I understand correctly, looking at patch [1] that introduced the flag, the shared buffer caveats mentioned there are not yet covered by this approach, so the flag should stay. I will make it consistent and cover both remote and local charge cases. [1] https://lore.kernel.org/all/[email protected]/ > > I don’t have a strong opinion here—it just feels a bit > strange, since its description is quite generic for memcg: > > "Enable cgroup-based memory accounting for dma-buf heap > allocations (default=false)." > > Best Regards > Barry >
