diff mbox series

[RFC,v3,01/11] cgroup: Introduce cgroup for drm subsystem

Message ID 20190626150522.11618-2-Kenny.Ho@amd.com (mailing list archive)
State New, archived
Headers show
Series new cgroup controller for gpu/drm subsystem | expand

Commit Message

Ho, Kenny June 26, 2019, 3:05 p.m. UTC 
Change-Id: I6830d3990f63f0c13abeba29b1d330cf28882831
Signed-off-by: Kenny Ho <Kenny.Ho@amd.com>
---
 include/linux/cgroup_drm.h    | 76 +++++++++++++++++++++++++++++++++++
 include/linux/cgroup_subsys.h |  4 ++
 init/Kconfig                  |  5 +++
 kernel/cgroup/Makefile        |  1 +
 kernel/cgroup/drm.c           | 42 +++++++++++++++++++
 5 files changed, 128 insertions(+)
 create mode 100644 include/linux/cgroup_drm.h
 create mode 100644 kernel/cgroup/drm.c

Comments

Daniel Vetter June 26, 2019, 3:49 p.m. UTC | #1 
On Wed, Jun 26, 2019 at 11:05:12AM -0400, Kenny Ho wrote:
Needs a bit more commit message here I htink.

> Change-Id: I6830d3990f63f0c13abeba29b1d330cf28882831
> Signed-off-by: Kenny Ho <Kenny.Ho@amd.com>

Bunch of naming bikesheds
> ---
>  include/linux/cgroup_drm.h    | 76 +++++++++++++++++++++++++++++++++++
>  include/linux/cgroup_subsys.h |  4 ++
>  init/Kconfig                  |  5 +++
>  kernel/cgroup/Makefile        |  1 +
>  kernel/cgroup/drm.c           | 42 +++++++++++++++++++
>  5 files changed, 128 insertions(+)
>  create mode 100644 include/linux/cgroup_drm.h
>  create mode 100644 kernel/cgroup/drm.c

> 
> diff --git a/include/linux/cgroup_drm.h b/include/linux/cgroup_drm.h
> new file mode 100644
> index 000000000000..9928e60037a5
> --- /dev/null
> +++ b/include/linux/cgroup_drm.h
> @@ -0,0 +1,76 @@
> +/* SPDX-License-Identifier: MIT
> + * Copyright 2019 Advanced Micro Devices, Inc.
> + */
> +#ifndef _CGROUP_DRM_H
> +#define _CGROUP_DRM_H
> +
> +#ifdef CONFIG_CGROUP_DRM
> +
> +#include <linux/cgroup.h>
> +
> +struct drmcgrp {

drm_cgroup for more consistency how we usually call these things.

> +	struct cgroup_subsys_state	css;
> +};
> +
> +static inline struct drmcgrp *css_drmcgrp(struct cgroup_subsys_state *css)

ccs_to_drm_cgroup

> +{
> +	return css ? container_of(css, struct drmcgrp, css) : NULL;
> +}
> +
> +static inline struct drmcgrp *drmcgrp_from(struct task_struct *task)

task_get_drm_cgroup for consistency with task_get_css?

> +{
> +	return css_drmcgrp(task_get_css(task, drm_cgrp_id));
> +}
> +
> +static inline struct drmcgrp *get_drmcgrp(struct task_struct *task)
> +{
> +	struct cgroup_subsys_state *css = task_get_css(task, drm_cgrp_id);
> +
> +	if (css)
> +		css_get(css);
> +
> +	return css_drmcgrp(css);
> +}
> +
> +static inline void put_drmcgrp(struct drmcgrp *drmcgrp)

In drm we generally put _get/_put at the end, cgroup seems to do the same.

> +{
> +	if (drmcgrp)
> +		css_put(&drmcgrp->css);
> +}
> +
> +static inline struct drmcgrp *parent_drmcgrp(struct drmcgrp *cg)

I'd also call this drm_cgroup_parent or so.

Also all the above needs a bit of nice kerneldoc for the final version.
-Daniel

