All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Welty <brian.welty@intel.com>
To: "Brian Welty" <brian.welty@intel.com>,
	cgroups@vger.kernel.org, "Tejun Heo" <tj@kernel.org>,
	dri-devel@lists.freedesktop.org,
	"David Airlie" <airlied@linux.ie>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Christian König" <christian.koenig@amd.com>,
	"Kenny Ho" <Kenny.Ho@amd.com>,
	amd-gfx@lists.freedesktop.org,
	"Chris Wilson" <chris@chris-wilson.co.uk>,
	"Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>,
	intel-gfx@lists.freedesktop.org,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Eero Tamminen" <eero.t.tamminen@intel.com>
Subject: [RFC PATCH 8/9] drm/gem: Associate GEM objects with drm cgroup
Date: Tue, 26 Jan 2021 13:46:25 -0800	[thread overview]
Message-ID: <20210126214626.16260-9-brian.welty@intel.com> (raw)
In-Reply-To: <20210126214626.16260-1-brian.welty@intel.com>

This patch adds tracking of which cgroup to make charges against for a
given GEM object.  We associate the current task's cgroup with GEM objects
as they are created.  First user of this is for charging DRM cgroup for
device memory allocations.  The intended behavior is for device drivers to
make the cgroup charging calls at the time that backing store is allocated
or deallocated for the object.

Exported functions are provided for charging memory allocations for a
GEM object to DRM cgroup. To aid in debugging, we store how many bytes
have been charged inside the GEM object.  Add helpers for setting and
clearing the object's associated cgroup which will check that charges are
not being leaked.

For shared objects, this may make the charge against a cgroup that is
potentially not the same cgroup as the process using the memory.  Based
on the memory cgroup's discussion of "memory ownership", this seems
acceptable [1].

[1] https://www.kernel.org/doc/Documentation/cgroup-v2.txt, "Memory Ownership"

Signed-off-by: Brian Welty <brian.welty@intel.com>
---
 drivers/gpu/drm/drm_gem.c | 89 +++++++++++++++++++++++++++++++++++++++
 include/drm/drm_gem.h     | 17 ++++++++
 2 files changed, 106 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index c2ce78c4edc3..a12da41eaafe 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -29,6 +29,7 @@
 #include <linux/slab.h>
 #include <linux/mm.h>
 #include <linux/uaccess.h>
+#include <linux/cgroup_drm.h>
 #include <linux/fs.h>
 #include <linux/file.h>
 #include <linux/module.h>
@@ -112,6 +113,89 @@ drm_gem_init(struct drm_device *dev)
 	return drmm_add_action(dev, drm_gem_init_release, NULL);
 }
 
+/**
+ * drm_gem_object_set_cgroup - associate GEM object with a cgroup
+ * @obj: GEM object which is being associated with a cgroup
+ * @task: task associated with process control group to use
+ *
+ * This will acquire a reference on cgroup and use for charging GEM
+ * memory allocations.
+ * This helper could be extended in future to migrate charges to another
+ * cgroup, print warning if this usage occurs.
+ */
+void drm_gem_object_set_cgroup(struct drm_gem_object *obj,
+			       struct task_struct *task)
+{
+	/* if object has existing cgroup, we migrate the charge... */
+	if (obj->drmcg) {
+		pr_warn("DRM: need to migrate cgroup charge of %lld\n",
+			atomic64_read(&obj->drmcg_bytes_charged));
+	}
+	obj->drmcg = drmcg_get(task);
+}
+EXPORT_SYMBOL(drm_gem_object_set_cgroup);
+
+/**
+ * drm_gem_object_unset_cgroup - clear GEM object's associated cgroup
+ * @obj: GEM object
+ *
+ * This will release a reference on cgroup.
+ */
+void drm_gem_object_unset_cgroup(struct drm_gem_object *obj)
+{
+	WARN_ON(atomic64_read(&obj->drmcg_bytes_charged));
+	drmcg_put(obj->drmcg);
+}
+EXPORT_SYMBOL(drm_gem_object_unset_cgroup);
+
+/**
+ * drm_gem_object_charge_mem - try charging size bytes to DRM cgroup
+ * @obj: GEM object which is being charged
+ * @size: number of bytes to charge
+ *
+ * Try to charge @size bytes to GEM object's associated DRM cgroup.  This
+ * will fail if a successful charge would cause the current device memory
+ * usage to go above the cgroup's GPU memory maximum limit.
+ *
+ * Returns 0 on success.  Otherwise, an error code is returned.
+ */
+int drm_gem_object_charge_mem(struct drm_gem_object *obj, u64 size)
+{
+	int ret;
+
+	ret = drm_cgroup_try_charge(obj->drmcg, obj->dev,
+				    DRMCG_TYPE_MEM_CURRENT, size);
+	if (!ret)
+		atomic64_add(size, &obj->drmcg_bytes_charged);
+	return ret;
+}
+EXPORT_SYMBOL(drm_gem_object_charge_mem);
+
+/**
+ * drm_gem_object_uncharge_mem - uncharge size bytes from DRM cgroup
+ * @obj: GEM object which is being uncharged
+ * @size: number of bytes to uncharge
+ *
+ * Uncharge @size bytes from the DRM cgroup associated with specified
+ * GEM object.
+ *
+ * Returns 0 on success.  Otherwise, an error code is returned.
+ */
+void drm_gem_object_uncharge_mem(struct drm_gem_object *obj, u64 size)
+{
+	u64 charged = atomic64_read(&obj->drmcg_bytes_charged);
+
+	if (WARN_ON(!charged))
+		return;
+	if (WARN_ON(size > charged))
+		size = charged;
+
+	atomic64_sub(size, &obj->drmcg_bytes_charged);
+	drm_cgroup_uncharge(obj->drmcg, obj->dev, DRMCG_TYPE_MEM_CURRENT,
+			    size);
+}
+EXPORT_SYMBOL(drm_gem_object_uncharge_mem);
+
 /**
  * drm_gem_object_init - initialize an allocated shmem-backed GEM object
  * @dev: drm_device the object should be initialized for
@@ -156,6 +240,8 @@ void drm_gem_private_object_init(struct drm_device *dev,
 	obj->dev = dev;
 	obj->filp = NULL;
 
+	drm_gem_object_set_cgroup(obj, current);
+
 	kref_init(&obj->refcount);
 	obj->handle_count = 0;
 	obj->size = size;
@@ -950,6 +1036,9 @@ drm_gem_object_release(struct drm_gem_object *obj)
 
 	dma_resv_fini(&obj->_resv);
 	drm_gem_free_mmap_offset(obj);
+
+	/* Release reference on cgroup used with GEM object charging */
+	drm_gem_object_unset_cgroup(obj);
 }
 EXPORT_SYMBOL(drm_gem_object_release);
 
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 240049566592..06ea10fc17bc 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -37,6 +37,7 @@
 #include <linux/kref.h>
 #include <linux/dma-resv.h>
 
