| 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 |
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
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
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 --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, +};
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