> +{
> +	return css_drmcgrp(cg->css.parent);
> +}
> +
> +#else /* CONFIG_CGROUP_DRM */
> +
> +struct drmcgrp {
> +};
> +
> +static inline struct drmcgrp *css_drmcgrp(struct cgroup_subsys_state *css)
> +{
> +	return NULL;
> +}
> +
> +static inline struct drmcgrp *drmcgrp_from(struct task_struct *task)
> +{
> +	return NULL;
> +}
> +
> +static inline struct drmcgrp *get_drmcgrp(struct task_struct *task)
> +{
> +	return NULL;
> +}
> +
> +static inline void put_drmcgrp(struct drmcgrp *drmcgrp)
> +{
> +}
> +
> +static inline struct drmcgrp *parent_drmcgrp(struct drmcgrp *cg)
> +{
> +	return NULL;
> +}
> +
> +#endif	/* CONFIG_CGROUP_DRM */
> +#endif	/* _CGROUP_DRM_H */
> diff --git a/include/linux/cgroup_subsys.h b/include/linux/cgroup_subsys.h
> index acb77dcff3b4..ddedad809e8b 100644
> --- a/include/linux/cgroup_subsys.h
> +++ b/include/linux/cgroup_subsys.h
> @@ -61,6 +61,10 @@ SUBSYS(pids)
>  SUBSYS(rdma)
>  #endif
>  
> +#if IS_ENABLED(CONFIG_CGROUP_DRM)
> +SUBSYS(drm)
> +#endif
> +
>  /*
>   * The following subsystems are not supported on the default hierarchy.
>   */
> diff --git a/init/Kconfig b/init/Kconfig
> index d47cb77a220e..0b0f112eb23b 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -839,6 +839,11 @@ 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_DRM
> +	bool "DRM controller (EXPERIMENTAL)"
> +	help
> +	  Provides accounting and enforcement of resources in the DRM subsystem.
> +
>  config CGROUP_FREEZER
>  	bool "Freezer controller"
>  	help
> diff --git a/kernel/cgroup/Makefile b/kernel/cgroup/Makefile
> index bfcdae896122..6af14bd93050 100644
> --- a/kernel/cgroup/Makefile
> +++ b/kernel/cgroup/Makefile
> @@ -4,5 +4,6 @@ obj-y := cgroup.o rstat.o namespace.o cgroup-v1.o
>  obj-$(CONFIG_CGROUP_FREEZER) += freezer.o
>  obj-$(CONFIG_CGROUP_PIDS) += pids.o
>  obj-$(CONFIG_CGROUP_RDMA) += rdma.o
> +obj-$(CONFIG_CGROUP_DRM) += drm.o
>  obj-$(CONFIG_CPUSETS) += cpuset.o
>  obj-$(CONFIG_CGROUP_DEBUG) += debug.o
> diff --git a/kernel/cgroup/drm.c b/kernel/cgroup/drm.c
> new file mode 100644
> index 000000000000..66cb1dda023d
> --- /dev/null
> +++ b/kernel/cgroup/drm.c
> @@ -0,0 +1,42 @@
> +// SPDX-License-Identifier: MIT
> +// Copyright 2019 Advanced Micro Devices, Inc.
> +#include <linux/slab.h>
> +#include <linux/cgroup.h>
> +#include <linux/cgroup_drm.h>
> +
> +static struct drmcgrp *root_drmcgrp __read_mostly;
> +
> +static void drmcgrp_css_free(struct cgroup_subsys_state *css)
> +{
> +	struct drmcgrp *drmcgrp = css_drmcgrp(css);
> +
> +	kfree(drmcgrp);
> +}
> +
> +static struct cgroup_subsys_state *
> +drmcgrp_css_alloc(struct cgroup_subsys_state *parent_css)
> +{
> +	struct drmcgrp *parent = css_drmcgrp(parent_css);
> +	struct drmcgrp *drmcgrp;
> +
> +	drmcgrp = kzalloc(sizeof(struct drmcgrp), GFP_KERNEL);
> +	if (!drmcgrp)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (!parent)
> +		root_drmcgrp = drmcgrp;
> +
> +	return &drmcgrp->css;
> +}
> +
> +struct cftype files[] = {
> +	{ }	/* terminate */
> +};
> +
> +struct cgroup_subsys drm_cgrp_subsys = {
> +	.css_alloc	= drmcgrp_css_alloc,
> +	.css_free	= drmcgrp_css_free,
> +	.early_init	= false,
> +	.legacy_cftypes	= files,
> +	.dfl_cftypes	= files,
> +};
> -- 
> 2.21.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Kenny Ho June 26, 2019, 7:35 p.m. UTC | #2 
On Wed, Jun 26, 2019 at 11:49 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> Bunch of naming bikesheds

I appreciate the suggestions, naming is hard :).

