Re: [Linaro-mm-sig] [PATCH v4 2/8] dma-heap: charge dma-buf memory via explicit memcg

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Thu, 2026-11-12 at 10:56 +0100, Albert Esteve wrote:
> On Thu, Nov 12, 2026 at 08:42:18AM +0100, Thomas Hellström wrote:
> > On Wed, 2026-11-04 at 12:09 +0100, Albert Esteve 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_fd to struct dma_heap_allocation_data. When set to
> > > a valid pidfd or cgroupfd, DMA_HEAP_IOCTL_ALLOC resolves the
> > > target
> > > task's or cgroup's memcg and charges the buffer there via
> > > mem_cgroup_reserve_dmabuf() inside dma_heap_ioctl_allocate(). The
> > > resolved memcg is passed through dma_buf_export_info so that
> > > dma_buf_export() stores it and updates the MEMCG_DMABUF stat.
> > > Without charge_fd, and with the mem_accounting module parameter
> > > enabled, the buffer is charged to the allocator's own cgroup.
> > > The __GPF_ACCOUNT flag is removed from system heap allocations
> > > to avoid double counting.
> > > 
> > > Usage examples:
> > > 
> > >   1. Central allocator charging to a client at allocation time.
> > >      The allocator knows the client's PID (e.g., from binder's
> > >      sender_pid) and uses pidfd to attribute the charge:
> > > 
> > >        pid_t client_pid = txn->sender_pid;
> > >        int pidfd = pidfd_open(client_pid, 0);
> > > 
> > >        struct dma_heap_allocation_data alloc = {
> > >            .len         = buffer_size,
> > >            .fd_flags    = O_RDWR | O_CLOEXEC,
> > >            .charge_fd   = pidfd,
> > >        };
> > >        ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc);
> > >        close(pidfd);
> > >        /* alloc.fd is now charged to client's cgroup */
> > > 
> > >   2. Default allocation (no pidfd, mem_accounting=1).
> > >      When charge_fd is not set and the mem_accounting module
> > >      parameter is enabled, the buffer is charged to the
> > > allocator's
> > >      own cgroup:
> > > 
> > >        struct dma_heap_allocation_data alloc = {
> > >            .len      = buffer_size,
> > >            .fd_flags = O_RDWR | O_CLOEXEC,
> > >        };
> > >        ioctl(heap_fd, DMA_HEAP_IOCTL_ALLOC, &alloc);
> > >        /* charged to current process's cgroup */
> > > 
> > > Current limitations:
> > > 
> > >  - Single-owner model: a dma-buf carries one memcg charge
> > > regardless
> > > of
> > >    how many processes share it. Means only the first owner (and
> > > exporter)
> > >    of the shared buffer bears the charge.
> > >  - Only memcg accounting supported. While this makes sense for
> > > system
> > >    heap buffers, other heaps (e.g., CMA heaps) will require
> > > selectively
> > >    charging also for the dmem controller.
> > > 
> > > Signed-off-by: Albert Esteve <aesteve@redhat.com>
> > > ---
> > >  Documentation/admin-guide/cgroup-v2.rst |  5 ++--
> > >  drivers/dma-buf/dma-buf.c               |  5 ++++
> > >  drivers/dma-buf/dma-heap.c              | 28 +++++++++++++++++--
> > > -
> > >  drivers/dma-buf/heaps/cma_heap.c        |  3 ++-
> > >  drivers/dma-buf/heaps/system_heap.c     | 21 +++++++++++----
> > >  drivers/tee/tee_heap.c                  |  3 ++-
> > >  include/linux/dma-buf.h                 |  4 +++
> > >  include/linux/dma-heap.h                |  3 ++-
> > >  include/linux/memcontrol.h              |  7 +++++
> > >  include/uapi/linux/dma-heap.h           |  7 +++++
> > >  mm/memcontrol.c                         | 47
> > > +++++++++++++++++++++++++++++++++
> > >  11 files changed, 120 insertions(+), 13 deletions(-)
> > > 
> > > diff --git a/Documentation/admin-guide/cgroup-v2.rst
> > > b/Documentation/admin-guide/cgroup-v2.rst
> > > index a3265a4a953e..66ddba678784 100644
> > > --- a/Documentation/admin-guide/cgroup-v2.rst
> > > +++ b/Documentation/admin-guide/cgroup-v2.rst
> > > @@ -1663,8 +1663,9 @@ The following nested keys are defined.
> > >  		structures.
> > >  
> > >  	  dmabuf (npn)
> > > -		Amount of memory used for exported DMA buffers
> > > allocated by the cgroup.
> > > -		Stays with the allocating cgroup regardless of
> > > how
> > > the buffer is shared.
> > > +		Amount of memory used for exported DMA buffers
> > > allocated by or on
> > > +		behalf of the cgroup. Stays with the allocating
> > > cgroup regardless
> > > +		of how the buffer is shared.
> > 
> > This doesn't sound right. Graphics buffers exported as DMA buffers
> > are
> > typically charged elsewhere, like gpu_active, and a transfer of
> > their
> > charge would likely stay in that key? To be consistent, shouldn't
> > this
> > be related to dma-heap memory and called dmaheap or something
> > similar.
> 
> Although this work initially targeted dma heaps only, in its current
> form is meant to stay generic (while solving the central allocator
> problem). The memory exported as dma-bufs has no memory.stat of its
> own, and an exporter allocating plain pages (e.g., V4L2) has nowhere
> else to report it.
> 
> > We should have a tighter definition of exactly what memory is to be
> > registered here.
> 
> Yes, I will be more specific and clearly state that an exporter
> whose memory is already accounted must not opt in. That also
> naturally disallows transfers.

