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

Daniel Vetter
2019-06-26T15:49:29Z

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

--
Daniel Vetter
Software Engineer, Intel Corporation