+#include <drm/drm_cgroup.h>
 #include <drm/drm_vma_manager.h>
 
 struct dma_buf_map;
@@ -222,6 +223,17 @@ struct drm_gem_object {
 	 */
 	struct file *filp;
 
+	/**
+	 * @drmcg:
+	 *
+	 * cgroup used for charging GEM object page allocations against. This
+	 * is set to the current cgroup during GEM object creation.
+	 * Charging policy is up to the DRM driver to implement and should be
+	 * charged when allocating backing store from device memory.
+	 */
+	struct drmcg *drmcg;
+	atomic64_t drmcg_bytes_charged;
+
 	/**
 	 * @vma_node:
 	 *
@@ -417,4 +429,9 @@ int drm_gem_fence_array_add_implicit(struct xarray *fence_array,
 int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
 			    u32 handle, u64 *offset);
 
+void drm_gem_object_set_cgroup(struct drm_gem_object *obj,
+			       struct task_struct *task);
+void drm_gem_object_unset_cgroup(struct drm_gem_object *obj);
+int drm_gem_object_charge_mem(struct drm_gem_object *obj, u64 size);
+void drm_gem_object_uncharge_mem(struct drm_gem_object *obj, u64 size);
 #endif /* __DRM_GEM_H__ */
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: Brian Welty <brian.welty@intel.com>
To: "Brian Welty" <brian.welty@intel.com>,
	cgroups@vger.kernel.org, "Tejun Heo" <tj@kernel.org>,
	dri-devel@lists.freedesktop.org,
	"David Airlie" <airlied@linux.ie>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Christian König" <christian.koenig@amd.com>,
	"Kenny Ho" <Kenny.Ho@amd.com>,
	amd-gfx@lists.freedesktop.org,
	"Chris Wilson" <chris@chris-wilson.co.uk>,
	"Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>,
	intel-gfx@lists.freedesktop.org,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Eero Tamminen" <eero.t.tamminen@intel.com>
Subject: [Intel-gfx] [RFC PATCH 8/9] drm/gem: Associate GEM objects with drm cgroup
Date: Tue, 26 Jan 2021 13:46:25 -0800	[thread overview]
Message-ID: <20210126214626.16260-9-brian.welty@intel.com> (raw)
In-Reply-To: <20210126214626.16260-1-brian.welty@intel.com>

This patch adds tracking of which cgroup to make charges against for a
given GEM object.  We associate the current task's cgroup with GEM objects
as they are created.  First user of this is for charging DRM cgroup for
device memory allocations.  The intended behavior is for device drivers to
make the cgroup charging calls at the time that backing store is allocated
or deallocated for the object.

Exported functions are provided for charging memory allocations for a
GEM object to DRM cgroup. To aid in debugging, we store how many bytes
have been charged inside the GEM object.  Add helpers for setting and
clearing the object's associated cgroup which will check that charges are
not being leaked.

For shared objects, this may make the charge against a cgroup that is
potentially not the same cgroup as the process using the memory.  Based
on the memory cgroup's discussion of "memory ownership", this seems
acceptable [1].

[1] https://www.kernel.org/doc/Documentation/cgroup-v2.txt, "Memory Ownership"

Signed-off-by: Brian Welty <brian.welty@intel.com>
---
 drivers/gpu/drm/drm_gem.c | 89 +++++++++++++++++++++++++++++++++++++++
 include/drm/drm_gem.h     | 17 ++++++++
 2 files changed, 106 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index c2ce78c4edc3..a12da41eaafe 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -29,6 +29,7 @@
 #include <linux/slab.h>
 #include <linux/mm.h>
 #include <linux/uaccess.h>
+#include <linux/cgroup_drm.h>
 #include <linux/fs.h>
 #include <linux/file.h>
 #include <linux/module.h>
@@ -112,6 +113,89 @@ drm_gem_init(struct drm_device *dev)
 	return drmm_add_action(dev, drm_gem_init_release, NULL);
 }
 
