* [PATCH v2 1/7] kernel/cgroup: Add "dmem" memory accounting cgroup
2024-12-04 13:44 [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup Maarten Lankhorst
@ 2024-12-04 13:44 ` Maarten Lankhorst
2024-12-04 14:31 ` [PATCH v2.1 1/1] " Maarten Lankhorst
2025-01-06 17:48 ` [PATCH v2 1/7] " Michal Koutný
2024-12-04 13:44 ` [PATCH v2 2/7] drm/drv: Add drmm managed registration helper for dmem cgroups Maarten Lankhorst
` (7 subsequent siblings)
8 siblings, 2 replies; 35+ messages in thread
From: Maarten Lankhorst @ 2024-12-04 13:44 UTC (permalink / raw)
To: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, Maxime Ripard
Cc: cgroups, linux-mm, Maarten Lankhorst, Maarten Lankhorst
This code is based on the RDMA and misc cgroup initially, but now
uses page_counter. It uses the same min/low/max semantics as the memory
cgroup as a result.
There's a small mismatch as TTM uses u64, and page_counter long pages.
In practice it's not a problem. 32-bits systems don't really come with
>=4GB cards and as long as we're consistently wrong with units, it's
fine. The device page size may not be in the same units as kernel page
size, and each region might also have a different page size (VRAM vs GART
for example).
The interface is simple:
- Call dmem_cgroup_register_region()
- Use dmem_cgroup_try_charge to check if you can allocate a chunk of memory,
use dmem_cgroup__uncharge when freeing it. This may return an error code,
or -EAGAIN when the cgroup limit is reached. In that case a reference
to the limiting pool is returned.
- The limiting cs can be used as compare function for
dmem_cgroup_state_evict_valuable.
- After having evicted enough, drop reference to limiting cs with
dmem_cgroup_pool_state_put.
This API allows you to limit device resources with cgroups.
You can see the supported cards in /sys/fs/cgroup/dmem.capacity
You need to echo +dmem to cgroup.subtree_control, and then you can
partition device memory.
Co-developed-by: Friedrich Vock <friedrich.vock@gmx.de>
Signed-off-by: Friedrich Vock <friedrich.vock@gmx.de>
Co-developed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
---
Documentation/admin-guide/cgroup-v2.rst | 58 +-
Documentation/core-api/cgroup.rst | 9 +
Documentation/core-api/index.rst | 1 +
Documentation/gpu/drm-compute.rst | 54 ++
include/linux/cgroup_dmem.h | 67 ++
include/linux/cgroup_subsys.h | 4 +
include/linux/page_counter.h | 2 +-
init/Kconfig | 10 +
kernel/cgroup/Makefile | 1 +
kernel/cgroup/dmem.c | 861 ++++++++++++++++++++++++
mm/page_counter.c | 4 +-
11 files changed, 1061 insertions(+), 10 deletions(-)
create mode 100644 Documentation/core-api/cgroup.rst
create mode 100644 Documentation/gpu/drm-compute.rst
create mode 100644 include/linux/cgroup_dmem.h
create mode 100644 kernel/cgroup/dmem.c
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 315ede811c9d0..cb1b4e759b7e2 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -64,13 +64,14 @@ v1 is available under :ref:`Documentation/admin-guide/cgroup-v1/index.rst <cgrou
5-6. Device
5-7. RDMA
5-7-1. RDMA Interface Files
- 5-8. HugeTLB
- 5.8-1. HugeTLB Interface Files
- 5-9. Misc
- 5.9-1 Miscellaneous cgroup Interface Files
- 5.9-2 Migration and Ownership
- 5-10. Others
- 5-10-1. perf_event
+ 5-8. DMEM
+ 5-9. HugeTLB
+ 5.9-1. HugeTLB Interface Files
+ 5-10. Misc
+ 5.10-1 Miscellaneous cgroup Interface Files
+ 5.10-2 Migration and Ownership
+ 5-11. Others
+ 5-11-1. perf_event
5-N. Non-normative information
5-N-1. CPU controller root cgroup process behaviour
5-N-2. IO controller root cgroup process behaviour
@@ -2626,6 +2627,49 @@ RDMA Interface Files
mlx4_0 hca_handle=1 hca_object=20
ocrdma1 hca_handle=1 hca_object=23
+DMEM
+----
+
+The "dmem" controller regulates the distribution and accounting of
+device memory regions. Because each memory region may have its own page size,
+which does not have to be equal to the system page size, the units are always bytes.
+
+DMEM Interface Files
+~~~~~~~~~~~~~~~~~~~~
+
+ dmem.max, dmem.min, dmem.low
+ A readwrite nested-keyed file that exists for all the cgroups
+ except root that describes current configured resource limit
+ for a region.
+
+ An example for xe follows::
+
+ drm/0000:03:00.0/vram0 1073741824
+ drm/0000:03:00.0/stolen max
+
+ The semantics are the same as for the memory cgroup controller, and are
+ calculated in the same way.
+
+ dmem.capacity
+ A read-only file that describes maximum region capacity.
+ It only exists on the root cgroup. Not all memory can be
+ allocated by cgroups, as the kernel reserves some for
+ internal use.
+
+ An example for xe follows::
+
+ drm/0000:03:00.0/vram0 8514437120
+ drm/0000:03:00.0/stolen 67108864
+
+ dmem.current
+ A read-only file that describes current resource usage.
+ It exists for all the cgroup except root.
+
+ An example for xe follows::
+
+ drm/0000:03:00.0/vram0 12550144
+ drm/0000:03:00.0/stolen 8650752
+
HugeTLB
-------
diff --git a/Documentation/core-api/cgroup.rst b/Documentation/core-api/cgroup.rst
new file mode 100644
index 0000000000000..8696e9513f511
--- /dev/null
+++ b/Documentation/core-api/cgroup.rst
@@ -0,0 +1,9 @@
+==================
+Cgroup Kernel APIs
+==================
+
+Device Memory Cgroup API (dmemcg)
+=========================
+.. kernel-doc:: kernel/cgroup/dmem.c
+ :export:
+
diff --git a/Documentation/core-api/index.rst b/Documentation/core-api/index.rst
index 563b8fc0002f7..913d91feaf760 100644
--- a/Documentation/core-api/index.rst
+++ b/Documentation/core-api/index.rst
@@ -109,6 +109,7 @@ more memory-management documentation in Documentation/mm/index.rst.
dma-isa-lpc
swiotlb
mm-api
+ cgroup
genalloc
pin_user_pages
boot-time-mm
diff --git a/Documentation/gpu/drm-compute.rst b/Documentation/gpu/drm-compute.rst
new file mode 100644
index 0000000000000..f90c3e63aa9e5
--- /dev/null
+++ b/Documentation/gpu/drm-compute.rst
@@ -0,0 +1,54 @@
+==================================
+Long running workloads and compute
+==================================
+
+Long running workloads (compute) are workloads that will not complete in 10
+seconds. (The time let the user wait before he reaches for the power button).
+This means that other techniques need to be used to manage those workloads,
+that cannot use fences.
+
+Some hardware may schedule compute jobs, and have no way to pre-empt them, or
+have their memory swapped out from them. Or they simply want their workload
+not to be preempted or swapped out at all.
+
+This means that it differs from what is described in driver-api/dma-buf.rst.
+
+As with normal compute jobs, dma-fence may not be used at all. In this case,
+not even to force preemption. The driver with is simply forced to unmap a BO
+from the long compute job's address space on unbind immediately, not even
+waiting for the workload to complete. Effectively this terminates the workload
+when there is no hardware support to recover.
+
+Since this is undesirable, there need to be mitigations to prevent a workload
+from being terminated. There are several possible approach, all with their
+advantages and drawbacks.
+
+The first approach you will likely try is to pin all buffers used by compute.
+This guarantees that the job will run uninterrupted, but also allows a very
+denial of service attack by pinning as much memory as possible, hogging the
+all GPU memory, and possibly a huge chunk of CPU memory.
+
+A second approach that will work slightly better on its own is adding an option
+not to evict when creating a new job (any kind). If all of userspace opts in
+to this flag, it would prevent cooperating userspace from forced terminating
+older compute jobs to start a new one.
+
+If job preemption and recoverable pagefaults are not available, those are the
+only approaches possible. So even with those, you want a separate way of
+controlling resources. The standard kernel way of doing so is cgroups.
+
+This creates a third option, using cgroups to prevent eviction. Both GPU and
+driver-allocated CPU memory would be accounted to the correct cgroup, and
+eviction would be made cgroup aware. This allows the GPU to be partitioned
+into cgroups, that will allow jobs to run next to each other without
+interference.
+
+The interface to the cgroup would be similar to the current CPU memory
+interface, with similar semantics for min/low/high/max, if eviction can
+be made cgroup aware.
+
+What should be noted is that each memory region (tiled memory for example)
+should have its own accounting.
+
+The key is set to the regionid set by the driver, for example "tile0".
+For the value of $card, we use drmGetUnique().
diff --git a/include/linux/cgroup_dmem.h b/include/linux/cgroup_dmem.h
new file mode 100644
index 0000000000000..48cb11cd40e9e
--- /dev/null
+++ b/include/linux/cgroup_dmem.h
@@ -0,0 +1,67 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2023-2024 Intel Corporation
+ */
+
+#ifndef _CGROUP_DMEM_H
+#define _CGROUP_DMEM_H
+
+#include <linux/types.h>
+#include <linux/llist.h>
+
+struct dmem_cgroup_pool_state;
+
+/* Opaque definition of a cgroup region, used internally */
+struct dmem_cgroup_region;
+
+#if IS_ENABLED(CONFIG_CGROUP_DMEM)
+struct dmem_cgroup_region *dmem_cgroup_register_region(u64 size, const char *name_fmt, ...) __printf(2,3);
+void dmem_cgroup_unregister_region(struct dmem_cgroup_region *region);
+int dmem_cgroup_try_charge(struct dmem_cgroup_region *region, u64 size,
+ struct dmem_cgroup_pool_state **ret_pool,
+ struct dmem_cgroup_pool_state **ret_limit_pool);
+void dmem_cgroup_uncharge(struct dmem_cgroup_pool_state *pool, u64 size);
+bool dmem_cgroup_state_evict_valuable(struct dmem_cgroup_pool_state *limit_pool,
+ struct dmem_cgroup_pool_state *test_pool,
+ bool ignore_low, bool *ret_hit_low);
+
+void dmem_cgroup_pool_state_put(struct dmem_cgroup_pool_state *pool);
+#else
+static inline struct dmem_cgroup_region *
+dmem_cgroup_register_region(u64 size, const char *name_fmt, ...) __printf(2,3);
+{
+ return NULL;
+}
+
+static inline void dmem_cgroup_unregister_region(struct dmem_cgroup_region *cgdev)
+{ }
+
+static int dmem_cgroup_try_charge(struct dmem_cgroup_device *cgdev, u64 size,
+ struct dmem_cgroup_pool_state **ret_pool,
+ struct dmem_cgroup_pool_state **ret_limit_pool);
+{
+ *ret_pool = NULL;
+
+ if (ret_limit_pool)
+ *ret_limit_pool = NULL;
+
+ return 0;
+}
+
+static inline void dmem_cgroup_uncharge(struct dmem_cgroup_pool_state *pool, u64 size)
+{ }
+
+static inline
+bool dmem_cgroup_state_evict_valuable(struct dmem_cgroup_device *region,
+ struct dmem_cgroup_pool_state *limit_pool,
+ struct dmem_cgroup_pool_state *test_pool,
+ bool ignore_low, bool *ret_hit_low)
+{
+ return true;
+}
+
+static inline void dmem_cgroup_pool_state_put(struct dmem_cgroup_pool_state *pool)
+{ }
+
+#endif
+#endif /* _CGROUP_DMEM_H */
diff --git a/include/linux/cgroup_subsys.h b/include/linux/cgroup_subsys.h
index 4452354872307..3fd0bcbf30803 100644
--- a/include/linux/cgroup_subsys.h
+++ b/include/linux/cgroup_subsys.h
@@ -65,6 +65,10 @@ SUBSYS(rdma)
SUBSYS(misc)
#endif
+#if IS_ENABLED(CONFIG_CGROUP_DMEM)
+SUBSYS(dmem)
+#endif
+
/*
* The following subsystems are not supported on the default hierarchy.
*/
diff --git a/include/linux/page_counter.h b/include/linux/page_counter.h
index 79dbd8bc35a72..46406f3fe34d0 100644
--- a/include/linux/page_counter.h
+++ b/include/linux/page_counter.h
@@ -96,7 +96,7 @@ static inline void page_counter_reset_watermark(struct page_counter *counter)
counter->watermark = usage;
}
-#ifdef CONFIG_MEMCG
+#if IS_ENABLED(CONFIG_MEMCG) || IS_ENABLED(CONFIG_CGROUP_DMEM)
void page_counter_calculate_protection(struct page_counter *root,
struct page_counter *counter,
bool recursive_protection);
diff --git a/init/Kconfig b/init/Kconfig
index a20e6efd3f0fb..61f50cafa8151 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1128,6 +1128,7 @@ config CGROUP_PIDS
config CGROUP_RDMA
bool "RDMA controller"
+ select PAGE_COUNTER
help
Provides enforcement of RDMA resources defined by IB stack.
It is fairly easy for consumers to exhaust RDMA resources, which
@@ -1136,6 +1137,15 @@ config CGROUP_RDMA
Attaching processes with active RDMA resources to the cgroup
hierarchy is allowed even if can cross the hierarchy's limit.
+config CGROUP_DMEM
+ bool "Device memory controller (DMEM)"
+ help
+ The DMEM controller allows compatible devices to restrict device
+ memory usage based on the cgroup hierarchy.
+
+ As an example, it allows you to restrict VRAM usage for applications
+ in the DRM subsystem.
+
config CGROUP_FREEZER
bool "Freezer controller"
help
diff --git a/kernel/cgroup/Makefile b/kernel/cgroup/Makefile
index a5c9359d516f8..ede31601a363a 100644
--- a/kernel/cgroup/Makefile
+++ b/kernel/cgroup/Makefile
@@ -7,4 +7,5 @@ obj-$(CONFIG_CGROUP_RDMA) += rdma.o
obj-$(CONFIG_CPUSETS) += cpuset.o
obj-$(CONFIG_CPUSETS_V1) += cpuset-v1.o
obj-$(CONFIG_CGROUP_MISC) += misc.o
+obj-$(CONFIG_CGROUP_DMEM) += dmem.o
obj-$(CONFIG_CGROUP_DEBUG) += debug.o
diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c
new file mode 100644
index 0000000000000..52736ef0ccf25
--- /dev/null
+++ b/kernel/cgroup/dmem.c
@@ -0,0 +1,861 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2023-2024 Intel Corporation (Maarten Lankhorst <dev@lankhorst.se>)
+ * Copyright 2024 Red Hat (Maxime Ripard <mripard@kernel.org>)
+ * Partially based on the rdma and misc controllers, which bear the following copyrights:
+ *
+ * Copyright 2020 Google LLC
+ * Copyright (C) 2016 Parav Pandit <pandit.parav@gmail.com>
+ */
+
+#include <linux/cgroup.h>
+#include <linux/cgroup_dmem.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/page_counter.h>
+#include <linux/parser.h>
+#include <linux/slab.h>
+
+struct dmem_cgroup_region {
+ /**
+ * @ref: References keeping the region alive.
+ * Keeps the region reference alive after a succesful RCU lookup.
+ */
+ struct kref ref;
+
+ /** @rcu: RCU head for freeing */
+ struct rcu_head rcu;
+
+ /**
+ * @region_node: Linked into &dmem_cgroup_regions list.
+ * Protected by RCU and global spinlock.
+ */
+ struct list_head region_node;
+
+ /**
+ * @pools: List of pools linked to this region.
+ * Protected by global spinlock only
+ */
+ struct list_head pools;
+
+ /** @size: Size of region, in bytes */
+ u64 size;
+
+ /** @name: Name describing the node, set by dmem_cgroup_register_region */
+ char *name;
+
+ /**
+ * @unregistered: Whether the region is unregistered by its caller.
+ * No new pools should be added to the region afterwards.
+ */
+ bool unregistered;
+};
+
+struct dmemcg_state {
+ struct cgroup_subsys_state css;
+
+ struct list_head pools;
+};
+
+struct dmem_cgroup_pool_state {
+ struct dmem_cgroup_region *region;
+ struct dmemcg_state *cs;
+
+ /* css node, RCU protected against region teardown */
+ struct list_head css_node;
+
+ /* dev node, no RCU protection required */
+ struct list_head region_node;
+
+ struct rcu_head rcu;
+
+ struct page_counter cnt;
+
+ bool inited;
+};
+
+/*
+ * 3 operations require locking protection:
+ * - Registering and unregistering region to/from list, requires global lock.
+ * - Adding a dmem_cgroup_pool_state to a CSS, removing when CSS is freed.
+ * - Adding a dmem_cgroup_pool_state to a region list.
+ *
+ * Since for the most common operations RCU provides enough protection, I
+ * do not think more granular locking makes sense. Most protection is offered
+ * by RCU and the lockless operating page_counter.
+ */
+static DEFINE_SPINLOCK(dmemcg_lock);
+static LIST_HEAD(dmem_cgroup_regions);
+
+static inline struct dmemcg_state *
+css_to_dmemcs(struct cgroup_subsys_state *css)
+{
+ return container_of(css, struct dmemcg_state, css);
+}
+
+static inline struct dmemcg_state *get_current_dmemcs(void)
+{
+ return css_to_dmemcs(task_get_css(current, dmem_cgrp_id));
+}
+
+static struct dmemcg_state *parent_dmemcs(struct dmemcg_state *cg)
+{
+ return cg->css.parent ? css_to_dmemcs(cg->css.parent) : NULL;
+}
+
+static void free_cg_pool(struct dmem_cgroup_pool_state *pool)
+{
+ list_del(&pool->region_node);
+ kfree(pool);
+}
+
+static void
+set_resource_min(struct dmem_cgroup_pool_state *pool, u64 val)
+{
+ page_counter_set_min(&pool->cnt, val);
+}
+
+static void
+set_resource_low(struct dmem_cgroup_pool_state *pool, u64 val)
+{
+ page_counter_set_low(&pool->cnt, val);
+}
+
+static void
+set_resource_max(struct dmem_cgroup_pool_state *pool, u64 val)
+{
+ page_counter_set_max(&pool->cnt, val);
+}
+
+static u64 get_resource_low(struct dmem_cgroup_pool_state *pool)
+{
+ return pool ? READ_ONCE(pool->cnt.low) : 0;
+}
+
+static u64 get_resource_min(struct dmem_cgroup_pool_state *pool)
+{
+ return pool ? READ_ONCE(pool->cnt.min) : 0;
+}
+
+static u64 get_resource_max(struct dmem_cgroup_pool_state *pool)
+{
+ return pool ? READ_ONCE(pool->cnt.max) : PAGE_COUNTER_MAX;
+}
+
+static u64 get_resource_current(struct dmem_cgroup_pool_state *pool)
+{
+ return pool ? page_counter_read(&pool->cnt) : 0;
+}
+
+static void reset_all_resource_limits(struct dmem_cgroup_pool_state *rpool)
+{
+ set_resource_min(rpool, 0);
+ set_resource_low(rpool, 0);
+ set_resource_max(rpool, PAGE_COUNTER_MAX);
+}
+
+static void dmemcs_offline(struct cgroup_subsys_state *css)
+{
+ struct dmemcg_state *dmemcs = css_to_dmemcs(css);
+ struct dmem_cgroup_pool_state *pool;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(pool, &dmemcs->pools, css_node)
+ reset_all_resource_limits(pool);
+ rcu_read_unlock();
+}
+
+static void dmemcs_free(struct cgroup_subsys_state *css)
+{
+ struct dmemcg_state *dmemcs = css_to_dmemcs(css);
+ struct dmem_cgroup_pool_state *pool, *next;
+
+ spin_lock(&dmemcg_lock);
+ list_for_each_entry_safe(pool, next, &dmemcs->pools, css_node) {
+ /*
+ *The pool is dead and all references are 0,
+ * no need for RCU protection with list_del_rcu or freeing.
+ */
+ list_del(&pool->css_node);
+ free_cg_pool(pool);
+ }
+ spin_unlock(&dmemcg_lock);
+
+ kfree(dmemcs);
+}
+
+static struct cgroup_subsys_state *
+dmemcs_alloc(struct cgroup_subsys_state *parent_css)
+{
+ struct dmemcg_state *dmemcs = kzalloc(sizeof(*dmemcs), GFP_KERNEL);
+ if (!dmemcs)
+ return ERR_PTR(-ENOMEM);
+
+ INIT_LIST_HEAD(&dmemcs->pools);
+ return &dmemcs->css;
+}
+
+static struct dmem_cgroup_pool_state *
+find_cg_pool_locked(struct dmemcg_state *dmemcs, struct dmem_cgroup_region *region)
+{
+ struct dmem_cgroup_pool_state *pool;
+
+ list_for_each_entry_rcu(pool, &dmemcs->pools, css_node, spin_is_locked(&dmemcg_lock))
+ if (pool->region == region)
+ return pool;
+
+ return NULL;
+}
+
+static struct dmem_cgroup_pool_state *pool_parent(struct dmem_cgroup_pool_state *pool)
+{
+ if (!pool->cnt.parent)
+ return NULL;
+
+ return container_of(pool->cnt.parent, typeof(*pool), cnt);
+}
+
+static void
+dmem_cgroup_calculate_protection(struct dmem_cgroup_pool_state *limit_pool,
+ struct dmem_cgroup_pool_state *test_pool)
+{
+ struct page_counter *climit;
+ struct cgroup_subsys_state *css, *next_css;
+ struct dmemcg_state *dmemcg_iter;
+ struct dmem_cgroup_pool_state *pool, *parent_pool;
+ bool found_descendant;
+
+ climit = &limit_pool->cnt;
+
+ rcu_read_lock();
+ parent_pool = pool = limit_pool;
+ css = &limit_pool->cs->css;
+
+ /*
+ * This logic is roughly equivalent to css_foreach_descendant_pre,
+ * except we also track the parent pool to find out which pool we need
+ * to calculate protection values for.
+ *
+ * We can stop the traversal once we find test_pool among the
+ * descendants since we don't really care about any others.
+ */
+ while (pool != test_pool) {
+ next_css = css_next_child(NULL, css);
+ if (next_css) {
+ parent_pool = pool;
+ } else {
+ while (css != &limit_pool->cs->css) {
+ next_css = css_next_child(css, css->parent);
+ if (next_css)
+ break;
+ css = css->parent;
+ parent_pool = pool_parent(parent_pool);
+ }
+ /*
+ * We can only hit this when test_pool is not a
+ * descendant of limit_pool.
+ */
+ if (WARN_ON_ONCE(css == &limit_pool->cs->css))
+ break;
+ }
+ css = next_css;
+
+ found_descendant = false;
+ dmemcg_iter = container_of(css, struct dmemcg_state, css);
+
+ list_for_each_entry_rcu(pool, &dmemcg_iter->pools, css_node) {
+ if (pool_parent(pool) == parent_pool) {
+ found_descendant = true;
+ break;
+ }
+ }
+ if (!found_descendant)
+ continue;
+
+ page_counter_calculate_protection(
+ climit, &pool->cnt, true);
+ }
+ rcu_read_unlock();
+}
+
+/**
+ * dmem_cgroup_state_evict_valuable() - Check if we should evict from test_pool
+ * @dev: &dmem_cgroup_region
+ * @index: The index number of the region being tested.
+ * @limit_pool: The pool for which we hit limits
+ * @test_pool: The pool for which to test
+ * @ignore_low: Whether we have to respect low watermarks.
+ * @ret_hit_low: Pointer to whether it makes sense to consider low watermark.
+ *
+ * This function returns true if we can evict from @test_pool, false if not.
+ * When returning false and @ignore_low is false, @ret_hit_low may
+ * be set to true to indicate this function can be retried with @ignore_low
+ * set to true.
+ *
+ * Return: bool
+ */
+bool dmem_cgroup_state_evict_valuable(struct dmem_cgroup_pool_state *limit_pool,
+ struct dmem_cgroup_pool_state *test_pool,
+ bool ignore_low, bool *ret_hit_low)
+{
+ struct dmem_cgroup_pool_state *pool = test_pool;
+ struct page_counter *climit, *ctest;
+ u64 used, min, low;
+
+ /* Can always evict from current pool, despite limits */
+ if (limit_pool == test_pool)
+ return true;
+
+ if (limit_pool) {
+ if (!parent_dmemcs(limit_pool->cs))
+ return true;
+
+ for (pool = test_pool; pool && limit_pool != pool; pool = pool_parent(pool))
+ {}
+
+ if (!pool)
+ return false;
+ } else {
+ /*
+ * If there is no cgroup limiting memory usage, use the root
+ * cgroup instead for limit calculations.
+ */
+ for (limit_pool = test_pool; pool_parent(limit_pool); limit_pool = pool_parent(limit_pool))
+ {}
+ }
+
+ climit = &limit_pool->cnt;
+ ctest = &test_pool->cnt;
+
+ dmem_cgroup_calculate_protection(limit_pool, test_pool);
+
+ used = page_counter_read(ctest);
+ min = READ_ONCE(ctest->emin);
+
+ if (used <= min)
+ return false;
+
+ if (!ignore_low) {
+ low = READ_ONCE(ctest->elow);
+ if (used > low)
+ return true;
+
+ *ret_hit_low = true;
+ return false;
+ }
+ return true;
+}
+EXPORT_SYMBOL_GPL(dmem_cgroup_state_evict_valuable);
+
+static struct dmem_cgroup_pool_state *
+alloc_pool_single(struct dmemcg_state *dmemcs, struct dmem_cgroup_region *region,
+ struct dmem_cgroup_pool_state **allocpool)
+{
+ struct dmemcg_state *parent = parent_dmemcs(dmemcs);
+ struct dmem_cgroup_pool_state *pool, *ppool = NULL;
+
+ if (!*allocpool) {
+ pool = kzalloc(sizeof(*pool), GFP_NOWAIT);
+ if (!pool)
+ return ERR_PTR(-ENOMEM);
+ } else {
+ pool = *allocpool;
+ *allocpool = NULL;
+ }
+
+ pool->region = region;
+ pool->cs = dmemcs;
+
+ if (parent)
+ ppool = find_cg_pool_locked(parent, region);
+
+ page_counter_init(&pool->cnt,
+ ppool ? &ppool->cnt : NULL, true);
+ reset_all_resource_limits(pool);
+
+ list_add_tail_rcu(&pool->css_node, &dmemcs->pools);
+ list_add_tail(&pool->region_node, ®ion->pools);
+
+ if (!parent)
+ pool->inited = true;
+ else
+ pool->inited = ppool ? ppool->inited : false;
+ return pool;
+}
+
+static struct dmem_cgroup_pool_state *
+get_cg_pool_locked(struct dmemcg_state *dmemcs, struct dmem_cgroup_region *region,
+ struct dmem_cgroup_pool_state **allocpool)
+{
+ struct dmem_cgroup_pool_state *pool, *ppool, *retpool;
+ struct dmemcg_state *p, *pp;
+
+ /*
+ * Recursively create pool, we may not initialize yet on
+ * recursion, this is done as a separate step.
+ */
+ for (p = dmemcs; p; p = parent_dmemcs(p)) {
+ pool = find_cg_pool_locked(p, region);
+ if (!pool)
+ pool = alloc_pool_single(p, region, allocpool);
+
+ if (IS_ERR(pool))
+ return pool;
+
+ if (p == dmemcs && pool->inited)
+ return pool;
+
+ if (pool->inited)
+ break;
+ }
+
+ retpool = pool = find_cg_pool_locked(dmemcs, region);
+ for (p = dmemcs, pp = parent_dmemcs(dmemcs); pp; p = pp, pp = parent_dmemcs(p)) {
+ if (pool->inited)
+ break;
+
+ /* ppool was created if it didn't exist by above loop. */
+ ppool = find_cg_pool_locked(pp, region);
+
+ /* Fix up parent links, mark as inited. */
+ pool->cnt.parent = &ppool->cnt;
+ pool->inited = true;
+
+ pool = ppool;
+ }
+
+ return retpool;
+}
+
+static void dmemcg_free_rcu(struct rcu_head *rcu)
+{
+ struct dmem_cgroup_region *region = container_of(rcu, typeof(*region), rcu);
+ struct dmem_cgroup_pool_state *pool, *next;
+
+ list_for_each_entry_safe(pool, next, ®ion->pools, region_node)
+ free_cg_pool(pool);
+ kfree(region->name);
+ kfree(region);
+}
+
+static void dmemcg_free_region(struct kref *ref)
+{
+ struct dmem_cgroup_region *cgregion = container_of(ref, typeof(*cgregion), ref);
+
+ call_rcu(&cgregion->rcu, dmemcg_free_rcu);
+}
+
+/**
+ * dmem_cgroup_unregister_region() - Unregister a previously registered region.
+ * @region: The region to unregister.
+ *
+ * This function undoes dmem_cgroup_register_region.
+ */
+void dmem_cgroup_unregister_region(struct dmem_cgroup_region *region)
+{
+ struct list_head *entry;
+
+ if (!region)
+ return;
+
+ spin_lock(&dmemcg_lock);
+
+ /* Remove from global region list */
+ list_del_rcu(®ion->region_node);
+
+ list_for_each_rcu(entry, ®ion->pools) {
+ struct dmem_cgroup_pool_state *pool =
+ container_of(entry, typeof(*pool), region_node);
+
+ list_del_rcu(&pool->css_node);
+ }
+
+ /*
+ * Ensure any RCU based lookups fail. Additionally,
+ * no new pools should be added to the dead region
+ * by get_cg_pool_unlocked.
+ */
+ region->unregistered = true;
+ spin_unlock(&dmemcg_lock);
+
+ kref_put(®ion->ref, dmemcg_free_region);
+}
+EXPORT_SYMBOL_GPL(dmem_cgroup_unregister_region);
+
+/**
+ * dmem_cgroup_register_region() - Register a regions for dev cgroup.
+ * @size: Size of region to register, in bytes.
+ * @fmt: Region parameters to register
+ *
+ * This function registers a node in the dmem cgroup with the
+ * name given. After calling this function, the region can be
+ * used for allocations.
+ *
+ * Return: NULL or a struct on success, PTR_ERR on failure.
+ */
+struct dmem_cgroup_region *dmem_cgroup_register_region(u64 size, const char *fmt, ...)
+{
+ struct dmem_cgroup_region *ret;
+ char *region_name;
+ va_list ap;
+
+ if (!size)
+ return NULL;
+
+ va_start(ap, fmt);
+ region_name = kvasprintf(GFP_KERNEL, fmt, ap);
+ va_end(ap);
+ if (!region_name)
+ return ERR_PTR(-ENOMEM);
+
+ ret = kzalloc(sizeof(*ret), GFP_KERNEL);
+ if (!ret) {
+ kfree(region_name);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ INIT_LIST_HEAD(&ret->pools);
+ ret->name = region_name;
+ ret->size = size;
+ kref_init(&ret->ref);
+
+ spin_lock(&dmemcg_lock);
+ list_add_tail_rcu(&ret->region_node, &dmem_cgroup_regions);
+ spin_unlock(&dmemcg_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(dmem_cgroup_register_region);
+
+static struct dmem_cgroup_region *dmemcg_get_region_by_name(const char *name)
+{
+ struct dmem_cgroup_region *region;
+
+ list_for_each_entry_rcu(region, &dmem_cgroup_regions, region_node, spin_is_locked(&dmemcg_lock))
+ if (!strcmp(name, region->name) &&
+ kref_get_unless_zero(®ion->ref))
+ return region;
+
+ return NULL;
+}
+
+/**
+ * dmem_cgroup_pool_state_put() - Drop a reference to a dmem_cgroup_pool_state
+ * @pool: &dmem_cgroup_pool_state
+ *
+ * Called to drop a reference to the limiting pool returned by
+ * dmem_cgroup_try_charge().
+ */
+void dmem_cgroup_pool_state_put(struct dmem_cgroup_pool_state *pool)
+{
+ if (pool)
+ css_put(&pool->cs->css);
+}
+EXPORT_SYMBOL_GPL(dmem_cgroup_pool_state_put);
+
+static struct dmem_cgroup_pool_state *
+get_cg_pool_unlocked(struct dmemcg_state *cg, struct dmem_cgroup_region *region)
+{
+ struct dmem_cgroup_pool_state *pool, *allocpool = NULL;
+
+ /* fastpath lookup? */
+ rcu_read_lock();
+ pool = find_cg_pool_locked(cg, region);
+ if (pool && !READ_ONCE(pool->inited))
+ pool = NULL;
+ rcu_read_unlock();
+
+ while (!pool) {
+ spin_lock(&dmemcg_lock);
+ if (!region->unregistered)
+ pool = get_cg_pool_locked(cg, region, &allocpool);
+ else
+ pool = ERR_PTR(-ENODEV);
+ spin_unlock(&dmemcg_lock);
+
+ if (pool == ERR_PTR(-ENOMEM)) {
+ pool = NULL;
+ if (WARN_ON(allocpool))
+ continue;
+
+ allocpool = kzalloc(sizeof(*allocpool), GFP_KERNEL);
+ if (allocpool) {
+ pool = NULL;
+ continue;
+ }
+ }
+ }
+
+ kfree(allocpool);
+ return pool;
+}
+
+/**
+ * dmem_cgroup_uncharge() - Uncharge a pool.
+ * @pool: Pool to uncharge.
+ * @size: Size to uncharge.
+ *
+ * Undoes the effects of dmem_cgroup_try_charge.
+ * Must be called with the returned pool as argument,
+ * and same @index and @size.
+ */
+void dmem_cgroup_uncharge(struct dmem_cgroup_pool_state *pool, u64 size)
+{
+ if (!pool)
+ return;
+
+ page_counter_uncharge(&pool->cnt, size);
+ css_put(&pool->cs->css);
+}
+EXPORT_SYMBOL_GPL(dmem_cgroup_uncharge);
+
+/**
+ * dmem_cgroup_try_charge() - Try charging a new allocation to a region.
+ * @dev: Device to charge
+ * @size: Size (in bytes) to charge.
+ * @ret_pool: On succesfull allocation, the pool that is charged.
+ * @ret_limit_pool: On a failed allocation, the limiting pool.
+ *
+ * This function charges the current pool for @dev with region at @index for a
+ * size of @size bytes.
+ *
+ * If the function succeeds, @ret_pool is set, which must be passed to
+ * dmem_cgroup_uncharge() when undoing the allocation.
+ *
+ * When this function fails with -EAGAIN and @ret_limit_pool is non-null, it
+ * will be set to the pool for which the limit is hit. This can be used for
+ * eviction as argument to dmem_cgroup_evict_valuable(). This reference must be freed
+ * with @dmem_cgroup_pool_state_put().
+ *
+ * Return: 0 on success, -EAGAIN on hitting a limit, or a negative errno on failure.
+ */
+int dmem_cgroup_try_charge(struct dmem_cgroup_region *region, u64 size,
+ struct dmem_cgroup_pool_state **ret_pool,
+ struct dmem_cgroup_pool_state **ret_limit_pool)
+{
+ struct dmemcg_state *cg;
+ struct dmem_cgroup_pool_state *pool;
+ struct page_counter *fail;
+ int ret;
+
+ *ret_pool = NULL;
+ if (ret_limit_pool)
+ *ret_limit_pool = NULL;
+
+ /*
+ * hold on to css, as cgroup can be removed but resource
+ * accounting happens on css.
+ */
+ cg = get_current_dmemcs();
+
+ pool = get_cg_pool_unlocked(cg, region);
+ if (IS_ERR(pool)) {
+ ret = PTR_ERR(pool);
+ goto err;
+ }
+
+ if (!page_counter_try_charge(&pool->cnt, size, &fail)) {
+ if (ret_limit_pool) {
+ *ret_limit_pool = container_of(fail, struct dmem_cgroup_pool_state, cnt);
+ css_get(&(*ret_limit_pool)->cs->css);
+ }
+ ret = -EAGAIN;
+ goto err;
+ }
+
+ /* On success, reference from get_current_dmemcs is transferred to *ret_pool */
+ *ret_pool = pool;
+ return 0;
+
+err:
+ css_put(&cg->css);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(dmem_cgroup_try_charge);
+
+static int dmem_cgroup_region_capacity_show(struct seq_file *sf, void *v)
+{
+ struct dmem_cgroup_region *region;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(region, &dmem_cgroup_regions, region_node) {
+ seq_puts(sf, region->name);
+ seq_printf(sf, " %llu\n", region->size);
+ }
+ rcu_read_unlock();
+ return 0;
+}
+
+static int dmemcg_parse_limit(char *options, struct dmem_cgroup_region *region,
+ u64 *new_limit)
+{
+ char *end;
+
+ if (!strcmp(options, "max")) {
+ *new_limit = PAGE_COUNTER_MAX;
+ return 0;
+ }
+
+ *new_limit = memparse(options, &end);
+ if (*end != '\0')
+ return -EINVAL;
+
+ return 0;
+}
+
+static ssize_t dmemcg_limit_write(struct kernfs_open_file *of,
+ char *buf, size_t nbytes, loff_t off,
+ void (*apply)(struct dmem_cgroup_pool_state *, u64))
+{
+ struct dmemcg_state *dmemcs = css_to_dmemcs(of_css(of));
+ int err = 0;
+
+ while (buf && !err) {
+ struct dmem_cgroup_pool_state *pool = NULL;
+ char *options, *region_name;
+ struct dmem_cgroup_region *region;
+ u64 new_limit;
+
+ options = buf;
+ buf = strchr(buf, '\n');
+ if (buf)
+ *buf++ = '\0';
+
+ options = strstrip(options);
+
+ /* eat empty lines */
+ if (!options[0])
+ continue;
+
+ region_name = strsep(&options, " \t");
+ if (!region_name[0])
+ continue;
+
+ rcu_read_lock();
+ region = dmemcg_get_region_by_name(region_name);
+ rcu_read_unlock();
+
+ if (!region)
+ return -EINVAL;
+
+ err = dmemcg_parse_limit(options, region, &new_limit);
+ if (err < 0)
+ goto out_put;
+
+ pool = get_cg_pool_unlocked(dmemcs, region);
+ if (IS_ERR(pool)) {
+ err = PTR_ERR(pool);
+ goto out_put;
+ }
+
+ /* And commit */
+ apply(pool, new_limit);
+
+out_put:
+ kref_put(®ion->ref, dmemcg_free_region);
+ }
+
+
+ return err ?: nbytes;
+}
+
+static int dmemcg_limit_show(struct seq_file *sf, void *v,
+ u64 (*fn)(struct dmem_cgroup_pool_state *))
+{
+ struct dmemcg_state *dmemcs = css_to_dmemcs(seq_css(sf));
+ struct dmem_cgroup_region *region;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(region, &dmem_cgroup_regions, region_node) {
+ struct dmem_cgroup_pool_state *pool = find_cg_pool_locked(dmemcs, region);
+ u64 val;
+
+ seq_puts(sf, region->name);
+
+ val = fn(pool);
+ if (val < PAGE_COUNTER_MAX)
+ seq_printf(sf, " %lld\n", val);
+ else
+ seq_puts(sf, " max\n");
+ }
+ rcu_read_unlock();
+
+ return 0;
+}
+
+static int dmem_cgroup_region_current_show(struct seq_file *sf, void *v)
+{
+ return dmemcg_limit_show(sf, v, get_resource_current);
+}
+
+static int dmem_cgroup_region_min_show(struct seq_file *sf, void *v)
+{
+ return dmemcg_limit_show(sf, v, get_resource_min);
+}
+
+static ssize_t dmem_cgroup_region_min_write(struct kernfs_open_file *of,
+ char *buf, size_t nbytes, loff_t off)
+{
+ return dmemcg_limit_write(of, buf, nbytes, off, set_resource_min);
+}
+
+static int dmem_cgroup_region_low_show(struct seq_file *sf, void *v)
+{
+ return dmemcg_limit_show(sf, v, get_resource_low);
+}
+
+static ssize_t dmem_cgroup_region_low_write(struct kernfs_open_file *of,
+ char *buf, size_t nbytes, loff_t off)
+{
+ return dmemcg_limit_write(of, buf, nbytes, off, set_resource_low);
+}
+
+static int dmem_cgroup_region_max_show(struct seq_file *sf, void *v)
+{
+ return dmemcg_limit_show(sf, v, get_resource_max);
+}
+
+static ssize_t dmem_cgroup_region_max_write(struct kernfs_open_file *of,
+ char *buf, size_t nbytes, loff_t off)
+{
+ return dmemcg_limit_write(of, buf, nbytes, off, set_resource_max);
+}
+
+static struct cftype files[] = {
+ {
+ .name = "capacity",
+ .seq_show = dmem_cgroup_region_capacity_show,
+ .flags = CFTYPE_ONLY_ON_ROOT,
+ },
+ {
+ .name = "current",
+ .seq_show = dmem_cgroup_region_current_show,
+ },
+ {
+ .name = "min",
+ .write = dmem_cgroup_region_min_write,
+ .seq_show = dmem_cgroup_region_min_show,
+ .flags = CFTYPE_NOT_ON_ROOT,
+ },
+ {
+ .name = "low",
+ .write = dmem_cgroup_region_low_write,
+ .seq_show = dmem_cgroup_region_low_show,
+ .flags = CFTYPE_NOT_ON_ROOT,
+ },
+ {
+ .name = "max",
+ .write = dmem_cgroup_region_max_write,
+ .seq_show = dmem_cgroup_region_max_show,
+ .flags = CFTYPE_NOT_ON_ROOT,
+ },
+ { } /* Zero entry terminates. */
+};
+
+struct cgroup_subsys dmem_cgrp_subsys = {
+ .css_alloc = dmemcs_alloc,
+ .css_free = dmemcs_free,
+ .css_offline = dmemcs_offline,
+ .legacy_cftypes = files,
+ .dfl_cftypes = files,
+};
diff --git a/mm/page_counter.c b/mm/page_counter.c
index b249d15af9dd8..af23f927611b7 100644
--- a/mm/page_counter.c
+++ b/mm/page_counter.c
@@ -288,7 +288,7 @@ int page_counter_memparse(const char *buf, const char *max,
}
-#ifdef CONFIG_MEMCG
+#if IS_ENABLED(CONFIG_MEMCG) || IS_ENABLED(CONFIG_CGROUP_DMEM)
/*
* This function calculates an individual page counter's effective
* protection which is derived from its own memory.min/low, its
@@ -460,4 +460,4 @@ void page_counter_calculate_protection(struct page_counter *root,
atomic_long_read(&parent->children_low_usage),
recursive_protection));
}
-#endif /* CONFIG_MEMCG */
+#endif /* CONFIG_MEMCG || CONFIG_CGROUP_DMEM */
--
2.43.0
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH v2.1 1/1] kernel/cgroup: Add "dmem" memory accounting cgroup
2024-12-04 13:44 ` [PATCH v2 1/7] kernel/cgroup: " Maarten Lankhorst
@ 2024-12-04 14:31 ` Maarten Lankhorst
2024-12-04 21:00 ` kernel test robot
` (2 more replies)
2025-01-06 17:48 ` [PATCH v2 1/7] " Michal Koutný
1 sibling, 3 replies; 35+ messages in thread
From: Maarten Lankhorst @ 2024-12-04 14:31 UTC (permalink / raw)
To: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, Maxime Ripard
Cc: cgroups, linux-mm, Maarten Lankhorst, Maarten Lankhorst
This code is based on the RDMA and misc cgroup initially, but now
uses page_counter. It uses the same min/low/max semantics as the memory
cgroup as a result.
There's a small mismatch as TTM uses u64, and page_counter long pages.
In practice it's not a problem. 32-bits systems don't really come with
>=4GB cards and as long as we're consistently wrong with units, it's
fine. The device page size may not be in the same units as kernel page
size, and each region might also have a different page size (VRAM vs GART
for example).
The interface is simple:
- Call dmem_cgroup_register_region()
- Use dmem_cgroup_try_charge to check if you can allocate a chunk of memory,
use dmem_cgroup__uncharge when freeing it. This may return an error code,
or -EAGAIN when the cgroup limit is reached. In that case a reference
to the limiting pool is returned.
- The limiting cs can be used as compare function for
dmem_cgroup_state_evict_valuable.
- After having evicted enough, drop reference to limiting cs with
dmem_cgroup_pool_state_put.
This API allows you to limit device resources with cgroups.
You can see the supported cards in /sys/fs/cgroup/dmem.capacity
You need to echo +dmem to cgroup.subtree_control, and then you can
partition device memory.
Co-developed-by: Friedrich Vock <friedrich.vock@gmx.de>
Signed-off-by: Friedrich Vock <friedrich.vock@gmx.de>
Co-developed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
---
I completely messed up the !CONFIG_CGROUP_DMEM path. Resending just this patch to compile cleanly without CONFIG_CGROUP_DMEM enabled.
Documentation/admin-guide/cgroup-v2.rst | 58 +-
Documentation/core-api/cgroup.rst | 9 +
Documentation/core-api/index.rst | 1 +
Documentation/gpu/drm-compute.rst | 54 ++
include/linux/cgroup_dmem.h | 66 ++
include/linux/cgroup_subsys.h | 4 +
include/linux/page_counter.h | 2 +-
init/Kconfig | 10 +
kernel/cgroup/Makefile | 1 +
kernel/cgroup/dmem.c | 861 ++++++++++++++++++++++++
mm/page_counter.c | 4 +-
11 files changed, 1060 insertions(+), 10 deletions(-)
create mode 100644 Documentation/core-api/cgroup.rst
create mode 100644 Documentation/gpu/drm-compute.rst
create mode 100644 include/linux/cgroup_dmem.h
create mode 100644 kernel/cgroup/dmem.c
diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 315ede811c9d0..cb1b4e759b7e2 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst
@@ -64,13 +64,14 @@ v1 is available under :ref:`Documentation/admin-guide/cgroup-v1/index.rst <cgrou
5-6. Device
5-7. RDMA
5-7-1. RDMA Interface Files
- 5-8. HugeTLB
- 5.8-1. HugeTLB Interface Files
- 5-9. Misc
- 5.9-1 Miscellaneous cgroup Interface Files
- 5.9-2 Migration and Ownership
- 5-10. Others
- 5-10-1. perf_event
+ 5-8. DMEM
+ 5-9. HugeTLB
+ 5.9-1. HugeTLB Interface Files
+ 5-10. Misc
+ 5.10-1 Miscellaneous cgroup Interface Files
+ 5.10-2 Migration and Ownership
+ 5-11. Others
+ 5-11-1. perf_event
5-N. Non-normative information
5-N-1. CPU controller root cgroup process behaviour
5-N-2. IO controller root cgroup process behaviour
@@ -2626,6 +2627,49 @@ RDMA Interface Files
mlx4_0 hca_handle=1 hca_object=20
ocrdma1 hca_handle=1 hca_object=23
+DMEM
+----
+
+The "dmem" controller regulates the distribution and accounting of
+device memory regions. Because each memory region may have its own page size,
+which does not have to be equal to the system page size, the units are always bytes.
+
+DMEM Interface Files
+~~~~~~~~~~~~~~~~~~~~
+
+ dmem.max, dmem.min, dmem.low
+ A readwrite nested-keyed file that exists for all the cgroups
+ except root that describes current configured resource limit
+ for a region.
+
+ An example for xe follows::
+
+ drm/0000:03:00.0/vram0 1073741824
+ drm/0000:03:00.0/stolen max
+
+ The semantics are the same as for the memory cgroup controller, and are
+ calculated in the same way.
+
+ dmem.capacity
+ A read-only file that describes maximum region capacity.
+ It only exists on the root cgroup. Not all memory can be
+ allocated by cgroups, as the kernel reserves some for
+ internal use.
+
+ An example for xe follows::
+
+ drm/0000:03:00.0/vram0 8514437120
+ drm/0000:03:00.0/stolen 67108864
+
+ dmem.current
+ A read-only file that describes current resource usage.
+ It exists for all the cgroup except root.
+
+ An example for xe follows::
+
+ drm/0000:03:00.0/vram0 12550144
+ drm/0000:03:00.0/stolen 8650752
+
HugeTLB
-------
diff --git a/Documentation/core-api/cgroup.rst b/Documentation/core-api/cgroup.rst
new file mode 100644
index 0000000000000..8696e9513f511
--- /dev/null
+++ b/Documentation/core-api/cgroup.rst
@@ -0,0 +1,9 @@
+==================
+Cgroup Kernel APIs
+==================
+
+Device Memory Cgroup API (dmemcg)
+=========================
+.. kernel-doc:: kernel/cgroup/dmem.c
+ :export:
+
diff --git a/Documentation/core-api/index.rst b/Documentation/core-api/index.rst
index 563b8fc0002f7..913d91feaf760 100644
--- a/Documentation/core-api/index.rst
+++ b/Documentation/core-api/index.rst
@@ -109,6 +109,7 @@ more memory-management documentation in Documentation/mm/index.rst.
dma-isa-lpc
swiotlb
mm-api
+ cgroup
genalloc
pin_user_pages
boot-time-mm
diff --git a/Documentation/gpu/drm-compute.rst b/Documentation/gpu/drm-compute.rst
new file mode 100644
index 0000000000000..f90c3e63aa9e5
--- /dev/null
+++ b/Documentation/gpu/drm-compute.rst
@@ -0,0 +1,54 @@
+==================================
+Long running workloads and compute
+==================================
+
+Long running workloads (compute) are workloads that will not complete in 10
+seconds. (The time let the user wait before he reaches for the power button).
+This means that other techniques need to be used to manage those workloads,
+that cannot use fences.
+
+Some hardware may schedule compute jobs, and have no way to pre-empt them, or
+have their memory swapped out from them. Or they simply want their workload
+not to be preempted or swapped out at all.
+
+This means that it differs from what is described in driver-api/dma-buf.rst.
+
+As with normal compute jobs, dma-fence may not be used at all. In this case,
+not even to force preemption. The driver with is simply forced to unmap a BO
+from the long compute job's address space on unbind immediately, not even
+waiting for the workload to complete. Effectively this terminates the workload
+when there is no hardware support to recover.
+
+Since this is undesirable, there need to be mitigations to prevent a workload
+from being terminated. There are several possible approach, all with their
+advantages and drawbacks.
+
+The first approach you will likely try is to pin all buffers used by compute.
+This guarantees that the job will run uninterrupted, but also allows a very
+denial of service attack by pinning as much memory as possible, hogging the
+all GPU memory, and possibly a huge chunk of CPU memory.
+
+A second approach that will work slightly better on its own is adding an option
+not to evict when creating a new job (any kind). If all of userspace opts in
+to this flag, it would prevent cooperating userspace from forced terminating
+older compute jobs to start a new one.
+
+If job preemption and recoverable pagefaults are not available, those are the
+only approaches possible. So even with those, you want a separate way of
+controlling resources. The standard kernel way of doing so is cgroups.
+
+This creates a third option, using cgroups to prevent eviction. Both GPU and
+driver-allocated CPU memory would be accounted to the correct cgroup, and
+eviction would be made cgroup aware. This allows the GPU to be partitioned
+into cgroups, that will allow jobs to run next to each other without
+interference.
+
+The interface to the cgroup would be similar to the current CPU memory
+interface, with similar semantics for min/low/high/max, if eviction can
+be made cgroup aware.
+
+What should be noted is that each memory region (tiled memory for example)
+should have its own accounting.
+
+The key is set to the regionid set by the driver, for example "tile0".
+For the value of $card, we use drmGetUnique().
diff --git a/include/linux/cgroup_dmem.h b/include/linux/cgroup_dmem.h
new file mode 100644
index 0000000000000..dd4869f1d736e
--- /dev/null
+++ b/include/linux/cgroup_dmem.h
@@ -0,0 +1,66 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2023-2024 Intel Corporation
+ */
+
+#ifndef _CGROUP_DMEM_H
+#define _CGROUP_DMEM_H
+
+#include <linux/types.h>
+#include <linux/llist.h>
+
+struct dmem_cgroup_pool_state;
+
+/* Opaque definition of a cgroup region, used internally */
+struct dmem_cgroup_region;
+
+#if IS_ENABLED(CONFIG_CGROUP_DMEM)
+struct dmem_cgroup_region *dmem_cgroup_register_region(u64 size, const char *name_fmt, ...) __printf(2,3);
+void dmem_cgroup_unregister_region(struct dmem_cgroup_region *region);
+int dmem_cgroup_try_charge(struct dmem_cgroup_region *region, u64 size,
+ struct dmem_cgroup_pool_state **ret_pool,
+ struct dmem_cgroup_pool_state **ret_limit_pool);
+void dmem_cgroup_uncharge(struct dmem_cgroup_pool_state *pool, u64 size);
+bool dmem_cgroup_state_evict_valuable(struct dmem_cgroup_pool_state *limit_pool,
+ struct dmem_cgroup_pool_state *test_pool,
+ bool ignore_low, bool *ret_hit_low);
+
+void dmem_cgroup_pool_state_put(struct dmem_cgroup_pool_state *pool);
+#else
+static inline __printf(2,3) struct dmem_cgroup_region *
+dmem_cgroup_register_region(u64 size, const char *name_fmt, ...)
+{
+ return NULL;
+}
+
+static inline void dmem_cgroup_unregister_region(struct dmem_cgroup_region *region)
+{ }
+
+static inline int dmem_cgroup_try_charge(struct dmem_cgroup_region *region, u64 size,
+ struct dmem_cgroup_pool_state **ret_pool,
+ struct dmem_cgroup_pool_state **ret_limit_pool)
+{
+ *ret_pool = NULL;
+
+ if (ret_limit_pool)
+ *ret_limit_pool = NULL;
+
+ return 0;
+}
+
+static inline void dmem_cgroup_uncharge(struct dmem_cgroup_pool_state *pool, u64 size)
+{ }
+
+static inline
+bool dmem_cgroup_state_evict_valuable(struct dmem_cgroup_pool_state *limit_pool,
+ struct dmem_cgroup_pool_state *test_pool,
+ bool ignore_low, bool *ret_hit_low)
+{
+ return true;
+}
+
+static inline void dmem_cgroup_pool_state_put(struct dmem_cgroup_pool_state *pool)
+{ }
+
+#endif
+#endif /* _CGROUP_DMEM_H */
diff --git a/include/linux/cgroup_subsys.h b/include/linux/cgroup_subsys.h
index 4452354872307..3fd0bcbf30803 100644
--- a/include/linux/cgroup_subsys.h
+++ b/include/linux/cgroup_subsys.h
@@ -65,6 +65,10 @@ SUBSYS(rdma)
SUBSYS(misc)
#endif
+#if IS_ENABLED(CONFIG_CGROUP_DMEM)
+SUBSYS(dmem)
+#endif
+
/*
* The following subsystems are not supported on the default hierarchy.
*/
diff --git a/include/linux/page_counter.h b/include/linux/page_counter.h
index 79dbd8bc35a72..46406f3fe34d0 100644
--- a/include/linux/page_counter.h
+++ b/include/linux/page_counter.h
@@ -96,7 +96,7 @@ static inline void page_counter_reset_watermark(struct page_counter *counter)
counter->watermark = usage;
}
-#ifdef CONFIG_MEMCG
+#if IS_ENABLED(CONFIG_MEMCG) || IS_ENABLED(CONFIG_CGROUP_DMEM)
void page_counter_calculate_protection(struct page_counter *root,
struct page_counter *counter,
bool recursive_protection);
diff --git a/init/Kconfig b/init/Kconfig
index a20e6efd3f0fb..61f50cafa8151 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1128,6 +1128,7 @@ config CGROUP_PIDS
config CGROUP_RDMA
bool "RDMA controller"
+ select PAGE_COUNTER
help
Provides enforcement of RDMA resources defined by IB stack.
It is fairly easy for consumers to exhaust RDMA resources, which
@@ -1136,6 +1137,15 @@ config CGROUP_RDMA
Attaching processes with active RDMA resources to the cgroup
hierarchy is allowed even if can cross the hierarchy's limit.
+config CGROUP_DMEM
+ bool "Device memory controller (DMEM)"
+ help
+ The DMEM controller allows compatible devices to restrict device
+ memory usage based on the cgroup hierarchy.
+
+ As an example, it allows you to restrict VRAM usage for applications
+ in the DRM subsystem.
+
config CGROUP_FREEZER
bool "Freezer controller"
help
diff --git a/kernel/cgroup/Makefile b/kernel/cgroup/Makefile
index a5c9359d516f8..ede31601a363a 100644
--- a/kernel/cgroup/Makefile
+++ b/kernel/cgroup/Makefile
@@ -7,4 +7,5 @@ obj-$(CONFIG_CGROUP_RDMA) += rdma.o
obj-$(CONFIG_CPUSETS) += cpuset.o
obj-$(CONFIG_CPUSETS_V1) += cpuset-v1.o
obj-$(CONFIG_CGROUP_MISC) += misc.o
+obj-$(CONFIG_CGROUP_DMEM) += dmem.o
obj-$(CONFIG_CGROUP_DEBUG) += debug.o
diff --git a/kernel/cgroup/dmem.c b/kernel/cgroup/dmem.c
new file mode 100644
index 0000000000000..52736ef0ccf25
--- /dev/null
+++ b/kernel/cgroup/dmem.c
@@ -0,0 +1,861 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2023-2024 Intel Corporation (Maarten Lankhorst <dev@lankhorst.se>)
+ * Copyright 2024 Red Hat (Maxime Ripard <mripard@kernel.org>)
+ * Partially based on the rdma and misc controllers, which bear the following copyrights:
+ *
+ * Copyright 2020 Google LLC
+ * Copyright (C) 2016 Parav Pandit <pandit.parav@gmail.com>
+ */
+
+#include <linux/cgroup.h>
+#include <linux/cgroup_dmem.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/page_counter.h>
+#include <linux/parser.h>
+#include <linux/slab.h>
+
+struct dmem_cgroup_region {
+ /**
+ * @ref: References keeping the region alive.
+ * Keeps the region reference alive after a succesful RCU lookup.
+ */
+ struct kref ref;
+
+ /** @rcu: RCU head for freeing */
+ struct rcu_head rcu;
+
+ /**
+ * @region_node: Linked into &dmem_cgroup_regions list.
+ * Protected by RCU and global spinlock.
+ */
+ struct list_head region_node;
+
+ /**
+ * @pools: List of pools linked to this region.
+ * Protected by global spinlock only
+ */
+ struct list_head pools;
+
+ /** @size: Size of region, in bytes */
+ u64 size;
+
+ /** @name: Name describing the node, set by dmem_cgroup_register_region */
+ char *name;
+
+ /**
+ * @unregistered: Whether the region is unregistered by its caller.
+ * No new pools should be added to the region afterwards.
+ */
+ bool unregistered;
+};
+
+struct dmemcg_state {
+ struct cgroup_subsys_state css;
+
+ struct list_head pools;
+};
+
+struct dmem_cgroup_pool_state {
+ struct dmem_cgroup_region *region;
+ struct dmemcg_state *cs;
+
+ /* css node, RCU protected against region teardown */
+ struct list_head css_node;
+
+ /* dev node, no RCU protection required */
+ struct list_head region_node;
+
+ struct rcu_head rcu;
+
+ struct page_counter cnt;
+
+ bool inited;
+};
+
+/*
+ * 3 operations require locking protection:
+ * - Registering and unregistering region to/from list, requires global lock.
+ * - Adding a dmem_cgroup_pool_state to a CSS, removing when CSS is freed.
+ * - Adding a dmem_cgroup_pool_state to a region list.
+ *
+ * Since for the most common operations RCU provides enough protection, I
+ * do not think more granular locking makes sense. Most protection is offered
+ * by RCU and the lockless operating page_counter.
+ */
+static DEFINE_SPINLOCK(dmemcg_lock);
+static LIST_HEAD(dmem_cgroup_regions);
+
+static inline struct dmemcg_state *
+css_to_dmemcs(struct cgroup_subsys_state *css)
+{
+ return container_of(css, struct dmemcg_state, css);
+}
+
+static inline struct dmemcg_state *get_current_dmemcs(void)
+{
+ return css_to_dmemcs(task_get_css(current, dmem_cgrp_id));
+}
+
+static struct dmemcg_state *parent_dmemcs(struct dmemcg_state *cg)
+{
+ return cg->css.parent ? css_to_dmemcs(cg->css.parent) : NULL;
+}
+
+static void free_cg_pool(struct dmem_cgroup_pool_state *pool)
+{
+ list_del(&pool->region_node);
+ kfree(pool);
+}
+
+static void
+set_resource_min(struct dmem_cgroup_pool_state *pool, u64 val)
+{
+ page_counter_set_min(&pool->cnt, val);
+}
+
+static void
+set_resource_low(struct dmem_cgroup_pool_state *pool, u64 val)
+{
+ page_counter_set_low(&pool->cnt, val);
+}
+
+static void
+set_resource_max(struct dmem_cgroup_pool_state *pool, u64 val)
+{
+ page_counter_set_max(&pool->cnt, val);
+}
+
+static u64 get_resource_low(struct dmem_cgroup_pool_state *pool)
+{
+ return pool ? READ_ONCE(pool->cnt.low) : 0;
+}
+
+static u64 get_resource_min(struct dmem_cgroup_pool_state *pool)
+{
+ return pool ? READ_ONCE(pool->cnt.min) : 0;
+}
+
+static u64 get_resource_max(struct dmem_cgroup_pool_state *pool)
+{
+ return pool ? READ_ONCE(pool->cnt.max) : PAGE_COUNTER_MAX;
+}
+
+static u64 get_resource_current(struct dmem_cgroup_pool_state *pool)
+{
+ return pool ? page_counter_read(&pool->cnt) : 0;
+}
+
+static void reset_all_resource_limits(struct dmem_cgroup_pool_state *rpool)
+{
+ set_resource_min(rpool, 0);
+ set_resource_low(rpool, 0);
+ set_resource_max(rpool, PAGE_COUNTER_MAX);
+}
+
+static void dmemcs_offline(struct cgroup_subsys_state *css)
+{
+ struct dmemcg_state *dmemcs = css_to_dmemcs(css);
+ struct dmem_cgroup_pool_state *pool;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(pool, &dmemcs->pools, css_node)
+ reset_all_resource_limits(pool);
+ rcu_read_unlock();
+}
+
+static void dmemcs_free(struct cgroup_subsys_state *css)
+{
+ struct dmemcg_state *dmemcs = css_to_dmemcs(css);
+ struct dmem_cgroup_pool_state *pool, *next;
+
+ spin_lock(&dmemcg_lock);
+ list_for_each_entry_safe(pool, next, &dmemcs->pools, css_node) {
+ /*
+ *The pool is dead and all references are 0,
+ * no need for RCU protection with list_del_rcu or freeing.
+ */
+ list_del(&pool->css_node);
+ free_cg_pool(pool);
+ }
+ spin_unlock(&dmemcg_lock);
+
+ kfree(dmemcs);
+}
+
+static struct cgroup_subsys_state *
+dmemcs_alloc(struct cgroup_subsys_state *parent_css)
+{
+ struct dmemcg_state *dmemcs = kzalloc(sizeof(*dmemcs), GFP_KERNEL);
+ if (!dmemcs)
+ return ERR_PTR(-ENOMEM);
+
+ INIT_LIST_HEAD(&dmemcs->pools);
+ return &dmemcs->css;
+}
+
+static struct dmem_cgroup_pool_state *
+find_cg_pool_locked(struct dmemcg_state *dmemcs, struct dmem_cgroup_region *region)
+{
+ struct dmem_cgroup_pool_state *pool;
+
+ list_for_each_entry_rcu(pool, &dmemcs->pools, css_node, spin_is_locked(&dmemcg_lock))
+ if (pool->region == region)
+ return pool;
+
+ return NULL;
+}
+
+static struct dmem_cgroup_pool_state *pool_parent(struct dmem_cgroup_pool_state *pool)
+{
+ if (!pool->cnt.parent)
+ return NULL;
+
+ return container_of(pool->cnt.parent, typeof(*pool), cnt);
+}
+
+static void
+dmem_cgroup_calculate_protection(struct dmem_cgroup_pool_state *limit_pool,
+ struct dmem_cgroup_pool_state *test_pool)
+{
+ struct page_counter *climit;
+ struct cgroup_subsys_state *css, *next_css;
+ struct dmemcg_state *dmemcg_iter;
+ struct dmem_cgroup_pool_state *pool, *parent_pool;
+ bool found_descendant;
+
+ climit = &limit_pool->cnt;
+
+ rcu_read_lock();
+ parent_pool = pool = limit_pool;
+ css = &limit_pool->cs->css;
+
+ /*
+ * This logic is roughly equivalent to css_foreach_descendant_pre,
+ * except we also track the parent pool to find out which pool we need
+ * to calculate protection values for.
+ *
+ * We can stop the traversal once we find test_pool among the
+ * descendants since we don't really care about any others.
+ */
+ while (pool != test_pool) {
+ next_css = css_next_child(NULL, css);
+ if (next_css) {
+ parent_pool = pool;
+ } else {
+ while (css != &limit_pool->cs->css) {
+ next_css = css_next_child(css, css->parent);
+ if (next_css)
+ break;
+ css = css->parent;
+ parent_pool = pool_parent(parent_pool);
+ }
+ /*
+ * We can only hit this when test_pool is not a
+ * descendant of limit_pool.
+ */
+ if (WARN_ON_ONCE(css == &limit_pool->cs->css))
+ break;
+ }
+ css = next_css;
+
+ found_descendant = false;
+ dmemcg_iter = container_of(css, struct dmemcg_state, css);
+
+ list_for_each_entry_rcu(pool, &dmemcg_iter->pools, css_node) {
+ if (pool_parent(pool) == parent_pool) {
+ found_descendant = true;
+ break;
+ }
+ }
+ if (!found_descendant)
+ continue;
+
+ page_counter_calculate_protection(
+ climit, &pool->cnt, true);
+ }
+ rcu_read_unlock();
+}
+
+/**
+ * dmem_cgroup_state_evict_valuable() - Check if we should evict from test_pool
+ * @dev: &dmem_cgroup_region
+ * @index: The index number of the region being tested.
+ * @limit_pool: The pool for which we hit limits
+ * @test_pool: The pool for which to test
+ * @ignore_low: Whether we have to respect low watermarks.
+ * @ret_hit_low: Pointer to whether it makes sense to consider low watermark.
+ *
+ * This function returns true if we can evict from @test_pool, false if not.
+ * When returning false and @ignore_low is false, @ret_hit_low may
+ * be set to true to indicate this function can be retried with @ignore_low
+ * set to true.
+ *
+ * Return: bool
+ */
+bool dmem_cgroup_state_evict_valuable(struct dmem_cgroup_pool_state *limit_pool,
+ struct dmem_cgroup_pool_state *test_pool,
+ bool ignore_low, bool *ret_hit_low)
+{
+ struct dmem_cgroup_pool_state *pool = test_pool;
+ struct page_counter *climit, *ctest;
+ u64 used, min, low;
+
+ /* Can always evict from current pool, despite limits */
+ if (limit_pool == test_pool)
+ return true;
+
+ if (limit_pool) {
+ if (!parent_dmemcs(limit_pool->cs))
+ return true;
+
+ for (pool = test_pool; pool && limit_pool != pool; pool = pool_parent(pool))
+ {}
+
+ if (!pool)
+ return false;
+ } else {
+ /*
+ * If there is no cgroup limiting memory usage, use the root
+ * cgroup instead for limit calculations.
+ */
+ for (limit_pool = test_pool; pool_parent(limit_pool); limit_pool = pool_parent(limit_pool))
+ {}
+ }
+
+ climit = &limit_pool->cnt;
+ ctest = &test_pool->cnt;
+
+ dmem_cgroup_calculate_protection(limit_pool, test_pool);
+
+ used = page_counter_read(ctest);
+ min = READ_ONCE(ctest->emin);
+
+ if (used <= min)
+ return false;
+
+ if (!ignore_low) {
+ low = READ_ONCE(ctest->elow);
+ if (used > low)
+ return true;
+
+ *ret_hit_low = true;
+ return false;
+ }
+ return true;
+}
+EXPORT_SYMBOL_GPL(dmem_cgroup_state_evict_valuable);
+
+static struct dmem_cgroup_pool_state *
+alloc_pool_single(struct dmemcg_state *dmemcs, struct dmem_cgroup_region *region,
+ struct dmem_cgroup_pool_state **allocpool)
+{
+ struct dmemcg_state *parent = parent_dmemcs(dmemcs);
+ struct dmem_cgroup_pool_state *pool, *ppool = NULL;
+
+ if (!*allocpool) {
+ pool = kzalloc(sizeof(*pool), GFP_NOWAIT);
+ if (!pool)
+ return ERR_PTR(-ENOMEM);
+ } else {
+ pool = *allocpool;
+ *allocpool = NULL;
+ }
+
+ pool->region = region;
+ pool->cs = dmemcs;
+
+ if (parent)
+ ppool = find_cg_pool_locked(parent, region);
+
+ page_counter_init(&pool->cnt,
+ ppool ? &ppool->cnt : NULL, true);
+ reset_all_resource_limits(pool);
+
+ list_add_tail_rcu(&pool->css_node, &dmemcs->pools);
+ list_add_tail(&pool->region_node, ®ion->pools);
+
+ if (!parent)
+ pool->inited = true;
+ else
+ pool->inited = ppool ? ppool->inited : false;
+ return pool;
+}
+
+static struct dmem_cgroup_pool_state *
+get_cg_pool_locked(struct dmemcg_state *dmemcs, struct dmem_cgroup_region *region,
+ struct dmem_cgroup_pool_state **allocpool)
+{
+ struct dmem_cgroup_pool_state *pool, *ppool, *retpool;
+ struct dmemcg_state *p, *pp;
+
+ /*
+ * Recursively create pool, we may not initialize yet on
+ * recursion, this is done as a separate step.
+ */
+ for (p = dmemcs; p; p = parent_dmemcs(p)) {
+ pool = find_cg_pool_locked(p, region);
+ if (!pool)
+ pool = alloc_pool_single(p, region, allocpool);
+
+ if (IS_ERR(pool))
+ return pool;
+
+ if (p == dmemcs && pool->inited)
+ return pool;
+
+ if (pool->inited)
+ break;
+ }
+
+ retpool = pool = find_cg_pool_locked(dmemcs, region);
+ for (p = dmemcs, pp = parent_dmemcs(dmemcs); pp; p = pp, pp = parent_dmemcs(p)) {
+ if (pool->inited)
+ break;
+
+ /* ppool was created if it didn't exist by above loop. */
+ ppool = find_cg_pool_locked(pp, region);
+
+ /* Fix up parent links, mark as inited. */
+ pool->cnt.parent = &ppool->cnt;
+ pool->inited = true;
+
+ pool = ppool;
+ }
+
+ return retpool;
+}
+
+static void dmemcg_free_rcu(struct rcu_head *rcu)
+{
+ struct dmem_cgroup_region *region = container_of(rcu, typeof(*region), rcu);
+ struct dmem_cgroup_pool_state *pool, *next;
+
+ list_for_each_entry_safe(pool, next, ®ion->pools, region_node)
+ free_cg_pool(pool);
+ kfree(region->name);
+ kfree(region);
+}
+
+static void dmemcg_free_region(struct kref *ref)
+{
+ struct dmem_cgroup_region *cgregion = container_of(ref, typeof(*cgregion), ref);
+
+ call_rcu(&cgregion->rcu, dmemcg_free_rcu);
+}
+
+/**
+ * dmem_cgroup_unregister_region() - Unregister a previously registered region.
+ * @region: The region to unregister.
+ *
+ * This function undoes dmem_cgroup_register_region.
+ */
+void dmem_cgroup_unregister_region(struct dmem_cgroup_region *region)
+{
+ struct list_head *entry;
+
+ if (!region)
+ return;
+
+ spin_lock(&dmemcg_lock);
+
+ /* Remove from global region list */
+ list_del_rcu(®ion->region_node);
+
+ list_for_each_rcu(entry, ®ion->pools) {
+ struct dmem_cgroup_pool_state *pool =
+ container_of(entry, typeof(*pool), region_node);
+
+ list_del_rcu(&pool->css_node);
+ }
+
+ /*
+ * Ensure any RCU based lookups fail. Additionally,
+ * no new pools should be added to the dead region
+ * by get_cg_pool_unlocked.
+ */
+ region->unregistered = true;
+ spin_unlock(&dmemcg_lock);
+
+ kref_put(®ion->ref, dmemcg_free_region);
+}
+EXPORT_SYMBOL_GPL(dmem_cgroup_unregister_region);
+
+/**
+ * dmem_cgroup_register_region() - Register a regions for dev cgroup.
+ * @size: Size of region to register, in bytes.
+ * @fmt: Region parameters to register
+ *
+ * This function registers a node in the dmem cgroup with the
+ * name given. After calling this function, the region can be
+ * used for allocations.
+ *
+ * Return: NULL or a struct on success, PTR_ERR on failure.
+ */
+struct dmem_cgroup_region *dmem_cgroup_register_region(u64 size, const char *fmt, ...)
+{
+ struct dmem_cgroup_region *ret;
+ char *region_name;
+ va_list ap;
+
+ if (!size)
+ return NULL;
+
+ va_start(ap, fmt);
+ region_name = kvasprintf(GFP_KERNEL, fmt, ap);
+ va_end(ap);
+ if (!region_name)
+ return ERR_PTR(-ENOMEM);
+
+ ret = kzalloc(sizeof(*ret), GFP_KERNEL);
+ if (!ret) {
+ kfree(region_name);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ INIT_LIST_HEAD(&ret->pools);
+ ret->name = region_name;
+ ret->size = size;
+ kref_init(&ret->ref);
+
+ spin_lock(&dmemcg_lock);
+ list_add_tail_rcu(&ret->region_node, &dmem_cgroup_regions);
+ spin_unlock(&dmemcg_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(dmem_cgroup_register_region);
+
+static struct dmem_cgroup_region *dmemcg_get_region_by_name(const char *name)
+{
+ struct dmem_cgroup_region *region;
+
+ list_for_each_entry_rcu(region, &dmem_cgroup_regions, region_node, spin_is_locked(&dmemcg_lock))
+ if (!strcmp(name, region->name) &&
+ kref_get_unless_zero(®ion->ref))
+ return region;
+
+ return NULL;
+}
+
+/**
+ * dmem_cgroup_pool_state_put() - Drop a reference to a dmem_cgroup_pool_state
+ * @pool: &dmem_cgroup_pool_state
+ *
+ * Called to drop a reference to the limiting pool returned by
+ * dmem_cgroup_try_charge().
+ */
+void dmem_cgroup_pool_state_put(struct dmem_cgroup_pool_state *pool)
+{
+ if (pool)
+ css_put(&pool->cs->css);
+}
+EXPORT_SYMBOL_GPL(dmem_cgroup_pool_state_put);
+
+static struct dmem_cgroup_pool_state *
+get_cg_pool_unlocked(struct dmemcg_state *cg, struct dmem_cgroup_region *region)
+{
+ struct dmem_cgroup_pool_state *pool, *allocpool = NULL;
+
+ /* fastpath lookup? */
+ rcu_read_lock();
+ pool = find_cg_pool_locked(cg, region);
+ if (pool && !READ_ONCE(pool->inited))
+ pool = NULL;
+ rcu_read_unlock();
+
+ while (!pool) {
+ spin_lock(&dmemcg_lock);
+ if (!region->unregistered)
+ pool = get_cg_pool_locked(cg, region, &allocpool);
+ else
+ pool = ERR_PTR(-ENODEV);
+ spin_unlock(&dmemcg_lock);
+
+ if (pool == ERR_PTR(-ENOMEM)) {
+ pool = NULL;
+ if (WARN_ON(allocpool))
+ continue;
+
+ allocpool = kzalloc(sizeof(*allocpool), GFP_KERNEL);
+ if (allocpool) {
+ pool = NULL;
+ continue;
+ }
+ }
+ }
+
+ kfree(allocpool);
+ return pool;
+}
+
+/**
+ * dmem_cgroup_uncharge() - Uncharge a pool.
+ * @pool: Pool to uncharge.
+ * @size: Size to uncharge.
+ *
+ * Undoes the effects of dmem_cgroup_try_charge.
+ * Must be called with the returned pool as argument,
+ * and same @index and @size.
+ */
+void dmem_cgroup_uncharge(struct dmem_cgroup_pool_state *pool, u64 size)
+{
+ if (!pool)
+ return;
+
+ page_counter_uncharge(&pool->cnt, size);
+ css_put(&pool->cs->css);
+}
+EXPORT_SYMBOL_GPL(dmem_cgroup_uncharge);
+
+/**
+ * dmem_cgroup_try_charge() - Try charging a new allocation to a region.
+ * @dev: Device to charge
+ * @size: Size (in bytes) to charge.
+ * @ret_pool: On succesfull allocation, the pool that is charged.
+ * @ret_limit_pool: On a failed allocation, the limiting pool.
+ *
+ * This function charges the current pool for @dev with region at @index for a
+ * size of @size bytes.
+ *
+ * If the function succeeds, @ret_pool is set, which must be passed to
+ * dmem_cgroup_uncharge() when undoing the allocation.
+ *
+ * When this function fails with -EAGAIN and @ret_limit_pool is non-null, it
+ * will be set to the pool for which the limit is hit. This can be used for
+ * eviction as argument to dmem_cgroup_evict_valuable(). This reference must be freed
+ * with @dmem_cgroup_pool_state_put().
+ *
+ * Return: 0 on success, -EAGAIN on hitting a limit, or a negative errno on failure.
+ */
+int dmem_cgroup_try_charge(struct dmem_cgroup_region *region, u64 size,
+ struct dmem_cgroup_pool_state **ret_pool,
+ struct dmem_cgroup_pool_state **ret_limit_pool)
+{
+ struct dmemcg_state *cg;
+ struct dmem_cgroup_pool_state *pool;
+ struct page_counter *fail;
+ int ret;
+
+ *ret_pool = NULL;
+ if (ret_limit_pool)
+ *ret_limit_pool = NULL;
+
+ /*
+ * hold on to css, as cgroup can be removed but resource
+ * accounting happens on css.
+ */
+ cg = get_current_dmemcs();
+
+ pool = get_cg_pool_unlocked(cg, region);
+ if (IS_ERR(pool)) {
+ ret = PTR_ERR(pool);
+ goto err;
+ }
+
+ if (!page_counter_try_charge(&pool->cnt, size, &fail)) {
+ if (ret_limit_pool) {
+ *ret_limit_pool = container_of(fail, struct dmem_cgroup_pool_state, cnt);
+ css_get(&(*ret_limit_pool)->cs->css);
+ }
+ ret = -EAGAIN;
+ goto err;
+ }
+
+ /* On success, reference from get_current_dmemcs is transferred to *ret_pool */
+ *ret_pool = pool;
+ return 0;
+
+err:
+ css_put(&cg->css);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(dmem_cgroup_try_charge);
+
+static int dmem_cgroup_region_capacity_show(struct seq_file *sf, void *v)
+{
+ struct dmem_cgroup_region *region;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(region, &dmem_cgroup_regions, region_node) {
+ seq_puts(sf, region->name);
+ seq_printf(sf, " %llu\n", region->size);
+ }
+ rcu_read_unlock();
+ return 0;
+}
+
+static int dmemcg_parse_limit(char *options, struct dmem_cgroup_region *region,
+ u64 *new_limit)
+{
+ char *end;
+
+ if (!strcmp(options, "max")) {
+ *new_limit = PAGE_COUNTER_MAX;
+ return 0;
+ }
+
+ *new_limit = memparse(options, &end);
+ if (*end != '\0')
+ return -EINVAL;
+
+ return 0;
+}
+
+static ssize_t dmemcg_limit_write(struct kernfs_open_file *of,
+ char *buf, size_t nbytes, loff_t off,
+ void (*apply)(struct dmem_cgroup_pool_state *, u64))
+{
+ struct dmemcg_state *dmemcs = css_to_dmemcs(of_css(of));
+ int err = 0;
+
+ while (buf && !err) {
+ struct dmem_cgroup_pool_state *pool = NULL;
+ char *options, *region_name;
+ struct dmem_cgroup_region *region;
+ u64 new_limit;
+
+ options = buf;
+ buf = strchr(buf, '\n');
+ if (buf)
+ *buf++ = '\0';
+
+ options = strstrip(options);
+
+ /* eat empty lines */
+ if (!options[0])
+ continue;
+
+ region_name = strsep(&options, " \t");
+ if (!region_name[0])
+ continue;
+
+ rcu_read_lock();
+ region = dmemcg_get_region_by_name(region_name);
+ rcu_read_unlock();
+
+ if (!region)
+ return -EINVAL;
+
+ err = dmemcg_parse_limit(options, region, &new_limit);
+ if (err < 0)
+ goto out_put;
+
+ pool = get_cg_pool_unlocked(dmemcs, region);
+ if (IS_ERR(pool)) {
+ err = PTR_ERR(pool);
+ goto out_put;
+ }
+
+ /* And commit */
+ apply(pool, new_limit);
+
+out_put:
+ kref_put(®ion->ref, dmemcg_free_region);
+ }
+
+
+ return err ?: nbytes;
+}
+
+static int dmemcg_limit_show(struct seq_file *sf, void *v,
+ u64 (*fn)(struct dmem_cgroup_pool_state *))
+{
+ struct dmemcg_state *dmemcs = css_to_dmemcs(seq_css(sf));
+ struct dmem_cgroup_region *region;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(region, &dmem_cgroup_regions, region_node) {
+ struct dmem_cgroup_pool_state *pool = find_cg_pool_locked(dmemcs, region);
+ u64 val;
+
+ seq_puts(sf, region->name);
+
+ val = fn(pool);
+ if (val < PAGE_COUNTER_MAX)
+ seq_printf(sf, " %lld\n", val);
+ else
+ seq_puts(sf, " max\n");
+ }
+ rcu_read_unlock();
+
+ return 0;
+}
+
+static int dmem_cgroup_region_current_show(struct seq_file *sf, void *v)
+{
+ return dmemcg_limit_show(sf, v, get_resource_current);
+}
+
+static int dmem_cgroup_region_min_show(struct seq_file *sf, void *v)
+{
+ return dmemcg_limit_show(sf, v, get_resource_min);
+}
+
+static ssize_t dmem_cgroup_region_min_write(struct kernfs_open_file *of,
+ char *buf, size_t nbytes, loff_t off)
+{
+ return dmemcg_limit_write(of, buf, nbytes, off, set_resource_min);
+}
+
+static int dmem_cgroup_region_low_show(struct seq_file *sf, void *v)
+{
+ return dmemcg_limit_show(sf, v, get_resource_low);
+}
+
+static ssize_t dmem_cgroup_region_low_write(struct kernfs_open_file *of,
+ char *buf, size_t nbytes, loff_t off)
+{
+ return dmemcg_limit_write(of, buf, nbytes, off, set_resource_low);
+}
+
+static int dmem_cgroup_region_max_show(struct seq_file *sf, void *v)
+{
+ return dmemcg_limit_show(sf, v, get_resource_max);
+}
+
+static ssize_t dmem_cgroup_region_max_write(struct kernfs_open_file *of,
+ char *buf, size_t nbytes, loff_t off)
+{
+ return dmemcg_limit_write(of, buf, nbytes, off, set_resource_max);
+}
+
+static struct cftype files[] = {
+ {
+ .name = "capacity",
+ .seq_show = dmem_cgroup_region_capacity_show,
+ .flags = CFTYPE_ONLY_ON_ROOT,
+ },
+ {
+ .name = "current",
+ .seq_show = dmem_cgroup_region_current_show,
+ },
+ {
+ .name = "min",
+ .write = dmem_cgroup_region_min_write,
+ .seq_show = dmem_cgroup_region_min_show,
+ .flags = CFTYPE_NOT_ON_ROOT,
+ },
+ {
+ .name = "low",
+ .write = dmem_cgroup_region_low_write,
+ .seq_show = dmem_cgroup_region_low_show,
+ .flags = CFTYPE_NOT_ON_ROOT,
+ },
+ {
+ .name = "max",
+ .write = dmem_cgroup_region_max_write,
+ .seq_show = dmem_cgroup_region_max_show,
+ .flags = CFTYPE_NOT_ON_ROOT,
+ },
+ { } /* Zero entry terminates. */
+};
+
+struct cgroup_subsys dmem_cgrp_subsys = {
+ .css_alloc = dmemcs_alloc,
+ .css_free = dmemcs_free,
+ .css_offline = dmemcs_offline,
+ .legacy_cftypes = files,
+ .dfl_cftypes = files,
+};
diff --git a/mm/page_counter.c b/mm/page_counter.c
index b249d15af9dd8..af23f927611b7 100644
--- a/mm/page_counter.c
+++ b/mm/page_counter.c
@@ -288,7 +288,7 @@ int page_counter_memparse(const char *buf, const char *max,
}
-#ifdef CONFIG_MEMCG
+#if IS_ENABLED(CONFIG_MEMCG) || IS_ENABLED(CONFIG_CGROUP_DMEM)
/*
* This function calculates an individual page counter's effective
* protection which is derived from its own memory.min/low, its
@@ -460,4 +460,4 @@ void page_counter_calculate_protection(struct page_counter *root,
atomic_long_read(&parent->children_low_usage),
recursive_protection));
}
-#endif /* CONFIG_MEMCG */
+#endif /* CONFIG_MEMCG || CONFIG_CGROUP_DMEM */
--
2.43.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v2.1 1/1] kernel/cgroup: Add "dmem" memory accounting cgroup
2024-12-04 14:31 ` [PATCH v2.1 1/1] " Maarten Lankhorst
@ 2024-12-04 21:00 ` kernel test robot
2024-12-05 2:27 ` kernel test robot
2025-01-14 10:16 ` Geert Uytterhoeven
2 siblings, 0 replies; 35+ messages in thread
From: kernel test robot @ 2024-12-04 21:00 UTC (permalink / raw)
To: Maarten Lankhorst, linux-kernel, intel-xe, dri-devel, Tejun Heo,
Zefan Li, Johannes Weiner, Andrew Morton, Friedrich Vock,
Maxime Ripard
Cc: oe-kbuild-all, Linux Memory Management List, cgroups, Maarten Lankhorst
Hi Maarten,
kernel test robot noticed the following build warnings:
[auto build test WARNING on tj-cgroup/for-next]
[also build test WARNING on akpm-mm/mm-everything linus/master v6.13-rc1 next-20241204]
[cannot apply to drm-misc/drm-misc-next drm-tip/drm-tip]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Maarten-Lankhorst/kernel-cgroup-Add-dmem-memory-accounting-cgroup/20241204-233207
base: https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-next
patch link: https://lore.kernel.org/r/20241204143112.1250983-1-dev%40lankhorst.se
patch subject: [PATCH v2.1 1/1] kernel/cgroup: Add "dmem" memory accounting cgroup
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20241205/202412050415.jf4sa0gH-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241205/202412050415.jf4sa0gH-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412050415.jf4sa0gH-lkp@intel.com/
All warnings (new ones prefixed by >>):
kernel/cgroup/dmem.c: In function 'dmem_cgroup_state_evict_valuable':
>> kernel/cgroup/dmem.c:302:30: warning: variable 'climit' set but not used [-Wunused-but-set-variable]
302 | struct page_counter *climit, *ctest;
| ^~~~~~
--
>> kernel/cgroup/dmem.c:300: warning: Excess function parameter 'dev' description in 'dmem_cgroup_state_evict_valuable'
>> kernel/cgroup/dmem.c:300: warning: Excess function parameter 'index' description in 'dmem_cgroup_state_evict_valuable'
>> kernel/cgroup/dmem.c:635: warning: Function parameter or struct member 'region' not described in 'dmem_cgroup_try_charge'
>> kernel/cgroup/dmem.c:635: warning: Excess function parameter 'dev' description in 'dmem_cgroup_try_charge'
vim +/climit +302 kernel/cgroup/dmem.c
280
281 /**
282 * dmem_cgroup_state_evict_valuable() - Check if we should evict from test_pool
283 * @dev: &dmem_cgroup_region
284 * @index: The index number of the region being tested.
285 * @limit_pool: The pool for which we hit limits
286 * @test_pool: The pool for which to test
287 * @ignore_low: Whether we have to respect low watermarks.
288 * @ret_hit_low: Pointer to whether it makes sense to consider low watermark.
289 *
290 * This function returns true if we can evict from @test_pool, false if not.
291 * When returning false and @ignore_low is false, @ret_hit_low may
292 * be set to true to indicate this function can be retried with @ignore_low
293 * set to true.
294 *
295 * Return: bool
296 */
297 bool dmem_cgroup_state_evict_valuable(struct dmem_cgroup_pool_state *limit_pool,
298 struct dmem_cgroup_pool_state *test_pool,
299 bool ignore_low, bool *ret_hit_low)
> 300 {
301 struct dmem_cgroup_pool_state *pool = test_pool;
> 302 struct page_counter *climit, *ctest;
303 u64 used, min, low;
304
305 /* Can always evict from current pool, despite limits */
306 if (limit_pool == test_pool)
307 return true;
308
309 if (limit_pool) {
310 if (!parent_dmemcs(limit_pool->cs))
311 return true;
312
313 for (pool = test_pool; pool && limit_pool != pool; pool = pool_parent(pool))
314 {}
315
316 if (!pool)
317 return false;
318 } else {
319 /*
320 * If there is no cgroup limiting memory usage, use the root
321 * cgroup instead for limit calculations.
322 */
323 for (limit_pool = test_pool; pool_parent(limit_pool); limit_pool = pool_parent(limit_pool))
324 {}
325 }
326
327 climit = &limit_pool->cnt;
328 ctest = &test_pool->cnt;
329
330 dmem_cgroup_calculate_protection(limit_pool, test_pool);
331
332 used = page_counter_read(ctest);
333 min = READ_ONCE(ctest->emin);
334
335 if (used <= min)
336 return false;
337
338 if (!ignore_low) {
339 low = READ_ONCE(ctest->elow);
340 if (used > low)
341 return true;
342
343 *ret_hit_low = true;
344 return false;
345 }
346 return true;
347 }
348 EXPORT_SYMBOL_GPL(dmem_cgroup_state_evict_valuable);
349
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v2.1 1/1] kernel/cgroup: Add "dmem" memory accounting cgroup
2024-12-04 14:31 ` [PATCH v2.1 1/1] " Maarten Lankhorst
2024-12-04 21:00 ` kernel test robot
@ 2024-12-05 2:27 ` kernel test robot
2024-12-05 12:07 ` Maarten Lankhorst
2025-01-14 10:16 ` Geert Uytterhoeven
2 siblings, 1 reply; 35+ messages in thread
From: kernel test robot @ 2024-12-05 2:27 UTC (permalink / raw)
To: Maarten Lankhorst, linux-kernel, intel-xe, dri-devel, Tejun Heo,
Zefan Li, Johannes Weiner, Andrew Morton, Friedrich Vock,
Maxime Ripard
Cc: oe-kbuild-all, Linux Memory Management List, cgroups, Maarten Lankhorst
Hi Maarten,
kernel test robot noticed the following build errors:
[auto build test ERROR on tj-cgroup/for-next]
[also build test ERROR on akpm-mm/mm-everything linus/master v6.13-rc1 next-20241204]
[cannot apply to drm-misc/drm-misc-next drm-tip/drm-tip]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Maarten-Lankhorst/kernel-cgroup-Add-dmem-memory-accounting-cgroup/20241204-233207
base: https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-next
patch link: https://lore.kernel.org/r/20241204143112.1250983-1-dev%40lankhorst.se
patch subject: [PATCH v2.1 1/1] kernel/cgroup: Add "dmem" memory accounting cgroup
config: um-randconfig-r061-20241205 (https://download.01.org/0day-ci/archive/20241205/202412051039.P06riwrP-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241205/202412051039.P06riwrP-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412051039.P06riwrP-lkp@intel.com/
All errors (new ones prefixed by >>):
/usr/bin/ld: kernel/cgroup/dmem.o: in function `set_resource_min':
>> kernel/cgroup/dmem.c:115: undefined reference to `page_counter_set_min'
/usr/bin/ld: kernel/cgroup/dmem.o: in function `set_resource_low':
>> kernel/cgroup/dmem.c:121: undefined reference to `page_counter_set_low'
/usr/bin/ld: kernel/cgroup/dmem.o: in function `set_resource_max':
>> kernel/cgroup/dmem.c:127: undefined reference to `page_counter_set_max'
/usr/bin/ld: kernel/cgroup/dmem.o: in function `reset_all_resource_limits':
>> kernel/cgroup/dmem.c:115: undefined reference to `page_counter_set_min'
>> /usr/bin/ld: kernel/cgroup/dmem.c:121: undefined reference to `page_counter_set_low'
>> /usr/bin/ld: kernel/cgroup/dmem.c:127: undefined reference to `page_counter_set_max'
/usr/bin/ld: kernel/cgroup/dmem.o: in function `dmem_cgroup_uncharge':
>> kernel/cgroup/dmem.c:607: undefined reference to `page_counter_uncharge'
/usr/bin/ld: kernel/cgroup/dmem.o: in function `dmem_cgroup_calculate_protection':
>> kernel/cgroup/dmem.c:275: undefined reference to `page_counter_calculate_protection'
/usr/bin/ld: kernel/cgroup/dmem.o: in function `dmem_cgroup_try_charge':
>> kernel/cgroup/dmem.c:657: undefined reference to `page_counter_try_charge'
collect2: error: ld returned 1 exit status
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for GET_FREE_REGION
Depends on [n]: SPARSEMEM [=n]
Selected by [y]:
- RESOURCE_KUNIT_TEST [=y] && RUNTIME_TESTING_MENU [=y] && KUNIT [=y]
vim +115 kernel/cgroup/dmem.c
111
112 static void
113 set_resource_min(struct dmem_cgroup_pool_state *pool, u64 val)
114 {
> 115 page_counter_set_min(&pool->cnt, val);
116 }
117
118 static void
119 set_resource_low(struct dmem_cgroup_pool_state *pool, u64 val)
120 {
> 121 page_counter_set_low(&pool->cnt, val);
122 }
123
124 static void
125 set_resource_max(struct dmem_cgroup_pool_state *pool, u64 val)
126 {
> 127 page_counter_set_max(&pool->cnt, val);
128 }
129
130 static u64 get_resource_low(struct dmem_cgroup_pool_state *pool)
131 {
132 return pool ? READ_ONCE(pool->cnt.low) : 0;
133 }
134
135 static u64 get_resource_min(struct dmem_cgroup_pool_state *pool)
136 {
137 return pool ? READ_ONCE(pool->cnt.min) : 0;
138 }
139
140 static u64 get_resource_max(struct dmem_cgroup_pool_state *pool)
141 {
142 return pool ? READ_ONCE(pool->cnt.max) : PAGE_COUNTER_MAX;
143 }
144
145 static u64 get_resource_current(struct dmem_cgroup_pool_state *pool)
146 {
147 return pool ? page_counter_read(&pool->cnt) : 0;
148 }
149
150 static void reset_all_resource_limits(struct dmem_cgroup_pool_state *rpool)
151 {
152 set_resource_min(rpool, 0);
153 set_resource_low(rpool, 0);
154 set_resource_max(rpool, PAGE_COUNTER_MAX);
155 }
156
157 static void dmemcs_offline(struct cgroup_subsys_state *css)
158 {
159 struct dmemcg_state *dmemcs = css_to_dmemcs(css);
160 struct dmem_cgroup_pool_state *pool;
161
162 rcu_read_lock();
163 list_for_each_entry_rcu(pool, &dmemcs->pools, css_node)
164 reset_all_resource_limits(pool);
165 rcu_read_unlock();
166 }
167
168 static void dmemcs_free(struct cgroup_subsys_state *css)
169 {
170 struct dmemcg_state *dmemcs = css_to_dmemcs(css);
171 struct dmem_cgroup_pool_state *pool, *next;
172
173 spin_lock(&dmemcg_lock);
174 list_for_each_entry_safe(pool, next, &dmemcs->pools, css_node) {
175 /*
176 *The pool is dead and all references are 0,
177 * no need for RCU protection with list_del_rcu or freeing.
178 */
179 list_del(&pool->css_node);
180 free_cg_pool(pool);
181 }
182 spin_unlock(&dmemcg_lock);
183
184 kfree(dmemcs);
185 }
186
187 static struct cgroup_subsys_state *
188 dmemcs_alloc(struct cgroup_subsys_state *parent_css)
189 {
190 struct dmemcg_state *dmemcs = kzalloc(sizeof(*dmemcs), GFP_KERNEL);
191 if (!dmemcs)
192 return ERR_PTR(-ENOMEM);
193
194 INIT_LIST_HEAD(&dmemcs->pools);
195 return &dmemcs->css;
196 }
197
198 static struct dmem_cgroup_pool_state *
199 find_cg_pool_locked(struct dmemcg_state *dmemcs, struct dmem_cgroup_region *region)
200 {
201 struct dmem_cgroup_pool_state *pool;
202
203 list_for_each_entry_rcu(pool, &dmemcs->pools, css_node, spin_is_locked(&dmemcg_lock))
204 if (pool->region == region)
205 return pool;
206
207 return NULL;
208 }
209
210 static struct dmem_cgroup_pool_state *pool_parent(struct dmem_cgroup_pool_state *pool)
211 {
212 if (!pool->cnt.parent)
213 return NULL;
214
215 return container_of(pool->cnt.parent, typeof(*pool), cnt);
216 }
217
218 static void
219 dmem_cgroup_calculate_protection(struct dmem_cgroup_pool_state *limit_pool,
220 struct dmem_cgroup_pool_state *test_pool)
221 {
222 struct page_counter *climit;
223 struct cgroup_subsys_state *css, *next_css;
224 struct dmemcg_state *dmemcg_iter;
225 struct dmem_cgroup_pool_state *pool, *parent_pool;
226 bool found_descendant;
227
228 climit = &limit_pool->cnt;
229
230 rcu_read_lock();
231 parent_pool = pool = limit_pool;
232 css = &limit_pool->cs->css;
233
234 /*
235 * This logic is roughly equivalent to css_foreach_descendant_pre,
236 * except we also track the parent pool to find out which pool we need
237 * to calculate protection values for.
238 *
239 * We can stop the traversal once we find test_pool among the
240 * descendants since we don't really care about any others.
241 */
242 while (pool != test_pool) {
243 next_css = css_next_child(NULL, css);
244 if (next_css) {
245 parent_pool = pool;
246 } else {
247 while (css != &limit_pool->cs->css) {
248 next_css = css_next_child(css, css->parent);
249 if (next_css)
250 break;
251 css = css->parent;
252 parent_pool = pool_parent(parent_pool);
253 }
254 /*
255 * We can only hit this when test_pool is not a
256 * descendant of limit_pool.
257 */
258 if (WARN_ON_ONCE(css == &limit_pool->cs->css))
259 break;
260 }
261 css = next_css;
262
263 found_descendant = false;
264 dmemcg_iter = container_of(css, struct dmemcg_state, css);
265
266 list_for_each_entry_rcu(pool, &dmemcg_iter->pools, css_node) {
267 if (pool_parent(pool) == parent_pool) {
268 found_descendant = true;
269 break;
270 }
271 }
272 if (!found_descendant)
273 continue;
274
> 275 page_counter_calculate_protection(
276 climit, &pool->cnt, true);
277 }
278 rcu_read_unlock();
279 }
280
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v2.1 1/1] kernel/cgroup: Add "dmem" memory accounting cgroup
2024-12-05 2:27 ` kernel test robot
@ 2024-12-05 12:07 ` Maarten Lankhorst
0 siblings, 0 replies; 35+ messages in thread
From: Maarten Lankhorst @ 2024-12-05 12:07 UTC (permalink / raw)
To: kernel test robot, linux-kernel, intel-xe, dri-devel, Tejun Heo,
Zefan Li, Johannes Weiner, Andrew Morton, Friedrich Vock,
Maxime Ripard
Cc: oe-kbuild-all, Linux Memory Management List, cgroups, Maarten Lankhorst
Hey,
Missing a select PAGE_COUNTER in init/Kconfig. I thought I had fixed it,
but I must have forgotten to commit those changes when developing
between 2 machines.
Cheers,
~Maarten
Den 2024-12-05 kl. 03:27, skrev kernel test robot:
> Hi Maarten,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on tj-cgroup/for-next]
> [also build test ERROR on akpm-mm/mm-everything linus/master v6.13-rc1 next-20241204]
> [cannot apply to drm-misc/drm-misc-next drm-tip/drm-tip]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Maarten-Lankhorst/kernel-cgroup-Add-dmem-memory-accounting-cgroup/20241204-233207
> base: https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-next
> patch link: https://lore.kernel.org/r/20241204143112.1250983-1-dev%40lankhorst.se
> patch subject: [PATCH v2.1 1/1] kernel/cgroup: Add "dmem" memory accounting cgroup
> config: um-randconfig-r061-20241205 (https://download.01.org/0day-ci/archive/20241205/202412051039.P06riwrP-lkp@intel.com/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241205/202412051039.P06riwrP-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202412051039.P06riwrP-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> /usr/bin/ld: kernel/cgroup/dmem.o: in function `set_resource_min':
>>> kernel/cgroup/dmem.c:115: undefined reference to `page_counter_set_min'
> /usr/bin/ld: kernel/cgroup/dmem.o: in function `set_resource_low':
>>> kernel/cgroup/dmem.c:121: undefined reference to `page_counter_set_low'
> /usr/bin/ld: kernel/cgroup/dmem.o: in function `set_resource_max':
>>> kernel/cgroup/dmem.c:127: undefined reference to `page_counter_set_max'
> /usr/bin/ld: kernel/cgroup/dmem.o: in function `reset_all_resource_limits':
>>> kernel/cgroup/dmem.c:115: undefined reference to `page_counter_set_min'
>>> /usr/bin/ld: kernel/cgroup/dmem.c:121: undefined reference to `page_counter_set_low'
>>> /usr/bin/ld: kernel/cgroup/dmem.c:127: undefined reference to `page_counter_set_max'
> /usr/bin/ld: kernel/cgroup/dmem.o: in function `dmem_cgroup_uncharge':
>>> kernel/cgroup/dmem.c:607: undefined reference to `page_counter_uncharge'
> /usr/bin/ld: kernel/cgroup/dmem.o: in function `dmem_cgroup_calculate_protection':
>>> kernel/cgroup/dmem.c:275: undefined reference to `page_counter_calculate_protection'
> /usr/bin/ld: kernel/cgroup/dmem.o: in function `dmem_cgroup_try_charge':
>>> kernel/cgroup/dmem.c:657: undefined reference to `page_counter_try_charge'
> collect2: error: ld returned 1 exit status
>
> Kconfig warnings: (for reference only)
> WARNING: unmet direct dependencies detected for GET_FREE_REGION
> Depends on [n]: SPARSEMEM [=n]
> Selected by [y]:
> - RESOURCE_KUNIT_TEST [=y] && RUNTIME_TESTING_MENU [=y] && KUNIT [=y]
>
>
> vim +115 kernel/cgroup/dmem.c
>
> 111
> 112 static void
> 113 set_resource_min(struct dmem_cgroup_pool_state *pool, u64 val)
> 114 {
> > 115 page_counter_set_min(&pool->cnt, val);
> 116 }
> 117
> 118 static void
> 119 set_resource_low(struct dmem_cgroup_pool_state *pool, u64 val)
> 120 {
> > 121 page_counter_set_low(&pool->cnt, val);
> 122 }
> 123
> 124 static void
> 125 set_resource_max(struct dmem_cgroup_pool_state *pool, u64 val)
> 126 {
> > 127 page_counter_set_max(&pool->cnt, val);
> 128 }
> 129
> 130 static u64 get_resource_low(struct dmem_cgroup_pool_state *pool)
> 131 {
> 132 return pool ? READ_ONCE(pool->cnt.low) : 0;
> 133 }
> 134
> 135 static u64 get_resource_min(struct dmem_cgroup_pool_state *pool)
> 136 {
> 137 return pool ? READ_ONCE(pool->cnt.min) : 0;
> 138 }
> 139
> 140 static u64 get_resource_max(struct dmem_cgroup_pool_state *pool)
> 141 {
> 142 return pool ? READ_ONCE(pool->cnt.max) : PAGE_COUNTER_MAX;
> 143 }
> 144
> 145 static u64 get_resource_current(struct dmem_cgroup_pool_state *pool)
> 146 {
> 147 return pool ? page_counter_read(&pool->cnt) : 0;
> 148 }
> 149
> 150 static void reset_all_resource_limits(struct dmem_cgroup_pool_state *rpool)
> 151 {
> 152 set_resource_min(rpool, 0);
> 153 set_resource_low(rpool, 0);
> 154 set_resource_max(rpool, PAGE_COUNTER_MAX);
> 155 }
> 156
> 157 static void dmemcs_offline(struct cgroup_subsys_state *css)
> 158 {
> 159 struct dmemcg_state *dmemcs = css_to_dmemcs(css);
> 160 struct dmem_cgroup_pool_state *pool;
> 161
> 162 rcu_read_lock();
> 163 list_for_each_entry_rcu(pool, &dmemcs->pools, css_node)
> 164 reset_all_resource_limits(pool);
> 165 rcu_read_unlock();
> 166 }
> 167
> 168 static void dmemcs_free(struct cgroup_subsys_state *css)
> 169 {
> 170 struct dmemcg_state *dmemcs = css_to_dmemcs(css);
> 171 struct dmem_cgroup_pool_state *pool, *next;
> 172
> 173 spin_lock(&dmemcg_lock);
> 174 list_for_each_entry_safe(pool, next, &dmemcs->pools, css_node) {
> 175 /*
> 176 *The pool is dead and all references are 0,
> 177 * no need for RCU protection with list_del_rcu or freeing.
> 178 */
> 179 list_del(&pool->css_node);
> 180 free_cg_pool(pool);
> 181 }
> 182 spin_unlock(&dmemcg_lock);
> 183
> 184 kfree(dmemcs);
> 185 }
> 186
> 187 static struct cgroup_subsys_state *
> 188 dmemcs_alloc(struct cgroup_subsys_state *parent_css)
> 189 {
> 190 struct dmemcg_state *dmemcs = kzalloc(sizeof(*dmemcs), GFP_KERNEL);
> 191 if (!dmemcs)
> 192 return ERR_PTR(-ENOMEM);
> 193
> 194 INIT_LIST_HEAD(&dmemcs->pools);
> 195 return &dmemcs->css;
> 196 }
> 197
> 198 static struct dmem_cgroup_pool_state *
> 199 find_cg_pool_locked(struct dmemcg_state *dmemcs, struct dmem_cgroup_region *region)
> 200 {
> 201 struct dmem_cgroup_pool_state *pool;
> 202
> 203 list_for_each_entry_rcu(pool, &dmemcs->pools, css_node, spin_is_locked(&dmemcg_lock))
> 204 if (pool->region == region)
> 205 return pool;
> 206
> 207 return NULL;
> 208 }
> 209
> 210 static struct dmem_cgroup_pool_state *pool_parent(struct dmem_cgroup_pool_state *pool)
> 211 {
> 212 if (!pool->cnt.parent)
> 213 return NULL;
> 214
> 215 return container_of(pool->cnt.parent, typeof(*pool), cnt);
> 216 }
> 217
> 218 static void
> 219 dmem_cgroup_calculate_protection(struct dmem_cgroup_pool_state *limit_pool,
> 220 struct dmem_cgroup_pool_state *test_pool)
> 221 {
> 222 struct page_counter *climit;
> 223 struct cgroup_subsys_state *css, *next_css;
> 224 struct dmemcg_state *dmemcg_iter;
> 225 struct dmem_cgroup_pool_state *pool, *parent_pool;
> 226 bool found_descendant;
> 227
> 228 climit = &limit_pool->cnt;
> 229
> 230 rcu_read_lock();
> 231 parent_pool = pool = limit_pool;
> 232 css = &limit_pool->cs->css;
> 233
> 234 /*
> 235 * This logic is roughly equivalent to css_foreach_descendant_pre,
> 236 * except we also track the parent pool to find out which pool we need
> 237 * to calculate protection values for.
> 238 *
> 239 * We can stop the traversal once we find test_pool among the
> 240 * descendants since we don't really care about any others.
> 241 */
> 242 while (pool != test_pool) {
> 243 next_css = css_next_child(NULL, css);
> 244 if (next_css) {
> 245 parent_pool = pool;
> 246 } else {
> 247 while (css != &limit_pool->cs->css) {
> 248 next_css = css_next_child(css, css->parent);
> 249 if (next_css)
> 250 break;
> 251 css = css->parent;
> 252 parent_pool = pool_parent(parent_pool);
> 253 }
> 254 /*
> 255 * We can only hit this when test_pool is not a
> 256 * descendant of limit_pool.
> 257 */
> 258 if (WARN_ON_ONCE(css == &limit_pool->cs->css))
> 259 break;
> 260 }
> 261 css = next_css;
> 262
> 263 found_descendant = false;
> 264 dmemcg_iter = container_of(css, struct dmemcg_state, css);
> 265
> 266 list_for_each_entry_rcu(pool, &dmemcg_iter->pools, css_node) {
> 267 if (pool_parent(pool) == parent_pool) {
> 268 found_descendant = true;
> 269 break;
> 270 }
> 271 }
> 272 if (!found_descendant)
> 273 continue;
> 274
> > 275 page_counter_calculate_protection(
> 276 climit, &pool->cnt, true);
> 277 }
> 278 rcu_read_unlock();
> 279 }
> 280
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2.1 1/1] kernel/cgroup: Add "dmem" memory accounting cgroup
2024-12-04 14:31 ` [PATCH v2.1 1/1] " Maarten Lankhorst
2024-12-04 21:00 ` kernel test robot
2024-12-05 2:27 ` kernel test robot
@ 2025-01-14 10:16 ` Geert Uytterhoeven
2025-01-14 18:01 ` Maxime Ripard
2 siblings, 1 reply; 35+ messages in thread
From: Geert Uytterhoeven @ 2025-01-14 10:16 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, Maxime Ripard,
cgroups, linux-mm, Maarten Lankhorst
Hi Maarten,
On Wed, Dec 4, 2024 at 3:32 PM Maarten Lankhorst <dev@lankhorst.se> wrote:
> This code is based on the RDMA and misc cgroup initially, but now
> uses page_counter. It uses the same min/low/max semantics as the memory
> cgroup as a result.
>
> There's a small mismatch as TTM uses u64, and page_counter long pages.
> In practice it's not a problem. 32-bits systems don't really come with
> >=4GB cards and as long as we're consistently wrong with units, it's
> fine. The device page size may not be in the same units as kernel page
> size, and each region might also have a different page size (VRAM vs GART
> for example).
>
> The interface is simple:
> - Call dmem_cgroup_register_region()
> - Use dmem_cgroup_try_charge to check if you can allocate a chunk of memory,
> use dmem_cgroup__uncharge when freeing it. This may return an error code,
> or -EAGAIN when the cgroup limit is reached. In that case a reference
> to the limiting pool is returned.
> - The limiting cs can be used as compare function for
> dmem_cgroup_state_evict_valuable.
> - After having evicted enough, drop reference to limiting cs with
> dmem_cgroup_pool_state_put.
>
> This API allows you to limit device resources with cgroups.
> You can see the supported cards in /sys/fs/cgroup/dmem.capacity
> You need to echo +dmem to cgroup.subtree_control, and then you can
> partition device memory.
>
> Co-developed-by: Friedrich Vock <friedrich.vock@gmx.de>
> Signed-off-by: Friedrich Vock <friedrich.vock@gmx.de>
> Co-developed-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
Thanks for your patch, which is now commit b168ed458ddecc17
("kernel/cgroup: Add "dmem" memory accounting cgroup") in drm/drm-next.
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1128,6 +1128,7 @@ config CGROUP_PIDS
>
> config CGROUP_RDMA
> bool "RDMA controller"
> + select PAGE_COUNTER
This change looks unrelated?
Oh, reading your response to the build error, this should have been below?
> help
> Provides enforcement of RDMA resources defined by IB stack.
> It is fairly easy for consumers to exhaust RDMA resources, which
> @@ -1136,6 +1137,15 @@ config CGROUP_RDMA
> Attaching processes with active RDMA resources to the cgroup
> hierarchy is allowed even if can cross the hierarchy's limit.
>
> +config CGROUP_DMEM
> + bool "Device memory controller (DMEM)"
> + help
> + The DMEM controller allows compatible devices to restrict device
> + memory usage based on the cgroup hierarchy.
> +
> + As an example, it allows you to restrict VRAM usage for applications
> + in the DRM subsystem.
> +
Do you envision other users than DRM?
Perhaps this should depend on DRM for now?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v2.1 1/1] kernel/cgroup: Add "dmem" memory accounting cgroup
2025-01-14 10:16 ` Geert Uytterhoeven
@ 2025-01-14 18:01 ` Maxime Ripard
0 siblings, 0 replies; 35+ messages in thread
From: Maxime Ripard @ 2025-01-14 18:01 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Maarten Lankhorst, linux-kernel, intel-xe, dri-devel, Tejun Heo,
Zefan Li, Johannes Weiner, Andrew Morton, Friedrich Vock,
cgroups, linux-mm, Maarten Lankhorst
[-- Attachment #1: Type: text/plain, Size: 3249 bytes --]
Hi Geert,
On Tue, Jan 14, 2025 at 11:16:43AM +0100, Geert Uytterhoeven wrote:
> Hi Maarten,
>
> On Wed, Dec 4, 2024 at 3:32 PM Maarten Lankhorst <dev@lankhorst.se> wrote:
> > This code is based on the RDMA and misc cgroup initially, but now
> > uses page_counter. It uses the same min/low/max semantics as the memory
> > cgroup as a result.
> >
> > There's a small mismatch as TTM uses u64, and page_counter long pages.
> > In practice it's not a problem. 32-bits systems don't really come with
> > >=4GB cards and as long as we're consistently wrong with units, it's
> > fine. The device page size may not be in the same units as kernel page
> > size, and each region might also have a different page size (VRAM vs GART
> > for example).
> >
> > The interface is simple:
> > - Call dmem_cgroup_register_region()
> > - Use dmem_cgroup_try_charge to check if you can allocate a chunk of memory,
> > use dmem_cgroup__uncharge when freeing it. This may return an error code,
> > or -EAGAIN when the cgroup limit is reached. In that case a reference
> > to the limiting pool is returned.
> > - The limiting cs can be used as compare function for
> > dmem_cgroup_state_evict_valuable.
> > - After having evicted enough, drop reference to limiting cs with
> > dmem_cgroup_pool_state_put.
> >
> > This API allows you to limit device resources with cgroups.
> > You can see the supported cards in /sys/fs/cgroup/dmem.capacity
> > You need to echo +dmem to cgroup.subtree_control, and then you can
> > partition device memory.
> >
> > Co-developed-by: Friedrich Vock <friedrich.vock@gmx.de>
> > Signed-off-by: Friedrich Vock <friedrich.vock@gmx.de>
> > Co-developed-by: Maxime Ripard <mripard@kernel.org>
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
>
> Thanks for your patch, which is now commit b168ed458ddecc17
> ("kernel/cgroup: Add "dmem" memory accounting cgroup") in drm/drm-next.
>
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -1128,6 +1128,7 @@ config CGROUP_PIDS
> >
> > config CGROUP_RDMA
> > bool "RDMA controller"
> > + select PAGE_COUNTER
>
> This change looks unrelated?
>
> Oh, reading your response to the build error, this should have been below?
Indeed, good catch.
> > help
> > Provides enforcement of RDMA resources defined by IB stack.
> > It is fairly easy for consumers to exhaust RDMA resources, which
> > @@ -1136,6 +1137,15 @@ config CGROUP_RDMA
> > Attaching processes with active RDMA resources to the cgroup
> > hierarchy is allowed even if can cross the hierarchy's limit.
> >
> > +config CGROUP_DMEM
> > + bool "Device memory controller (DMEM)"
> > + help
> > + The DMEM controller allows compatible devices to restrict device
> > + memory usage based on the cgroup hierarchy.
> > +
> > + As an example, it allows you to restrict VRAM usage for applications
> > + in the DRM subsystem.
> > +
>
> Do you envision other users than DRM?
> Perhaps this should depend on DRM for now?
dma-buf heaps and v4l2 support are in progress right now.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 1/7] kernel/cgroup: Add "dmem" memory accounting cgroup
2024-12-04 13:44 ` [PATCH v2 1/7] kernel/cgroup: " Maarten Lankhorst
2024-12-04 14:31 ` [PATCH v2.1 1/1] " Maarten Lankhorst
@ 2025-01-06 17:48 ` Michal Koutný
1 sibling, 0 replies; 35+ messages in thread
From: Michal Koutný @ 2025-01-06 17:48 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, Maxime Ripard,
cgroups, linux-mm, Maarten Lankhorst
[-- Attachment #1: Type: text/plain, Size: 1713 bytes --]
Hello.
On Wed, Dec 04, 2024 at 02:44:01PM +0100, Maarten Lankhorst <dev@lankhorst.se> wrote:
> +bool dmem_cgroup_state_evict_valuable(struct dmem_cgroup_pool_state *limit_pool,
> + struct dmem_cgroup_pool_state *test_pool,
> + bool ignore_low, bool *ret_hit_low)
> +{
> + struct dmem_cgroup_pool_state *pool = test_pool;
> + struct page_counter *climit, *ctest;
> + u64 used, min, low;
> +
> + /* Can always evict from current pool, despite limits */
> + if (limit_pool == test_pool)
> + return true;
> +
> + if (limit_pool) {
> + if (!parent_dmemcs(limit_pool->cs))
> + return true;
> +
> + for (pool = test_pool; pool && limit_pool != pool; pool = pool_parent(pool))
> + {}
> +
> + if (!pool)
> + return false;
> + } else {
> + /*
> + * If there is no cgroup limiting memory usage, use the root
> + * cgroup instead for limit calculations.
> + */
> + for (limit_pool = test_pool; pool_parent(limit_pool); limit_pool = pool_parent(limit_pool))
> + {}
> + }
I'm trying to understand the two branches above. If limit_pool is a root
one, eviction is granted and no protection is evaluated.
Then it checks that test_pool is below limit_pool (can this ever fail,
given the limit_pool must have been above when charging in test_pool?).
(OK, this may be called arbitrarily by modules.)
I think it could be simplified and corrected like this:
/* Resolve NULL limit_pool */
if (!limit_pool)
for (limit_pool = test_pool; pool_parent(limit_pool); limit_pool = pool_parent(limit_pool));
/* Check ancestry */
if (!cgroup_is_descendant(test_pool->cs->css.cgroup, limit_pool->cs->css.cgroup))
return false;
HTH,
Michal
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2 2/7] drm/drv: Add drmm managed registration helper for dmem cgroups.
2024-12-04 13:44 [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup Maarten Lankhorst
2024-12-04 13:44 ` [PATCH v2 1/7] kernel/cgroup: " Maarten Lankhorst
@ 2024-12-04 13:44 ` Maarten Lankhorst
2024-12-04 13:44 ` [PATCH v2 3/7] drm/ttm: Handle cgroup based eviction in TTM Maarten Lankhorst
` (6 subsequent siblings)
8 siblings, 0 replies; 35+ messages in thread
From: Maarten Lankhorst @ 2024-12-04 13:44 UTC (permalink / raw)
To: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, Maxime Ripard
Cc: cgroups, linux-mm, Maarten Lankhorst, Maarten Lankhorst
From: Maxime Ripard <mripard@kernel.org>
Drivers will need to register dmem regions at probe time, so let's
give them a drm-managed helper.
Signed-off-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
---
drivers/gpu/drm/drm_drv.c | 32 ++++++++++++++++++++++++++++++++
include/drm/drm_drv.h | 5 +++++
2 files changed, 37 insertions(+)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index c2c172eb25df7..3cf440eee8a2a 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -26,6 +26,7 @@
* DEALINGS IN THE SOFTWARE.
*/
+#include <linux/cgroup_dmem.h>
#include <linux/debugfs.h>
#include <linux/fs.h>
#include <linux/module.h>
@@ -820,6 +821,37 @@ void drm_dev_put(struct drm_device *dev)
}
EXPORT_SYMBOL(drm_dev_put);
+static void drmm_cg_unregister_region(struct drm_device *dev, void *arg)
+{
+ dmem_cgroup_unregister_region(arg);
+}
+
+/**
+ * drmm_cgroup_register_region - Register a region of a DRM device to cgroups
+ * @dev: device for region
+ * @region_name: Region name for registering
+ * @size: Size of region in bytes
+ *
+ * This decreases the ref-count of @dev by one. The device is destroyed if the
+ * ref-count drops to zero.
+ */
+struct dmem_cgroup_region *drmm_cgroup_register_region(struct drm_device *dev, const char *region_name, u64 size)
+{
+ struct dmem_cgroup_region *region;
+ int ret;
+
+ region = dmem_cgroup_register_region(size, "drm/%s/%s", dev->unique, region_name);
+ if (IS_ERR_OR_NULL(region))
+ return region;
+
+ ret = drmm_add_action_or_reset(dev, drmm_cg_unregister_region, region);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return region;
+}
+EXPORT_SYMBOL_GPL(drmm_cgroup_register_region);
+
static int create_compat_control_link(struct drm_device *dev)
{
struct drm_minor *minor;
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 1bbbcb8e2d231..7dd49d9ab5058 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -34,6 +34,7 @@
#include <drm/drm_device.h>
+struct dmem_cgroup_region;
struct drm_fb_helper;
struct drm_fb_helper_surface_size;
struct drm_file;
@@ -438,6 +439,10 @@ void *__devm_drm_dev_alloc(struct device *parent,
const struct drm_driver *driver,
size_t size, size_t offset);
+struct dmem_cgroup_region *
+drmm_cgroup_register_region(struct drm_device *dev,
+ const char *region_name, u64 size);
+
/**
* devm_drm_dev_alloc - Resource managed allocation of a &drm_device instance
* @parent: Parent device object
--
2.43.0
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH v2 3/7] drm/ttm: Handle cgroup based eviction in TTM
2024-12-04 13:44 [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup Maarten Lankhorst
2024-12-04 13:44 ` [PATCH v2 1/7] kernel/cgroup: " Maarten Lankhorst
2024-12-04 13:44 ` [PATCH v2 2/7] drm/drv: Add drmm managed registration helper for dmem cgroups Maarten Lankhorst
@ 2024-12-04 13:44 ` Maarten Lankhorst
2024-12-04 13:44 ` [PATCH v2 4/7] drm/xe: Implement cgroup for vram Maarten Lankhorst
` (5 subsequent siblings)
8 siblings, 0 replies; 35+ messages in thread
From: Maarten Lankhorst @ 2024-12-04 13:44 UTC (permalink / raw)
To: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, Maxime Ripard
Cc: cgroups, linux-mm, Maarten Lankhorst, Maarten Lankhorst
cgroup resource allocation has to be handled in TTM, so -EAGAIN from
cgroups can be converted into -ENOSPC, and the limitcg can be properly
evicted in ttm code.
When hitting a resource limit through -EAGAIN, the cgroup for which the
limit is hit is also returned. This allows eviction to delete only from
cgroups which are a subgroup of the current cgroup.
The returned CSS is used to determine if eviction is valuable for a
given resource, and allows TTM to only target specific resources to
lower memory usage.
Co-developed-by: Friedrich Vock <friedrich.vock@gmx.de>
Signed-off-by: Friedrich Vock <friedrich.vock@gmx.de>
Co-developed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
---
drivers/gpu/drm/ttm/tests/ttm_bo_test.c | 18 +++----
.../gpu/drm/ttm/tests/ttm_bo_validate_test.c | 4 +-
drivers/gpu/drm/ttm/tests/ttm_resource_test.c | 2 +-
drivers/gpu/drm/ttm/ttm_bo.c | 54 ++++++++++++++++---
drivers/gpu/drm/ttm/ttm_resource.c | 23 +++++++-
include/drm/ttm/ttm_resource.h | 12 ++++-
6 files changed, 90 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
index 3139fd9128d84..f8f20d2f61740 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
@@ -258,13 +258,13 @@ static void ttm_bo_unreserve_basic(struct kunit *test)
bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
bo->priority = bo_prio;
- err = ttm_resource_alloc(bo, place, &res1);
+ err = ttm_resource_alloc(bo, place, &res1, NULL);
KUNIT_ASSERT_EQ(test, err, 0);
bo->resource = res1;
/* Add a dummy resource to populate LRU */
- ttm_resource_alloc(bo, place, &res2);
+ ttm_resource_alloc(bo, place, &res2, NULL);
dma_resv_lock(bo->base.resv, NULL);
ttm_bo_unreserve(bo);
@@ -300,12 +300,12 @@ static void ttm_bo_unreserve_pinned(struct kunit *test)
dma_resv_lock(bo->base.resv, NULL);
ttm_bo_pin(bo);
- err = ttm_resource_alloc(bo, place, &res1);
+ err = ttm_resource_alloc(bo, place, &res1, NULL);
KUNIT_ASSERT_EQ(test, err, 0);
bo->resource = res1;
/* Add a dummy resource to the pinned list */
- err = ttm_resource_alloc(bo, place, &res2);
+ err = ttm_resource_alloc(bo, place, &res2, NULL);
KUNIT_ASSERT_EQ(test, err, 0);
KUNIT_ASSERT_EQ(test,
list_is_last(&res2->lru.link, &priv->ttm_dev->unevictable), 1);
@@ -355,7 +355,7 @@ static void ttm_bo_unreserve_bulk(struct kunit *test)
ttm_bo_set_bulk_move(bo1, &lru_bulk_move);
dma_resv_unlock(bo1->base.resv);
- err = ttm_resource_alloc(bo1, place, &res1);
+ err = ttm_resource_alloc(bo1, place, &res1, NULL);
KUNIT_ASSERT_EQ(test, err, 0);
bo1->resource = res1;
@@ -363,7 +363,7 @@ static void ttm_bo_unreserve_bulk(struct kunit *test)
ttm_bo_set_bulk_move(bo2, &lru_bulk_move);
dma_resv_unlock(bo2->base.resv);
- err = ttm_resource_alloc(bo2, place, &res2);
+ err = ttm_resource_alloc(bo2, place, &res2, NULL);
KUNIT_ASSERT_EQ(test, err, 0);
bo2->resource = res2;
@@ -401,7 +401,7 @@ static void ttm_bo_put_basic(struct kunit *test)
bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
bo->type = ttm_bo_type_device;
- err = ttm_resource_alloc(bo, place, &res);
+ err = ttm_resource_alloc(bo, place, &res, NULL);
KUNIT_ASSERT_EQ(test, err, 0);
bo->resource = res;
@@ -518,7 +518,7 @@ static void ttm_bo_pin_unpin_resource(struct kunit *test)
bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
- err = ttm_resource_alloc(bo, place, &res);
+ err = ttm_resource_alloc(bo, place, &res, NULL);
KUNIT_ASSERT_EQ(test, err, 0);
bo->resource = res;
@@ -569,7 +569,7 @@ static void ttm_bo_multiple_pin_one_unpin(struct kunit *test)
bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
- err = ttm_resource_alloc(bo, place, &res);
+ err = ttm_resource_alloc(bo, place, &res, NULL);
KUNIT_ASSERT_EQ(test, err, 0);
bo->resource = res;
diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
index 1adf18481ea05..3148f5d3dbd66 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_validate_test.c
@@ -542,7 +542,7 @@ static void ttm_bo_validate_no_placement_signaled(struct kunit *test)
bo->ttm = old_tt;
}
- err = ttm_resource_alloc(bo, place, &bo->resource);
+ err = ttm_resource_alloc(bo, place, &bo->resource, NULL);
KUNIT_EXPECT_EQ(test, err, 0);
KUNIT_ASSERT_EQ(test, man->usage, size);
@@ -603,7 +603,7 @@ static void ttm_bo_validate_no_placement_not_signaled(struct kunit *test)
bo = ttm_bo_kunit_init(test, test->priv, size, NULL);
bo->type = params->bo_type;
- err = ttm_resource_alloc(bo, place, &bo->resource);
+ err = ttm_resource_alloc(bo, place, &bo->resource, NULL);
KUNIT_EXPECT_EQ(test, err, 0);
placement = kunit_kzalloc(test, sizeof(*placement), GFP_KERNEL);
diff --git a/drivers/gpu/drm/ttm/tests/ttm_resource_test.c b/drivers/gpu/drm/ttm/tests/ttm_resource_test.c
index a9f4b81921c3c..e6ea2bd01f07a 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_resource_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_resource_test.c
@@ -302,7 +302,7 @@ static void ttm_sys_man_free_basic(struct kunit *test)
res = kunit_kzalloc(test, sizeof(*res), GFP_KERNEL);
KUNIT_ASSERT_NOT_NULL(test, res);
- ttm_resource_alloc(bo, place, &res);
+ ttm_resource_alloc(bo, place, &res, NULL);
man = ttm_manager_type(priv->devs->ttm_dev, mem_type);
man->func->free(man, res);
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 48c5365efca1c..aab49dee65a76 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -42,6 +42,7 @@
#include <linux/file.h>
#include <linux/module.h>
#include <linux/atomic.h>
+#include <linux/cgroup_dmem.h>
#include <linux/dma-resv.h>
#include "ttm_module.h"
@@ -447,7 +448,7 @@ int ttm_bo_evict_first(struct ttm_device *bdev, struct ttm_resource_manager *man
struct ttm_buffer_object *bo;
struct ttm_resource *res;
unsigned int mem_type;
- int ret = 0;
+ int ret;
spin_lock(&bdev->lru_lock);
res = ttm_resource_manager_first(man, &cursor);
@@ -499,6 +500,13 @@ struct ttm_bo_evict_walk {
struct ttm_resource **res;
/** @evicted: Number of successful evictions. */
unsigned long evicted;
+
+ /** @limit_pool: Which pool limit we should test against */
+ struct dmem_cgroup_pool_state *limit_pool;
+ /** @try_low: Whether we should attempt to evict BO's with low watermark threshold */
+ bool try_low;
+ /** @hit_low: If we cannot evict a bo when @try_low is false (first pass) */
+ bool hit_low;
};
static s64 ttm_bo_evict_cb(struct ttm_lru_walk *walk, struct ttm_buffer_object *bo)
@@ -507,6 +515,10 @@ static s64 ttm_bo_evict_cb(struct ttm_lru_walk *walk, struct ttm_buffer_object *
container_of(walk, typeof(*evict_walk), walk);
s64 lret;
+ if (!dmem_cgroup_state_evict_valuable(evict_walk->limit_pool, bo->resource->css,
+ evict_walk->try_low, &evict_walk->hit_low))
+ return 0;
+
if (bo->pin_count || !bo->bdev->funcs->eviction_valuable(bo, evict_walk->place))
return 0;
@@ -524,7 +536,7 @@ static s64 ttm_bo_evict_cb(struct ttm_lru_walk *walk, struct ttm_buffer_object *
evict_walk->evicted++;
if (evict_walk->res)
lret = ttm_resource_alloc(evict_walk->evictor, evict_walk->place,
- evict_walk->res);
+ evict_walk->res, NULL);
if (lret == 0)
return 1;
out:
@@ -545,7 +557,8 @@ static int ttm_bo_evict_alloc(struct ttm_device *bdev,
struct ttm_buffer_object *evictor,
struct ttm_operation_ctx *ctx,
struct ww_acquire_ctx *ticket,
- struct ttm_resource **res)
+ struct ttm_resource **res,
+ struct dmem_cgroup_pool_state *limit_pool)
{
struct ttm_bo_evict_walk evict_walk = {
.walk = {
@@ -556,22 +569,39 @@ static int ttm_bo_evict_alloc(struct ttm_device *bdev,
.place = place,
.evictor = evictor,
.res = res,
+ .limit_pool = limit_pool,
};
s64 lret;
evict_walk.walk.trylock_only = true;
lret = ttm_lru_walk_for_evict(&evict_walk.walk, bdev, man, 1);
+
+ /* One more attempt if we hit low limit? */
+ if (!lret && evict_walk.hit_low) {
+ evict_walk.try_low = true;
+ lret = ttm_lru_walk_for_evict(&evict_walk.walk, bdev, man, 1);
+ }
if (lret || !ticket)
goto out;
+ /* Reset low limit */
+ evict_walk.try_low = evict_walk.hit_low = false;
/* If ticket-locking, repeat while making progress. */
evict_walk.walk.trylock_only = false;
+
+retry:
do {
/* The walk may clear the evict_walk.walk.ticket field */
evict_walk.walk.ticket = ticket;
evict_walk.evicted = 0;
lret = ttm_lru_walk_for_evict(&evict_walk.walk, bdev, man, 1);
} while (!lret && evict_walk.evicted);
+
+ /* We hit the low limit? Try once more */
+ if (!lret && evict_walk.hit_low && !evict_walk.try_low) {
+ evict_walk.try_low = true;
+ goto retry;
+ }
out:
if (lret < 0)
return lret;
@@ -689,6 +719,7 @@ static int ttm_bo_alloc_resource(struct ttm_buffer_object *bo,
for (i = 0; i < placement->num_placement; ++i) {
const struct ttm_place *place = &placement->placement[i];
+ struct dmem_cgroup_pool_state *limit_pool = NULL;
struct ttm_resource_manager *man;
bool may_evict;
@@ -701,15 +732,20 @@ static int ttm_bo_alloc_resource(struct ttm_buffer_object *bo,
continue;
may_evict = (force_space && place->mem_type != TTM_PL_SYSTEM);
- ret = ttm_resource_alloc(bo, place, res);
+ ret = ttm_resource_alloc(bo, place, res, force_space ? &limit_pool : NULL);
if (ret) {
- if (ret != -ENOSPC)
+ if (ret != -ENOSPC && ret != -EAGAIN) {
+ dmem_cgroup_pool_state_put(limit_pool);
return ret;
- if (!may_evict)
+ }
+ if (!may_evict) {
+ dmem_cgroup_pool_state_put(limit_pool);
continue;
+ }
ret = ttm_bo_evict_alloc(bdev, man, place, bo, ctx,
- ticket, res);
+ ticket, res, limit_pool);
+ dmem_cgroup_pool_state_put(limit_pool);
if (ret == -EBUSY)
continue;
if (ret)
@@ -1056,6 +1092,8 @@ struct ttm_bo_swapout_walk {
struct ttm_lru_walk walk;
/** @gfp_flags: The gfp flags to use for ttm_tt_swapout() */
gfp_t gfp_flags;
+
+ bool hit_low, evict_low;
};
static s64
@@ -1106,7 +1144,7 @@ ttm_bo_swapout_cb(struct ttm_lru_walk *walk, struct ttm_buffer_object *bo)
memset(&hop, 0, sizeof(hop));
place.mem_type = TTM_PL_SYSTEM;
- ret = ttm_resource_alloc(bo, &place, &evict_mem);
+ ret = ttm_resource_alloc(bo, &place, &evict_mem, NULL);
if (ret)
goto out;
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index a87665eb28a62..cc29bbf3eabb1 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -26,6 +26,7 @@
#include <linux/io-mapping.h>
#include <linux/iosys-map.h>
#include <linux/scatterlist.h>
+#include <linux/cgroup_dmem.h>
#include <drm/ttm/ttm_bo.h>
#include <drm/ttm/ttm_placement.h>
@@ -350,15 +351,28 @@ EXPORT_SYMBOL(ttm_resource_fini);
int ttm_resource_alloc(struct ttm_buffer_object *bo,
const struct ttm_place *place,
- struct ttm_resource **res_ptr)
+ struct ttm_resource **res_ptr,
+ struct dmem_cgroup_pool_state **ret_limit_pool)
{
struct ttm_resource_manager *man =
ttm_manager_type(bo->bdev, place->mem_type);
+ struct dmem_cgroup_pool_state *pool = NULL;
int ret;
+ if (man->cg) {
+ ret = dmem_cgroup_try_charge(man->cg, bo->base.size, &pool, ret_limit_pool);
+ if (ret)
+ return ret;
+ }
+
ret = man->func->alloc(man, bo, place, res_ptr);
- if (ret)
+ if (ret) {
+ if (pool)
+ dmem_cgroup_uncharge(pool, bo->base.size);
return ret;
+ }
+
+ (*res_ptr)->css = pool;
spin_lock(&bo->bdev->lru_lock);
ttm_resource_add_bulk_move(*res_ptr, bo);
@@ -370,6 +384,7 @@ EXPORT_SYMBOL_FOR_TESTS_ONLY(ttm_resource_alloc);
void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res)
{
struct ttm_resource_manager *man;
+ struct dmem_cgroup_pool_state *pool;
if (!*res)
return;
@@ -377,9 +392,13 @@ void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res)
spin_lock(&bo->bdev->lru_lock);
ttm_resource_del_bulk_move(*res, bo);
spin_unlock(&bo->bdev->lru_lock);
+
+ pool = (*res)->css;
man = ttm_manager_type(bo->bdev, (*res)->mem_type);
man->func->free(man, *res);
*res = NULL;
+ if (man->cg)
+ dmem_cgroup_uncharge(pool, bo->base.size);
}
EXPORT_SYMBOL(ttm_resource_free);
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index be034be56ba1b..ee688d0c029b3 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -38,6 +38,7 @@
#define TTM_MAX_BO_PRIORITY 4U
#define TTM_NUM_MEM_TYPES 8
+struct dmem_cgroup_device;
struct ttm_device;
struct ttm_resource_manager;
struct ttm_resource;
@@ -211,6 +212,11 @@ struct ttm_resource_manager {
* bdev->lru_lock.
*/
uint64_t usage;
+
+ /**
+ * @cg: &dmem_cgroup_region used for memory accounting, if not NULL.
+ */
+ struct dmem_cgroup_region *cg;
};
/**
@@ -239,6 +245,7 @@ struct ttm_bus_placement {
* @placement: Placement flags.
* @bus: Placement on io bus accessible to the CPU
* @bo: weak reference to the BO, protected by ttm_device::lru_lock
+ * @css: cgroup state this resource is charged to
*
* Structure indicating the placement and space resources used by a
* buffer object.
@@ -251,6 +258,8 @@ struct ttm_resource {
struct ttm_bus_placement bus;
struct ttm_buffer_object *bo;
+ struct dmem_cgroup_pool_state *css;
+
/**
* @lru: Least recently used list, see &ttm_resource_manager.lru
*/
@@ -432,7 +441,8 @@ void ttm_resource_fini(struct ttm_resource_manager *man,
int ttm_resource_alloc(struct ttm_buffer_object *bo,
const struct ttm_place *place,
- struct ttm_resource **res);
+ struct ttm_resource **res,
+ struct dmem_cgroup_pool_state **ret_limit_pool);
void ttm_resource_free(struct ttm_buffer_object *bo, struct ttm_resource **res);
bool ttm_resource_intersects(struct ttm_device *bdev,
struct ttm_resource *res,
--
2.43.0
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH v2 4/7] drm/xe: Implement cgroup for vram
2024-12-04 13:44 [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup Maarten Lankhorst
` (2 preceding siblings ...)
2024-12-04 13:44 ` [PATCH v2 3/7] drm/ttm: Handle cgroup based eviction in TTM Maarten Lankhorst
@ 2024-12-04 13:44 ` Maarten Lankhorst
2024-12-19 12:03 ` Maxime Ripard
2024-12-04 13:44 ` [PATCH v2 5/7] drm/amdgpu: Add cgroups implementation Maarten Lankhorst
` (4 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: Maarten Lankhorst @ 2024-12-04 13:44 UTC (permalink / raw)
To: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, Maxime Ripard
Cc: cgroups, linux-mm, Maarten Lankhorst, Maarten Lankhorst
Add vram based cgroup eviction to Xe.
Most hardware with VRAM uses TTM for its management, and can be
similarly trivially enabled.
Co-developed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
---
drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
index c95728c45ea48..f4a16e5fa7700 100644
--- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
+++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
@@ -5,6 +5,7 @@
*/
#include <drm/drm_managed.h>
+#include <drm/drm_drv.h>
#include <drm/ttm/ttm_placement.h>
#include <drm/ttm/ttm_range_manager.h>
@@ -311,6 +312,13 @@ int __xe_ttm_vram_mgr_init(struct xe_device *xe, struct xe_ttm_vram_mgr *mgr,
struct ttm_resource_manager *man = &mgr->manager;
int err;
+ if (mem_type != XE_PL_STOLEN) {
+ const char *name = mem_type == XE_PL_VRAM0 ? "vram0" : "vram1";
+ man->cg = drmm_cgroup_register_region(&xe->drm, name, size);
+ if (IS_ERR(man->cg))
+ return PTR_ERR(man->cg);
+ }
+
man->func = &xe_ttm_vram_mgr_func;
mgr->mem_type = mem_type;
mutex_init(&mgr->lock);
--
2.43.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v2 4/7] drm/xe: Implement cgroup for vram
2024-12-04 13:44 ` [PATCH v2 4/7] drm/xe: Implement cgroup for vram Maarten Lankhorst
@ 2024-12-19 12:03 ` Maxime Ripard
2024-12-20 14:22 ` Rodrigo Vivi
0 siblings, 1 reply; 35+ messages in thread
From: Maxime Ripard @ 2024-12-19 12:03 UTC (permalink / raw)
To: Lucas De Marchi, Thomas Hellström, Rodrigo Vivi
Cc: Maarten Lankhorst, linux-kernel, intel-xe, dri-devel, Tejun Heo,
Zefan Li, Johannes Weiner, Andrew Morton, Friedrich Vock,
cgroups, linux-mm, Maarten Lankhorst
[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]
Hi Lucas, Thomas, Rodrigo,
We forgot to Cc you on this series, sorry. Could you have a look at it,
and especially the following patch?
On Wed, Dec 04, 2024 at 02:44:04PM +0100, Maarten Lankhorst wrote:
> Add vram based cgroup eviction to Xe.
> Most hardware with VRAM uses TTM for its management, and can be
> similarly trivially enabled.
>
> Co-developed-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> ---
> drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> index c95728c45ea48..f4a16e5fa7700 100644
> --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> @@ -5,6 +5,7 @@
> */
>
> #include <drm/drm_managed.h>
> +#include <drm/drm_drv.h>
>
> #include <drm/ttm/ttm_placement.h>
> #include <drm/ttm/ttm_range_manager.h>
> @@ -311,6 +312,13 @@ int __xe_ttm_vram_mgr_init(struct xe_device *xe, struct xe_ttm_vram_mgr *mgr,
> struct ttm_resource_manager *man = &mgr->manager;
> int err;
>
> + if (mem_type != XE_PL_STOLEN) {
> + const char *name = mem_type == XE_PL_VRAM0 ? "vram0" : "vram1";
> + man->cg = drmm_cgroup_register_region(&xe->drm, name, size);
> + if (IS_ERR(man->cg))
> + return PTR_ERR(man->cg);
> + }
> +
> man->func = &xe_ttm_vram_mgr_func;
> mgr->mem_type = mem_type;
> mutex_init(&mgr->lock);
> --
> 2.43.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v2 4/7] drm/xe: Implement cgroup for vram
2024-12-19 12:03 ` Maxime Ripard
@ 2024-12-20 14:22 ` Rodrigo Vivi
0 siblings, 0 replies; 35+ messages in thread
From: Rodrigo Vivi @ 2024-12-20 14:22 UTC (permalink / raw)
To: Maxime Ripard
Cc: Lucas De Marchi, Thomas Hellström, Maarten Lankhorst,
linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, cgroups,
linux-mm, Maarten Lankhorst
On Thu, Dec 19, 2024 at 01:03:48PM +0100, Maxime Ripard wrote:
> Hi Lucas, Thomas, Rodrigo,
>
> We forgot to Cc you on this series, sorry. Could you have a look at it,
> and especially the following patch?
I'm sorry for the delay here.
I was following the thread on the side.
I'm glad to see that we are finally going to get some cgroups support!
Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Fell free to push this through any tree...
>
>
>
> On Wed, Dec 04, 2024 at 02:44:04PM +0100, Maarten Lankhorst wrote:
> > Add vram based cgroup eviction to Xe.
> > Most hardware with VRAM uses TTM for its management, and can be
> > similarly trivially enabled.
> >
> > Co-developed-by: Maxime Ripard <mripard@kernel.org>
> > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> > ---
> > drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> > index c95728c45ea48..f4a16e5fa7700 100644
> > --- a/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> > +++ b/drivers/gpu/drm/xe/xe_ttm_vram_mgr.c
> > @@ -5,6 +5,7 @@
> > */
> >
> > #include <drm/drm_managed.h>
> > +#include <drm/drm_drv.h>
> >
> > #include <drm/ttm/ttm_placement.h>
> > #include <drm/ttm/ttm_range_manager.h>
> > @@ -311,6 +312,13 @@ int __xe_ttm_vram_mgr_init(struct xe_device *xe, struct xe_ttm_vram_mgr *mgr,
> > struct ttm_resource_manager *man = &mgr->manager;
> > int err;
> >
> > + if (mem_type != XE_PL_STOLEN) {
> > + const char *name = mem_type == XE_PL_VRAM0 ? "vram0" : "vram1";
> > + man->cg = drmm_cgroup_register_region(&xe->drm, name, size);
> > + if (IS_ERR(man->cg))
> > + return PTR_ERR(man->cg);
> > + }
> > +
> > man->func = &xe_ttm_vram_mgr_func;
> > mgr->mem_type = mem_type;
> > mutex_init(&mgr->lock);
> > --
> > 2.43.0
> >
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2 5/7] drm/amdgpu: Add cgroups implementation
2024-12-04 13:44 [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup Maarten Lankhorst
` (3 preceding siblings ...)
2024-12-04 13:44 ` [PATCH v2 4/7] drm/xe: Implement cgroup for vram Maarten Lankhorst
@ 2024-12-04 13:44 ` Maarten Lankhorst
2024-12-19 12:01 ` Maxime Ripard
2024-12-04 13:44 ` [PATCH v2 6/7] drm/xe: Hack to test with mapped pages instead of vram Maarten Lankhorst
` (3 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: Maarten Lankhorst @ 2024-12-04 13:44 UTC (permalink / raw)
To: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, Maxime Ripard
Cc: cgroups, linux-mm, Maarten Lankhorst, Maarten Lankhorst
Similar to xe, enable some simple management of VRAM only.
Co-developed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 7d26a962f811c..f1703a746cadd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -24,6 +24,7 @@
#include <linux/dma-mapping.h>
#include <drm/ttm/ttm_range_manager.h>
+#include <drm/drm_drv.h>
#include "amdgpu.h"
#include "amdgpu_vm.h"
@@ -908,6 +909,9 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev)
struct ttm_resource_manager *man = &mgr->manager;
int err;
+ man->cg = drmm_cgroup_register_region(adev_to_drm(adev), "vram", adev->gmc.real_vram_size);
+ if (IS_ERR(man->cg))
+ return PTR_ERR(man->cg);
ttm_resource_manager_init(man, &adev->mman.bdev,
adev->gmc.real_vram_size);
--
2.43.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v2 5/7] drm/amdgpu: Add cgroups implementation
2024-12-04 13:44 ` [PATCH v2 5/7] drm/amdgpu: Add cgroups implementation Maarten Lankhorst
@ 2024-12-19 12:01 ` Maxime Ripard
0 siblings, 0 replies; 35+ messages in thread
From: Maxime Ripard @ 2024-12-19 12:01 UTC (permalink / raw)
To: Alex Deucher, Christian König, Xinhui Pan
Cc: Maarten Lankhorst, linux-kernel, intel-xe, dri-devel, Tejun Heo,
Zefan Li, Johannes Weiner, Andrew Morton, Friedrich Vock,
cgroups, linux-mm, Maarten Lankhorst, amd-gfx
[-- Attachment #1: Type: text/plain, Size: 1415 bytes --]
Hi Alex, Christian, Xinhui,
We forgot to Cc you on that series, sorry. Could you have a look at the following patch?
Thanks!
Maxime
On Wed, Dec 04, 2024 at 02:44:05PM +0100, Maarten Lankhorst wrote:
> Similar to xe, enable some simple management of VRAM only.
>
> Co-developed-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> index 7d26a962f811c..f1703a746cadd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
> @@ -24,6 +24,7 @@
>
> #include <linux/dma-mapping.h>
> #include <drm/ttm/ttm_range_manager.h>
> +#include <drm/drm_drv.h>
>
> #include "amdgpu.h"
> #include "amdgpu_vm.h"
> @@ -908,6 +909,9 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev)
> struct ttm_resource_manager *man = &mgr->manager;
> int err;
>
> + man->cg = drmm_cgroup_register_region(adev_to_drm(adev), "vram", adev->gmc.real_vram_size);
> + if (IS_ERR(man->cg))
> + return PTR_ERR(man->cg);
> ttm_resource_manager_init(man, &adev->mman.bdev,
> adev->gmc.real_vram_size);
>
> --
> 2.43.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v2 6/7] drm/xe: Hack to test with mapped pages instead of vram.
2024-12-04 13:44 [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup Maarten Lankhorst
` (4 preceding siblings ...)
2024-12-04 13:44 ` [PATCH v2 5/7] drm/amdgpu: Add cgroups implementation Maarten Lankhorst
@ 2024-12-04 13:44 ` Maarten Lankhorst
2024-12-04 13:44 ` [PATCH v2 7/7] drm/gem: Add cgroup memory accounting for VRAM helper Maarten Lankhorst
` (2 subsequent siblings)
8 siblings, 0 replies; 35+ messages in thread
From: Maarten Lankhorst @ 2024-12-04 13:44 UTC (permalink / raw)
To: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, Maxime Ripard
Cc: cgroups, linux-mm, Maarten Lankhorst, Maarten Lankhorst
We will probably want to make this a proper region in TTM for
everything, so that we can charge VRAM twice, once for mapped
in sysmem, once for mapped in vram. That way we don't need to
deal with evict failing from lack of available memory in mapped.
Signed-off-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
---
drivers/gpu/drm/xe/xe_ttm_sys_mgr.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/xe/xe_ttm_sys_mgr.c b/drivers/gpu/drm/xe/xe_ttm_sys_mgr.c
index 9844a8edbfe19..5450caaef52ad 100644
--- a/drivers/gpu/drm/xe/xe_ttm_sys_mgr.c
+++ b/drivers/gpu/drm/xe/xe_ttm_sys_mgr.c
@@ -11,6 +11,7 @@
#include <drm/ttm/ttm_placement.h>
#include <drm/ttm/ttm_range_manager.h>
#include <drm/ttm/ttm_tt.h>
+#include <drm/drm_drv.h>
#include "xe_bo.h"
#include "xe_gt.h"
@@ -112,6 +113,10 @@ int xe_ttm_sys_mgr_init(struct xe_device *xe)
/* TTM limits allocation of all TTM devices by 50% of system memory */
gtt_size /= 2;
+ man->cg = drmm_cgroup_register_region(&xe->drm, "mapped", gtt_size);
+ if (IS_ERR(man->cg))
+ return PTR_ERR(man->cg);
+
man->use_tt = true;
man->func = &xe_ttm_sys_mgr_func;
ttm_resource_manager_init(man, &xe->ttm, gtt_size >> PAGE_SHIFT);
--
2.43.0
^ permalink raw reply [flat|nested] 35+ messages in thread* [PATCH v2 7/7] drm/gem: Add cgroup memory accounting for VRAM helper.
2024-12-04 13:44 [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup Maarten Lankhorst
` (5 preceding siblings ...)
2024-12-04 13:44 ` [PATCH v2 6/7] drm/xe: Hack to test with mapped pages instead of vram Maarten Lankhorst
@ 2024-12-04 13:44 ` Maarten Lankhorst
2024-12-08 12:15 ` [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup Friedrich Vock
2024-12-13 13:03 ` Maxime Ripard
8 siblings, 0 replies; 35+ messages in thread
From: Maarten Lankhorst @ 2024-12-04 13:44 UTC (permalink / raw)
To: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, Maxime Ripard
Cc: cgroups, linux-mm, Maarten Lankhorst, Maarten Lankhorst
From: Maxime Ripard <mripard@kernel.org>
This allows any driver using the VRAM helper to set limits on VRAM using
cgroup.
Signed-off-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Maarten Lankhorst <dev@lankhorst.se>
---
drivers/gpu/drm/drm_gem_vram_helper.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 22b1fe9c03b81..70979523ee984 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -925,6 +925,7 @@ EXPORT_SYMBOL(drm_vram_mm_debugfs_init);
static int drm_vram_mm_init(struct drm_vram_mm *vmm, struct drm_device *dev,
uint64_t vram_base, size_t vram_size)
{
+ struct ttm_resource_manager *man;
int ret;
vmm->vram_base = vram_base;
@@ -939,8 +940,20 @@ static int drm_vram_mm_init(struct drm_vram_mm *vmm, struct drm_device *dev,
ret = ttm_range_man_init(&vmm->bdev, TTM_PL_VRAM,
false, vram_size >> PAGE_SHIFT);
- if (ret)
+ if (ret) {
+ ttm_device_fini(&vmm->bdev);
return ret;
+ }
+
+ man = ttm_manager_type(&vmm->bdev, TTM_PL_VRAM);
+ man->cg = drmm_register_region(dev, "vram", size);
+ if (IS_ERR(man->cg)) {
+ ret = PTR_ERR(man->cg);
+
+ ttm_range_man_fini(&vmm->bdev, TTM_PL_VRAM);
+ ttm_device_fini(&vmm->bdev);
+ return ret;
+ }
return 0;
}
--
2.43.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-04 13:44 [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup Maarten Lankhorst
` (6 preceding siblings ...)
2024-12-04 13:44 ` [PATCH v2 7/7] drm/gem: Add cgroup memory accounting for VRAM helper Maarten Lankhorst
@ 2024-12-08 12:15 ` Friedrich Vock
2024-12-13 13:07 ` Maxime Ripard
2024-12-13 13:03 ` Maxime Ripard
8 siblings, 1 reply; 35+ messages in thread
From: Friedrich Vock @ 2024-12-08 12:15 UTC (permalink / raw)
To: Maarten Lankhorst, linux-kernel, intel-xe, dri-devel, Tejun Heo,
Zefan Li, Johannes Weiner, Andrew Morton, Maxime Ripard
Cc: cgroups, linux-mm, Maarten Lankhorst
Hi,
On 04.12.24 14:44, Maarten Lankhorst wrote:
> New update. Instead of calling it the 'dev' cgroup, it's now the 'dmem' cgroup.
Thanks! I think this version looks pretty good.
>
> Because it only deals with memory regions, the UAPI has been updated to use dmem.min/low/max/current, and to make the API cleaner, the names are changed too.
>
> dmem.current could contain a line like:
> "drm/0000:03:00.0/vram0 1073741824"
>
> But I think using "drm/card0/vram0" instead of PCIID would perhaps be good too. I'm open to changing it to that based on feedback.
Agree, allowing userspace to reference DRM devices via "cardN" syntax
sounds good. What about other subsystems potentially using dmem cgroups?
I'm not familiar with the media subsystem, but I imagine we might be
dealing with things like USB devices there? Is something like a
"deviceN" possible there as well, or would device IDs look completely
different?
Regards,
Friedrich
>
> I've created an IGT test for min and max, and found the changes
> from Friedrich Vock sent as feedback were needed.
> I've integrated those into the first patch.
>
> Maarten Lankhorst (5):
> kernel/cgroup: Add "dmem" memory accounting cgroup
> drm/ttm: Handle cgroup based eviction in TTM
> drm/xe: Implement cgroup for vram
> drm/amdgpu: Add cgroups implementation
> drm/xe: Hack to test with mapped pages instead of vram.
>
> Maxime Ripard (2):
> drm/drv: Add drmm managed registration helper for dmem cgroups.
> drm/gem: Add cgroup memory accounting for VRAM helper.
>
> Documentation/admin-guide/cgroup-v2.rst | 58 +-
> Documentation/core-api/cgroup.rst | 9 +
> Documentation/core-api/index.rst | 1 +
> Documentation/gpu/drm-compute.rst | 54 ++
> drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 4 +
> drivers/gpu/drm/drm_drv.c | 32 +
> drivers/gpu/drm/drm_gem_vram_helper.c | 15 +-
> drivers/gpu/drm/ttm/tests/ttm_bo_test.c | 18 +-
> .../gpu/drm/ttm/tests/ttm_bo_validate_test.c | 4 +-
> drivers/gpu/drm/ttm/tests/ttm_resource_test.c | 2 +-
> drivers/gpu/drm/ttm/ttm_bo.c | 54 +-
> drivers/gpu/drm/ttm/ttm_resource.c | 23 +-
> drivers/gpu/drm/xe/xe_ttm_sys_mgr.c | 5 +
> drivers/gpu/drm/xe/xe_ttm_vram_mgr.c | 8 +
> include/drm/drm_drv.h | 5 +
> include/drm/ttm/ttm_resource.h | 12 +-
> include/linux/cgroup_dmem.h | 67 ++
> include/linux/cgroup_subsys.h | 4 +
> include/linux/page_counter.h | 2 +-
> init/Kconfig | 10 +
> kernel/cgroup/Makefile | 1 +
> kernel/cgroup/dmem.c | 861 ++++++++++++++++++
> mm/page_counter.c | 4 +-
> 23 files changed, 1219 insertions(+), 34 deletions(-)
> create mode 100644 Documentation/core-api/cgroup.rst
> create mode 100644 Documentation/gpu/drm-compute.rst
> create mode 100644 include/linux/cgroup_dmem.h
> create mode 100644 kernel/cgroup/dmem.c
>
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-08 12:15 ` [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup Friedrich Vock
@ 2024-12-13 13:07 ` Maxime Ripard
2024-12-13 14:13 ` Maarten Lankhorst
0 siblings, 1 reply; 35+ messages in thread
From: Maxime Ripard @ 2024-12-13 13:07 UTC (permalink / raw)
To: Friedrich Vock
Cc: Maarten Lankhorst, linux-kernel, intel-xe, dri-devel, Tejun Heo,
Zefan Li, Johannes Weiner, Andrew Morton, cgroups, linux-mm,
Maarten Lankhorst
[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]
On Sun, Dec 08, 2024 at 01:15:34PM +0100, Friedrich Vock wrote:
> Hi,
>
> On 04.12.24 14:44, Maarten Lankhorst wrote:
>
> > Because it only deals with memory regions, the UAPI has been updated
> > to use dmem.min/low/max/current, and to make the API cleaner, the
> > names are changed too.
> >
> > dmem.current could contain a line like:
> > "drm/0000:03:00.0/vram0 1073741824"
> >
> > But I think using "drm/card0/vram0" instead of PCIID would perhaps
> > be good too. I'm open to changing it to that based on feedback.
>
> Agree, allowing userspace to reference DRM devices via "cardN" syntax
> sounds good.
>
> What about other subsystems potentially using dmem cgroups?
> I'm not familiar with the media subsystem, but I imagine we might be
> dealing with things like USB devices there? Is something like a
> "deviceN" possible there as well, or would device IDs look completely
> different?
I have some patches to enable the cgroup in GEM-based drivers, media
ones and dma-buf heaps. The dma-buf heaps are simple enough since the
heaps names are supposed to be stable.
I don't think using card0 vs card1 (or v4l0 vs v4l1 for example) will
work because I don't think we have any sort of guarantee that these
names will always point to the same devices across reboots or updates.
If the module is loaded later than it used to for example, we could very
well end up in a situation where card0 and card1 are swapped, while the
constraints apply to the previous situation.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-13 13:07 ` Maxime Ripard
@ 2024-12-13 14:13 ` Maarten Lankhorst
2024-12-13 15:22 ` Maxime Ripard
0 siblings, 1 reply; 35+ messages in thread
From: Maarten Lankhorst @ 2024-12-13 14:13 UTC (permalink / raw)
To: Maxime Ripard, Friedrich Vock
Cc: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, cgroups, linux-mm,
Maarten Lankhorst
Hey,
Den 2024-12-13 kl. 14:07, skrev Maxime Ripard:
> On Sun, Dec 08, 2024 at 01:15:34PM +0100, Friedrich Vock wrote:
>> Hi,
>>
>> On 04.12.24 14:44, Maarten Lankhorst wrote:
>>
>>> Because it only deals with memory regions, the UAPI has been updated
>>> to use dmem.min/low/max/current, and to make the API cleaner, the
>>> names are changed too.
>>>
>>> dmem.current could contain a line like:
>>> "drm/0000:03:00.0/vram0 1073741824"
>>>
>>> But I think using "drm/card0/vram0" instead of PCIID would perhaps
>>> be good too. I'm open to changing it to that based on feedback.
>>
>> Agree, allowing userspace to reference DRM devices via "cardN" syntax
>> sounds good.
>>
>> What about other subsystems potentially using dmem cgroups?
>> I'm not familiar with the media subsystem, but I imagine we might be
>> dealing with things like USB devices there? Is something like a
>> "deviceN" possible there as well, or would device IDs look completely
>> different?
I'd just take what makes sense for each driver. dev_name() would be a
good approximation.
I agree that cardN is not stable.
> > I have some patches to enable the cgroup in GEM-based drivers, media
> ones and dma-buf heaps. The dma-buf heaps are simple enough since the
> heaps names are supposed to be stable.
I've used your patch as a base enable cgroup in drivers that use the
VRAM manager. I didn't want to enable it for all of GEM, because it
would conflict with drivers using TTM. Some more discussion is needed first.
For DMA-BUF heaps, I think it's fine and there is a lot less need of
discussion. I just felt it should be sent separately from the initial
enablement.
> I don't think using card0 vs card1 (or v4l0 vs v4l1 for example) will
> work because I don't think we have any sort of guarantee that these
> names will always point to the same devices across reboots or updates.
>
> If the module is loaded later than it used to for example, we could very
> well end up in a situation where card0 and card1 are swapped, while the
> constraints apply to the previous situation.
I agree, just put it out there for discussion. I don't think the
benefits weigh up against the downsides :-)
Cheers,
~Maarten
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-13 14:13 ` Maarten Lankhorst
@ 2024-12-13 15:22 ` Maxime Ripard
0 siblings, 0 replies; 35+ messages in thread
From: Maxime Ripard @ 2024-12-13 15:22 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: Friedrich Vock, linux-kernel, intel-xe, dri-devel, Tejun Heo,
Zefan Li, Johannes Weiner, Andrew Morton, cgroups, linux-mm,
Maarten Lankhorst
[-- Attachment #1: Type: text/plain, Size: 2648 bytes --]
On Fri, Dec 13, 2024 at 03:13:23PM +0100, Maarten Lankhorst wrote:
> Hey,
>
> Den 2024-12-13 kl. 14:07, skrev Maxime Ripard:
> > On Sun, Dec 08, 2024 at 01:15:34PM +0100, Friedrich Vock wrote:
> > > Hi,
> > >
> > > On 04.12.24 14:44, Maarten Lankhorst wrote:
> > >
> > > > Because it only deals with memory regions, the UAPI has been updated
> > > > to use dmem.min/low/max/current, and to make the API cleaner, the
> > > > names are changed too.
> > > >
> > > > dmem.current could contain a line like:
> > > > "drm/0000:03:00.0/vram0 1073741824"
> > > >
> > > > But I think using "drm/card0/vram0" instead of PCIID would perhaps
> > > > be good too. I'm open to changing it to that based on feedback.
> > >
> > > Agree, allowing userspace to reference DRM devices via "cardN" syntax
> > > sounds good.
> > >
> > > What about other subsystems potentially using dmem cgroups?
> > > I'm not familiar with the media subsystem, but I imagine we might be
> > > dealing with things like USB devices there? Is something like a
> > > "deviceN" possible there as well, or would device IDs look completely
> > > different?
>
> I'd just take what makes sense for each driver. dev_name() would be a good
> approximation.
Yeah, dev_name() seems good enough to me too.
> I agree that cardN is not stable.
>
> > > I have some patches to enable the cgroup in GEM-based drivers, media
> > ones and dma-buf heaps. The dma-buf heaps are simple enough since the
> > heaps names are supposed to be stable.
>
> I've used your patch as a base enable cgroup in drivers that use the VRAM
> manager. I didn't want to enable it for all of GEM, because it would
> conflict with drivers using TTM. Some more discussion is needed first.
>
> For DMA-BUF heaps, I think it's fine and there is a lot less need of
> discussion. I just felt it should be sent separately from the initial
> enablement.
Definitely.
> > I don't think using card0 vs card1 (or v4l0 vs v4l1 for example) will
> > work because I don't think we have any sort of guarantee that these
> > names will always point to the same devices across reboots or updates.
> >
> > If the module is loaded later than it used to for example, we could very
> > well end up in a situation where card0 and card1 are swapped, while the
> > constraints apply to the previous situation.
>
> I agree, just put it out there for discussion. I don't think the benefits
> weigh up against the downsides :-)
Oh absolutely. The way to define a stable name is going to be framework
specific anyway. My point was that we wanted to have a stable name.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-04 13:44 [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup Maarten Lankhorst
` (7 preceding siblings ...)
2024-12-08 12:15 ` [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup Friedrich Vock
@ 2024-12-13 13:03 ` Maxime Ripard
2024-12-13 14:53 ` Maarten Lankhorst
8 siblings, 1 reply; 35+ messages in thread
From: Maxime Ripard @ 2024-12-13 13:03 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, cgroups,
linux-mm, Maarten Lankhorst
[-- Attachment #1: Type: text/plain, Size: 876 bytes --]
Hi,
Thanks for the new update!
On Wed, Dec 04, 2024 at 02:44:00PM +0100, Maarten Lankhorst wrote:
> New update. Instead of calling it the 'dev' cgroup, it's now the
> 'dmem' cgroup.
>
> Because it only deals with memory regions, the UAPI has been updated
> to use dmem.min/low/max/current, and to make the API cleaner, the
> names are changed too.
The API is much nicer, and fits much better into other frameworks too.
> dmem.current could contain a line like:
> "drm/0000:03:00.0/vram0 1073741824"
>
> But I think using "drm/card0/vram0" instead of PCIID would perhaps be
> good too. I'm open to changing it to that based on feedback.
Do we have any sort of guarantee over the name card0 being stable across
reboots?
I also wonder if we should have a "total" device that limits the amount
of memory we can allocate from any region?
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-13 13:03 ` Maxime Ripard
@ 2024-12-13 14:53 ` Maarten Lankhorst
2024-12-13 15:21 ` Maxime Ripard
0 siblings, 1 reply; 35+ messages in thread
From: Maarten Lankhorst @ 2024-12-13 14:53 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, cgroups,
linux-mm, Maarten Lankhorst
Den 2024-12-13 kl. 14:03, skrev Maxime Ripard:
> Hi,
>
> Thanks for the new update!
>
> On Wed, Dec 04, 2024 at 02:44:00PM +0100, Maarten Lankhorst wrote:
>> New update. Instead of calling it the 'dev' cgroup, it's now the
>> 'dmem' cgroup.
>>
>> Because it only deals with memory regions, the UAPI has been updated
>> to use dmem.min/low/max/current, and to make the API cleaner, the
>> names are changed too.
>
> The API is much nicer, and fits much better into other frameworks too.
>
>> dmem.current could contain a line like:
>> "drm/0000:03:00.0/vram0 1073741824"
>>
>> But I think using "drm/card0/vram0" instead of PCIID would perhaps be
>> good too. I'm open to changing it to that based on feedback.
>
> Do we have any sort of guarantee over the name card0 being stable across
> reboots?
>
> I also wonder if we should have a "total" device that limits the amount
> of memory we can allocate from any region?
I don't think it is useful. Say your app can use 1 GB of main memory or
2 GB of VRAM, it wouldn't make sense to limit the total of those. In a
lot of cases there is only 1 region, so the total of that would still be
the same.
On top, we just separated the management of each region, adding a
'total' would require unseparating it again. :-)
I'm happy with this version I think. I don't think more changes for the
base are needed.
Cheers,
~Maarten
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-13 14:53 ` Maarten Lankhorst
@ 2024-12-13 15:21 ` Maxime Ripard
2024-12-13 16:06 ` Maarten Lankhorst
0 siblings, 1 reply; 35+ messages in thread
From: Maxime Ripard @ 2024-12-13 15:21 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, cgroups,
linux-mm, Maarten Lankhorst
[-- Attachment #1: Type: text/plain, Size: 1848 bytes --]
On Fri, Dec 13, 2024 at 03:53:13PM +0100, Maarten Lankhorst wrote:
>
>
> Den 2024-12-13 kl. 14:03, skrev Maxime Ripard:
> > Hi,
> >
> > Thanks for the new update!
> >
> > On Wed, Dec 04, 2024 at 02:44:00PM +0100, Maarten Lankhorst wrote:
> > > New update. Instead of calling it the 'dev' cgroup, it's now the
> > > 'dmem' cgroup.
> > >
> > > Because it only deals with memory regions, the UAPI has been updated
> > > to use dmem.min/low/max/current, and to make the API cleaner, the
> > > names are changed too.
> >
> > The API is much nicer, and fits much better into other frameworks too.
> >
> > > dmem.current could contain a line like:
> > > "drm/0000:03:00.0/vram0 1073741824"
> > >
> > > But I think using "drm/card0/vram0" instead of PCIID would perhaps be
> > > good too. I'm open to changing it to that based on feedback.
> >
> > Do we have any sort of guarantee over the name card0 being stable across
> > reboots?
> >
> > I also wonder if we should have a "total" device that limits the amount
> > of memory we can allocate from any region?
>
> I don't think it is useful. Say your app can use 1 GB of main memory or 2 GB
> of VRAM, it wouldn't make sense to limit the total of those. In a lot of
> cases there is only 1 region, so the total of that would still be the same.
>
> On top, we just separated the management of each region, adding a 'total'
> would require unseparating it again. :-)
I didn't mean the total for a device, but for the system. It would
definitely not make sense for a VRAM, but for CMA for example, you have
a single, limited, allocator that will be accessible from heaps, v4l2
and DRM devices.
If an application has to allocate both from v4l2 and DRM buffers, we
should be able to limit its total usage of CMA, not just on a single
device.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-13 15:21 ` Maxime Ripard
@ 2024-12-13 16:06 ` Maarten Lankhorst
2024-12-17 7:46 ` Maxime Ripard
0 siblings, 1 reply; 35+ messages in thread
From: Maarten Lankhorst @ 2024-12-13 16:06 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, cgroups,
linux-mm, Maarten Lankhorst
Hey,
Den 2024-12-13 kl. 16:21, skrev Maxime Ripard:
> On Fri, Dec 13, 2024 at 03:53:13PM +0100, Maarten Lankhorst wrote:
>>
>>
>> Den 2024-12-13 kl. 14:03, skrev Maxime Ripard:
>>> Hi,
>>>
>>> Thanks for the new update!
>>>
>>> On Wed, Dec 04, 2024 at 02:44:00PM +0100, Maarten Lankhorst wrote:
>>>> New update. Instead of calling it the 'dev' cgroup, it's now the
>>>> 'dmem' cgroup.
>>>>
>>>> Because it only deals with memory regions, the UAPI has been updated
>>>> to use dmem.min/low/max/current, and to make the API cleaner, the
>>>> names are changed too.
>>>
>>> The API is much nicer, and fits much better into other frameworks too.
>>>
>>>> dmem.current could contain a line like:
>>>> "drm/0000:03:00.0/vram0 1073741824"
>>>>
>>>> But I think using "drm/card0/vram0" instead of PCIID would perhaps be
>>>> good too. I'm open to changing it to that based on feedback.
>>>
>>> Do we have any sort of guarantee over the name card0 being stable across
>>> reboots?
>>>
>>> I also wonder if we should have a "total" device that limits the amount
>>> of memory we can allocate from any region?
>>
>> I don't think it is useful. Say your app can use 1 GB of main memory or 2 GB
>> of VRAM, it wouldn't make sense to limit the total of those. In a lot of
>> cases there is only 1 region, so the total of that would still be the same.
>>
>> On top, we just separated the management of each region, adding a 'total'
>> would require unseparating it again. :-)
>
> I didn't mean the total for a device, but for the system. It would
> definitely not make sense for a VRAM, but for CMA for example, you have
> a single, limited, allocator that will be accessible from heaps, v4l2
> and DRM devices.
>
> If an application has to allocate both from v4l2 and DRM buffers, we
> should be able to limit its total usage of CMA, not just on a single
> device.
In this case, I think it makes more sense if CMA creates a region, then
use that region in both v4l2 and DRM instead of a separate region for
both, with CMA being responsible for lifetime.
Cheers,
~Maarten
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-13 16:06 ` Maarten Lankhorst
@ 2024-12-17 7:46 ` Maxime Ripard
2024-12-17 14:28 ` Maarten Lankhorst
0 siblings, 1 reply; 35+ messages in thread
From: Maxime Ripard @ 2024-12-17 7:46 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, cgroups,
linux-mm, Maarten Lankhorst
[-- Attachment #1: Type: text/plain, Size: 2396 bytes --]
On Fri, Dec 13, 2024 at 05:06:05PM +0100, Maarten Lankhorst wrote:
> Hey,
>
> Den 2024-12-13 kl. 16:21, skrev Maxime Ripard:
> > On Fri, Dec 13, 2024 at 03:53:13PM +0100, Maarten Lankhorst wrote:
> > >
> > >
> > > Den 2024-12-13 kl. 14:03, skrev Maxime Ripard:
> > > > Hi,
> > > >
> > > > Thanks for the new update!
> > > >
> > > > On Wed, Dec 04, 2024 at 02:44:00PM +0100, Maarten Lankhorst wrote:
> > > > > New update. Instead of calling it the 'dev' cgroup, it's now the
> > > > > 'dmem' cgroup.
> > > > >
> > > > > Because it only deals with memory regions, the UAPI has been updated
> > > > > to use dmem.min/low/max/current, and to make the API cleaner, the
> > > > > names are changed too.
> > > >
> > > > The API is much nicer, and fits much better into other frameworks too.
> > > >
> > > > > dmem.current could contain a line like:
> > > > > "drm/0000:03:00.0/vram0 1073741824"
> > > > >
> > > > > But I think using "drm/card0/vram0" instead of PCIID would perhaps be
> > > > > good too. I'm open to changing it to that based on feedback.
> > > >
> > > > Do we have any sort of guarantee over the name card0 being stable across
> > > > reboots?
> > > >
> > > > I also wonder if we should have a "total" device that limits the amount
> > > > of memory we can allocate from any region?
> > >
> > > I don't think it is useful. Say your app can use 1 GB of main memory or 2 GB
> > > of VRAM, it wouldn't make sense to limit the total of those. In a lot of
> > > cases there is only 1 region, so the total of that would still be the same.
> > >
> > > On top, we just separated the management of each region, adding a 'total'
> > > would require unseparating it again. :-)
> >
> > I didn't mean the total for a device, but for the system. It would
> > definitely not make sense for a VRAM, but for CMA for example, you have
> > a single, limited, allocator that will be accessible from heaps, v4l2
> > and DRM devices.
> >
> > If an application has to allocate both from v4l2 and DRM buffers, we
> > should be able to limit its total usage of CMA, not just on a single
> > device.
>
> In this case, I think it makes more sense if CMA creates a region, then use
> that region in both v4l2 and DRM instead of a separate region for both, with
> CMA being responsible for lifetime.
Ack, thanks for your feedback :)
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-17 7:46 ` Maxime Ripard
@ 2024-12-17 14:28 ` Maarten Lankhorst
2024-12-17 17:11 ` Tejun Heo
0 siblings, 1 reply; 35+ messages in thread
From: Maarten Lankhorst @ 2024-12-17 14:28 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-kernel, intel-xe, dri-devel, Tejun Heo, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, cgroups,
linux-mm, Maarten Lankhorst
Hey,
Now that all patches look good, what is needed to merge the series?
Without patch 6/7 as it is a hack for testing.
I've also posted a IGT for verifying read/write works (rule out
copy/paste errors) and min, max semantics work as intended.
https://lists.freedesktop.org/archives/igt-dev/2024-December/083345.html
Cheers,
~Maarten
Den 2024-12-17 kl. 08:46, skrev Maxime Ripard:
> On Fri, Dec 13, 2024 at 05:06:05PM +0100, Maarten Lankhorst wrote:
>> Hey,
>>
>> Den 2024-12-13 kl. 16:21, skrev Maxime Ripard:
>>> On Fri, Dec 13, 2024 at 03:53:13PM +0100, Maarten Lankhorst wrote:
>>>>
>>>>
>>>> Den 2024-12-13 kl. 14:03, skrev Maxime Ripard:
>>>>> Hi,l
>>>>>
>>>>> Thanks for the new update!
>>>>>
>>>>> On Wed, Dec 04, 2024 at 02:44:00PM +0100, Maarten Lankhorst wrote:
>>>>>> New update. Instead of calling it the 'dev' cgroup, it's now the
>>>>>> 'dmem' cgroup.
>>>>>>
>>>>>> Because it only deals with memory regions, the UAPI has been updated
>>>>>> to use dmem.min/low/max/current, and to make the API cleaner, the
>>>>>> names are changed too.
>>>>>
>>>>> The API is much nicer, and fits much better into other frameworks too.
>>>>>
>>>>>> dmem.current could contain a line like:
>>>>>> "drm/0000:03:00.0/vram0 1073741824"
>>>>>>
>>>>>> But I think using "drm/card0/vram0" instead of PCIID would perhaps be
>>>>>> good too. I'm open to changing it to that based on feedback.
>>>>>
>>>>> Do we have any sort of guarantee over the name card0 being stable across
>>>>> reboots?
>>>>>
>>>>> I also wonder if we should have a "total" device that limits the amount
>>>>> of memory we can allocate from any region?
>>>>
>>>> I don't think it is useful. Say your app can use 1 GB of main memory or 2 GB
>>>> of VRAM, it wouldn't make sense to limit the total of those. In a lot of
>>>> cases there is only 1 region, so the total of that would still be the same.
>>>>
>>>> On top, we just separated the management of each region, adding a 'total'
>>>> would require unseparating it again. :-)
>>>
>>> I didn't mean the total for a device, but for the system. It would
>>> definitely not make sense for a VRAM, but for CMA for example, you have
>>> a single, limited, allocator that will be accessible from heaps, v4l2
>>> and DRM devices.
>>>
>>> If an application has to allocate both from v4l2 and DRM buffers, we
>>> should be able to limit its total usage of CMA, not just on a single
>>> device.
>>
>> In this case, I think it makes more sense if CMA creates a region, then use
>> that region in both v4l2 and DRM instead of a separate region for both, with
>> CMA being responsible for lifetime.
>
> Ack, thanks for your feedback :)
>
> Maxime
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-17 14:28 ` Maarten Lankhorst
@ 2024-12-17 17:11 ` Tejun Heo
2024-12-17 17:34 ` Maxime Ripard
2024-12-17 17:37 ` Maarten Lankhorst
0 siblings, 2 replies; 35+ messages in thread
From: Tejun Heo @ 2024-12-17 17:11 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: Maxime Ripard, linux-kernel, intel-xe, dri-devel, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, cgroups,
linux-mm, Maarten Lankhorst
On Tue, Dec 17, 2024 at 03:28:50PM +0100, Maarten Lankhorst wrote:
> Now that all patches look good, what is needed to merge the series? Without
> patch 6/7 as it is a hack for testing.
There were some questions raised about device naming. One thing we want to
get right from the beginning is the basic interface.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-17 17:11 ` Tejun Heo
@ 2024-12-17 17:34 ` Maxime Ripard
2024-12-17 17:37 ` Maarten Lankhorst
1 sibling, 0 replies; 35+ messages in thread
From: Maxime Ripard @ 2024-12-17 17:34 UTC (permalink / raw)
To: Tejun Heo
Cc: Maarten Lankhorst, linux-kernel, intel-xe, dri-devel, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, cgroups,
linux-mm, Maarten Lankhorst
[-- Attachment #1: Type: text/plain, Size: 532 bytes --]
On Tue, Dec 17, 2024 at 07:11:00AM -1000, Tejun Heo wrote:
> On Tue, Dec 17, 2024 at 03:28:50PM +0100, Maarten Lankhorst wrote:
> > Now that all patches look good, what is needed to merge the series? Without
> > patch 6/7 as it is a hack for testing.
>
> There were some questions raised about device naming. One thing we want to
> get right from the beginning is the basic interface.
We decided on the previous version to use dmem and I believe this
version has switched to it. Do you have any concern still?
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-17 17:11 ` Tejun Heo
2024-12-17 17:34 ` Maxime Ripard
@ 2024-12-17 17:37 ` Maarten Lankhorst
2024-12-17 18:23 ` Tejun Heo
2024-12-18 10:28 ` Friedrich Vock
1 sibling, 2 replies; 35+ messages in thread
From: Maarten Lankhorst @ 2024-12-17 17:37 UTC (permalink / raw)
To: Tejun Heo
Cc: Maxime Ripard, linux-kernel, intel-xe, dri-devel, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, cgroups,
linux-mm, Maarten Lankhorst
Den 2024-12-17 kl. 18:11, skrev Tejun Heo:
> On Tue, Dec 17, 2024 at 03:28:50PM +0100, Maarten Lankhorst wrote:
>> Now that all patches look good, what is needed to merge the series? Without
>> patch 6/7 as it is a hack for testing.
>
> There were some questions raised about device naming. One thing we want to
> get right from the beginning is the basic interface.
>
> Thanks.
>
I believe it was solved. The conclusion appears to be that we go with
how we defined it in this series. drm/$pciid/$regionname. With the only
regions defined now being VRAM. Main memory will be a followup, but
requires some discussions on hwo to be prevent double accounting, and
what to do with the limited amount of mappable memory.
Cheers,
~Maarten
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-17 17:37 ` Maarten Lankhorst
@ 2024-12-17 18:23 ` Tejun Heo
2024-12-17 20:17 ` Maarten Lankhorst
2024-12-18 10:28 ` Friedrich Vock
1 sibling, 1 reply; 35+ messages in thread
From: Tejun Heo @ 2024-12-17 18:23 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: Maxime Ripard, linux-kernel, intel-xe, dri-devel, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, cgroups,
linux-mm, Maarten Lankhorst
Hello,
On Tue, Dec 17, 2024 at 06:37:22PM +0100, Maarten Lankhorst wrote:
> Den 2024-12-17 kl. 18:11, skrev Tejun Heo:
> > On Tue, Dec 17, 2024 at 03:28:50PM +0100, Maarten Lankhorst wrote:
> > > Now that all patches look good, what is needed to merge the series? Without
> > > patch 6/7 as it is a hack for testing.
> >
> > There were some questions raised about device naming. One thing we want to
> > get right from the beginning is the basic interface.
> >
> > Thanks.
> >
> I believe it was solved. The conclusion appears to be that we go with how we
> defined it in this series. drm/$pciid/$regionname. With the only regions
> defined now being VRAM. Main memory will be a followup, but requires some
> discussions on hwo to be prevent double accounting, and what to do with the
> limited amount of mappable memory.
Provided Johannes is okay with the series, how do you want to route the
series? If you want to route it through drm, that's fine by me and please
feel free to add:
Acked-by: Tejun Heo <tj@kernel.org>
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-17 18:23 ` Tejun Heo
@ 2024-12-17 20:17 ` Maarten Lankhorst
2025-01-07 15:13 ` Maxime Ripard
0 siblings, 1 reply; 35+ messages in thread
From: Maarten Lankhorst @ 2024-12-17 20:17 UTC (permalink / raw)
To: Tejun Heo, Simona Vetter, Dave Airlie
Cc: Maxime Ripard, linux-kernel, intel-xe, dri-devel, Zefan Li,
Johannes Weiner, Andrew Morton, Friedrich Vock, cgroups,
linux-mm, Maarten Lankhorst
Hey,
Den 2024-12-17 kl. 19:23, skrev Tejun Heo:
> Hello,
>
> On Tue, Dec 17, 2024 at 06:37:22PM +0100, Maarten Lankhorst wrote:
>> Den 2024-12-17 kl. 18:11, skrev Tejun Heo:
>>> On Tue, Dec 17, 2024 at 03:28:50PM +0100, Maarten Lankhorst wrote:
>>>> Now that all patches look good, what is needed to merge the series? Without
>>>> patch 6/7 as it is a hack for testing.
>>>
>>> There were some questions raised about device naming. One thing we want to
>>> get right from the beginning is the basic interface.
>>>
>>> Thanks.
>>>
>> I believe it was solved. The conclusion appears to be that we go with how we
>> defined it in this series. drm/$pciid/$regionname. With the only regions
>> defined now being VRAM. Main memory will be a followup, but requires some
>> discussions on hwo to be prevent double accounting, and what to do with the
>> limited amount of mappable memory.
>
> Provided Johannes is okay with the series, how do you want to route the
> series? If you want to route it through drm, that's fine by me and please
> feel free to add:
>
> Acked-by: Tejun Heo <tj@kernel.org>
>
> Thanks.
>
Thank you!
I've discussed this with the DRM maintainers. What was suggested is to
create a topic branch, merge it to drm-misc whichwhere it will be picked
up into drm.git during the next pull request. At the same time the topic
branch can be also be merged into the cgroup tree if needed.
The drm-misc tree already handles dma-buf and fbdev core, think DMEM
could fit in there too.
Cheers,
~Maarten
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-17 20:17 ` Maarten Lankhorst
@ 2025-01-07 15:13 ` Maxime Ripard
0 siblings, 0 replies; 35+ messages in thread
From: Maxime Ripard @ 2025-01-07 15:13 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: Tejun Heo, Simona Vetter, Dave Airlie, linux-kernel, intel-xe,
dri-devel, Zefan Li, Johannes Weiner, Andrew Morton,
Friedrich Vock, cgroups, linux-mm, Maarten Lankhorst
[-- Attachment #1: Type: text/plain, Size: 1858 bytes --]
On Tue, Dec 17, 2024 at 09:17:24PM +0100, Maarten Lankhorst wrote:
> Hey,
>
> Den 2024-12-17 kl. 19:23, skrev Tejun Heo:
> > Hello,
> >
> > On Tue, Dec 17, 2024 at 06:37:22PM +0100, Maarten Lankhorst wrote:
> > > Den 2024-12-17 kl. 18:11, skrev Tejun Heo:
> > > > On Tue, Dec 17, 2024 at 03:28:50PM +0100, Maarten Lankhorst wrote:
> > > > > Now that all patches look good, what is needed to merge the series? Without
> > > > > patch 6/7 as it is a hack for testing.
> > > >
> > > > There were some questions raised about device naming. One thing we want to
> > > > get right from the beginning is the basic interface.
> > > >
> > > > Thanks.
> > > >
> > > I believe it was solved. The conclusion appears to be that we go with how we
> > > defined it in this series. drm/$pciid/$regionname. With the only regions
> > > defined now being VRAM. Main memory will be a followup, but requires some
> > > discussions on hwo to be prevent double accounting, and what to do with the
> > > limited amount of mappable memory.
> >
> > Provided Johannes is okay with the series, how do you want to route the
> > series? If you want to route it through drm, that's fine by me and please
> > feel free to add:
> >
> > Acked-by: Tejun Heo <tj@kernel.org>
> >
> > Thanks.
> >
>
> Thank you!
>
> I've discussed this with the DRM maintainers. What was suggested is to
> create a topic branch, merge it to drm-misc whichwhere it will be picked up
> into drm.git during the next pull request. At the same time the topic branch
> can be also be merged into the cgroup tree if needed.
>
> The drm-misc tree already handles dma-buf and fbdev core, think DMEM could
> fit in there too.
FTR, I sent the PR Maarten mentioned yesterday:
https://lore.kernel.org/dri-devel/20250106-shaggy-solid-dogfish-e88ebc@houat/
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH v2 0/7] kernel/cgroups: Add "dmem" memory accounting cgroup.
2024-12-17 17:37 ` Maarten Lankhorst
2024-12-17 18:23 ` Tejun Heo
@ 2024-12-18 10:28 ` Friedrich Vock
1 sibling, 0 replies; 35+ messages in thread
From: Friedrich Vock @ 2024-12-18 10:28 UTC (permalink / raw)
To: Maarten Lankhorst, Tejun Heo
Cc: Maxime Ripard, linux-kernel, intel-xe, dri-devel, Zefan Li,
Johannes Weiner, Andrew Morton, cgroups, linux-mm,
Maarten Lankhorst
On 17.12.24 18:37, Maarten Lankhorst wrote:
>
>
> Den 2024-12-17 kl. 18:11, skrev Tejun Heo:
>> On Tue, Dec 17, 2024 at 03:28:50PM +0100, Maarten Lankhorst wrote:
>>> Now that all patches look good, what is needed to merge the series?
>>> Without
>>> patch 6/7 as it is a hack for testing.
>>
>> There were some questions raised about device naming. One thing we
>> want to
>> get right from the beginning is the basic interface.
>>
>> Thanks.
>>
> I believe it was solved. The conclusion appears to be that we go with
> how we defined it in this series. drm/$pciid/$regionname.
Yeah, I'd agree. Using PCI IDs works, and the objection about cardN
syntax being unstable when driver load order changes is a good point.
Friedrich
> With the only
> regions defined now being VRAM. Main memory will be a followup, but
> requires some discussions on hwo to be prevent double accounting, and
> what to do with the limited amount of mappable memory.
>
> Cheers,
> ~Maarten
^ permalink raw reply [flat|nested] 35+ messages in thread