[08/17] drm/cgroup: Track DRM clients per cgroup
Commit Message
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
To enable propagation of settings from the cgroup DRM controller to DRM
and vice-versa, we need to start tracking to which cgroups DRM clients
belong.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/drm_file.c | 6 ++++
include/drm/drm_file.h | 6 ++++
include/linux/cgroup_drm.h | 20 ++++++++++++
kernel/cgroup/drm.c | 62 +++++++++++++++++++++++++++++++++++++-
4 files changed, 93 insertions(+), 1 deletion(-)
Comments
Hello,
On Wed, Jul 12, 2023 at 12:45:56PM +0100, Tvrtko Ursulin wrote:
> +void drmcgroup_client_migrate(struct drm_file *file_priv)
> +{
> + struct drm_cgroup_state *src, *dst;
> + struct cgroup_subsys_state *old;
> +
> + mutex_lock(&drmcg_mutex);
> +
> + old = file_priv->__css;
> + src = css_to_drmcs(old);
> + dst = css_to_drmcs(task_get_css(current, drm_cgrp_id));
> +
> + if (src != dst) {
> + file_priv->__css = &dst->css; /* Keeps the reference. */
> + list_move_tail(&file_priv->clink, &dst->clients);
> + }
> +
> + mutex_unlock(&drmcg_mutex);
> +
> + css_put(old);
> +}
> +EXPORT_SYMBOL_GPL(drmcgroup_client_migrate);
So, you're implicitly migrating the fd to the latest ioctl user on the first
access. While this may work for state-less control like usage time. This
likely will cause problem if you later want to add stateful control for
memory consumption. e.g. What happens if the new destination doesn't have
enough budget to accommodate the fd's usage? Let's say we allow over-commit
with follow-up reclaim or oom kill actions, if so, can we guarantee that all
memory ownership for can always be updated? If not, what happens after
failure?
If DRM needs to transfer fd ownership with resources attached to it, it
likely would be a good idea to make that an explicit operation so that the
attempt can be verified and failed if necessary.
Thanks.
@@ -32,6 +32,7 @@
*/
#include <linux/anon_inodes.h>
+#include <linux/cgroup_drm.h>
#include <linux/dma-fence.h>
#include <linux/file.h>
#include <linux/module.h>
@@ -304,6 +305,8 @@ static void drm_close_helper(struct file *filp)
list_del(&file_priv->lhead);
mutex_unlock(&dev->filelist_mutex);
+ drmcgroup_client_close(file_priv);
+
drm_file_free(file_priv);
}
@@ -367,6 +370,8 @@ int drm_open_helper(struct file *filp, struct drm_minor *minor)
list_add(&priv->lhead, &dev->filelist);
mutex_unlock(&dev->filelist_mutex);
+ drmcgroup_client_open(priv);
+
#ifdef CONFIG_DRM_LEGACY
#ifdef __alpha__
/*
@@ -533,6 +538,7 @@ void drm_file_update_pid(struct drm_file *filp)
mutex_unlock(&dev->filelist_mutex);
if (pid != old) {
+ drmcgroup_client_migrate(filp);
get_pid(pid);
synchronize_rcu();
put_pid(old);
@@ -30,6 +30,7 @@
#ifndef _DRM_FILE_H_
#define _DRM_FILE_H_
+#include <linux/cgroup.h>
#include <linux/types.h>
#include <linux/completion.h>
#include <linux/idr.h>
@@ -283,6 +284,11 @@ struct drm_file {
/** @minor: &struct drm_minor for this file. */
struct drm_minor *minor;
+#if IS_ENABLED(CONFIG_CGROUP_DRM)
+ struct cgroup_subsys_state *__css;
+ struct list_head clink;
+#endif
+
/**
* @object_idr:
*
@@ -6,4 +6,24 @@
#ifndef _CGROUP_DRM_H
#define _CGROUP_DRM_H
+#include <drm/drm_file.h>
+
+#if IS_ENABLED(CONFIG_CGROUP_DRM)
+void drmcgroup_client_open(struct drm_file *file_priv);
+void drmcgroup_client_close(struct drm_file *file_priv);
+void drmcgroup_client_migrate(struct drm_file *file_priv);
+#else
+static inline void drmcgroup_client_open(struct drm_file *file_priv)
+{
+}
+
+static inline void drmcgroup_client_close(struct drm_file *file_priv)
+{
+}
+
+static void drmcgroup_client_migrate(struct drm_file *file_priv)
+{
+}
+#endif
+
#endif /* _CGROUP_DRM_H */
@@ -5,17 +5,25 @@
#include <linux/cgroup.h>
#include <linux/cgroup_drm.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
#include <linux/slab.h>
struct drm_cgroup_state {
struct cgroup_subsys_state css;
+
+ struct list_head clients;
};
struct drm_root_cgroup_state {
struct drm_cgroup_state drmcs;
};
-static struct drm_root_cgroup_state root_drmcs;
+static struct drm_root_cgroup_state root_drmcs = {
+ .drmcs.clients = LIST_HEAD_INIT(root_drmcs.drmcs.clients),
+};
+
+static DEFINE_MUTEX(drmcg_mutex);
static inline struct drm_cgroup_state *
css_to_drmcs(struct cgroup_subsys_state *css)
@@ -42,11 +50,63 @@ drmcs_alloc(struct cgroup_subsys_state *parent_css)
drmcs = kzalloc(sizeof(*drmcs), GFP_KERNEL);
if (!drmcs)
return ERR_PTR(-ENOMEM);
+
+ INIT_LIST_HEAD(&drmcs->clients);
}
return &drmcs->css;
}
+void drmcgroup_client_open(struct drm_file *file_priv)
+{
+ struct drm_cgroup_state *drmcs;
+
+ drmcs = css_to_drmcs(task_get_css(current, drm_cgrp_id));
+
+ mutex_lock(&drmcg_mutex);
+ file_priv->__css = &drmcs->css; /* Keeps the reference. */
+ list_add_tail(&file_priv->clink, &drmcs->clients);
+ mutex_unlock(&drmcg_mutex);
+}
+EXPORT_SYMBOL_GPL(drmcgroup_client_open);
+
+void drmcgroup_client_close(struct drm_file *file_priv)
+{
+ struct drm_cgroup_state *drmcs;
+
+ drmcs = css_to_drmcs(file_priv->__css);
+
+ mutex_lock(&drmcg_mutex);
+ list_del(&file_priv->clink);
+ file_priv->__css = NULL;
+ mutex_unlock(&drmcg_mutex);
+
+ css_put(&drmcs->css);
+}
+EXPORT_SYMBOL_GPL(drmcgroup_client_close);
+
+void drmcgroup_client_migrate(struct drm_file *file_priv)
+{
+ struct drm_cgroup_state *src, *dst;
+ struct cgroup_subsys_state *old;
+
+ mutex_lock(&drmcg_mutex);
+
+ old = file_priv->__css;
+ src = css_to_drmcs(old);
+ dst = css_to_drmcs(task_get_css(current, drm_cgrp_id));
+
+ if (src != dst) {
+ file_priv->__css = &dst->css; /* Keeps the reference. */
+ list_move_tail(&file_priv->clink, &dst->clients);
+ }
+
+ mutex_unlock(&drmcg_mutex);
+
+ css_put(old);
+}
+EXPORT_SYMBOL_GPL(drmcgroup_client_migrate);
+
struct cftype files[] = {
{ } /* Zero entry terminates. */
};