+/**
+ * drm_gem_object_set_cgroup - associate GEM object with a cgroup
+ * @obj: GEM object which is being associated with a cgroup
+ * @task: task associated with process control group to use
+ *
+ * This will acquire a reference on cgroup and use for charging GEM
+ * memory allocations.
+ * This helper could be extended in future to migrate charges to another
+ * cgroup, print warning if this usage occurs.
+ */
+void drm_gem_object_set_cgroup(struct drm_gem_object *obj,
+			       struct task_struct *task)
+{
+	/* if object has existing cgroup, we migrate the charge... */
+	if (obj->drmcg) {
+		pr_warn("DRM: need to migrate cgroup charge of %lld\n",
+			atomic64_read(&obj->drmcg_bytes_charged));
+	}
+	obj->drmcg = drmcg_get(task);
+}
+EXPORT_SYMBOL(drm_gem_object_set_cgroup);
+
+/**
+ * drm_gem_object_unset_cgroup - clear GEM object's associated cgroup
+ * @obj: GEM object
+ *
+ * This will release a reference on cgroup.
+ */
+void drm_gem_object_unset_cgroup(struct drm_gem_object *obj)
+{
+	WARN_ON(atomic64_read(&obj->drmcg_bytes_charged));
+	drmcg_put(obj->drmcg);
+}
+EXPORT_SYMBOL(drm_gem_object_unset_cgroup);
+
+/**
+ * drm_gem_object_charge_mem - try charging size bytes to DRM cgroup
+ * @obj: GEM object which is being charged
+ * @size: number of bytes to charge
+ *
+ * Try to charge @size bytes to GEM object's associated DRM cgroup.  This
+ * will fail if a successful charge would cause the current device memory
+ * usage to go above the cgroup's GPU memory maximum limit.
+ *
+ * Returns 0 on success.  Otherwise, an error code is returned.
+ */
+int drm_gem_object_charge_mem(struct drm_gem_object *obj, u64 size)
+{
+	int ret;
+
+	ret = drm_cgroup_try_charge(obj->drmcg, obj->dev,
+				    DRMCG_TYPE_MEM_CURRENT, size);
+	if (!ret)
+		atomic64_add(size, &obj->drmcg_bytes_charged);
+	return ret;
+}
+EXPORT_SYMBOL(drm_gem_object_charge_mem);
+
+/**
+ * drm_gem_object_uncharge_mem - uncharge size bytes from DRM cgroup
+ * @obj: GEM object which is being uncharged
+ * @size: number of bytes to uncharge
+ *
+ * Uncharge @size bytes from the DRM cgroup associated with specified
+ * GEM object.
+ *
+ * Returns 0 on success.  Otherwise, an error code is returned.
+ */
+void drm_gem_object_uncharge_mem(struct drm_gem_object *obj, u64 size)
+{
+	u64 charged = atomic64_read(&obj->drmcg_bytes_charged);
+
+	if (WARN_ON(!charged))
+		return;
+	if (WARN_ON(size > charged))
+		size = charged;
+
+	atomic64_sub(size, &obj->drmcg_bytes_charged);
+	drm_cgroup_uncharge(obj->drmcg, obj->dev, DRMCG_TYPE_MEM_CURRENT,
+			    size);
+}
+EXPORT_SYMBOL(drm_gem_object_uncharge_mem);
+
 /**
  * drm_gem_object_init - initialize an allocated shmem-backed GEM object
  * @dev: drm_device the object should be initialized for
@@ -156,6 +240,8 @@ void drm_gem_private_object_init(struct drm_device *dev,
 	obj->dev = dev;
 	obj->filp = NULL;
 
+	drm_gem_object_set_cgroup(obj, current);
+
 	kref_init(&obj->refcount);
 	obj->handle_count = 0;
 	obj->size = size;
@@ -950,6 +1036,9 @@ drm_gem_object_release(struct drm_gem_object *obj)
 
 	dma_resv_fini(&obj->_resv);
 	drm_gem_free_mmap_offset(obj);
+
+	/* Release reference on cgroup used with GEM object charging */
+	drm_gem_object_unset_cgroup(obj);
 }
 EXPORT_SYMBOL(drm_gem_object_release);
 
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 240049566592..06ea10fc17bc 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -37,6 +37,7 @@
 #include <linux/kref.h>
 #include <linux/dma-resv.h>
 