I would suspect (although not sure TBH) a system that uses a
centralized allocator for gpu memory (even if it's accounted as
gpu_active) might want to be able to transfer the charge as well, so I
think the transfer mechanism might want to be generic enough to handle
that. At least the initial design should IMO allow for a future hook to
handle such transfers even for !dma-buf memory?

Thanks,
Thomas


> 
> > 
> > Also says "Stays with the allocating cgroup regardless of how the
> > buffer is shared", which isn't correct after the series? Perhaps
> > that's
> > changed in a later patch.
> 
> I missed updating the text after having added XFER_CHARGE to the
> series. I'll fix the wording in this patch and mention the transfer
> in the next one.
> 
> BR,
> Albert
> 
> > 
> > Thanks,
> > Thomas
> > 
> > 
> > 
> > 
> > >  
> > >  	  workingset_refault_anon
> > >  		Number of refaults of previously evicted
> > > anonymous
> > > pages.
> > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-
> > > buf.c
> > > index 1c6b9cee7ac2..837666273550 100644
> > > --- a/drivers/dma-buf/dma-buf.c
> > > +++ b/drivers/dma-buf/dma-buf.c
> > > @@ -714,6 +714,7 @@ struct dma_buf *dma_buf_export(const struct
> > > dma_buf_export_info *exp_info)
> > >  	struct dma_resv *resv = exp_info->resv;
> > >  	struct file *file;
> > >  	size_t alloc_size = sizeof(struct dma_buf);
> > > +	unsigned int nr_pages = PAGE_ALIGN(exp_info->size) >>
> > > PAGE_SHIFT;
> > >  	int ret;
> > >  
> > >  	if (WARN_ON(!exp_info->priv || !exp_info->ops
> > > @@ -750,6 +751,7 @@ struct dma_buf *dma_buf_export(const struct
> > > dma_buf_export_info *exp_info)
> > >  	dmabuf->size = exp_info->size;
> > >  	dmabuf->exp_name = exp_info->exp_name;
> > >  	dmabuf->owner = exp_info->owner;
> > > +	dmabuf->memcg = exp_info->memcg;
> > >  	spin_lock_init(&dmabuf->name_lock);
> > >  	init_waitqueue_head(&dmabuf->poll);
> > >  	dmabuf->cb_in.poll = dmabuf->cb_out.poll = &dmabuf-
> > > >poll;
> > > @@ -771,6 +773,9 @@ struct dma_buf *dma_buf_export(const struct
> > > dma_buf_export_info *exp_info)
> > >  
> > >  	DMA_BUF_TRACE(trace_dma_buf_export, dmabuf);
> > >  
> > > +	if (dmabuf->memcg)
> > > +		mod_memcg_state(dmabuf->memcg, MEMCG_DMABUF,
> > > nr_pages);
> > > +
> > >  	return dmabuf;
> > >  
> > >  err_file:
> > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-
> > > heap.c
> > > index 8a6c2b6e0cc6..7bd416ae2c23 100644
> > > --- a/drivers/dma-buf/dma-heap.c
> > > +++ b/drivers/dma-buf/dma-heap.c
> > > @@ -10,6 +10,8 @@
> > >  #include <linux/device.h>
> > >  #include <linux/dma-buf.h>
> > >  #include <linux/dma-heap.h>
> > > +#include <linux/memcontrol.h>
> > > +#include <linux/sched/mm.h>
> > >  #include <linux/err.h>
> > >  #include <linux/export.h>
> > >  #include <linux/list.h>
> > > @@ -78,6 +80,7 @@ static struct dma_buf
> > > *dma_heap_ioctl_allocate(struct file *file, void *data)
> > >  	struct dma_heap_allocation_data *heap_allocation = data;
> > >  	struct dma_heap *heap = file->private_data;
> > >  	struct dma_buf *dmabuf;
> > > +	struct mem_cgroup *memcg = NULL;
> > >  	int fd;
> > >  	size_t len;
> > >  
> > > @@ -90,15 +93,34 @@ static struct dma_buf
> > > *dma_heap_ioctl_allocate(struct file *file, void *data)
> > >  	if (heap_allocation->heap_flags &
> > > ~DMA_HEAP_VALID_HEAP_FLAGS)
> > >  		return ERR_PTR(-EINVAL);
> > >  
> > > +	if (heap_allocation->__padding != 0)
> > > +		return ERR_PTR(-EINVAL);
> > > +
> > > +	if (heap_allocation->charge_fd) {
> > > +		memcg = mem_cgroup_get_from_fd(heap_allocation-
> > > > charge_fd);
> > > +		if (IS_ERR(memcg))
> > > +			return ERR_CAST(memcg);
> > > +	}
> > > +
> > > +	if (!memcg && mem_accounting)
> > > +		memcg = get_mem_cgroup_from_mm(current->mm);
> > > +
> > >  	len = PAGE_ALIGN(heap_allocation->len);
> > > -	if (!len)
> > > +	if (!len) {
> > > +		mem_cgroup_put(memcg);
> > >  		return ERR_PTR(-EINVAL);
> > > +	}
> > >  
> > >  	dmabuf = heap->ops->allocate(heap, len, heap_allocation-
> > > > fd_flags,
> > > -				     heap_allocation-
> > > >heap_flags);
> > > +				     heap_allocation-
> > > >heap_flags,
> > > memcg);
> > >  
> > > -	if (IS_ERR(dmabuf))
> > > +	if (IS_ERR(dmabuf)) {
> > > +		mem_cgroup_put(memcg);
> > >  		return dmabuf;
> > > +	}
> > > +
> > > +	if (memcg && !dmabuf->memcg)
> > > +		mem_cgroup_put(memcg);
> > >  
> > >  	fd = get_unused_fd_flags(heap_allocation->fd_flags);
> > >  	if (fd < 0) {
> > > diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-
> > > buf/heaps/cma_heap.c
> > > index 3fb4b946c91a..04de88b468a8 100644
> > > --- a/drivers/dma-buf/heaps/cma_heap.c
> > > +++ b/drivers/dma-buf/heaps/cma_heap.c
> > > @@ -281,7 +281,8 @@ static const struct dma_buf_ops
> > > cma_heap_buf_ops
> > > = {
> > >  static struct dma_buf *cma_heap_allocate(struct dma_heap *heap,
> > >  					 unsigned long len,
> > >  					 u32 fd_flags,
> > > -					 u64 heap_flags)
> > > +					 u64 heap_flags,
> > > +					 struct mem_cgroup
> > > *memcg)
> > >  {
> > >  	struct cma_heap *cma_heap = dma_heap_get_drvdata(heap);
> > >  	struct cma_heap_buffer *buffer;
> > > diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-
> > > buf/heaps/system_heap.c
> > > index c8959eadc71d..2bd245e1eab8 100644
> > > --- a/drivers/dma-buf/heaps/system_heap.c
> > > +++ b/drivers/dma-buf/heaps/system_heap.c
> > > @@ -17,6 +17,7 @@
> > >  #include <linux/err.h>
> > >  #include <linux/highmem.h>
> > >  #include <linux/mem_encrypt.h>
> > > +#include <linux/memcontrol.h>
> > >  #include <linux/mm.h>
> > >  #include <linux/set_memory.h>
> > >  #include <linux/module.h>
> > > @@ -388,8 +389,6 @@ static struct page
> > > *alloc_largest_available(unsigned long size,
> > >  		if (max_order < orders[i])
> > >  			continue;
> > >  		flags = order_flags[i];
> > > -		if (mem_accounting)
> > > -			flags |= __GFP_ACCOUNT;
> > >  		page = alloc_pages(flags, orders[i]);
> > >  		if (!page)
> > >  			continue;
> > > @@ -401,11 +400,13 @@ static struct page
> > > *alloc_largest_available(unsigned long size,
> > >  static struct dma_buf *system_heap_allocate(struct dma_heap
> > > *heap,
> > >  					    unsigned long len,
> > >  					    u32 fd_flags,
> > > -					    u64 heap_flags)
> > > +					    u64 heap_flags,
> > > +					    struct mem_cgroup
> > > *memcg)
> > >  {
> > >  	struct system_heap_buffer *buffer;
> > >  	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> > >  	unsigned long size_remaining = len;
> > > +	unsigned int nr_pages = PAGE_ALIGN(len) >> PAGE_SHIFT;
> > >  	unsigned int max_order = orders[0];
> > >  	struct system_heap_priv *priv =
> > > dma_heap_get_drvdata(heap);
> > >  	bool cc_shared = priv->cc_shared;
> > > @@ -416,10 +417,15 @@ static struct dma_buf
> > > *system_heap_allocate(struct dma_heap *heap,
> > >  	struct page *page, *tmp_page;
> > >  	int i, ret = -ENOMEM;
> > >  
> > > -	buffer = kzalloc_obj(*buffer);
> > > -	if (!buffer)
> > > +	if (!mem_cgroup_reserve_dmabuf(memcg, GFP_KERNEL,
> > > nr_pages))
> > >  		return ERR_PTR(-ENOMEM);
> > >  
> > > +	buffer = kzalloc_obj(*buffer);
> > > +	if (!buffer) {
> > > +		ret = -ENOMEM;
> > > +		goto cancel_charge;
> > > +	}
> > > +
> > >  	INIT_LIST_HEAD(&buffer->attachments);
> > >  	mutex_init(&buffer->lock);
> > >  	buffer->heap = heap;
> > > @@ -473,11 +479,14 @@ static struct dma_buf
> > > *system_heap_allocate(struct dma_heap *heap,
> > >  	exp_info.size = buffer->len;
> > >  	exp_info.flags = fd_flags;
> > >  	exp_info.priv = buffer;
> > > +	if (memcg)
> > > +		exp_info.memcg = memcg;
> > >  	dmabuf = dma_buf_export(&exp_info);
> > >  	if (IS_ERR(dmabuf)) {
> > >  		ret = PTR_ERR(dmabuf);
> > >  		goto free_pages;
> > >  	}
> > > +
> > >  	return dmabuf;
> > >  
> > >  free_pages:
> > > @@ -498,6 +507,8 @@ static struct dma_buf
> > > *system_heap_allocate(struct dma_heap *heap,
> > >  	list_for_each_entry_safe(page, tmp_page, &pages, lru)
> > >  		__free_pages(page, compound_order(page));
> > >  	kfree(buffer);
> > > +cancel_charge:
> > > +	mem_cgroup_cancel_dmabuf(memcg, nr_pages);
> > >  
> > >  	return ERR_PTR(ret);
> > >  }
> > > diff --git a/drivers/tee/tee_heap.c b/drivers/tee/tee_heap.c
> > > index 007c00156520..83e2fbd6960b 100644
> > > --- a/drivers/tee/tee_heap.c
> > > +++ b/drivers/tee/tee_heap.c
> > > @@ -167,7 +167,8 @@ static const struct dma_buf_ops
> > > tee_heap_buf_ops
> > > = {
> > >  
> > >  static struct dma_buf *tee_dma_heap_alloc(struct dma_heap *heap,
> > >  					  unsigned long len, u32
> > > fd_flags,
> > > -					  u64 heap_flags)
> > > +					  u64 heap_flags,
> > > +					  struct mem_cgroup
> > > *memcg)
> > >  {
> > >  	struct tee_dma_heap *h = dma_heap_get_drvdata(heap);
> > >  	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> > > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > > index 23ec0ff23fba..85dd747c9b36 100644
> > > --- a/include/linux/dma-buf.h
> > > +++ b/include/linux/dma-buf.h
> > > @@ -505,6 +505,9 @@ struct dma_buf_attachment {
> > >   * @flags:	mode flags for the file
> > >   * @resv:	reservation-object, NULL to allocate default one
> > >   * @priv:	Attach private data of allocator to this buffer
> > > + * @memcg:	memcg to charge for this buffer's memory. Set
> > > only
> > > for
> > > + *		buffers backed by system memory where @size
> > > reflects
> > > the
> > > + *		actual allocation in bytes.
> > >   *
> > >   * This structure holds the information required to export the
> > > buffer. Used
> > >   * with dma_buf_export() only.
> > > @@ -517,6 +520,7 @@ struct dma_buf_export_info {
> > >  	int flags;
> > >  	struct dma_resv *resv;
> > >  	void *priv;
> > > +	struct mem_cgroup *memcg;
> > >  };
> > >  
> > >  /**
> > > diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
> > > index 648328a64b27..2d69f8432bd2 100644
> > > --- a/include/linux/dma-heap.h
> > > +++ b/include/linux/dma-heap.h
> > > @@ -23,7 +23,8 @@ struct dma_heap_ops {
> > >  	struct dma_buf *(*allocate)(struct dma_heap *heap,
> > >  				    unsigned long len,
> > >  				    u32 fd_flags,
> > > -				    u64 heap_flags);
> > > +				    u64 heap_flags,
> > > +				    struct mem_cgroup *memcg);
> > >  };
> > >  
> > >  /**
> > > diff --git a/include/linux/memcontrol.h
> > > b/include/linux/memcontrol.h
> > > index 4d1e82cf0833..f8ec751b7d7b 100644
> > > --- a/include/linux/memcontrol.h
> > > +++ b/include/linux/memcontrol.h
> > > @@ -671,6 +671,8 @@ int mem_cgroup_charge_hugetlb(struct folio*
> > > folio, gfp_t gfp);
> > >  int mem_cgroup_swapin_charge_folio(struct folio *folio, unsigned
> > > short id,
> > >  				   struct mm_struct *mm, gfp_t
> > > gfp);
> > >  
> > > +struct mem_cgroup *mem_cgroup_get_from_fd(int fd);
> > > +
> > >  bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t
> > > gfp,
> > >  			       unsigned int nr_pages);
> > >  
> > > @@ -1178,6 +1180,11 @@ static inline int
> > > mem_cgroup_swapin_charge_folio(struct folio *folio,
> > >  	return 0;
> > >  }
> > >  
> > > +static inline struct mem_cgroup *mem_cgroup_get_from_fd(int fd)
> > > +{
> > > +	return ERR_PTR(-EOPNOTSUPP);
> > > +}
> > > +
> > >  static inline bool mem_cgroup_reserve_dmabuf(struct mem_cgroup
> > > *memcg,
> > >  					     gfp_t gfp,
> > >  					     unsigned int
> > > nr_pages)
> > > diff --git a/include/uapi/linux/dma-heap.h
> > > b/include/uapi/linux/dma-
> > > heap.h
> > > index a4cf716a49fa..bfcd1c25bc7e 100644
> > > --- a/include/uapi/linux/dma-heap.h
> > > +++ b/include/uapi/linux/dma-heap.h
> > > @@ -29,6 +29,11 @@
> > >   *			handle to the allocated dma-buf
> > >   * @fd_flags:		file descriptor flags used when
> > > allocating
> > >   * @heap_flags:		flags passed to heap
> > > + * @charge_fd:		optional pidfd or cgroupfd whose cgroup
> > > should be
> > > + *			charged for this allocation; 0 means
> > > charge
> > > the calling
> > > + *			process's cgroup when the mem_accounting
> > > module parameter
> > > + *			is enabled, or skip charging otherwise
> > > + * @__padding:		reserved, must be zero
> > >   *
> > >   * Provided by userspace as an argument to the ioctl
> > >   */
> > > @@ -37,6 +42,8 @@ struct dma_heap_allocation_data {
> > >  	__u32 fd;
> > >  	__u32 fd_flags;
> > >  	__u64 heap_flags;
> > > +	__u32 charge_fd;
> > > +	__u32 __padding;
> > >  };
> > >  
> > >  #define DMA_HEAP_IOC_MAGIC		'H'
> > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > > index e53dd7eebc3b..17fdefc56b97 100644
> > > --- a/mm/memcontrol.c
> > > +++ b/mm/memcontrol.c
> > > @@ -5661,6 +5661,53 @@ void mem_cgroup_flush_workqueue(void)
> > >  	flush_workqueue(memcg_wq);
> > >  }
> > >  
> > > +/**
> > > + * mem_cgroup_get_from_fd - resolve a memcg from a cgroupfd or
> > > pidfd
> > > + * @fd: file descriptor (cgroupfd or pidfd)
> > > + *
> > > + * Tries @fd as a cgroupfd first; if that fails, tries it as a
> > > pidfd.
> > > + * On success returns a referenced memcg pointer. On failure
> > > returns
> > > + * an ERR_PTR.
> > > + */
> > > +struct mem_cgroup *mem_cgroup_get_from_fd(int fd)
> > > +{
> > > +	struct cgroup_subsys_state *css;
> > > +	struct mem_cgroup *memcg;
> > > +	struct cgroup *cgrp;
> > > +	struct task_struct *task;
> > > +	struct mm_struct *mm;
> > > +	unsigned int pidfd_flags;
> > > +
> > > +	cgrp = cgroup_get_from_fd(fd);
> > > +	if (!IS_ERR(cgrp)) {
> > > +		rcu_read_lock();
> > > +		css = cgroup_css(cgrp, &memory_cgrp_subsys);
> > > +		if (!css || !css_tryget_online(css)) {
> > > +			rcu_read_unlock();
> > > +			cgroup_put(cgrp);
> > > +			return ERR_PTR(-ENOENT);
> > > +		}
> > > +
> > > +		memcg = mem_cgroup_from_css(css);
> > > +		rcu_read_unlock();
> > > +		cgroup_put(cgrp);
> > > +		return memcg;
> > > +	}
> > > +
> > > +	task = pidfd_get_task(fd, &pidfd_flags);
> > > +	if (IS_ERR(task))
> > > +		return ERR_CAST(task);
> > > +
> > > +	mm = get_task_mm(task);
> > > +	put_task_struct(task);
> > > +	if (!mm)
> > > +		return ERR_PTR(-ESRCH);
> > > +
> > > +	memcg = get_mem_cgroup_from_mm(mm);
> > > +	mmput(mm);
> > > +	return memcg ?: ERR_PTR(-ENOENT);
> > > +}
> > > +
> > >  /**
> > >   * mem_cgroup_reserve_dmabuf - charge a memcg for a dma-buf
> > > allocation
> > >   * @memcg: memcg to charge (may be %NULL)