On Tue, May 19, 2026 at 9:53 AM Christian König <[email protected]> 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. > > > > > 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.
Yeah, true. > > > 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). So you favor having only the XFER_CHARGE variant. That is fine with me. If that is fine for others also that could be the way forward. If we extend it to accept either a pidfd or a cgroup fd (as commented previously), we can cover all dma-buf use cases with a single primitive: ``` ioctl(buf_fd, DMA_BUF_IOCTL_XFER_CHARGE, charge_fd); ``` With the daemon invoking this ioctl before handing out the buf_fd. This should cover most usecases? Except for the memfd case, which requires a separate mechanism. That would be follow-up work. > > 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. > > We have tons of other interfaces to allocate DMA-bufs which need to stay > around because of HW restrictions and we do need a solution for them as well. > > Regards, > Christian. > > >, and scenario 3 is addressed by adding the app's pid_fd > > to SurfaceFlinger, then `charge_pid_fd` alone is a coherent/sufficient > > approach despite the uAPI change. > > > >> > >> Regards, > >> Christian. > >> > >>> > >>> Thanks > >>> Barry > >> > > >