+#include <drm/drm_cgroup.h>
 #include <drm/drm_vma_manager.h>
 
 struct dma_buf_map;
@@ -222,6 +223,17 @@ struct drm_gem_object {
 	 */
 	struct file *filp;
 
+	/**
+	 * @drmcg:
+	 *
+	 * cgroup used for charging GEM object page allocations against. This
+	 * is set to the current cgroup during GEM object creation.
+	 * Charging policy is up to the DRM driver to implement and should be
+	 * charged when allocating backing store from device memory.
+	 */
+	struct drmcg *drmcg;
+	atomic64_t drmcg_bytes_charged;
+
 	/**
 	 * @vma_node:
 	 *
@@ -417,4 +429,9 @@ int drm_gem_fence_array_add_implicit(struct xarray *fence_array,
 int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
 			    u32 handle, u64 *offset);
 
+void drm_gem_object_set_cgroup(struct drm_gem_object *obj,
+			       struct task_struct *task);
+void drm_gem_object_unset_cgroup(struct drm_gem_object *obj);
+int drm_gem_object_charge_mem(struct drm_gem_object *obj, u64 size);
+void drm_gem_object_uncharge_mem(struct drm_gem_object *obj, u64 size);
 #endif /* __DRM_GEM_H__ */
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

WARNING: multiple messages have this Message-ID (diff)
From: Brian Welty <brian.welty@intel.com>
To: "Brian Welty" <brian.welty@intel.com>,
	cgroups@vger.kernel.org, "Tejun Heo" <tj@kernel.org>,
	dri-devel@lists.freedesktop.org,
	"David Airlie" <airlied@linux.ie>,
	"Daniel Vetter" <daniel@ffwll.ch>,
	"Christian König" <christian.koenig@amd.com>,
	"Kenny Ho" <Kenny.Ho@amd.com>,
	amd-gfx@lists.freedesktop.org,
	"Chris Wilson" <chris@chris-wilson.co.uk>,
	"Tvrtko Ursulin" <tvrtko.ursulin@linux.intel.com>,
	intel-gfx@lists.freedesktop.org,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Eero Tamminen" <eero.t.tamminen@intel.com>
Subject: [RFC PATCH 8/9] drm/gem: Associate GEM objects with drm cgroup
Date: Tue, 26 Jan 2021 13:46:25 -0800	[thread overview]
Message-ID: <20210126214626.16260-9-brian.welty@intel.com> (raw)
In-Reply-To: <20210126214626.16260-1-brian.welty@intel.com>

This patch adds tracking of which cgroup to make charges against for a
given GEM object.  We associate the current task's cgroup with GEM objects
as they are created.  First user of this is for charging DRM cgroup for
device memory allocations.  The intended behavior is for device drivers to
make the cgroup charging calls at the time that backing store is allocated
or deallocated for the object.

Exported functions are provided for charging memory allocations for a
GEM object to DRM cgroup. To aid in debugging, we store how many bytes
have been charged inside the GEM object.  Add helpers for setting and
clearing the object's associated cgroup which will check that charges are
not being leaked.

For shared objects, this may make the charge against a cgroup that is
potentially not the same cgroup as the process using the memory.  Based
on the memory cgroup's discussion of "memory ownership", this seems
acceptable [1].

[1] https://www.kernel.org/doc/Documentation/cgroup-v2.txt, "Memory Ownership"

Signed-off-by: Brian Welty <brian.welty@intel.com>
---
 drivers/gpu/drm/drm_gem.c | 89 +++++++++++++++++++++++++++++++++++++++
 include/drm/drm_gem.h     | 17 ++++++++
 2 files changed, 106 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index c2ce78c4edc3..a12da41eaafe 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -29,6 +29,7 @@
 #include <linux/slab.h>
 #include <linux/mm.h>
 #include <linux/uaccess.h>
+#include <linux/cgroup_drm.h>
 #include <linux/fs.h>
 #include <linux/file.h>
 #include <linux/module.h>
@@ -112,6 +113,89 @@ drm_gem_init(struct drm_device *dev)
 	return drmm_add_action(dev, drm_gem_init_release, NULL);
 }
 
