Hi Chritian, On Tue, May 19, 2026 at 09:53:19AM +0200, Christian König wrote: > On 5/18/26 14:06, Albert Esteve wrote: > >>>>> udmabufs are already > >>>>> memcg-charged, so adding a separate MEMCG_DMABUF would double count. > >>>>> Are there any other exporters you had in mind that would benefit from > >>>>> this approach? > >> > >> Well apart from DMA-buf memfd_create() is one of the things which as > >> broken our neck in the past a couple of times. > >> > >> But thinking more about it what if instead of making this DMA-buf heaps > >> specific what if we have a general cgroups function which allows to change > >> accounting of a buffer referenced by a file descriptor to a different > >> process? > >> > >> That would cover not only the DMA-buf heaps use case, but also all other > >> DMA-buf with dmem and whatever we come up in the future as well. > > > > I removed a draft adding an ioctl for charge transfer from the series > > before sending because I wanted to focus on the charge_pid_fd approach > > and keep things simple, deferring the recharge path to a follow-up > > depending on feedback. > > > > The main difference between my removed draft and what you're > > describing, iiuc, is scope and layer: my draft was an explicit ioctl > > on the dma-buf fd that the consumer calls to claim the charge (see > > below), while you seem to be suggesting a more general kernel-internal > > function that could work across buffer types and cgroup controllers, > > so not necessarily userspace-initiated? A kernel-internal function > > will need a way to identify the target process, which sounds similar > > to the binder-backed approach from TJ [1]. For everything else, the > > receiver still needs to declare itself, which the ioctl accomplishes. > > > > ``` > > # When an app imports a daemon-allocated buffer, it can transfer the > > charge to itself: > > int buf_fd = receive_dmabuf_from_daemon(); > > ioctl(buf_fd, DMA_BUF_IOCTL_XFER_CHARGE); /* charge now attributed to > > apps's cgroup */ > > Well that thinking goes into the right direction, but the requirements are > still not completely > covered as far as I can see. > > Let me explain below a bit more. > > > > > [1] > > https://lore.kernel.org/cgroups/[email protected]/ > > > >> > >> The only drawback I can see is that DMA-buf heap allocations would be > >> temporarily accounted to the memory allocation daemon, but I don't think > >> that this would be a problem. > > > > The main reasons we moved away from TJ's transfer-based approach > > toward `charge_pid_fd` are: avoid the transient charge window on the > > daemon's cgroup; and to decouple from Binder, allowing any allocator > > to use it. > > Yeah those concerns are completely correct. > > The application should not volunteering says 'Charge that buffer to > me.', but rather that the daemon says force charge that buffer to this > application and tell me when the application is over its limit.
I would agree, but with a caveat: how do we want to deal with malicious applications here? The application should have expressed that it's okay for it to be charged by a different process, otherwise it becomes trivial for a malicious app to create arbitrary charges against another application in the system and DoS it. But then, that means that an application could arbitrarily charge the daemon as well if it doesn't opt-in but asks for allocations. So maybe we should have an opt-in for the caller, and a way for the daemon to check if the caller has indeed opted in before performing the allocation (and the charge transfer)? > > Technically, both approaches could coexist, though. Of the three > > scenarios TJ described: > > - Scenario 2 is directly addressed by charge_pid_fd approach without > > any transient charge on the daemon at the cost of one extra field in > > the heap ioctl uAPI struct. > > Yeah extending the uAPI to pass in the pid on allocation time is not > much of a problem, but you also need to modify the whole stack above > it and that is a bit more trickier. > > > - Scenario 3 can be handled by the charge transfer function without > > changes to SurfaceFlinger. The app or dequeueBuffer claims the charge > > for itself or the app, respectively (depending on whether we include a > > pid_fd field in the transfer ioctl). It also covers non-heap > > exporters. The con in both variants is the transient charge window on > > the daemon. > > It should be trivial for the deamon to charge the buffer to an > application before handing it out. > > > Both approaches shift the responsibility for correct charging > > attribution to userspace: first, 'charge_pid_fd` on the allocator's > > side, and the transfer charge on the consumer's side. > > Yeah that's why I said it would be better if we do that without any > uAPI change, but with all the uAPI we have to transfer file > descriptors (dup(), fork(), passing FDs over sockets etc...) it could > be really tricky to implement that. > > > Deciding on one, the other or both depends on how much we value > > avoiding transient attribution, and how much we need a non-heap > > generic solution. With the XFER_CHARGE we can cover both. Thus, the > > `charge_pid_fd` approach in this RFC can be seen as a > > performance/strictness optimisation, eliminating transient charges to > > the daemon at the cost of a permanent uAPI addition to the heap ioctl > > struct, but not strictly required for correctness. > > Well all we need is a uAPI which says charge this buffer (file > descriptor) to that cgroup (pidfd). > > With this at hand we should be able to handle all use cases at the > same time. > > > On the other hand, if we agree on the end goal of migrating other > > exporters to use dma-buf heaps > > That won't work. DMA-buf heaps is actually only a rather small and > Anroid specific use case. I don't think that's true anymore. heaps are used in lots of different use cases now in the embedded space, including in regular, generic, components not specifically used for embedded systems. Maxime
signature.asc
Description: PGP signature
