2022-05-03 01:26:07

by T.J. Mercier

[permalink] [raw]
Subject: [PATCH v6 3/6] dmabuf: heaps: export system_heap buffers with GPU cgroup charging

All DMA heaps now register a new GPU cgroup bucket upon creation, and the
system_heap now exports buffers associated with its GPU cgroup bucket for
tracking purposes.

In order to support GPU cgroup charge transfer on a dma-buf, the current
GPU cgroup information must be stored inside the dma-buf struct. For
tracked buffers, exporters include the struct gpucg and struct
gpucg_bucket pointers in the export info which can later be modified if
the charge is migrated to another cgroup.

Signed-off-by: Hridya Valsaraju <[email protected]>
Signed-off-by: T.J. Mercier <[email protected]>

---
v5 changes
Merge dmabuf: Use the GPU cgroup charge/uncharge APIs into this patch.

Remove all GPU cgroup code from dma-buf except what's necessary to support
charge transfer. Previously charging was done in export, but for
non-Android graphics use-cases this is not ideal since there may be a
dealy between allocation and export, during which time there is no
accounting.

Append "-heap" to gpucg_bucket names.

Charge on allocation instead of export. This should more closely mirror
non-Android use-cases where there is potentially a delay between allocation
and export.

Put the charge and uncharge code in the same file (system_heap_allocate,
system_heap_dma_buf_release) instead of splitting them between the heap and
the dma_buf_release.

Move no-op code to header file to match other files in the series.

v3 changes
Use more common dual author commit message format per John Stultz.

v2 changes
Move dma-buf cgroup charge transfer from a dma_buf_op defined by every
heap to a single dma-buf function for all heaps per Daniel Vetter and
Christian König.
---
drivers/dma-buf/dma-buf.c | 19 +++++++++++++
drivers/dma-buf/dma-heap.c | 39 +++++++++++++++++++++++++++
drivers/dma-buf/heaps/system_heap.c | 28 +++++++++++++++++---
include/linux/dma-buf.h | 41 +++++++++++++++++++++++------
include/linux/dma-heap.h | 15 +++++++++++
5 files changed, 130 insertions(+), 12 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index df23239b04fc..bc89c44bd9b9 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -462,6 +462,24 @@ static struct file *dma_buf_getfile(struct dma_buf *dmabuf, int flags)
* &dma_buf_ops.
*/

+#ifdef CONFIG_CGROUP_GPU
+static void dma_buf_set_gpucg(struct dma_buf *dmabuf, const struct dma_buf_export_info *exp)
+{
+ dmabuf->gpucg = exp->gpucg;
+ dmabuf->gpucg_bucket = exp->gpucg_bucket;
+}
+
+void dma_buf_exp_info_set_gpucg(struct dma_buf_export_info *exp_info,
+ struct gpucg *gpucg,
+ struct gpucg_bucket *gpucg_bucket)
+{
+ exp_info->gpucg = gpucg;
+ exp_info->gpucg_bucket = gpucg_bucket;
+}
+#else
+static void dma_buf_set_gpucg(struct dma_buf *dmabuf, struct dma_buf_export_info *exp) {}
+#endif
+
/**
* dma_buf_export - Creates a new dma_buf, and associates an anon file
* with this buffer, so it can be exported.
@@ -527,6 +545,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
init_waitqueue_head(&dmabuf->poll);
dmabuf->cb_in.poll = dmabuf->cb_out.poll = &dmabuf->poll;
dmabuf->cb_in.active = dmabuf->cb_out.active = 0;
+ dma_buf_set_gpucg(dmabuf, exp_info);

if (!resv) {
resv = (struct dma_resv *)&dmabuf[1];
diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
index 8f5848aa144f..b81015548314 100644
--- a/drivers/dma-buf/dma-heap.c
+++ b/drivers/dma-buf/dma-heap.c
@@ -7,10 +7,12 @@
*/

#include <linux/cdev.h>
+#include <linux/cgroup_gpu.h>
#include <linux/debugfs.h>
#include <linux/device.h>
#include <linux/dma-buf.h>
#include <linux/err.h>
+#include <linux/kconfig.h>
#include <linux/xarray.h>
#include <linux/list.h>
#include <linux/slab.h>
@@ -21,6 +23,7 @@
#include <uapi/linux/dma-heap.h>

#define DEVNAME "dma_heap"
+#define HEAP_NAME_SUFFIX "-heap"

#define NUM_HEAP_MINORS 128

@@ -31,6 +34,7 @@
* @heap_devt heap device node
* @list list head connecting to list of heaps
* @heap_cdev heap char device
+ * @gpucg_bucket gpu cgroup bucket for memory accounting
*
* Represents a heap of memory from which buffers can be made.
*/
@@ -41,6 +45,9 @@ struct dma_heap {
dev_t heap_devt;
struct list_head list;
struct cdev heap_cdev;
+#ifdef CONFIG_CGROUP_GPU
+ struct gpucg_bucket gpucg_bucket;
+#endif
};