+/**
+ * drm_gem_object_set_cgroup - associate GEM object with a cgroup
+ * @obj: GEM object which is being associated with a cgroup
+ * @task: task associated with process control group to use
+ *
+ * This will acquire a reference on cgroup and use for charging GEM
+ * memory allocations.
+ * This helper could be extended in future to migrate charges to another
+ * cgroup, print warning if this usage occurs.
+ */
+void drm_gem_object_set_cgroup(struct drm_gem_object *obj,
+			       struct task_struct *task)
+{
+	/* if object has existing cgroup, we migrate the charge... */
+	if (obj->drmcg) {
+		pr_warn("DRM: need to migrate cgroup charge of %lld\n",
+			atomic64_read(&obj->drmcg_bytes_charged));
+	}
+	obj->drmcg = drmcg_get(task);
+}
+EXPORT_SYMBOL(drm_gem_object_set_cgroup);
+
+/**
+ * drm_gem_object_unset_cgroup - clear GEM object's associated cgroup
+ * @obj: GEM object
+ *
+ * This will release a reference on cgroup.
+ */
+void drm_gem_object_unset_cgroup(struct drm_gem_object *obj)
+{
+	WARN_ON(atomic64_read(&obj->drmcg_bytes_charged));
+	drmcg_put(obj->drmcg);
+}
+EXPORT_SYMBOL(drm_gem_object_unset_cgroup);
+
+/**
+ * drm_gem_object_charge_mem - try charging size bytes to DRM cgroup
+ * @obj: GEM object which is being charged
+ * @size: number of bytes to charge
+ *
+ * Try to charge @size bytes to GEM object's associated DRM cgroup.  This
+ * will fail if a successful charge would cause the current device memory
+ * usage to go above the cgroup's GPU memory maximum limit.
+ *
+ * Returns 0 on success.  Otherwise, an error code is returned.
+ */
+int drm_gem_object_charge_mem(struct drm_gem_object *obj, u64 size)
+{
+	int ret;
+
+	ret = drm_cgroup_try_charge(obj->drmcg, obj->dev,
+				    DRMCG_TYPE_MEM_CURRENT, size);
+	if (!ret)
+		atomic64_add(size, &obj->drmcg_bytes_charged);
+	return ret;
+}
+EXPORT_SYMBOL(drm_gem_object_charge_mem);
+
+/**
+ * drm_gem_object_uncharge_mem - uncharge size bytes from DRM cgroup
+ * @obj: GEM object which is being uncharged
+ * @size: number of bytes to uncharge
+ *
+ * Uncharge @size bytes from the DRM cgroup associated with specified
+ * GEM object.
+ *
+ * Returns 0 on success.  Otherwise, an error code is returned.
+ */
+void drm_gem_object_uncharge_mem(struct drm_gem_object *obj, u64 size)
+{
+	u64 charged = atomic64_read(&obj->drmcg_bytes_charged);
+
+	if (WARN_ON(!charged))
+		return;
+	if (WARN_ON(size > charged))
+		size = charged;
+
+	atomic64_sub(size, &obj->drmcg_bytes_charged);
+	drm_cgroup_uncharge(obj->drmcg, obj->dev, DRMCG_TYPE_MEM_CURRENT,
+			    size);
+}
+EXPORT_SYMBOL(drm_gem_object_uncharge_mem);
+
 /**
  * drm_gem_object_init - initialize an allocated shmem-backed GEM object
  * @dev: drm_device the object should be initialized for
@@ -156,6 +240,8 @@ void drm_gem_private_object_init(struct drm_device *dev,
 	obj->dev = dev;
 	obj->filp = NULL;
 
+	drm_gem_object_set_cgroup(obj, current);
+
 	kref_init(&obj->refcount);
 	obj->handle_count = 0;
 	obj->size = size;
@@ -950,6 +1036,9 @@ drm_gem_object_release(struct drm_gem_object *obj)
 
 	dma_resv_fini(&obj->_resv);
 	drm_gem_free_mmap_offset(obj);
+
+	/* Release reference on cgroup used with GEM object charging */
+	drm_gem_object_unset_cgroup(obj);
 }
 EXPORT_SYMBOL(drm_gem_object_release);
 
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 240049566592..06ea10fc17bc 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -37,6 +37,7 @@
 #include <linux/kref.h>
 #include <linux/dma-resv.h>
 
+#include <drm/drm_cgroup.h>
 #include <drm/drm_vma_manager.h>
 
 struct dma_buf_map;