> > +#include <linux/cgroup.h>
> > +
> > +struct drmcgrp {
>
> drm_cgroup for more consistency how we usually call these things.

I was hoping to keep the symbol short if possible.  I started with
drmcg (following blkcg),  but I believe that causes confusion with
other aspect of the drm subsystem.  I don't have too strong of an
opinion on this but I'd prefer not needing to keep refactoring.  So if
there are other opinions on this, please speak up.

> > +
> > +static inline void put_drmcgrp(struct drmcgrp *drmcgrp)
>
> In drm we generally put _get/_put at the end, cgroup seems to do the same.

ok, I will refactor.

> > +{
> > +     if (drmcgrp)
> > +             css_put(&drmcgrp->css);
> > +}
> > +
> > +static inline struct drmcgrp *parent_drmcgrp(struct drmcgrp *cg)
>
> I'd also call this drm_cgroup_parent or so.
>
> Also all the above needs a bit of nice kerneldoc for the final version.
> -Daniel

Noted, will do, thanks.

Regards,
Kenny
Daniel Vetter June 26, 2019, 8:12 p.m. UTC | #3 
On Wed, Jun 26, 2019 at 9:35 PM Kenny Ho <y2kenny@gmail.com> wrote:
>
> On Wed, Jun 26, 2019 at 11:49 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > Bunch of naming bikesheds
>
> I appreciate the suggestions, naming is hard :).
>
> > > +#include <linux/cgroup.h>
> > > +
> > > +struct drmcgrp {
> >
> > drm_cgroup for more consistency how we usually call these things.
>
> I was hoping to keep the symbol short if possible.  I started with
> drmcg (following blkcg),  but I believe that causes confusion with
> other aspect of the drm subsystem.  I don't have too strong of an
> opinion on this but I'd prefer not needing to keep refactoring.  So if
> there are other opinions on this, please speak up.

I think drmcg sounds good to me. That aligns at least with memcg,
blkcg in cgroups, so as good reason as any. drmcgrp just felt kinda
awkward in-between solution, not as easy to read as drm_cgroup, but
also not as short as drmcg and cgrp is just letter jumbo I can never
remember anyway what it means :-)
-Daniel

> > > +
> > > +static inline void put_drmcgrp(struct drmcgrp *drmcgrp)
> >
> > In drm we generally put _get/_put at the end, cgroup seems to do the same.
>
> ok, I will refactor.
>
> > > +{
> > > +     if (drmcgrp)
> > > +             css_put(&drmcgrp->css);
> > > +}
> > > +
> > > +static inline struct drmcgrp *parent_drmcgrp(struct drmcgrp *cg)
> >
> > I'd also call this drm_cgroup_parent or so.
> >
> > Also all the above needs a bit of nice kerneldoc for the final version.
> > -Daniel
>
> Noted, will do, thanks.
>
> Regards,
> Kenny
diff mbox series

Patch

diff --git a/include/linux/cgroup_drm.h b/include/linux/cgroup_drm.h
new file mode 100644
index 000000000000..9928e60037a5
--- /dev/null
+++ b/include/linux/cgroup_drm.h
@@ -0,0 +1,76 @@ 
+/* SPDX-License-Identifier: MIT
+ * Copyright 2019 Advanced Micro Devices, Inc.
+ */
+#ifndef _CGROUP_DRM_H
+#define _CGROUP_DRM_H
+
+#ifdef CONFIG_CGROUP_DRM
+
+#include <linux/cgroup.h>
+
+struct drmcgrp {
+	struct cgroup_subsys_state	css;
+};
+
+static inline struct drmcgrp *css_drmcgrp(struct cgroup_subsys_state *css)
+{
+	return css ? container_of(css, struct drmcgrp, css) : NULL;
+}
+
+static inline struct drmcgrp *drmcgrp_from(struct task_struct *task)
+{
+	return css_drmcgrp(task_get_css(task, drm_cgrp_id));
+}
+
+static inline struct drmcgrp *get_drmcgrp(struct task_struct *task)
+{
+	struct cgroup_subsys_state *css = task_get_css(task, drm_cgrp_id);
+
+	if (css)
+		css_get(css);
+
+	return css_drmcgrp(css);
+}
+
+static inline void put_drmcgrp(struct drmcgrp *drmcgrp)
+{
+	if (drmcgrp)
+		css_put(&drmcgrp->css);
+}
+
+static inline struct drmcgrp *parent_drmcgrp(struct drmcgrp *cg)
+{
+	return css_drmcgrp(cg->css.parent);
+}
+
+#else /* CONFIG_CGROUP_DRM */
+
+struct drmcgrp {
+};
+
+static inline struct drmcgrp *css_drmcgrp(struct cgroup_subsys_state *css)
+{
+	return NULL;
+}
+
+static inline struct drmcgrp *drmcgrp_from(struct task_struct *task)
+{
+	return NULL;
+}
+
+static inline struct drmcgrp *get_drmcgrp(struct task_struct *task)
+{
+	return NULL;
+}
+
+static inline void put_drmcgrp(struct drmcgrp *drmcgrp)
+{
+}
+
+static inline struct drmcgrp *parent_drmcgrp(struct drmcgrp *cg)
+{
+	return NULL;
+}
+
+#endif	/* CONFIG_CGROUP_DRM */
+#endif	/* _CGROUP_DRM_H */
diff --git a/include/linux/cgroup_subsys.h b/include/linux/cgroup_subsys.h
index acb77dcff3b4..ddedad809e8b 100644
--- a/include/linux/cgroup_subsys.h
+++ b/include/linux/cgroup_subsys.h
@@ -61,6 +61,10 @@  SUBSYS(pids)
 SUBSYS(rdma)
 #endif
 
