Re: [PATCH RFC v2 5/7] drm/i915: cgroup integration

2018-02-01 Thread Chris Wilson
Quoting Matt Roper (2018-02-01 19:53:13)
> +#include 
> +
> +#include "i915_drv.h"
> +
> +struct i915_cgroup_data {
> +   struct cgroup_driver_data base;
> +};
> +
> +static inline struct i915_cgroup_data *
> +cgrp_to_i915(struct cgroup_driver_data *data)
> +{

Document your requirement that base is offset 0 (required for both
i915_cgroup_alloc and i915_cgroup_free).

BUILD_BUG_ON(offsetof(struct i915_cgroup_data, base));

(or make said code flexible)

> +   return container_of(data, struct i915_cgroup_data, base);
> +}
> +
> +static struct cgroup_driver_data *
> +i915_cgroup_alloc(struct cgroup_driver *drv)
> +{
> +   struct i915_cgroup_data *data;
> +
> +   data = kzalloc(sizeof *data, GFP_KERNEL);
> +   return >base;
> +}
> +
> +static void
> +i915_cgroup_free(struct cgroup_driver_data *data)
> +{
> +   kfree(data);
> +}
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH RFC v2 5/7] drm/i915: cgroup integration

2018-02-01 Thread Chris Wilson
Quoting Matt Roper (2018-02-01 19:53:13)
> Register i915 as a consumer of the cgroup_driver framework and add an ioctl to
> allow userspace to set i915-specific parameters associated with cgroups.
> 
> Signed-off-by: Matt Roper 
> ---
>  drivers/gpu/drm/i915/Kconfig   |   1 +
>  drivers/gpu/drm/i915/Makefile  |   1 +
>  drivers/gpu/drm/i915/i915_cgroup.c | 134 
> +
>  drivers/gpu/drm/i915/i915_drv.c|   7 ++
>  drivers/gpu/drm/i915/i915_drv.h|  24 +++
>  include/uapi/drm/i915_drm.h|  12 
>  6 files changed, 179 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/i915_cgroup.c
> 
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index dfd95889f4b7..1c6b53ee76cd 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -23,6 +23,7 @@ config DRM_I915
> select SYNC_FILE
> select IOSF_MBI
> select CRC32
> +   select CGROUP_DRIVER if CGROUPS
> help
>   Choose this option if you have a system that has "Intel Graphics
>   Media Accelerator" or "HD Graphics" integrated graphics,
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 3bddd8a06806..5f4a21f1a9df 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -47,6 +47,7 @@ i915-y := i915_drv.o \
>  i915-$(CONFIG_COMPAT)   += i915_ioc32.o
>  i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o
>  i915-$(CONFIG_PERF_EVENTS) += i915_pmu.o
> +i915-$(CONFIG_CGROUPS) += i915_cgroup.o

CONFIG_CGROUP_DRIVER since that's strictly what the code depends on?
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH RFC v2 5/7] drm/i915: cgroup integration

2018-02-01 Thread Chris Wilson
Quoting Matt Roper (2018-02-01 19:53:13)
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index dfd95889f4b7..1c6b53ee76cd 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -23,6 +23,7 @@ config DRM_I915
> select SYNC_FILE
> select IOSF_MBI
> select CRC32
> +   select CGROUP_DRIVER if CGROUPS

Hmm, I was expecting a DRM_CGROUP around here?

Was it too small to be worth building conditionally?
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH RFC v2 5/7] drm/i915: cgroup integration

2018-02-01 Thread Matt Roper
Register i915 as a consumer of the cgroup_driver framework and add an ioctl to
allow userspace to set i915-specific parameters associated with cgroups.

Signed-off-by: Matt Roper 
---
 drivers/gpu/drm/i915/Kconfig   |   1 +
 drivers/gpu/drm/i915/Makefile  |   1 +
 drivers/gpu/drm/i915/i915_cgroup.c | 134 +
 drivers/gpu/drm/i915/i915_drv.c|   7 ++
 drivers/gpu/drm/i915/i915_drv.h|  24 +++
 include/uapi/drm/i915_drm.h|  12 
 6 files changed, 179 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/i915_cgroup.c

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index dfd95889f4b7..1c6b53ee76cd 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -23,6 +23,7 @@ config DRM_I915
select SYNC_FILE
select IOSF_MBI
select CRC32
+   select CGROUP_DRIVER if CGROUPS
help
  Choose this option if you have a system that has "Intel Graphics
  Media Accelerator" or "HD Graphics" integrated graphics,
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 3bddd8a06806..5f4a21f1a9df 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -47,6 +47,7 @@ i915-y := i915_drv.o \
 i915-$(CONFIG_COMPAT)   += i915_ioc32.o
 i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o
 i915-$(CONFIG_PERF_EVENTS) += i915_pmu.o