@@ -222,6 +223,17 @@ struct drm_gem_object {
 	 */
 	struct file *filp;
 
+	/**
+	 * @drmcg:
+	 *
+	 * cgroup used for charging GEM object page allocations against. This
+	 * is set to the current cgroup during GEM object creation.
+	 * Charging policy is up to the DRM driver to implement and should be
+	 * charged when allocating backing store from device memory.
+	 */
+	struct drmcg *drmcg;
+	atomic64_t drmcg_bytes_charged;
+
 	/**
 	 * @vma_node:
 	 *
@@ -417,4 +429,9 @@ int drm_gem_fence_array_add_implicit(struct xarray *fence_array,
 int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
 			    u32 handle, u64 *offset);
 
+void drm_gem_object_set_cgroup(struct drm_gem_object *obj,
+			       struct task_struct *task);
+void drm_gem_object_unset_cgroup(struct drm_gem_object *obj);
+int drm_gem_object_charge_mem(struct drm_gem_object *obj, u64 size);
+void drm_gem_object_uncharge_mem(struct drm_gem_object *obj, u64 size);
 #endif /* __DRM_GEM_H__ */
-- 
2.20.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

WARNING: multiple messages have this Message-ID (diff)
From: Brian Welty <brian.welty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
To: "Brian Welty"
	<brian.welty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	"Tejun Heo" <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	"David Airlie" <airlied-cv59FeDIM0c@public.gmane.org>,
	"Daniel Vetter" <daniel-/w4YWyX8dFk@public.gmane.org>,
	"Christian König" <christian.koenig-5C7GfCeVMHo@public.gmane.org>,
	"Kenny Ho" <Kenny.Ho-5C7GfCeVMHo@public.gmane.org>,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	"Chris Wilson"
	<chris-Y6uKTt2uX1cEflXRtASbqLVCufUGDwFn@public.gmane.org>,
	"Tvrtko Ursulin"
	<tvrtko.ursulin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
	intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	"Joonas Lahtinen"
	<joonas.lahtinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
	"Eero Tamminen"
	<eero.t.tamminen-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Subject: [RFC PATCH 8/9] drm/gem: Associate GEM objects with drm cgroup
Date: Tue, 26 Jan 2021 13:46:25 -0800	[thread overview]
Message-ID: <20210126214626.16260-9-brian.welty@intel.com> (raw)
In-Reply-To: <20210126214626.16260-1-brian.welty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

This patch adds tracking of which cgroup to make charges against for a
given GEM object.  We associate the current task's cgroup with GEM objects
as they are created.  First user of this is for charging DRM cgroup for
device memory allocations.  The intended behavior is for device drivers to
make the cgroup charging calls at the time that backing store is allocated
or deallocated for the object.

Exported functions are provided for charging memory allocations for a
GEM object to DRM cgroup. To aid in debugging, we store how many bytes
have been charged inside the GEM object.  Add helpers for setting and
clearing the object's associated cgroup which will check that charges are
not being leaked.

For shared objects, this may make the charge against a cgroup that is
potentially not the same cgroup as the process using the memory.  Based
on the memory cgroup's discussion of "memory ownership", this seems
acceptable [1].

[1] https://www.kernel.org/doc/Documentation/cgroup-v2.txt, "Memory Ownership"

Signed-off-by: Brian Welty <brian.welty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/drm_gem.c | 89 +++++++++++++++++++++++++++++++++++++++
 include/drm/drm_gem.h     | 17 ++++++++
 2 files changed, 106 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index c2ce78c4edc3..a12da41eaafe 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -29,6 +29,7 @@
 #include <linux/slab.h>
 #include <linux/mm.h>
 #include <linux/uaccess.h>
+#include <linux/cgroup_drm.h>
 #include <linux/fs.h>
 #include <linux/file.h>
 #include <linux/module.h>
@@ -112,6 +113,89 @@ drm_gem_init(struct drm_device *dev)
 	return drmm_add_action(dev, drm_gem_init_release, NULL);
 }
 
+/**
+ * drm_gem_object_set_cgroup - associate GEM object with a cgroup
+ * @obj: GEM object which is being associated with a cgroup
+ * @task: task associated with process control group to use
+ *
+ * This will acquire a reference on cgroup and use for charging GEM
+ * memory allocations.
+ * This helper could be extended in future to migrate charges to another
+ * cgroup, print warning if this usage occurs.
+ */
+void drm_gem_object_set_cgroup(struct drm_gem_object *obj,
+			       struct task_struct *task)
+{
+	/* if object has existing cgroup, we migrate the charge... */
+	if (obj->drmcg) {
+		pr_warn("DRM: need to migrate cgroup charge of %lld\n",
+			atomic64_read(&obj->drmcg_bytes_charged));
+	}
+	obj->drmcg = drmcg_get(task);
+}
+EXPORT_SYMBOL(drm_gem_object_set_cgroup);
+
+/**
+ * drm_gem_object_unset_cgroup - clear GEM object's associated cgroup
+ * @obj: GEM object
+ *
+ * This will release a reference on cgroup.
+ */
+void drm_gem_object_unset_cgroup(struct drm_gem_object *obj)
+{
+	WARN_ON(atomic64_read(&obj->drmcg_bytes_charged));
+	drmcg_put(obj->drmcg);
+}
+EXPORT_SYMBOL(drm_gem_object_unset_cgroup);
+
+/**
+ * drm_gem_object_charge_mem - try charging size bytes to DRM cgroup
+ * @obj: GEM object which is being charged
+ * @size: number of bytes to charge
+ *
+ * Try to charge @size bytes to GEM object's associated DRM cgroup.  This
+ * will fail if a successful charge would cause the current device memory
+ * usage to go above the cgroup's GPU memory maximum limit.
+ *
+ * Returns 0 on success.  Otherwise, an error code is returned.
+ */
+int drm_gem_object_charge_mem(struct drm_gem_object *obj, u64 size)
+{
+	int ret;
+
+	ret = drm_cgroup_try_charge(obj->drmcg, obj->dev,
+				    DRMCG_TYPE_MEM_CURRENT, size);
+	if (!ret)
+		atomic64_add(size, &obj->drmcg_bytes_charged);
+	return ret;
+}
+EXPORT_SYMBOL(drm_gem_object_charge_mem);
+
+/**
+ * drm_gem_object_uncharge_mem - uncharge size bytes from DRM cgroup
+ * @obj: GEM object which is being uncharged
+ * @size: number of bytes to uncharge
+ *
+ * Uncharge @size bytes from the DRM cgroup associated with specified
+ * GEM object.
+ *
+ * Returns 0 on success.  Otherwise, an error code is returned.
+ */
+void drm_gem_object_uncharge_mem(struct drm_gem_object *obj, u64 size)
+{
+	u64 charged = atomic64_read(&obj->drmcg_bytes_charged);
+
+	if (WARN_ON(!charged))
+		return;
+	if (WARN_ON(size > charged))
+		size = charged;
+
+	atomic64_sub(size, &obj->drmcg_bytes_charged);
+	drm_cgroup_uncharge(obj->drmcg, obj->dev, DRMCG_TYPE_MEM_CURRENT,
+			    size);
+}
+EXPORT_SYMBOL(drm_gem_object_uncharge_mem);
+
 /**
  * drm_gem_object_init - initialize an allocated shmem-backed GEM object
  * @dev: drm_device the object should be initialized for
@@ -156,6 +240,8 @@ void drm_gem_private_object_init(struct drm_device *dev,
 	obj->dev = dev;
 	obj->filp = NULL;
 
+	drm_gem_object_set_cgroup(obj, current);
+
 	kref_init(&obj->refcount);
 	obj->handle_count = 0;
 	obj->size = size;
@@ -950,6 +1036,9 @@ drm_gem_object_release(struct drm_gem_object *obj)
 
 	dma_resv_fini(&obj->_resv);
 	drm_gem_free_mmap_offset(obj);
+
+	/* Release reference on cgroup used with GEM object charging */
+	drm_gem_object_unset_cgroup(obj);
 }
 EXPORT_SYMBOL(drm_gem_object_release);
 
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 240049566592..06ea10fc17bc 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -37,6 +37,7 @@
 #include <linux/kref.h>
 #include <linux/dma-resv.h>
 
