On Fri, Oct 30, 2026 at 11:14:50AM +0100, Christian König wrote:
> On 10/29/26 17:23, Albert Esteve wrote:
> > On Thu, Oct 29, 2026 at 03:16:12PM +0100, Christian König wrote:
> >> On 10/28/26 17:09, Albert Esteve wrote:
> >>> On Wed, Oct 28, 2026 at 10:08:19AM +0100, Christian König wrote:
> >>>> On 10/27/26 16:02, Albert Esteve wrote:
> >>>>> 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 <[email protected]>
> >>>>> ---
> >>>>> 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.
> >>
> >> 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.
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.
>
> >>> 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.
BR,
Albert.
>
> Regards,
> Christian.
>
> >
> >>
> >> Regards,
> >> Christian.
> >>
> >>>
> >>>>
> >>>>> 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!
> >>>
> >>>>
> >>>>> +{
> >>>>> + struct mem_cgroup *old_memcg = READ_ONCE(dmabuf->memcg);
> >>>>
> >>>>> + 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.
> >>>>
> >>>>> +
> >>>>> + 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
> >>>>>
> >>>>
> >>>
> >>
> >
>