+i915-$(CONFIG_CGROUPS) += i915_cgroup.o
 
 # GEM code
 i915-y += i915_cmd_parser.o \
diff --git a/drivers/gpu/drm/i915/i915_cgroup.c 
b/drivers/gpu/drm/i915/i915_cgroup.c
new file mode 100644
index ..778af895fc00
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_cgroup.c
@@ -0,0 +1,134 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * i915_cgroup.c - Linux cgroups integration for i915
+ *
+ * Copyright (C) 2018 Intel Corporation
+ */
+
+#include 
+
+#include "i915_drv.h"
+
+struct i915_cgroup_data {
+   struct cgroup_driver_data base;
+};
+
+static inline struct i915_cgroup_data *
+cgrp_to_i915(struct cgroup_driver_data *data)
+{
+   return container_of(data, struct i915_cgroup_data, base);
+}
+
+static struct cgroup_driver_data *
+i915_cgroup_alloc(struct cgroup_driver *drv)
+{
+   struct i915_cgroup_data *data;
+
+   data = kzalloc(sizeof *data, GFP_KERNEL);
+   return >base;
+}
+
+static void
+i915_cgroup_free(struct cgroup_driver_data *data)
+{
+   kfree(data);
+}
+
+static struct cgroup_driver_funcs i915_cgroup_funcs = {
+   .alloc_data = i915_cgroup_alloc,
+   .free_data = i915_cgroup_free,
+};
+
+int
+i915_cgroup_init(struct drm_i915_private *dev_priv)
+{
+   dev_priv->i915_cgroups = cgroup_driver_init(_cgroup_funcs);
+   if (IS_ERR(dev_priv->i915_cgroups))
+   return PTR_ERR(dev_priv->i915_cgroups);
+
+   return 0;
+}
+
+void
+i915_cgroup_shutdown(struct drm_i915_private *dev_priv)
+{
+   cgroup_driver_release(dev_priv->i915_cgroups);
+}
+
+/**
+ * i915_cgroup_setparam_ioctl - ioctl to alter i915 settings for a cgroup
+ * @dev: DRM device
+ * @data: data pointer for the ioctl
+ * @file_priv: DRM file handle for the ioctl call
+ *
+ * Allows i915-specific parameters to be set for a Linux cgroup.
+ */
+int
+i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file)
+{
+   struct drm_i915_private *dev_priv = to_i915(dev);
+   struct drm_i915_cgroup_param *req = data;
+   struct cgroup *cgrp;
+   struct file *f;
+   struct inode *inode = NULL;
+   int ret;
+
+   if (!dev_priv->i915_cgroups) {
+   DRM_DEBUG_DRIVER("No support for driver-specific cgroup 
data\n");
+   return -EINVAL;
+   }
+
+   /* We don't actually support any flags yet. */
+   if (req->flags) {
+   DRM_DEBUG_DRIVER("Invalid flags\n");
+   return -EINVAL;
+   }
+
+   /*
+* Make sure the file descriptor really is a cgroup fd and is on the
+* v2 hierarchy.
+*/
+   cgrp = cgroup_get_from_fd(req->cgroup_fd);
+   if (IS_ERR(cgrp)) {
+   DRM_DEBUG_DRIVER("Invalid cgroup file descriptor\n");
+   return PTR_ERR(cgrp);
+   }
+
+   /*
+* Access control:  The strategy for using cgroups in a given
+* environment is generally determined by the system integrator
+* and/or OS vendor, so the specific policy about who can/can't
+* manipulate them tends to be domain-specific (and may vary
+* depending on the location in the cgroup hierarchy).  Rather than
+* trying to tie permission on this ioctl to a DRM-specific concepts
+* like DRM master, we'll allow cgroup parameters to be set by any
+* process that has been granted write access on the cgroup's
+* virtual file system (i.e., the same permissions that would
+* generally be needed to update the virtual files