+#include <drm/drm_cgroup.h>
 #include <drm/drm_vma_manager.h>
 
 struct dma_buf_map;
@@ -222,6 +223,17 @@ struct drm_gem_object {
 	 */
 	struct file *filp;
 
+	/**
+	 * @drmcg:
+	 *
+	 * cgroup used for charging GEM object page allocations against. This
+	 * is set to the current cgroup during GEM object creation.
+	 * Charging policy is up to the DRM driver to implement and should be
+	 * charged when allocating backing store from device memory.
+	 */
+	struct drmcg *drmcg;
+	atomic64_t drmcg_bytes_charged;
+
 	/**
 	 * @vma_node:
 	 *
@@ -417,4 +429,9 @@ int drm_gem_fence_array_add_implicit(struct xarray *fence_array,
 int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
 			    u32 handle, u64 *offset);
 
+void drm_gem_object_set_cgroup(struct drm_gem_object *obj,
+			       struct task_struct *task);
+void drm_gem_object_unset_cgroup(struct drm_gem_object *obj);
+int drm_gem_object_charge_mem(struct drm_gem_object *obj, u64 size);
+void drm_gem_object_uncharge_mem(struct drm_gem_object *obj, u64 size);
 #endif /* __DRM_GEM_H__ */
-- 
2.20.1

  parent reply	other threads:[~2021-01-26 21:45 UTC|newest]

Thread overview: 110+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-26 21:46 [RFC PATCH 0/9] cgroup support for GPU devices Brian Welty
2021-01-26 21:46 ` Brian Welty
2021-01-26 21:46 ` Brian Welty
2021-01-26 21:46 ` [Intel-gfx] " Brian Welty
2021-01-26 21:46 ` [RFC PATCH 1/9] cgroup: Introduce cgroup for drm subsystem Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` [Intel-gfx] " Brian Welty
2021-01-26 21:46 ` [RFC PATCH 2/9] drm, cgroup: Bind drm and cgroup subsystem Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` [Intel-gfx] " Brian Welty
2021-01-26 21:46 ` [RFC PATCH 3/9] drm, cgroup: Initialize drmcg properties Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` [Intel-gfx] " Brian Welty
2021-01-26 21:46 ` [RFC PATCH 4/9] drmcg: Add skeleton seq_show and write for drmcg files Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` [Intel-gfx] " Brian Welty
2021-01-26 21:46 ` [RFC PATCH 5/9] drmcg: Add support for device memory accounting via page counter Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` [Intel-gfx] " Brian Welty
2021-01-27  9:14   ` kernel test robot
2021-01-26 21:46 ` [RFC PATCH 6/9] drmcg: Add memory.total file Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` [Intel-gfx] " Brian Welty
2021-01-26 21:46 ` [RFC PATCH 7/9] drmcg: Add initial support for tracking gpu time usage Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` [Intel-gfx] " Brian Welty
2021-02-03 13:25   ` Joonas Lahtinen
2021-02-03 13:25     ` Joonas Lahtinen
2021-02-03 13:25     ` [Intel-gfx] " Joonas Lahtinen
2021-02-04  2:23     ` Brian Welty
2021-02-04  2:23       ` Brian Welty
2021-02-04  2:23       ` Brian Welty
2021-02-04  2:23       ` [Intel-gfx] " Brian Welty
2021-01-26 21:46 ` Brian Welty [this message]
2021-01-26 21:46   ` [RFC PATCH 8/9] drm/gem: Associate GEM objects with drm cgroup Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` [Intel-gfx] " Brian Welty
2021-02-09 10:54   ` Daniel Vetter
2021-02-09 10:54     ` Daniel Vetter
2021-02-09 10:54     ` Daniel Vetter
2021-02-09 10:54     ` [Intel-gfx] " Daniel Vetter
2021-02-10  7:52     ` Thomas Zimmermann
2021-02-10  7:52       ` Thomas Zimmermann
2021-02-10  7:52       ` Thomas Zimmermann
2021-02-10  7:52       ` [Intel-gfx] " Thomas Zimmermann
2021-02-10 12:45       ` Daniel Vetter
2021-02-10 12:45         ` Daniel Vetter
2021-02-10 12:45         ` Daniel Vetter
2021-02-10 12:45         ` [Intel-gfx] " Daniel Vetter
2021-02-10 22:00     ` Brian Welty
2021-02-10 22:00       ` Brian Welty
2021-02-10 22:00       ` Brian Welty
2021-02-10 22:00       ` [Intel-gfx] " Brian Welty
2021-02-11 15:34       ` Daniel Vetter
2021-02-11 15:34         ` Daniel Vetter
2021-02-11 15:34         ` Daniel Vetter
2021-02-11 15:34         ` [Intel-gfx] " Daniel Vetter
2021-03-06  0:44         ` Brian Welty
2021-03-06  0:44           ` Brian Welty
2021-03-06  0:44           ` Brian Welty
2021-03-06  0:44           ` [Intel-gfx] " Brian Welty
2021-03-18 10:16           ` Daniel Vetter
2021-03-18 10:16             ` Daniel Vetter
2021-03-18 10:16             ` Daniel Vetter
2021-03-18 10:16             ` [Intel-gfx] " Daniel Vetter
2021-03-18 19:20             ` Brian Welty
2021-03-18 19:20               ` Brian Welty
2021-03-18 19:20               ` Brian Welty
2021-03-18 19:20               ` [Intel-gfx] " Brian Welty
2021-05-10 15:36               ` Daniel Vetter
2021-05-10 15:36                 ` Daniel Vetter
2021-05-10 15:36                 ` Daniel Vetter
2021-05-10 15:36                 ` [Intel-gfx] " Daniel Vetter
2021-05-10 16:06                 ` Tamminen, Eero T
2021-05-10 16:06                   ` Tamminen, Eero T
2021-05-10 16:06                   ` Tamminen, Eero T
2021-05-10 16:06                   ` [Intel-gfx] " Tamminen, Eero T
2021-01-26 21:46 ` [RFC PATCH 9/9] drm/i915: Use memory cgroup for enforcing device memory limit Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` Brian Welty
2021-01-26 21:46   ` [Intel-gfx] " Brian Welty
2021-01-27  6:04   ` kernel test robot
2021-01-26 22:37 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for cgroup support for GPU devices (rev3) Patchwork
2021-01-26 22:40 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2021-01-26 23:07 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2021-01-26 23:07 ` [Intel-gfx] ✗ Fi.CI.BUILD: warning " Patchwork
2021-01-27  4:55 ` [Intel-gfx] ✓ Fi.CI.IGT: success " Patchwork
2021-01-29  2:45 ` [RFC PATCH 0/9] cgroup support for GPU devices Xingyou Chen
2021-01-29  2:45   ` Xingyou Chen
2021-01-29  2:45   ` Xingyou Chen
2021-01-29  2:45   ` [Intel-gfx] " Xingyou Chen
2021-01-29  3:00 ` Xingyou Chen
2021-01-29  3:00   ` Xingyou Chen
2021-01-29  3:00   ` Xingyou Chen
2021-01-29  3:00   ` [Intel-gfx] " Xingyou Chen
2021-02-01 23:21   ` Brian Welty
2021-02-01 23:21     ` Brian Welty
2021-02-01 23:21     ` Brian Welty
2021-02-01 23:21     ` [Intel-gfx] " Brian Welty
2021-02-03 10:18     ` Daniel Vetter
2021-02-03 10:18       ` Daniel Vetter
2021-02-03 10:18       ` Daniel Vetter
2021-02-03 10:18       ` [Intel-gfx] " Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210126214626.16260-9-brian.welty@intel.com \
    --to=brian.welty@intel.com \
    --cc=Kenny.Ho@amd.com \
    --cc=airlied@linux.ie \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=cgroups@vger.kernel.org \
    --cc=chris@chris-wilson.co.uk \
    --cc=christian.koenig@amd.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=eero.t.tamminen@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=tj@kernel.org \
    --cc=tvrtko.ursulin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.