+#if IS_ENABLED(CONFIG_CGROUP_DRM)
+SUBSYS(drm)
+#endif
+
 /*
  * The following subsystems are not supported on the default hierarchy.
  */
diff --git a/init/Kconfig b/init/Kconfig
index d47cb77a220e..0b0f112eb23b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -839,6 +839,11 @@  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_DRM
+	bool "DRM controller (EXPERIMENTAL)"
+	help
+	  Provides accounting and enforcement of resources in the DRM subsystem.
+
 config CGROUP_FREEZER
 	bool "Freezer controller"
 	help
diff --git a/kernel/cgroup/Makefile b/kernel/cgroup/Makefile
index bfcdae896122..6af14bd93050 100644
--- a/kernel/cgroup/Makefile
+++ b/kernel/cgroup/Makefile
@@ -4,5 +4,6 @@  obj-y := cgroup.o rstat.o namespace.o cgroup-v1.o
 obj-$(CONFIG_CGROUP_FREEZER) += freezer.o
 obj-$(CONFIG_CGROUP_PIDS) += pids.o
 obj-$(CONFIG_CGROUP_RDMA) += rdma.o
+obj-$(CONFIG_CGROUP_DRM) += drm.o
 obj-$(CONFIG_CPUSETS) += cpuset.o
 obj-$(CONFIG_CGROUP_DEBUG) += debug.o
diff --git a/kernel/cgroup/drm.c b/kernel/cgroup/drm.c
new file mode 100644
index 000000000000..66cb1dda023d
--- /dev/null
+++ b/kernel/cgroup/drm.c
@@ -0,0 +1,42 @@ 
+// SPDX-License-Identifier: MIT
+// Copyright 2019 Advanced Micro Devices, Inc.
+#include <linux/slab.h>
+#include <linux/cgroup.h>
+#include <linux/cgroup_drm.h>
+
+static struct drmcgrp *root_drmcgrp __read_mostly;
+
+static void drmcgrp_css_free(struct cgroup_subsys_state *css)
+{
+	struct drmcgrp *drmcgrp = css_drmcgrp(css);
+
+	kfree(drmcgrp);
+}
+
+static struct cgroup_subsys_state *
+drmcgrp_css_alloc(struct cgroup_subsys_state *parent_css)
+{
+	struct drmcgrp *parent = css_drmcgrp(parent_css);
+	struct drmcgrp *drmcgrp;
+
+	drmcgrp = kzalloc(sizeof(struct drmcgrp), GFP_KERNEL);
+	if (!drmcgrp)
+		return ERR_PTR(-ENOMEM);
+
+	if (!parent)
+		root_drmcgrp = drmcgrp;
+
+	return &drmcgrp->css;
+}
+
+struct cftype files[] = {
+	{ }	/* terminate */
+};
+
+struct cgroup_subsys drm_cgrp_subsys = {
+	.css_alloc	= drmcgrp_css_alloc,
+	.css_free	= drmcgrp_css_free,
+	.early_init	= false,
+	.legacy_cftypes	= files,
+	.dfl_cftypes	= files,
+};