Re: [PATCH v3 7/8] dma-buf: transfer memcg charges via DMA_BUF_IOCTL_XFER_CHARGE

From
Christian König <christian.koenig@amd.com>
Date
2026-11-02 14:03:03
Also in
cgroups, dri-devel, linux-doc, linux-kselftest, linux-media, linux-mm, lkml, op-tee, selinux
Reply to
Albert Esteve, 2026-11-02
On 11/2/26 10:45, Albert Esteve wrote:
On Fri, Oct 30, 2026 at 11:14:50AM +0100, Christian König wrote:
quoted
On 10/29/26 17:23, Albert Esteve wrote:
quoted
On Thu, Oct 29, 2026 at 03:16:12PM +0100, Christian König wrote:
quoted
On 10/28/26 17:09, Albert Esteve wrote:
quoted
On Wed, Oct 28, 2026 at 10:08:19AM +0100, Christian König wrote:
quoted
On 10/27/26 16:02, Albert Esteve wrote:
quoted
dma-heap already supports charging a specific cgroup at allocation
time via charge_fd in DMA_HEAP_IOCTL_ALLOC. However, dma-bufs can
originate from any exporter (DRM, V4L2, etc.), and the allocator may
not know the final owner at allocation time.

Add DMA_BUF_IOCTL_XFER_CHARGE to transfer a dma-buf's memcg charge
to a different cgroup after allocation. The target is identified by
charge_fd, which may be a cgroupfd or a pidfd.

The transfer charges the new cgroup before uncharging the old one,
preserving the original charge on failure. An atomic cmpxchg on
dmabuf->memcg prevents races with concurrent transfers.

Not all dma-buf exporters back their buffers with reclaimable system
memory. Some represent device registers, VRAM, or carved-out pools
where size is not a meaningful memory cost. Add a memcg_account flag
to dma_buf_export_info that exporters set when their buffers should
participate in memcg accounting. The xfer ioctl returns -EOPNOTSUPP
for buffers that do not set the flag. Currently only the system heap
sets it, other exporters can opt in as needed.

Usage example:
  int cg_fd = open("/sys/fs/cgroup/clients/app1", O_RDONLY | O_DIRECTORY);

  struct dma_buf_transfer_charge xfer = {
      .charge_fd = cg_fd,
  };
  ioctl(dmabuf_fd, DMA_BUF_IOCTL_XFER_CHARGE, &xfer);
  /* charge now on app1's cgroup */
  close(cg_fd);

Signed-off-by: Albert Esteve <redacted>
---
 drivers/dma-buf/dma-buf.c           | 43 +++++++++++++++++++++++++++++++++++++
 drivers/dma-buf/heaps/system_heap.c |  1 +
 include/linux/dma-buf.h             |  5 +++++
 include/linux/memcontrol.h          |  9 ++++++++
 include/uapi/linux/dma-buf.h        | 26 ++++++++++++++++++++++
 mm/memcontrol.c                     | 41 +++++++++++++++++++++++++++++++++++
 6 files changed, 125 insertions(+)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index f31f49ebd0af..07513ecedc10 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -540,6 +540,45 @@ static long dma_buf_import_sync_file(struct dma_buf *dmabuf,
 }
 #endif
 