static LIST_HEAD(heap_list);
@@ -216,6 +223,19 @@ const char *dma_heap_get_name(struct dma_heap *heap)
return heap->name;
}

+/**
+ * dma_heap_get_gpucg_bucket() - get struct gpucg_bucket for the heap.
+ * @heap: DMA-Heap to get the gpucg_bucket struct for.
+ *
+ * Returns:
+ * The gpucg_bucket struct for the heap. NULL if the GPU cgroup controller is
+ * not enabled.
+ */
+struct gpucg_bucket *dma_heap_get_gpucg_bucket(struct dma_heap *heap)
+{
+ return &heap->gpucg_bucket;
+}
+
struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
{
struct dma_heap *heap, *h, *err_ret;
@@ -228,6 +248,12 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
return ERR_PTR(-EINVAL);
}

+ if (IS_ENABLED(CONFIG_CGROUP_GPU) && strlen(exp_info->name) + strlen(HEAP_NAME_SUFFIX) >=
+ GPUCG_BUCKET_NAME_MAX_LEN) {
+ pr_err("dma_heap: Name is too long for GPU cgroup\n");
+ return ERR_PTR(-ENAMETOOLONG);
+ }
+
if (!exp_info->ops || !exp_info->ops->allocate) {
pr_err("dma_heap: Cannot add heap with invalid ops struct\n");
return ERR_PTR(-EINVAL);
@@ -253,6 +279,19 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
heap->ops = exp_info->ops;
heap->priv = exp_info->priv;

+ if (IS_ENABLED(CONFIG_CGROUP_GPU)) {
+ char gpucg_bucket_name[GPUCG_BUCKET_NAME_MAX_LEN];
+
+ snprintf(gpucg_bucket_name, sizeof(gpucg_bucket_name), "%s%s",
+ exp_info->name, HEAP_NAME_SUFFIX);
+
+ ret = gpucg_register_bucket(dma_heap_get_gpucg_bucket(heap), gpucg_bucket_name);
+ if (ret < 0) {
+ err_ret = ERR_PTR(ret);
+ goto err0;
+ }
+ }
+
/* Find unused minor number */
ret = xa_alloc(&dma_heap_minors, &minor, heap,
XA_LIMIT(0, NUM_HEAP_MINORS - 1), GFP_KERNEL);
diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index fcf836ba9c1f..27f686faef00 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -297,6 +297,11 @@ static void system_heap_dma_buf_release(struct dma_buf *dmabuf)
}
sg_free_table(table);
kfree(buffer);
+
+ if (dmabuf->gpucg && dmabuf->gpucg_bucket) {
+ gpucg_uncharge(dmabuf->gpucg, dmabuf->gpucg_bucket, dmabuf->size);
+ gpucg_put(dmabuf->gpucg);
+ }
}

