On Tue, Jan 18, 2022 at 10:54:16AM -0800, Hridya Valsaraju wrote:
On Sun, Jan 16, 2022 at 11:46 PM Christian König christian.koenig@amd.com wrote:
Am 15.01.22 um 02:06 schrieb Hridya Valsaraju:
The optional exporter op provides a way for processes to transfer charge of a buffer to a different process. This is essential for the cases where a central allocator process does allocations for various subsystems, hands over the fd to the client who requested the memory and drops all references to the allocated memory.
Signed-off-by: Hridya Valsaraju hridya@google.com
include/linux/dma-buf.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 7ab50076e7a6..d5e52f81cc6f 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -13,6 +13,7 @@ #ifndef __DMA_BUF_H__ #define __DMA_BUF_H__
+#include <linux/cgroup_gpu.h> #include <linux/dma-buf-map.h> #include <linux/file.h> #include <linux/err.h> @@ -285,6 +286,23 @@ struct dma_buf_ops {
int (*vmap)(struct dma_buf *dmabuf, struct dma_buf_map *map); void (*vunmap)(struct dma_buf *dmabuf, struct dma_buf_map *map);
/*** @charge_to_cgroup:** This is called by an exporter to charge a buffer to the specified* cgroup.Well that sentence makes absolutely no sense at all.
The dma_buf_ops are supposed to be called by the DMA-buf subsystem on behalves of the importer and never by the exporter itself.
I hope that this is just a documentation mixup.
Thank you for taking a look Christian!
Yes, that was poor wording, sorry about that. It should instead say that the op would be called by the process the buffer is currently charged to in order to transfer the buffer's charge to a different cgroup. This is helpful in the case where a process acts as an allocator for multiple client processes and we would like the allocated buffers to be charged to the clients who requested their allocation(instead of the allocating process as is the default behavior). In Android, the graphics allocator HAL process[1] does most of the graphics allocations on behalf of various clients. After allocation, the HAL process passes the fd to the client over binder IPC and the binder driver invokes the charge_to_cgroup() DMA-BUF op to uncharge the buffer from the HAL process and charge it to the client process instead.
For that use-case, do we really need to have the vfunc abstraction and force all exporters to do something reasonable with it?
I think just storing the cgrpus gpu memory bucket this is charged against and doing this in a generic way would be a lot better.
That way we can also easily add other neat features in the future, like e.g. ttm could take care of charge-assignement automatically maybe, or we could print the current cgroups charge relationship in the sysfs info file. Or anything else really.
I do feel that in general for gpu memory cgroups to be useful, we should really have memory pools as a fairly strong concept. Otherwise every driver/allocator/thing is going to come up with their own, and very likely incompatible interpretation. And we end up with a supposed generic cgroups interface which cannot actually be used in a driver/vendor agnostic way at all. -Daniel
Regards, Hridya
Regards, Christian.
The caller must hold a reference to @gpucg obtained via
* gpucg_get(). The DMA-BUF will be uncharged from the cgroup it is* currently charged to before being charged to @gpucg. The caller must* belong to the cgroup the buffer is currently charged to.** This callback is optional.** Returns:** 0 on success or negative error code on failure.*/int (*charge_to_cgroup)(struct dma_buf *dmabuf, struct gpucg *gpucg);};
/**