+/* Transfer a dma-buf's memcg charge; see struct dma_buf_transfer_charge */
+static long dma_buf_xfer_charge(struct dma_buf *dmabuf,
+				const void __user *user_data)
+{
+	struct dma_buf_transfer_charge arg;
+	struct mem_cgroup *memcg;
+	int ret = 0;
+
+	if (copy_from_user(&arg, user_data, sizeof(arg)))
+		return -EFAULT;
+
+	if (!IS_ENABLED(CONFIG_MEMCG))
+		return 0;
+
+	if (arg.flags != 0)
+		return -EINVAL;
+
+	if (!dmabuf->memcg_account)
+		return -EOPNOTSUPP;
+
+	memcg = mem_cgroup_get_from_fd(arg.charge_fd);
+	if (IS_ERR(memcg))
+		return PTR_ERR(memcg);
+
+	if (dmabuf->memcg == memcg)
+		goto skip_transfer;
+
+	if (!mem_cgroup_transfer_dmabuf_charge(dmabuf, memcg)) {
+		ret = -ENOMEM;
+		goto skip_transfer;
+	}
+
+	return 0;
+
+skip_transfer:
+	mem_cgroup_put(memcg);
+	return ret;
+}
+
 static long dma_buf_ioctl(struct file *file,
 			  unsigned int cmd, unsigned long arg)
 {
@@ -590,6 +629,9 @@ static long dma_buf_ioctl(struct file *file,
 		return dma_buf_import_sync_file(dmabuf, (const void __user *)arg);
 #endif
 
+	case DMA_BUF_IOCTL_XFER_CHARGE:
+		return dma_buf_xfer_charge(dmabuf, (const void __user *)arg);
+
 	default:
 		return -ENOTTY;
 	}
@@ -750,6 +792,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_account = exp_info->memcg_account;
The initial charging should probably be done here as well.

This way DMA-buf heaps only needs to pass in the charge_fd and we can re-use the functionality for other DMA-buf exporters as well.
The problem with doing the charge here is that it happens too late.
That is the reasoning for this proposal to move all the charging logic
into heap allocators before pages are allocated.
Oh, that is a really good point we need to talk about.

DMA-buf heaps is actually a rather simple use case, but we have way more complex things.

For example in some use cases the backing store of a DMA-buf is only allocated on first use and only then it needs to be comitted to it's memcg or dmem.
Yes. The idea is focus on heaps for now. But the uAPI is actually
agnostic of the time when the charge happens. When we deal with
lazy-allocation in the future we can just call the reserve/cancel
helpers wherever is more appropiate. As long as the pages are not
allocated, exporters do not set dmabuf->memcg, and XFER does not
work (returns -EOPNOTSUPP).
Yeah exactly that's the reason why I'm raising this topic. That approach won't work in all cases.

See userspace is not aware that the kernel is delaying allocation or that the kernel is allocating the backing store asynchroniously in a background worker for example.

So returning -EOPNOTSUPP when the backing store is not allocated yet and might be allocated later on is rather questionable uAPI because that is racy by design.
Ah! I see what you meant now.
quoted
quoted
quoted
I don't mind implementing it for DMA-buf heaps first, but we should have the uAPI and it's handling so solid that it also works for future use cases.
Agreed. I think we should cover all usecases with the current status.
Your feedback helped a lot in getting there!
I mean we can postpone that until we finally need it, but essentially we need to be able to distinct between charging, e.g. making sure the cgroup has enough free limit, and committing, e.g. allowing the resource to be reclaimed.

And then it is possible that when the memcg of a DMA-buf changes that only the charging changes, but there isn't any comitted backing store yet...
Right. That would need adding a specific flag for committed pages or
a similar solution. When transfering an uncommited dmabuf only the
charge moves (limit reservation and dmabuf->memcg), the stat is
untouched. Once the backing store exists, the exporter commits, it
sets the flag and bumps MEMCG_DMABUF stat. For memory where its
backing store can go away (e.g., dynamic dma-buf sharing), it can
have a mirror function that drops the stat but keeps the charge.
That would resolve the -EOPNOTSUPP thingy, as transfer succeeds
when dmabuf->memcg is set even for uncommitted pages.

My question would be if all exporters know when memory is committed.
Yes, the exporter "owns" the backing store and is the only one who can allocate it based on requests from importers (or itself).
For those that only get, e.g., a firmware handle and cannot observe
the allocation, I guess the buffer can just remain reserved without
even being committed. Or in case of protected memory like OP-TEE
they would not even opt-in to begin with.

I do not mind adding that for next version, or leaving it for when
lazy exporters actually opt-in. It is not a big change either way.
Although, it may look weird while the system heap is the only user
and commits right away. But either way I can try to document the
flag / explain this discussion in the commit.
I think we should keep it simple and stupid for now, just not make decisions which would block extending it in the future.
quoted
quoted
quoted
quoted
What we could do is fill the dmabuf->memcg field when the flag is set
(i.e., committing into the dmabuf in the patch semantics). But for that
we'd need adding the memcg we want to charge it to into exp_info in
order to support charging to cgroups other than the allocator's (for
the common case would be simpler as we can just look at current).
Maybe that's what you meant?
At least for the DMA-buf heaps use case that sounds reasonable for now.
Thanks! Next version will have the flag removed and just use
dmabuf->memcg as indicator, set within dma_buf_export().
Also, `nr_pages` will be computed within dma_buf_export(), the
contract being that exporters setting exp_info.memcg assert that
size reflects actual system memory bytes.
Thinking more about it I would also put changing dmabuf->memcg under the dmabuf->resv lock instead of doing it with cmpxchg.