static const struct dma_buf_ops system_heap_buf_ops = {
@@ -346,11 +351,21 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
struct scatterlist *sg;
struct list_head pages;
struct page *page, *tmp_page;
- int i, ret = -ENOMEM;
+ struct gpucg *gpucg;
+ struct gpucg_bucket *gpucg_bucket;
+ int i, ret;
+
+ gpucg = gpucg_get(current);
+ gpucg_bucket = dma_heap_get_gpucg_bucket(heap);
+ ret = gpucg_charge(gpucg, gpucg_bucket, len);
+ if (ret)
+ goto put_gpucg;

buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
- if (!buffer)
- return ERR_PTR(-ENOMEM);
+ if (!buffer) {
+ ret = -ENOMEM;
+ goto uncharge_gpucg;
+ }

INIT_LIST_HEAD(&buffer->attachments);
mutex_init(&buffer->lock);
@@ -396,6 +411,8 @@ 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;
+ dma_buf_exp_info_set_gpucg(&exp_info, gpucg, gpucg_bucket);
+
dmabuf = dma_buf_export(&exp_info);
if (IS_ERR(dmabuf)) {
ret = PTR_ERR(dmabuf);
@@ -414,7 +431,10 @@ static struct dma_buf *system_heap_allocate(struct dma_heap *heap,
list_for_each_entry_safe(page, tmp_page, &pages, lru)
__free_pages(page, compound_order(page));
kfree(buffer);
-
+uncharge_gpucg:
+ gpucg_uncharge(gpucg, gpucg_bucket, len);
+put_gpucg:
+ gpucg_put(gpucg);
return ERR_PTR(ret);
}

diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 2097760e8e95..8e7c55c830b3 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/iosys-map.h>
#include <linux/file.h>
#include <linux/err.h>
@@ -303,7 +304,7 @@ struct dma_buf {
/**
* @size:
*
- * Size of the buffer; invariant over the lifetime of the buffer.
+ * Size of the buffer in bytes; invariant over the lifetime of the buffer.
*/
size_t size;

@@ -453,6 +454,14 @@ struct dma_buf {
struct dma_buf *dmabuf;
} *sysfs_entry;
#endif
+
+#ifdef CONFIG_CGROUP_GPU
+ /** @gpucg: Pointer to the GPU cgroup this buffer currently belongs to. */
+ struct gpucg *gpucg;
+
+ /* @gpucg_bucket: Pointer to the GPU cgroup bucket whence this buffer originates. */
+ struct gpucg_bucket *gpucg_bucket;
+#endif
};

/**
@@ -526,13 +535,15 @@ struct dma_buf_attachment {

/**
* struct dma_buf_export_info - holds information needed to export a dma_buf
- * @exp_name: name of the exporter - useful for debugging.
- * @owner: pointer to exporter module - used for refcounting kernel module
- * @ops: Attach allocator-defined dma buf ops to the new buffer
- * @size: Size of the buffer - invariant over the lifetime of the buffer
- * @flags: mode flags for the file
- * @resv: reservation-object, NULL to allocate default one
- * @priv: Attach private data of allocator to this buffer
+ * @exp_name: name of the exporter - useful for debugging.
+ * @owner: pointer to exporter module - used for refcounting kernel module
+ * @ops: Attach allocator-defined dma buf ops to the new buffer
+ * @size: Size of the buffer in bytes - invariant over the lifetime of the buffer
+ * @flags: mode flags for the file
+ * @resv: reservation-object, NULL to allocate default one
+ * @priv: Attach private data of allocator to this buffer
+ * @gpucg: Pointer to GPU cgroup this buffer is charged to, or NULL if not charged
+ * @gpucg_bucket: Pointer to GPU cgroup bucket this buffer comes from, or NULL if not charged
*
* This structure holds the information required to export the buffer. Used
* with dma_buf_export() only.
@@ -545,6 +556,10 @@ struct dma_buf_export_info {
int flags;
struct dma_resv *resv;
void *priv;
+#ifdef CONFIG_CGROUP_GPU
+ struct gpucg *gpucg;
+ struct gpucg_bucket *gpucg_bucket;
+#endif
};

/**
@@ -630,4 +645,14 @@ int dma_buf_mmap(struct dma_buf *, struct vm_area_struct *,
unsigned long);
int dma_buf_vmap(struct dma_buf *dmabuf, struct iosys_map *map);
void dma_buf_vunmap(struct dma_buf *dmabuf, struct iosys_map *map);
+
+#ifdef CONFIG_CGROUP_GPU
+void dma_buf_exp_info_set_gpucg(struct dma_buf_export_info *exp_info,
+ struct gpucg *gpucg,
+ struct gpucg_bucket *gpucg_bucket);
+#else/* CONFIG_CGROUP_GPU */
+static inline void dma_buf_exp_info_set_gpucg(struct dma_buf_export_info *exp_info,
+ struct gpucg *gpucg,
+ struct gpucg_bucket *gpucg_bucket) {}
+#endif /* CONFIG_CGROUP_GPU */
#endif /* __DMA_BUF_H__ */
diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
index 0c05561cad6e..6321e7636538 100644
--- a/include/linux/dma-heap.h
+++ b/include/linux/dma-heap.h
@@ -10,6 +10,7 @@
#define _DMA_HEAPS_H

#include <linux/cdev.h>
+#include <linux/cgroup_gpu.h>
#include <linux/types.h>

struct dma_heap;
@@ -59,6 +60,20 @@ void *dma_heap_get_drvdata(struct dma_heap *heap);
*/
const char *dma_heap_get_name(struct dma_heap *heap);

+#ifdef CONFIG_CGROUP_GPU
+/**
+ * dma_heap_get_gpucg_bucket() - get a pointer to the struct gpucg_bucket for the heap.
+ * @heap: DMA-Heap to retrieve gpucg_bucket for
+ *
+ * Returns:
+ * The gpucg_bucket struct for the heap.
+ */
+struct gpucg_bucket *dma_heap_get_gpucg_bucket(struct dma_heap *heap);
+#else /* CONFIG_CGROUP_GPU */
+static inline struct gpucg_bucket *dma_heap_get_gpucg_bucket(struct dma_heap *heap)
+{ return NULL; }
+#endif /* CONFIG_CGROUP_GPU */
+
/**
* dma_heap_add - adds a heap to dmabuf heaps
* @exp_info: information needed to register this heap
--
2.36.0.464.gb9c8b46e94-goog