We don't need it currently, but the dmabuf->resv lock is used for all state changes of the DMA-buf object and this operation is not performance critical in any way possible.

If there is a problem with locking order we at least get lockdep screaming at us very early and not when we want to extend the functionality in the future.
Sure, will do.
Thanks,
Christian.
BR,
Albert.
quoted
Regards,
Christian.
quoted
quoted
Regards,
Christian.
quoted
quoted
quoted
 	spin_lock_init(&dmabuf->name_lock);
 	init_waitqueue_head(&dmabuf->poll);
 	dmabuf->cb_in.poll = dmabuf->cb_out.poll = &dmabuf->poll;
diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index adeea8cd328b..a764167167ba 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -479,6 +479,7 @@ 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;
+	exp_info.memcg_account = true;
 	dmabuf = dma_buf_export(&exp_info);
 	if (IS_ERR(dmabuf)) {
 		ret = PTR_ERR(dmabuf);
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 23ec0ff23fba..ac0ec88cab93 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -433,6 +433,9 @@ struct dma_buf {
 
 	/** @memcg: the cgroup to which this buffer is currently attributed */
 	struct mem_cgroup *memcg;
+
+	/** @memcg_account: account buffer memory to cgroup */
+	bool memcg_account;
 };
 
 /**
@@ -505,6 +508,7 @@ 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_account:	account buffer memory to cgroup
  *
  * This structure holds the information required to export the buffer. Used
  * with dma_buf_export() only.
@@ -517,6 +521,7 @@ struct dma_buf_export_info {
 	int flags;
 	struct dma_resv *resv;
 	void *priv;
+	bool memcg_account;
 };
 
 /**
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 6b30baa79fa9..36fddafbef4e 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -679,6 +679,9 @@ bool mem_cgroup_reserve_dmabuf(struct mem_cgroup *memcg, gfp_t gfp,
 void mem_cgroup_commit_dmabuf(struct dma_buf *dmabuf, struct mem_cgroup *memcg,
 			      unsigned int nr_pages);
 
+bool mem_cgroup_transfer_dmabuf_charge(struct dma_buf *dmabuf,
+				       struct mem_cgroup *new_memcg);
+
 void __mem_cgroup_uncharge(struct folio *folio);
 
 /**
@@ -1206,6 +1209,12 @@ static inline void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg,
 {
 }
 
+static inline bool mem_cgroup_transfer_dmabuf_charge(struct dma_buf *dmabuf,
+						     struct mem_cgroup *new_memcg)
+{
+	return true;
+}
+
 static inline void mem_cgroup_uncharge(struct folio *folio)
 {
 }
diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h
index e827c9d20c5d..ea957f4b1d38 100644
--- a/include/uapi/linux/dma-buf.h
+++ b/include/uapi/linux/dma-buf.h
@@ -168,6 +168,31 @@ struct dma_buf_import_sync_file {
 	__s32 fd;
 };
 
+/**
+ * struct dma_buf_transfer_charge - Transfer a dma-buf's memcg charge
+ *
+ * Any holder of a dma-buf file descriptor can perform a
+ * DMA_BUF_IOCTL_XFER_CHARGE to attribute the buffer's memory
+ * cost to a specific cgroup, or move it from one cgroup to
+ * another.  The ioctl charges the new cgroup before uncharging
+ * the old one atomically.  On failure the original charge is
+ * preserved.  If the buffer has no existing charge, a new
+ * charge is established.
+ */
+struct dma_buf_transfer_charge {
+	/** @flags: Reserved, must be zero */
+	__u32 flags; /* unused for now */
+	/**
+	 * @charge_fd: File descriptor identifying the target cgroup.
+	 *
+	 * May be either a pidfd, in which case the charge moves to the
+	 * memory cgroup of the referenced process, or a cgroupfd obtained
+	 * by opening a cgroup v2 directory, in which case the charge moves
+	 * directly to that cgroup's memory controller.
+	 */
+	__u32 charge_fd;
+};
+
 #define DMA_BUF_BASE		'b'
 #define DMA_BUF_IOCTL_SYNC	_IOW(DMA_BUF_BASE, 0, struct dma_buf_sync)
 
@@ -179,5 +204,6 @@ struct dma_buf_import_sync_file {
 #define DMA_BUF_SET_NAME_B	_IOW(DMA_BUF_BASE, 1, __u64)
 #define DMA_BUF_IOCTL_EXPORT_SYNC_FILE	_IOWR(DMA_BUF_BASE, 2, struct dma_buf_export_sync_file)
 #define DMA_BUF_IOCTL_IMPORT_SYNC_FILE	_IOW(DMA_BUF_BASE, 3, struct dma_buf_import_sync_file)
+#define DMA_BUF_IOCTL_XFER_CHARGE	_IOW(DMA_BUF_BASE, 4, struct dma_buf_transfer_charge)
 
 #endif
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 7b841b905759..3adc84fe6668 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5786,6 +5786,47 @@ void mem_cgroup_cancel_dmabuf(struct mem_cgroup *memcg, unsigned int nr_pages)
 		refill_stock(memcg, nr_pages);
 }
 
+/**
+ * mem_cgroup_transfer_dmabuf_charge - move a dma-buf charge
+ * @dmabuf: buffer to transfer charge
+ * @new_memcg: target memcg; caller must hold a reference
+ *
+ * Transfers the charge for @dmabuf to @new_memcg and, on
+ * success, stores @new_memcg in @dmabuf->memcg.
+ * Returns %true on success, %false if the charge against @new_memcg
+ * failed or a concurrent transfer raced on @dmabuf->memcg.
+ * On failure the caller retains its reference to @new_memcg.
+ */
+bool mem_cgroup_transfer_dmabuf_charge(struct dma_buf *dmabuf,
+				       struct mem_cgroup *new_memcg)
I would make that more generic instead of dmabuf specific. E.g. something like this:

bool mem_cgroup_transfer_charge(struct struct mem_cgroup **ptr_memcg, struct mem_cgroup *new_memcg, enum memcg_stat_item item, unsigned int nr_pages)
Ah yes, I like this generalization. And the implementation remains
mostly the same. Thanks!
quoted
quoted
+{
+	struct mem_cgroup *old_memcg = READ_ONCE(dmabuf->memcg);
quoted
+	unsigned int nr_pages = PAGE_ALIGN(dmabuf->size) >> PAGE_SHIFT;
This way we can also document in the DMA-buf caller that this conversation must be ok when memcg_account is true.

Regards,
Christian.
quoted
+
+	if (mem_cgroup_disabled())
+		return true;
+
+	if (!mem_cgroup_reserve_dmabuf(new_memcg, GFP_KERNEL, nr_pages))
+		return false;
+
+	mod_memcg_state(new_memcg, MEMCG_DMABUF, nr_pages);
+
+	if (cmpxchg(&dmabuf->memcg, old_memcg, new_memcg) != old_memcg) {
+		mod_memcg_state(new_memcg, MEMCG_DMABUF, -nr_pages);
+		mem_cgroup_cancel_dmabuf(new_memcg, nr_pages);
+		return false;
+	}
+
+	if (old_memcg) {
+		/* Uncharge old cgroup */
+		mod_memcg_state(old_memcg, MEMCG_DMABUF, -nr_pages);
+		mem_cgroup_cancel_dmabuf(old_memcg, nr_pages);
+		mem_cgroup_put(old_memcg);
+	}
+
+	return true;
+}
+
 /**
  * mem_cgroup_uncharge_dmabuf - uncharge a dma-buf from its memcg
  * @dmabuf: buffer to uncharge
  
Thread (25 messages) 25 messages, 3 authors, last reply 2d ago
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help