[PATCH RFC v2 0/7] DRM management via cgroups

[PATCH RFC v2 0/7] DRM management via cgroups

[Matt Roper]

This is a second iteration of the patches originally posted here:
        https://lists.freedesktop.org/archives/intel-gfx/2018-January/153156.html

Please see the original cover letter to understand the general goal and
justification for this work.

This series makes the following important changes to the design from
the first version:

 * The "cgroup helper library" has been removed from the DRM core; the
   early patches of this series introduce an enhancement to the cgroup
   subsystem itself which will grant drivers the ability to save/restore
   their own driver-specific data structures associated with individual
   cgroups.  Drivers may subclass the cgroup_driver_data structure to
   store any kind of per-cgroup data they want, not just integer
   key/value pairs as the helper library from version 1 of this series
   did.  Moving this functionality directly into cgroups will also allow
   other parts of the kernel to potentially benefit from this
   functionality, not just DRM drivers.

 * The general graphics-specific ioctl to set cgroup parameters has
   moved from the DRM core to i915.  This was pretty much an arbitrary
   choice I made while re-writing the new version of the series; I don't
   have a strong opinion over whether the ioctl goes in the DRM core or
   stays i915-specific.  I'd appreciate feedback from other driver teams
   as to whether they anticipate cgroup integration being useful for
   their drivers and use cases (e.g., for controlling things like GPU
   memory usage, priority settings, etc.).  I'll move the ioctl back to
   the DRM core in the next iteration if there's interest from other
   driver teams.

 * The i915-specific usage of this functionality is to adjust GPU
   priority for groups of processes.  Based on Chris' feedback, I've
   made this control a "priority offset" rather than just a starting
   priority, so that the change applied by cgroup will be added to the
   explicitly-set context priority rather than being overwritten by it.
   At the moment i915 still just assigns the priority offset one time at
   context creation and doesn't alter it if the process migrates between
   cgroups at runtime or if the cgroup has a new value assigned to it;
   such changes only affect the priority offset of new contexts created
   by a process.  I'm open to feedback on whether we should also make
   cgroup changes affect existing GPU context(s) from a process.

As noted on v1 of my patch series, cgroup control of graphics driver
concepts is expected to be done mostly during system startup (e.g., a
sysv-init script or a systemd recipe written by a system integrator and
custom to their own setup), so we don't really need a terribly
complicated userspace, just a simple tool that can be called by
appropriate scripts/recipes to set the desired policy.  I've re-written
the tool from v1 of the patch series as an i-g-t tool this time and will
send it as followup email shortly.

Still on my TODO list (once I gather some more general feedback and move
out of the early RFC stage):
 * Document this more fully in both the cgroups documentation and the
   i915 kerneldoc.
 * Write some i-g-t tests to exercise the ioctl and all the corner
   cases.


Matt Roper (7):
  cgroup: Allow drivers to store data associated with a cgroup
  kernfs: Export kernfs_get_inode
  cgroup: Add interface to allow drivers to lookup process cgroup
    membership
  drm: Add helper to obtain cgroup of drm_file's owning process
  drm/i915: cgroup integration
  drm/i915: Introduce 'priority offset' for GPU contexts
  drm/i915: Add context priority & priority offset to debugfs

 drivers/gpu/drm/i915/Kconfig            |   1 +
 drivers/gpu/drm/i915/Makefile           |   1 +
 drivers/gpu/drm/i915/i915_cgroup.c      | 180 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_debugfs.c     |   3 +
 drivers/gpu/drm/i915/i915_drv.c         |   7 ++
 drivers/gpu/drm/i915/i915_drv.h         |  31 ++++++
 drivers/gpu/drm/i915/i915_gem_context.c |   6 +-
 drivers/gpu/drm/i915/i915_gem_context.h |   9 ++
 fs/kernfs/inode.c                       |   1 +
 include/drm/drm_file.h                  |  20 ++++
 include/linux/cgroup-defs.h             |  37 +++++++
 include/linux/cgroup.h                  |  33 ++++++
 include/uapi/drm/i915_drm.h             |  13 +++
 init/Kconfig                            |   4 +
 kernel/cgroup/Makefile                  |   1 +
 kernel/cgroup/cgroup.c                  |   1 +
 kernel/cgroup/cgroup_driver.c           | 162 ++++++++++++++++++++++++++++
 17 files changed, 508 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_cgroup.c
 create mode 100644 kernel/cgroup/cgroup_driver.c

-- 
2.14.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH RFC v2 1/7] cgroup: Allow drivers to store data associated with a cgroup

[Matt Roper]

There are cases where drivers need to adjust behavior or control
device-specific resources according to the type of clients (processes)
submitting requests.  Linux cgroups are a natural fit for this type of
resource control and policy management, so we need a way for drivers to
associate their own driver-specific and device-specific data with
individual cgroups.

This is different than the cgroup controller support that exists today
in several important ways:
 * Drivers may be built as modules (and unloaded/reloaded) which is not
   something cgroup controllers support today.
 * Drivers may wish to provide their own interface to allow userspace to
   adjust driver-specific settings (e.g., via a driver ioctl rather than
   via the kernfs filesystem).
 * A single driver may be managing multiple devices and wish to maintain
   different driver-specific cgroup data for each.

To use this mechanism, drivers should call cgroup_driver_init() to
register themselves and provide provide handler functions for allocating
driver-specific data structures; this call will return a handle that can
be used to lookup the driver-specific data associated with the device.
Drivers managing multiple devices that wish to track separate data for
each device may register themselves multiple times (e.g., a graphics
driver might call cgroup_driver_init() for each drm_device it manages).

At runtime, drivers may call cgroup_driver_get_data() to fetch the
driver-specific data associated with a cgroup.  The driver-specific data
(which should be a driver-defined subclass of 'struct
cgroup_driver_data') will be allocated if one doesn't already exist.
Management of driver-specific data by the cgroups framework is protected
by cgroup_mutex, but drivers are responsible for performing their own
synchronization on the per-cgroup data they receive, if necessary.  Note
that driver-specific data for a cgroup will only be allocated if/when
the driver first requests data for that cgroup.  The driver data will
also be automatically destroyed if the cgroup it belongs to is removed.

Finally, drivers should cgroup_driver_release() on driver unload to
destroy all of their driver-specific data.

Note that technically these interfaces aren't restricted to drivers
(other non-driver parts of the kernel could make use of them as well).
I expect drivers to be the primary consumers of this interface and
couldn't think of a more appropriate generic name (the term "subsystem"
would probably be more accurate, but that's already used by cgroup
controllers).

Cc: Tejun Heo <tj@kernel.org>
Cc: cgroups@vger.kernel.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 include/linux/cgroup-defs.h   |  37 ++++++++++++
 include/linux/cgroup.h        |  27 +++++++++
 init/Kconfig                  |   4 ++
 kernel/cgroup/Makefile        |   1 +
 kernel/cgroup/cgroup.c        |   1 +
 kernel/cgroup/cgroup_driver.c | 130 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 200 insertions(+)
 create mode 100644 kernel/cgroup/cgroup_driver.c

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 8b7fd8eeccee..5728e3afc95f 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -8,6 +8,7 @@
 #ifndef _LINUX_CGROUP_DEFS_H
 #define _LINUX_CGROUP_DEFS_H
 
+#include <linux/hashtable.h>
 #include <linux/limits.h>
 #include <linux/list.h>
 #include <linux/idr.h>
@@ -27,6 +28,7 @@ struct cgroup;
 struct cgroup_root;
 struct cgroup_subsys;
 struct cgroup_taskset;
+struct cgroup_driver;
 struct kernfs_node;
 struct kernfs_ops;
 struct kernfs_open_file;
@@ -307,6 +309,22 @@ struct cgroup_stat {
 	struct prev_cputime prev_cputime;
 };
 
+/*
+ * Driver-specific cgroup data.  Drivers should subclass this structure with
+ * their own fields for data that should be stored alongside individual
+ * cgroups.
+ */
+struct cgroup_driver_data {
+	/* Driver this data structure is associated with */
+	struct cgroup_driver *drv;
+
+	/* Node in cgroup's data hashtable */
+	struct hlist_node cgroupnode;
+
+	/* Node in driver's data list; used to cleanup on driver unload */
+	struct list_head drivernode;
+};
+
 struct cgroup {
 	/* self css with NULL ->ss, points back to this cgroup */
 	struct cgroup_subsys_state self;
@@ -427,6 +445,12 @@ struct cgroup {
 	/* used to store eBPF programs */
 	struct cgroup_bpf bpf;
 
+	/*
+	 * list of cgroup_driver_data structures; used to manage
+	 * driver-specific data associated with individual cgroups
+	 */
+	DECLARE_HASHTABLE(driver_data, 4);
+
 	/* ids of the ancestors at each level including self */
 	int ancestor_ids[];
 };
@@ -662,6 +686,19 @@ struct cgroup_subsys {
 	unsigned int depends_on;
 };
 
+/* Function table for handling driver-specific cgroup data */
+struct cgroup_driver_funcs {
+	/* Allocates driver-specific data for a cgroup */
+	struct cgroup_driver_data *(*alloc_data)(struct cgroup_driver *drv);
+
+	/*
+	 * Frees a driver-specific datastructure.
+	 *
+	 * This function is optional; if NULL, data will be kvfree()'d.
+	 */
+	void (*free_data)(struct cgroup_driver_data *data);
+};
+
 extern struct percpu_rw_semaphore cgroup_threadgroup_rwsem;
 
 /**
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 473e0c0abb86..0ba1374122c7 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -833,4 +833,31 @@ static inline void put_cgroup_ns(struct cgroup_namespace *ns)
 		free_cgroup_ns(ns);
 }
 
+struct cgroup_driver_funcs;
+
+#ifdef CONFIG_CGROUP_DRIVER
+
+struct cgroup_driver *cgroup_driver_init(struct cgroup_driver_funcs *funcs);
+void cgroup_driver_release(struct cgroup_driver *drv);
+struct cgroup_driver_data * cgroup_driver_get_data(struct cgroup_driver *drv,
+						   struct cgroup *cgrp,
+						   bool *is_new);
+
+#else /* !CONFIG_CGROUP_DRIVER */
+
+static inline struct cgroup_driver *
+cgroup_driver_init(struct cgroup_driver_funcs *funcs) {
+	return NULL;
+}
+static inline void cgroup_driver_release(struct cgroup_driver *drv) {}
+static inline struct cgroup_driver_data *
+cgroup_driver_get_data(struct cgroup_driver *drv,
+		       struct cgroup *cgrp,
+		       bool *is_new)
+{
+	return ERR_PTR(-EINVAL);
+}
+
+#endif /* !CONFIG_CGROUP_DRIVER */
+
 #endif /* _LINUX_CGROUP_H */
diff --git a/init/Kconfig b/init/Kconfig
index a9a2e2c86671..5f942ed8d248 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -873,6 +873,10 @@ config SOCK_CGROUP_DATA
 	bool
 	default n
 
+config CGROUP_DRIVER
+	bool
+	default n
+
 endif # CGROUPS
 
 menuconfig NAMESPACES
diff --git a/kernel/cgroup/Makefile b/kernel/cgroup/Makefile
index 2be89a003185..669ef66da3a2 100644
--- a/kernel/cgroup/Makefile
+++ b/kernel/cgroup/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_CGROUP_PIDS) += pids.o
 obj-$(CONFIG_CGROUP_RDMA) += rdma.o
 obj-$(CONFIG_CPUSETS) += cpuset.o
 obj-$(CONFIG_CGROUP_DEBUG) += debug.o
+obj-$(CONFIG_CGROUP_DRIVER) += cgroup_driver.o
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 7e4c44538119..b926713a1860 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -1838,6 +1838,7 @@ static void init_cgroup_housekeeping(struct cgroup *cgrp)
 	INIT_LIST_HEAD(&cgrp->self.children);
 	INIT_LIST_HEAD(&cgrp->cset_links);
 	INIT_LIST_HEAD(&cgrp->pidlists);
+	hash_init(cgrp->driver_data);
 	mutex_init(&cgrp->pidlist_mutex);
 	cgrp->self.cgroup = cgrp;
 	cgrp->self.flags |= CSS_ONLINE;
diff --git a/kernel/cgroup/cgroup_driver.c b/kernel/cgroup/cgroup_driver.c
new file mode 100644
index 000000000000..0d893395dc7b
--- /dev/null
+++ b/kernel/cgroup/cgroup_driver.c
@@ -0,0 +1,130 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include "cgroup-internal.h"
+
+#include <linux/hashtable.h>
+#include <linux/mm.h>
+
+/*
+ * General data structure returned by cgroup_driver_init() and used as a
+ * hashtable key to lookup driver-specific data.
+ */
+struct cgroup_driver {
+	/* Functions this driver uses to manage its data */
+	struct cgroup_driver_funcs *funcs;
+
+	/*
+	 * List of driver-specific data structures that need to be cleaned up
+	 * if driver is unloaded.
+	 */
+	struct list_head datalist;
+};
+
+/**
+ * cgroup_driver_init - initialize cgroups driver-specific data management
+ * @funcs: driver-specific data management functions
+ *
+ * Drivers that wish to store driver-specific data alongside individual
+ * cgroups should call this and provide a function table of driver-specific
+ * data operations.
+ *
+ * RETURNS:
+ * An instance of 'struct cgroup_driver' that will be used to help manage
+ * data storage for the invoking driver.  If an error occurs, a negative
+ * error code will be returned.  If CONFIG_CGROUP_DRIVER is not set, NULL
+ * will be returned.
+ */
+struct cgroup_driver *
+cgroup_driver_init(struct cgroup_driver_funcs *funcs)
+{
+	struct cgroup_driver *drv;
+
+	drv = kzalloc(sizeof *drv, GFP_KERNEL);
+	if (!drv)
+		return ERR_PTR(-ENOMEM);
+
+	drv->funcs = funcs;
+	INIT_LIST_HEAD(&drv->datalist);
+
+	return drv;
+}
+EXPORT_SYMBOL(cgroup_driver_init);
+
+/**
+ * cgroup_driver_release - release all driver-specific data for a driver
+ * @drv: driver to release data for
+ *
+ * Drivers storing their own data alongside cgroups should call this function
+ * when unloaded to ensure all driver-specific data is released.
+ */
+void
+cgroup_driver_release(struct cgroup_driver *drv)
+{
+	struct cgroup_driver_data *data, *tmp;
+
+	mutex_lock(&cgroup_mutex);
+	list_for_each_entry_safe(data, tmp, &drv->datalist, drivernode) {
+		hlist_del(&data->cgroupnode);
+		list_del(&data->drivernode);
+		if (drv->funcs && drv->funcs->free_data)
+			drv->funcs->free_data(data);
+		else
+			kvfree(data);
+	}
+	mutex_unlock(&cgroup_mutex);
+
+	kfree(drv);
+}
+EXPORT_SYMBOL(cgroup_driver_release);
+
+/**
+ * cgroup_driver_get_data - retrieve/allocate driver-specific data for a cgroup
+ * @drv: driver wishing to fetch data
+ * @cgrp: cgroup to fetch data for
+ * @is_new: will be set to true if a new structure is allocated
+ *
+ * Fetches the driver-specific data structure associated with a cgroup, if one
+ * has previously been set.  If no driver data has been associated with this
+ * cgroup, a new driver-specific data structure is allocated and returned.
+ *
+ * RETURNS:
+ * The driver data previously associated with this cgroup, or a fresh data
+ * structure allocated via drv->funcs->alloc_data() if no data has previously
+ * been associated.  On error, a negative error code is returned.
+ */
+struct cgroup_driver_data *
+cgroup_driver_get_data(struct cgroup_driver *drv,
+		       struct cgroup *cgrp,
+		       bool *is_new)
+{
+	struct cgroup_driver_data *data;
+
+	/* We only support driver-specific data on the cgroup-v2 hierarchy */
+	if (!cgroup_on_dfl(cgrp))
+		return ERR_PTR(-EINVAL);
+
+	mutex_lock(&cgroup_mutex);
+
+	if (is_new)
+		*is_new = false;
+	hash_for_each_possible(cgrp->driver_data, data, cgroupnode,
+			       (unsigned long)drv)
+		if (data->drv == drv)
+			goto out;
+
+	/* First time for this cgroup; alloc and store new data */
+	data = drv->funcs->alloc_data(drv);
+	if (!IS_ERR(data)) {
+		data->drv = drv;
+		hash_add(cgrp->driver_data, &data->cgroupnode,
+			 (unsigned long)drv);
+		list_add(&data->drivernode, &drv->datalist);
+		if (is_new)
+			*is_new = true;
+	}
+
+out:
+	mutex_unlock(&cgroup_mutex);
+	return data;
+}
+EXPORT_SYMBOL(cgroup_driver_get_data);
-- 
2.14.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC v2 1/7] cgroup: Allow drivers to store data associated with a cgroup

[Tejun Heo]

Hello,

On Thu, Feb 01, 2018 at 11:53:09AM -0800, Matt Roper wrote:
>  * Drivers may be built as modules (and unloaded/reloaded) which is not
>    something cgroup controllers support today.

As discussed in the other subthread, this shouldn't be a concern.

>  * Drivers may wish to provide their own interface to allow userspace to
>    adjust driver-specific settings (e.g., via a driver ioctl rather than
>    via the kernfs filesystem).
>  * A single driver may be managing multiple devices and wish to maintain
>    different driver-specific cgroup data for each.

If you look at io and rdma controllers, they already do this.

> Note that technically these interfaces aren't restricted to drivers
> (other non-driver parts of the kernel could make use of them as well).
> I expect drivers to be the primary consumers of this interface and
> couldn't think of a more appropriate generic name (the term "subsystem"
> would probably be more accurate, but that's already used by cgroup
> controllers).

Let's please not do "driver", it's really confusing.  Just coming up
with a made-up word would be fine as long as the connection can be
made and the word is easily identifiable.  e.g. cgroup cdata / pdata for
cgroup custom / priv data.

> +/*
> + * Driver-specific cgroup data.  Drivers should subclass this structure with
> + * their own fields for data that should be stored alongside individual
> + * cgroups.
> + */
> +struct cgroup_driver_data {
> +	/* Driver this data structure is associated with */
> +	struct cgroup_driver *drv;
> +
> +	/* Node in cgroup's data hashtable */
> +	struct hlist_node cgroupnode;
> +
> +	/* Node in driver's data list; used to cleanup on driver unload */
> +	struct list_head drivernode;
> +};
...
> +struct cgroup_driver {
> +	/* Functions this driver uses to manage its data */
> +	struct cgroup_driver_funcs *funcs;
> +
> +	/*
> +	 * List of driver-specific data structures that need to be cleaned up
> +	 * if driver is unloaded.
> +	 */
> +	struct list_head datalist;
> +};

It generally looks great but can we do something like the following in
terms of interface?

  struct cgroup_cdata {
	  const void *key;
	  void (*free)(struct cgroup_cdata *cdata);
	  /* whatever other necessary fields */
	  char data[];
  };

  int cgroup_cdata_install(struct cgroup *cgrp, struct cgroup_cdata *cdata);
  struct cgroup_cdata *cgroup_cdata_lookup(struct cgroup *cgrp, const void *key);
  int cgroup_cdata_free(struct cgroup *cgrp, const void *key);
  /* free is also automatically called when the cgroup is released */

And please use a separate lock or mutex for managing them.

Thanks.

-- 
tejun

[PATCH RFC v2 2/7] kernfs: Export kernfs_get_inode

[Matt Roper]

Drivers may wish to access a cgroup's inode to perform permission checks on
driver-specific operations.

Cc: Tejun Heo <tj@kernel.org>
Cc: cgroups@vger.kernel.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 fs/kernfs/inode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index a34303981deb..e1e8a0404c5b 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -272,6 +272,7 @@ struct inode *kernfs_get_inode(struct super_block *sb, struct kernfs_node *kn)
 
 	return inode;
 }
+EXPORT_SYMBOL(kernfs_get_inode);
 
 /*
  * The kernfs_node serves as both an inode and a directory entry for
-- 
2.14.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH RFC v2 2/7] kernfs: Export kernfs_get_inode

[Chris Wilson]

Quoting Matt Roper (2018-02-01 19:53:10)
> Drivers may wish to access a cgroup's inode to perform permission checks on
> driver-specific operations.
> 
> Cc: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Matt Roper <matthew.d.roper-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  fs/kernfs/inode.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
> index a34303981deb..e1e8a0404c5b 100644
> --- a/fs/kernfs/inode.c
> +++ b/fs/kernfs/inode.c
> @@ -272,6 +272,7 @@ struct inode *kernfs_get_inode(struct super_block *sb, struct kernfs_node *kn)
>  
>         return inode;
>  }
> +EXPORT_SYMBOL(kernfs_get_inode);

Will want the _GPL suffix to be in keeping with the other kernfs
exports.
-Chris

[PATCH RFC v2 3/7] cgroup: Add interface to allow drivers to lookup process cgroup membership

[Matt Roper]

Drivers will need to save/restore the appropriate cgroup data for the process
submitting a driver request.  Add an interface for drivers to determine the
cgroup for the userspace process submitting a driver request.

Cc: Tejun Heo <tj@kernel.org>
Cc: cgroups@vger.kernel.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 include/linux/cgroup.h        |  6 ++++++
 kernel/cgroup/cgroup_driver.c | 24 ++++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 0ba1374122c7..05ebfb97bcde 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -842,6 +842,7 @@ void cgroup_driver_release(struct cgroup_driver *drv);
 struct cgroup_driver_data * cgroup_driver_get_data(struct cgroup_driver *drv,
 						   struct cgroup *cgrp,
 						   bool *is_new);
+struct cgroup *cgroup_for_driver_process(struct pid *pid);
 
 #else /* !CONFIG_CGROUP_DRIVER */
 
@@ -857,6 +858,11 @@ cgroup_driver_get_data(struct cgroup_driver *drv,
 {
 	return ERR_PTR(-EINVAL);
 }
+static inline struct cgroup *
+cgroup_for_driver_process(struct pid *pid)
+{
+	return NULL;
+}
 
 #endif /* !CONFIG_CGROUP_DRIVER */
 
diff --git a/kernel/cgroup/cgroup_driver.c b/kernel/cgroup/cgroup_driver.c
index 0d893395dc7b..4f870cbb9212 100644
--- a/kernel/cgroup/cgroup_driver.c
+++ b/kernel/cgroup/cgroup_driver.c
@@ -128,3 +128,27 @@ cgroup_driver_get_data(struct cgroup_driver *drv,
 	return data;
 }
 EXPORT_SYMBOL(cgroup_driver_get_data);
+
+/**
+ * cgroup_for_driver_process - return the cgroup for a process
+ * @pid: process to lookup cgroup for
+ *
+ * Returns the cgroup from the v2 hierarchy that a process belongs to.
+ * This function is intended to be called from drivers and will obtain
+ * the necessary cgroup locks.
+ *
+ * RETURNS:
+ * Process' cgroup in the default (v2) hierarchy
+ */
+struct cgroup *
+cgroup_for_driver_process(struct pid *pid)
+{
+	struct task_struct *task = pid_task(pid, PIDTYPE_PID);
+
+	mutex_lock(&cgroup_mutex);
+	spin_lock_irq(&css_set_lock);
+	task_cgroup_from_root(task, &cgrp_dfl_root);
+	spin_unlock_irq(&css_set_lock);
+	mutex_unlock(&cgroup_mutex);
+}
+EXPORT_SYMBOL(cgroup_for_driver_process);
-- 
2.14.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH RFC v2 3/7] cgroup: Add interface to allow drivers to lookup process cgroup membership

[Chris Wilson]

Quoting Matt Roper (2018-02-01 19:53:11)
> Drivers will need to save/restore the appropriate cgroup data for the process
> submitting a driver request.  Add an interface for drivers to determine the
> cgroup for the userspace process submitting a driver request.
> 
> Cc: Tejun Heo <tj@kernel.org>
> Cc: cgroups@vger.kernel.org
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  include/linux/cgroup.h        |  6 ++++++
>  kernel/cgroup/cgroup_driver.c | 24 ++++++++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index 0ba1374122c7..05ebfb97bcde 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -842,6 +842,7 @@ void cgroup_driver_release(struct cgroup_driver *drv);
>  struct cgroup_driver_data * cgroup_driver_get_data(struct cgroup_driver *drv,
>                                                    struct cgroup *cgrp,
>                                                    bool *is_new);
> +struct cgroup *cgroup_for_driver_process(struct pid *pid);
>  
>  #else /* !CONFIG_CGROUP_DRIVER */
>  
> @@ -857,6 +858,11 @@ cgroup_driver_get_data(struct cgroup_driver *drv,
>  {
>         return ERR_PTR(-EINVAL);
>  }
> +static inline struct cgroup *
> +cgroup_for_driver_process(struct pid *pid)
> +{
> +       return NULL;
> +}
>  
>  #endif /* !CONFIG_CGROUP_DRIVER */
>  
> diff --git a/kernel/cgroup/cgroup_driver.c b/kernel/cgroup/cgroup_driver.c
> index 0d893395dc7b..4f870cbb9212 100644
> --- a/kernel/cgroup/cgroup_driver.c
> +++ b/kernel/cgroup/cgroup_driver.c
> @@ -128,3 +128,27 @@ cgroup_driver_get_data(struct cgroup_driver *drv,
>         return data;
>  }
>  EXPORT_SYMBOL(cgroup_driver_get_data);
> +
> +/**
> + * cgroup_for_driver_process - return the cgroup for a process
> + * @pid: process to lookup cgroup for
> + *
> + * Returns the cgroup from the v2 hierarchy that a process belongs to.
> + * This function is intended to be called from drivers and will obtain
> + * the necessary cgroup locks.
> + *
> + * RETURNS:
> + * Process' cgroup in the default (v2) hierarchy
> + */
> +struct cgroup *
> +cgroup_for_driver_process(struct pid *pid)
> +{
> +       struct task_struct *task = pid_task(pid, PIDTYPE_PID);
> +
> +       mutex_lock(&cgroup_mutex);
> +       spin_lock_irq(&css_set_lock);
> +       task_cgroup_from_root(task, &cgrp_dfl_root);
> +       spin_unlock_irq(&css_set_lock);
> +       mutex_unlock(&cgroup_mutex);

Gut feeling says use task = get_pid_task(); if (!task) return NULL and
put_task_struct(task); for safety.

Shouldn't there be a return here?
> +}
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC v2 3/7] cgroup: Add interface to allow drivers to lookup process cgroup membership

[Matt Roper]

On Thu, Feb 01, 2018 at 08:49:32PM +0000, Chris Wilson wrote:
> Quoting Matt Roper (2018-02-01 19:53:11)
...
> > +struct cgroup *
> > +cgroup_for_driver_process(struct pid *pid)
> > +{
> > +       struct task_struct *task = pid_task(pid, PIDTYPE_PID);
> > +
> > +       mutex_lock(&cgroup_mutex);
> > +       spin_lock_irq(&css_set_lock);
> > +       task_cgroup_from_root(task, &cgrp_dfl_root);
> > +       spin_unlock_irq(&css_set_lock);
> > +       mutex_unlock(&cgroup_mutex);
> 
> Gut feeling says use task = get_pid_task(); if (!task) return NULL and
> put_task_struct(task); for safety.
> 
> Shouldn't there be a return here?

Woops, looks like I mis-squashed some of my fixes for this patch in to
patch #4 instead.  But yeah, get_pid_task() is better in general.


Matt

> > +}
> -Chris
> --
> To unsubscribe from this list: send the line "unsubscribe cgroups" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH RFC v2 3/7] cgroup: Add interface to allow drivers to lookup process cgroup membership

[Tejun Heo]

Hello,

On Thu, Feb 01, 2018 at 11:53:11AM -0800, Matt Roper wrote:
> +/**
> + * cgroup_for_driver_process - return the cgroup for a process
> + * @pid: process to lookup cgroup for
> + *
> + * Returns the cgroup from the v2 hierarchy that a process belongs to.
> + * This function is intended to be called from drivers and will obtain
> + * the necessary cgroup locks.
> + *
> + * RETURNS:
> + * Process' cgroup in the default (v2) hierarchy
> + */
> +struct cgroup *
> +cgroup_for_driver_process(struct pid *pid)

I think you probably want to use task_dfl_cgroup().  We can add
task_get_dfl_cgroup() in a similar fashion to task_get_css() if
necessary.

Thanks.

-- 
tejun
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH RFC v2 4/7] drm: Add helper to obtain cgroup of drm_file's owning process

[Matt Roper]

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 include/drm/drm_file.h        | 20 ++++++++++++++++++++
 kernel/cgroup/cgroup_driver.c | 12 ++++++++++--
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
index 0e0c868451a5..72ac40530ad3 100644
--- a/include/drm/drm_file.h
+++ b/include/drm/drm_file.h
@@ -32,6 +32,7 @@
 
 #include <linux/types.h>
 #include <linux/completion.h>
+#include <linux/cgroup.h>
 
 #include <uapi/drm/drm.h>
 
@@ -378,4 +379,23 @@ void drm_event_cancel_free(struct drm_device *dev,
 void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e);
 void drm_send_event(struct drm_device *dev, struct drm_pending_event *e);
 
+#ifdef CONFIG_CGROUPS
+/**
+ * drm_file_get_cgroup - obtain cgroup for drm_file's owning process
+ * @file_priv: DRM file
+ *
+ * Obtains the cgroup from a specific hierarchy that the drm_file's owning
+ * process belongs to.  The cgroup may be used to set driver-specific
+ * policy (priority, vram usage, etc.).
+ */
+static inline struct cgroup *
+drm_file_get_cgroup(struct drm_file *file_priv)
+{
+	return cgroup_for_driver_process(file_priv->pid);
+}
+#else
+static inline struct cgroup *
+drm_file_get_cgroup(struct drm_file *file_priv) { return NULL; }
+#endif
+
 #endif /* _DRM_FILE_H_ */
diff --git a/kernel/cgroup/cgroup_driver.c b/kernel/cgroup/cgroup_driver.c
index 4f870cbb9212..db0e268b9546 100644
--- a/kernel/cgroup/cgroup_driver.c
+++ b/kernel/cgroup/cgroup_driver.c
@@ -4,6 +4,7 @@
 
 #include <linux/hashtable.h>
 #include <linux/mm.h>
+#include <linux/sched/task.h>
 
 /*
  * General data structure returned by cgroup_driver_init() and used as a
@@ -143,12 +144,19 @@ EXPORT_SYMBOL(cgroup_driver_get_data);
 struct cgroup *
 cgroup_for_driver_process(struct pid *pid)
 {
-	struct task_struct *task = pid_task(pid, PIDTYPE_PID);
+	struct task_struct *task;
+	struct cgroup *cgrp;
+
+	read_lock(&tasklist_lock);
+	task = pid_task(pid, PIDTYPE_PID);
+	read_unlock(&tasklist_lock);
 
 	mutex_lock(&cgroup_mutex);
 	spin_lock_irq(&css_set_lock);
-	task_cgroup_from_root(task, &cgrp_dfl_root);
+	cgrp = task_cgroup_from_root(task, &cgrp_dfl_root);
 	spin_unlock_irq(&css_set_lock);
 	mutex_unlock(&cgroup_mutex);
+
+	return cgrp;
 }
 EXPORT_SYMBOL(cgroup_for_driver_process);
-- 
2.14.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH RFC v2 4/7] drm: Add helper to obtain cgroup of drm_file's owning process

[Chris Wilson]

Quoting Matt Roper (2018-02-01 19:53:12)
> +/**
> + * drm_file_get_cgroup - obtain cgroup for drm_file's owning process
> + * @file_priv: DRM file
> + *
> + * Obtains the cgroup from a specific hierarchy that the drm_file's owning
> + * process belongs to.  The cgroup may be used to set driver-specific
> + * policy (priority, vram usage, etc.).
> + */
> +static inline struct cgroup *
> +drm_file_get_cgroup(struct drm_file *file_priv)
> +{
> +       return cgroup_for_driver_process(file_priv->pid);

Just a word of warning file_priv->pid isn't a true indicator of the
attached process's pid. E.g. DRI3 passes the struct drm_file over a UNIX
socket, so pid is Xorg but the actual process is the GL client.

Not sure if we can detect the transfer over the socket, but one
suggestion has been to update file->pid based on who creates a context.
-Chris

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

[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 <matthew.d.roper@intel.com>
---
 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 000000000000..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 <linux/cgroup.h>
+
+#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 &data->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(&i915_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 provided by
+	 * cgroup controllers).
+	 */
+	f = fget_raw(req->cgroup_fd);
+	if (WARN_ON(!f))
+		return -EBADF;
+
+	inode = kernfs_get_inode(f->f_path.dentry->d_sb, cgrp->kn);
+	if (inode)
+		ret = inode_permission(inode, MAY_WRITE);
+	else
+		ret = -ENOMEM;
+
+	iput(inode);
+	fput(f);
+
+	if (ret)
+		return ret;
+
+	switch (req->param) {
+	default:
+		DRM_DEBUG_DRIVER("Invalid cgroup parameter %lld\n", req->param);
+		return -EINVAL;
+	}
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1ec12add34b2..5d5e652598fb 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1398,6 +1398,10 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	intel_runtime_pm_put(dev_priv);
 
+	ret = i915_cgroup_init(dev_priv);
+	if (ret < 0)
+	    goto out_cleanup_hw;
+
 	i915_welcome_messages(dev_priv);
 
 	return 0;
@@ -1424,6 +1428,8 @@ void i915_driver_unload(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct pci_dev *pdev = dev_priv->drm.pdev;
 
+	i915_cgroup_shutdown(dev_priv);
+
 	i915_driver_unregister(dev_priv);
 
 	if (i915_gem_suspend(dev_priv))
@@ -2835,6 +2841,7 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl, DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG, i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_CGROUP_SETPARAM, i915_cgroup_setparam_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 };
 
 static struct drm_driver driver = {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c676269ed843..60e3ff6a48bb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2357,6 +2357,9 @@ struct drm_i915_private {
 
 	struct i915_pmu pmu;
 
+	/* Linux cgroup integration */
+	struct cgroup_driver *i915_cgroups;
+
 	/*
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
@@ -2911,6 +2914,27 @@ intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *dev_priv)
 int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
 				int enable_ppgtt);
 
+/* i915_cgroup.c */
+#ifdef CONFIG_CGROUPS
+int i915_cgroup_init(struct drm_i915_private *dev_priv);
+int i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
+			       struct drm_file *file);
+void i915_cgroup_shutdown(struct drm_i915_private *dev_priv);
+#else
+static inline int
+i915_cgroup_init(struct drm_i915_private *dev_priv)
+{
+	return 0;
+}
+static inline void i915_cgroup_shutdown(struct drm_i915_private *dev_priv) {}
+static inline int
+i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
+			   struct drm_file *file)
+{
+	return -EINVAL;
+}
+#endif
+
 /* i915_drv.c */
 void __printf(3, 4)
 __i915_printk(struct drm_i915_private *dev_priv, const char *level,
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 536ee4febd74..fe333ae1f0c6 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -318,6 +318,7 @@ typedef struct _drm_i915_sarea {
 #define DRM_I915_PERF_OPEN		0x36
 #define DRM_I915_PERF_ADD_CONFIG	0x37
 #define DRM_I915_PERF_REMOVE_CONFIG	0x38
+#define DRM_I915_CGROUP_SETPARAM	0x39
 
 #define DRM_IOCTL_I915_INIT		DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
 #define DRM_IOCTL_I915_FLUSH		DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
@@ -375,6 +376,7 @@ typedef struct _drm_i915_sarea {
 #define DRM_IOCTL_I915_PERF_OPEN	DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_OPEN, struct drm_i915_perf_open_param)
 #define DRM_IOCTL_I915_PERF_ADD_CONFIG	DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_ADD_CONFIG, struct drm_i915_perf_oa_config)
 #define DRM_IOCTL_I915_PERF_REMOVE_CONFIG	DRM_IOW(DRM_COMMAND_BASE + DRM_I915_PERF_REMOVE_CONFIG, __u64)
+#define DRM_IOCTL_I915_CGROUP_SETPARAM		DRM_IOW(DRM_COMMAND_BASE + DRM_I915_CGROUP_SETPARAM, struct drm_i915_cgroup_param)
 
 /* Allow drivers to submit batchbuffers directly to hardware, relying
  * on the security mechanisms provided by hardware.
@@ -1613,6 +1615,16 @@ struct drm_i915_perf_oa_config {
 	__u64 flex_regs_ptr;
 };
 
+/**
+ * Structure to set i915 parameter on a cgroup.
+ */
+struct drm_i915_cgroup_param {
+	__s32 cgroup_fd;
+	__u32 flags;
+	__u64 param;
+	__s64 value;
+};
+
 #if defined(__cplusplus)
 }
 #endif
-- 
2.14.3

_______________________________________________
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

[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

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

[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 <matthew.d.roper-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  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

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

[Chris Wilson]

Quoting Matt Roper (2018-02-01 19:53:13)
> +#include <linux/cgroup.h>
> +
> +#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 &data->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

[PATCH RFC v2 6/7] drm/i915: Introduce 'priority offset' for GPU contexts

[Matt Roper]

There are cases where a system integrator may wish to raise/lower the
priority of GPU workloads being submitted by specific OS process(es),
independently of how the software self-classifies its own priority.
Exposing "priority offset" as an i915-specific cgroup parameter will
enable such system-level configuration.

Normally GPU contexts start with a priority value of 0
(I915_CONTEXT_DEFAULT_PRIORITY) and then may be adjusted up/down from
there via other mechanisms.  We'd like to provide a system-level input
to the priority decision that will be taken into consideration, even
when userspace later attempts to set an absolute priority value via
I915_CONTEXT_PARAM_PRIORITY.  The priority offset introduced here
provides a base value that will always be added to (or subtracted from)
the software's self-assigned priority value.

This patch makes priority offset a cgroup-specific value; contexts will
be created with a priority offset based on the cgroup membership of the
process creating the context at the time the context is created.  Note
that priority offset is assigned at context creation time; migrating a
process to a different cgroup or changing the offset associated with a
cgroup will only affect new context creation and will not alter the
behavior of existing contexts previously created by the process.

Signed-off-by: Matt Roper <matthew.d.roper-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/i915/i915_cgroup.c      | 46 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h         |  7 +++++
 drivers/gpu/drm/i915/i915_gem_context.c |  6 +++--
 drivers/gpu/drm/i915/i915_gem_context.h |  9 +++++++
 include/uapi/drm/i915_drm.h             |  1 +
 5 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cgroup.c b/drivers/gpu/drm/i915/i915_cgroup.c
index 778af895fc00..73f3cfe31d66 100644
--- a/drivers/gpu/drm/i915/i915_cgroup.c
+++ b/drivers/gpu/drm/i915/i915_cgroup.c
@@ -11,6 +11,8 @@
 
 struct i915_cgroup_data {
 	struct cgroup_driver_data base;
+
+	int priority_offset;
 };
 
 static inline struct i915_cgroup_data *
@@ -72,6 +74,8 @@ i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
 	struct cgroup *cgrp;
 	struct file *f;
 	struct inode *inode = NULL;
+	struct cgroup_driver_data *cgrpdata;
+	struct i915_cgroup_data *i915data;
 	int ret;
 
 	if (!dev_priv->i915_cgroups) {
@@ -125,6 +129,17 @@ i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
 		return ret;
 
 	switch (req->param) {
+	case I915_CGROUP_PARAM_PRIORITY_OFFSET:
+		cgrpdata = cgroup_driver_get_data(dev_priv->i915_cgroups, cgrp,
+						  NULL);
+		if (IS_ERR(cgrpdata))
+			return PTR_ERR(cgrpdata);
+
+		DRM_DEBUG_DRIVER("Setting cgroup priority offset to %lld\n",
+				 req->value);
+		i915data = cgrp_to_i915(cgrpdata);
+		i915data->priority_offset = req->value;
+		break;
 	default:
 		DRM_DEBUG_DRIVER("Invalid cgroup parameter %lld\n", req->param);
 		return -EINVAL;
@@ -132,3 +147,34 @@ i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
 
 	return 0;
 }
+
+/**
+ * i915_cgroup_get_prio_offset() - get prio offset for current proc's cgroup
+ * @dev_priv: drm device
+ * @file_priv: file handle for calling process
+ *
+ * RETURNS:
+ * Priority offset associated with the calling process' cgroup in the default
+ * (v2) hierarchy, otherwise 0 if no explicit priority has been assigned.
+ */
+int
+i915_cgroup_get_prio_offset(struct drm_i915_private *dev_priv,
+			    struct drm_i915_file_private *file_priv)
+{
+       struct cgroup *cgrp;
+       struct cgroup_driver_data *cgrpdata;
+
+       /* Ignore internally-created contexts not associated with a process */
+       if (!file_priv)
+               return 0;
+
+       cgrp = drm_file_get_cgroup(file_priv->file);
+       if (WARN_ON(!cgrp))
+               return 0;
+
+       cgrpdata = cgroup_driver_get_data(dev_priv->i915_cgroups, cgrp, NULL);
+       if (IS_ERR(cgrpdata))
+	       return 0;
+
+       return cgrp_to_i915(cgrpdata)->priority_offset;
+}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 60e3ff6a48bb..313191e5278a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2920,6 +2920,8 @@ int i915_cgroup_init(struct drm_i915_private *dev_priv);
 int i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
 			       struct drm_file *file);
 void i915_cgroup_shutdown(struct drm_i915_private *dev_priv);
+int i915_cgroup_get_prio_offset(struct drm_i915_private *dev_priv,
+				struct drm_i915_file_private *file_priv);
 #else
 static inline int
 i915_cgroup_init(struct drm_i915_private *dev_priv)
@@ -2933,6 +2935,11 @@ i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
 {
 	return -EINVAL;
 }
+static inline int
+i915_cgroup_get_prio_offset(struct drm_i915_private *dev_priv,
+			    struct drm_i915_file_private *file_priv) {
+	return 0;
+}
 #endif
 
 /* i915_drv.c */
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 648e7536ff51..700d7bc7cfc8 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -274,7 +274,9 @@ __create_hw_context(struct drm_i915_private *dev_priv,
 	kref_init(&ctx->ref);
 	list_add_tail(&ctx->link, &dev_priv->contexts.list);
 	ctx->i915 = dev_priv;
-	ctx->priority = I915_PRIORITY_NORMAL;
+	ctx->priority_offset =
+		i915_cgroup_get_prio_offset(dev_priv, file_priv);
+	ctx->priority = I915_PRIORITY_NORMAL + ctx->priority_offset;
 
 	INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL);
 	INIT_LIST_HEAD(&ctx->handles_list);
@@ -816,7 +818,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
 				 !capable(CAP_SYS_NICE))
 				ret = -EPERM;
 			else
-				ctx->priority = priority;
+				ctx->priority = priority + ctx->priority_offset;
 		}
 		break;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index 4bfb72f8e1cb..4f1c95fca5c2 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -147,6 +147,15 @@ struct i915_gem_context {
 	 */
 	int priority;
 
+	/**
+	 * @priority_offset: priority offset
+	 *
+	 * A value, configured via cgroup, that sets the starting priority
+	 * of the context.  Any priority set explicitly via context parameter
+	 * will be added to the priority offset.
+	 */
+	int priority_offset;
+
 	/** ggtt_offset_bias: placement restriction for context objects */
 	u32 ggtt_offset_bias;
 
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index fe333ae1f0c6..1c1988e9fba5 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1622,6 +1622,7 @@ struct drm_i915_cgroup_param {
 	__s32 cgroup_fd;
 	__u32 flags;
 	__u64 param;
+#define I915_CGROUP_PARAM_PRIORITY_OFFSET	0x1
 	__s64 value;
 };
 
-- 
2.14.3

Re: [PATCH RFC v2 6/7] drm/i915: Introduce 'priority offset' for GPU contexts

[Chris Wilson]

Quoting Matt Roper (2018-02-01 19:53:14)
> There are cases where a system integrator may wish to raise/lower the
> priority of GPU workloads being submitted by specific OS process(es),
> independently of how the software self-classifies its own priority.
> Exposing "priority offset" as an i915-specific cgroup parameter will
> enable such system-level configuration.
> 
> Normally GPU contexts start with a priority value of 0
> (I915_CONTEXT_DEFAULT_PRIORITY) and then may be adjusted up/down from
> there via other mechanisms.  We'd like to provide a system-level input
> to the priority decision that will be taken into consideration, even
> when userspace later attempts to set an absolute priority value via
> I915_CONTEXT_PARAM_PRIORITY.  The priority offset introduced here
> provides a base value that will always be added to (or subtracted from)
> the software's self-assigned priority value.
> 
> This patch makes priority offset a cgroup-specific value; contexts will
> be created with a priority offset based on the cgroup membership of the
> process creating the context at the time the context is created.  Note
> that priority offset is assigned at context creation time; migrating a
> process to a different cgroup or changing the offset associated with a
> cgroup will only affect new context creation and will not alter the
> behavior of existing contexts previously created by the process.
> 
> Signed-off-by: Matt Roper <matthew.d.roper-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/gpu/drm/i915/i915_cgroup.c      | 46 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h         |  7 +++++
>  drivers/gpu/drm/i915/i915_gem_context.c |  6 +++--
>  drivers/gpu/drm/i915/i915_gem_context.h |  9 +++++++
>  include/uapi/drm/i915_drm.h             |  1 +
>  5 files changed, 67 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_cgroup.c b/drivers/gpu/drm/i915/i915_cgroup.c
> index 778af895fc00..73f3cfe31d66 100644
> --- a/drivers/gpu/drm/i915/i915_cgroup.c
> +++ b/drivers/gpu/drm/i915/i915_cgroup.c
> @@ -11,6 +11,8 @@
>  
>  struct i915_cgroup_data {
>         struct cgroup_driver_data base;
> +
> +       int priority_offset;
>  };
>  
>  static inline struct i915_cgroup_data *
> @@ -72,6 +74,8 @@ i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
>         struct cgroup *cgrp;
>         struct file *f;
>         struct inode *inode = NULL;
> +       struct cgroup_driver_data *cgrpdata;
> +       struct i915_cgroup_data *i915data;
>         int ret;
>  
>         if (!dev_priv->i915_cgroups) {
> @@ -125,6 +129,17 @@ i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
>                 return ret;
>  
>         switch (req->param) {
> +       case I915_CGROUP_PARAM_PRIORITY_OFFSET:
> +               cgrpdata = cgroup_driver_get_data(dev_priv->i915_cgroups, cgrp,
> +                                                 NULL);

dev_priv is i915. So i915->i915_cgroups?

Could you just have a i915_get_cgroup_data() returning struct
i915_croup_data?


> +               if (IS_ERR(cgrpdata))
> +                       return PTR_ERR(cgrpdata);
> +
> +               DRM_DEBUG_DRIVER("Setting cgroup priority offset to %lld\n",
> +                                req->value);
> +               i915data = cgrp_to_i915(cgrpdata);
> +               i915data->priority_offset = req->value;

We'll have to consider what bounds we think are sensible, and how
internal policy interacts. Presumably another cgroup-param :)

> +               break;
>         default:
>                 DRM_DEBUG_DRIVER("Invalid cgroup parameter %lld\n", req->param);
>                 return -EINVAL;
> @@ -132,3 +147,34 @@ i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
>  
>         return 0;
>  }
> +
> +/**
> + * i915_cgroup_get_prio_offset() - get prio offset for current proc's cgroup
> + * @dev_priv: drm device
> + * @file_priv: file handle for calling process
> + *
> + * RETURNS:
> + * Priority offset associated with the calling process' cgroup in the default
> + * (v2) hierarchy, otherwise 0 if no explicit priority has been assigned.
> + */
> +int
> +i915_cgroup_get_prio_offset(struct drm_i915_private *dev_priv,
> +                           struct drm_i915_file_private *file_priv)
> +{
> +       struct cgroup *cgrp;
> +       struct cgroup_driver_data *cgrpdata;
> +
> +       /* Ignore internally-created contexts not associated with a process */
> +       if (!file_priv)
> +               return 0;
> +
> +       cgrp = drm_file_get_cgroup(file_priv->file);
> +       if (WARN_ON(!cgrp))
> +               return 0;

Ah, this needs to pull from current then not drm_file->pid.

> +
> +       cgrpdata = cgroup_driver_get_data(dev_priv->i915_cgroups, cgrp, NULL);
> +       if (IS_ERR(cgrpdata))
> +              return 0;
> +
> +       return cgrp_to_i915(cgrpdata)->priority_offset;
> +}
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 60e3ff6a48bb..313191e5278a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2920,6 +2920,8 @@ int i915_cgroup_init(struct drm_i915_private *dev_priv);
>  int i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
>                                struct drm_file *file);
>  void i915_cgroup_shutdown(struct drm_i915_private *dev_priv);
> +int i915_cgroup_get_prio_offset(struct drm_i915_private *dev_priv,
> +                               struct drm_i915_file_private *file_priv);
>  #else
>  static inline int
>  i915_cgroup_init(struct drm_i915_private *dev_priv)
> @@ -2933,6 +2935,11 @@ i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
>  {
>         return -EINVAL;
>  }
> +static inline int
> +i915_cgroup_get_prio_offset(struct drm_i915_private *dev_priv,
> +                           struct drm_i915_file_private *file_priv) {
> +       return 0;
> +}
>  #endif
>  
>  /* i915_drv.c */
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 648e7536ff51..700d7bc7cfc8 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -274,7 +274,9 @@ __create_hw_context(struct drm_i915_private *dev_priv,
>         kref_init(&ctx->ref);
>         list_add_tail(&ctx->link, &dev_priv->contexts.list);
>         ctx->i915 = dev_priv;
> -       ctx->priority = I915_PRIORITY_NORMAL;
> +       ctx->priority_offset =
> +               i915_cgroup_get_prio_offset(dev_priv, file_priv);
> +       ctx->priority = I915_PRIORITY_NORMAL + ctx->priority_offset;
>  
>         INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL);
>         INIT_LIST_HEAD(&ctx->handles_list);
> @@ -816,7 +818,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
>                                  !capable(CAP_SYS_NICE))
>                                 ret = -EPERM;
>                         else
> -                               ctx->priority = priority;
> +                               ctx->priority = priority + ctx->priority_offset;

And subtract the offset in getparam. So user priority in == reported
priority out, and they are none the wiser that it is being adjusted
internally (out of their direct control).
-Chris

[PATCH RFC v2 7/7] drm/i915: Add context priority & priority offset to debugfs

[Matt Roper]

Update i915_context_status to include priority information.

Signed-off-by: Matt Roper <matthew.d.roper-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3849ded354e3..e9d8e20155b1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1915,6 +1915,9 @@ static int i915_context_status(struct seq_file *m, void *unused)
 		seq_putc(m, ctx->remap_slice ? 'R' : 'r');
 		seq_putc(m, '\n');
 
+		seq_printf(m, "Priority %d (prio offset = %d)\n",
+			   ctx->priority, ctx->priority_offset);
+
 		for_each_engine(engine, dev_priv, id) {
 			struct intel_context *ce = &ctx->engine[engine->id];
 
-- 
2.14.3

Re: [PATCH RFC v2 7/7] drm/i915: Add context priority & priority offset to debugfs

[Chris Wilson]

Quoting Matt Roper (2018-02-01 19:53:15)
> Update i915_context_status to include priority information.
> 
> Signed-off-by: Matt Roper <matthew.d.roper-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3849ded354e3..e9d8e20155b1 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1915,6 +1915,9 @@ static int i915_context_status(struct seq_file *m, void *unused)
>                 seq_putc(m, ctx->remap_slice ? 'R' : 'r');
>                 seq_putc(m, '\n');
>  
> +               seq_printf(m, "Priority %d (prio offset = %d)\n",
> +                          ctx->priority, ctx->priority_offset);

s/prio offset/cgroup offset/ ?
"prio" itself I think is redundant given that we are providing further
details of "Priority"
-Chris

[IGT PATCH RFC] tools: Introduce intel_cgroup tool

[Matt Roper]

intel_cgroup is used to modify i915 cgroup parameters.  At the moment only a
single parameter is supported (GPU priority offset).  In the future the driver
may support additional parameters as well (e.g., limits on memory usage).

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 tools/Makefile.sources |   1 +
 tools/intel_cgroup.c   | 103 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+)
 create mode 100644 tools/intel_cgroup.c

diff --git a/tools/Makefile.sources b/tools/Makefile.sources
index abd23a0f..b30216c4 100644
--- a/tools/Makefile.sources
+++ b/tools/Makefile.sources
@@ -11,6 +11,7 @@ tools_prog_lists =		\
 	intel_reg		\
 	intel_backlight		\
 	intel_bios_dumper	\
+	intel_cgroup		\
 	intel_display_crc	\
 	intel_display_poller	\
 	intel_forcewaked	\
diff --git a/tools/intel_cgroup.c b/tools/intel_cgroup.c
new file mode 100644
index 00000000..ce781b08
--- /dev/null
+++ b/tools/intel_cgroup.c
@@ -0,0 +1,103 @@
+/*
+ * Copyright © 2018 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include <fcntl.h>
+#include <getopt.h>
+#include <i915_drm.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "igt.h"
+#include "xf86drm.h"
+#include "xf86drmMode.h"
+
+#define I915_CGROUP_PARAM_PRIORITY_OFFSET       0x1
+
+char short_opts[] = "p:";
+struct option long_opts[] = {
+	{ "set-prio",	required_argument, NULL, 'p'},
+	{ 0 },
+};
+
+static void usage(void)
+{
+	puts("Usage:");
+	printf("  intel_cgroup --set-prio=<val> <cgroup-v2>\n");
+}
+
+int main(int argc, char **argv)
+{
+	int drm_fd, cgrp_fd;
+	struct drm_i915_cgroup_param req;
+	int opt, ret;
+	struct {
+		bool do_prio;
+		int prio_val;
+	} updates = { 0 };
+
+	while ((opt = getopt_long(argc, argv, short_opts, long_opts, NULL)) != -1) {
+		switch (opt) {
+		case 'p':
+			updates.do_prio = true;
+			updates.prio_val = atoi(optarg);
+			break;
+		default:
+			igt_assert(false);
+		}
+	}
+
+	if (argc - optind != 1) {
+		usage();
+		return 1;
+	}
+
+	drm_fd = drm_open_driver(DRIVER_INTEL);
+	if (drm_fd < 0) {
+		perror("Invalid DRM device");
+		return 1;
+	}
+
+	cgrp_fd = open(argv[optind], O_RDONLY|O_DIRECTORY, 0);
+	if (cgrp_fd < 0) {
+		perror("Invalid cgroup directory");
+		return 1;
+	}
+
+	req.cgroup_fd = cgrp_fd;
+	req.flags = 0;
+
+	if (updates.do_prio) {
+		req.param = I915_CGROUP_PARAM_PRIORITY_OFFSET;
+		req.value = updates.prio_val;
+
+		ret = drmIoctl(drm_fd, DRM_IOCTL_I915_CGROUP_SETPARAM, &req);
+		if (ret)
+			perror("Failed to set cgroup parameter");
+	}
+
+	close(cgrp_fd);
+	close(drm_fd);
+
+	return ret;
+}
-- 
2.14.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [IGT PATCH RFC] tools: Introduce intel_cgroup tool

[Chris Wilson]

Quoting Matt Roper (2018-02-01 19:56:15)
> intel_cgroup is used to modify i915 cgroup parameters.  At the moment only a
> single parameter is supported (GPU priority offset).  In the future the driver
> may support additional parameters as well (e.g., limits on memory usage).
> 
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  tools/Makefile.sources |   1 +
>  tools/intel_cgroup.c   | 103 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 104 insertions(+)
>  create mode 100644 tools/intel_cgroup.c
> 
> diff --git a/tools/Makefile.sources b/tools/Makefile.sources
> index abd23a0f..b30216c4 100644
> --- a/tools/Makefile.sources
> +++ b/tools/Makefile.sources
> @@ -11,6 +11,7 @@ tools_prog_lists =            \
>         intel_reg               \
>         intel_backlight         \
>         intel_bios_dumper       \
> +       intel_cgroup            \
>         intel_display_crc       \
>         intel_display_poller    \
>         intel_forcewaked        \
> diff --git a/tools/intel_cgroup.c b/tools/intel_cgroup.c
> new file mode 100644
> index 00000000..ce781b08
> --- /dev/null
> +++ b/tools/intel_cgroup.c
> @@ -0,0 +1,103 @@
> +/*
> + * Copyright © 2018 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include <fcntl.h>
> +#include <getopt.h>
> +#include <i915_drm.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +#include "igt.h"
> +#include "xf86drm.h"
> +#include "xf86drmMode.h"
> +
> +#define I915_CGROUP_PARAM_PRIORITY_OFFSET       0x1

Hmm. Could we not avoid drm_ioctl + well known param names and use a
more generic tool to set cgroup attributes? Just feels wrong that a
such a generic interface boils down to a driver specific ioctl.
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [IGT PATCH RFC] tools: Introduce intel_cgroup tool

[Matt Roper]

+cgroups list since this discussion goes back to the general design

On Thu, Feb 01, 2018 at 08:27:33PM +0000, Chris Wilson wrote:
> Quoting Matt Roper (2018-02-01 19:56:15)
> > intel_cgroup is used to modify i915 cgroup parameters.  At the moment only a
> > single parameter is supported (GPU priority offset).  In the future the driver
> > may support additional parameters as well (e.g., limits on memory usage).
> > 
> > Signed-off-by: Matt Roper <matthew.d.roper-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
<snip>
> > +#define I915_CGROUP_PARAM_PRIORITY_OFFSET       0x1
> 
> Hmm. Could we not avoid drm_ioctl + well known param names and use a
> more generic tool to set cgroup attributes? Just feels wrong that a
> such a generic interface boils down to a driver specific ioctl.
> -Chris

There are a few different design choices here, so feedback like this is
definitely what I'm looking for with this series.  A few goals we should
keep in mind as we consider options:

 * I'd like to keep the attributes we set tied to a driver/device in
   some manner.  E.g., if we have a machine with multiple GPU's, we
   should be able to set card0 priority independently of card1 priority
   A DRM or driver ioctl makes this easy, but it certainly isn't the
   only approach.

 * Some of the attributes we want to manage don't fall under the design
   goals of formal cgroup controllers.  I.e., they don't care about the
   hierarchy layout of cgroups at all; they only care about the details
   of the process' final leaf node.  GPU priority as implemented in this
   series is an example of that.

 * Other attributes we'll eventually want to control (such as graphics
   memory usage), probably _will_ want to take the hierarchy design into
   account, the same way real cgroup controllers like the mem controller
   do.

 * The drivers that want to make use of this functionality may be built
   as modules rather than compiled directly into the kernel.  This is
   important because the cgroups subsystem removed the ability to have
   cgroup controllers in modules a few years ago.  While it might be
   possible to resurrect module-based cgroup controller support, my
   impression is that the cgroups community would prefer a separate
   non-controller mechanism for doing what we want to do.  E.g., see
   https://www.spinics.net/lists/cgroups/msg18674.html for some brief
   discussion on this topic.

I think the nicest interface for setting cgroup attributes would be to
find a way to add our own kernfs entries to the cgroup filesystem, the
same way real cgroup controllers do.  Then we could do something like
"echo 123 > /cgroup2/apps/highprio/i915.card0.priority" and not need to
call any ioctl's at all.  Without creating an actual cgroup controller,
I think this might be challenging, but I'm investigating it on the side
right now to see if it's a viable option.

There are other options that might be suitable as well, but I haven't
throught through them in details:

 * Hang the ioctl() off the cgroup filesystem entry rather than the DRM
   device node.  Not sure if this gains us anything given that we still
   want to track data on a per-device basis.

 * Add a cgroup filesystem entry that just handles write() events of the
   form "drivername:key:value" to trigger driver callbacks.  This might
   be a decent compromise.  E.g., we could issue a command like:
      echo "i915.card0:prio_base:123" > /cgroup2/foo/cgroup.driver_attr
   and the kernfs handler for that file would looks for a cgroup_driver
   registered under the name "i915.card0" to figure out what driver
   callback to call for the key/value data.

The second bullet above is growing on me as I think more about it, so I
might try implementing that approach in the third version of my series
if nobody has other suggestions or feedback.


Matt

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795

Re: [IGT PATCH RFC] tools: Introduce intel_cgroup tool

[Tejun Heo]

Hello, Matt, Chris.

On Thu, Feb 01, 2018 at 03:14:38PM -0800, Matt Roper wrote:
> > Hmm. Could we not avoid drm_ioctl + well known param names and use a
> > more generic tool to set cgroup attributes? Just feels wrong that a
> > such a generic interface boils down to a driver specific ioctl.

So, everything which shows up in the cgroup hierarchy should satisfy
the following two conditions.

* The control mechanism should adhere to one of the resource
  distribution models defined in Documentation/cgroup-v2.txt.  This is
  to ensure consistency across different resources which in turn
  allows things like delegation.

* This one is a bit vague and I probably should find a better way to
  describe it but each controller should encapsulate a generic core
  resource.  Here, I don't think it makes sense to have intel gfx
  controller when there are a lot of commmonalities in the graphics
  hardware across different vendors.  It should be better abstracted.

It's true that it's difficult to figure out the right generic design
without actually trying, and I think that's why it'd be better to
start scoped in the scope of the specific driver.  The smaller scope
would allow for less strict expectations, more latitude, and easier
experimentations.

> I think the nicest interface for setting cgroup attributes would be to
> find a way to add our own kernfs entries to the cgroup filesystem, the
> same way real cgroup controllers do.  Then we could do something like
> "echo 123 > /cgroup2/apps/highprio/i915.card0.priority" and not need to
> call any ioctl's at all.  Without creating an actual cgroup controller,
> I think this might be challenging, but I'm investigating it on the side
> right now to see if it's a viable option.

So, I strongly believe that it isn't the right approach to add i915
prefixed interface files to cgroup interface.

Thanks.

-- 
tejun
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [IGT PATCH RFC] tools: Introduce intel_cgroup tool

[Tejun Heo]

Hello,

Forgot to respond to one point.

On Thu, Feb 01, 2018 at 03:14:38PM -0800, Matt Roper wrote:
>  * The drivers that want to make use of this functionality may be built
>    as modules rather than compiled directly into the kernel.  This is
>    important because the cgroups subsystem removed the ability to have
>    cgroup controllers in modules a few years ago.  While it might be
>    possible to resurrect module-based cgroup controller support, my
>    impression is that the cgroups community would prefer a separate
>    non-controller mechanism for doing what we want to do.  E.g., see
>    https://www.spinics.net/lists/cgroups/msg18674.html for some brief
>    discussion on this topic.

So, this isn't a concern.  If we need to have modular controllers, we
can resurrect module support, hopefully in a simpler way, or could do
something similar to what rdma controller did.  We shouldn't make
userland interface decisions based on this sort of implementation
details.

Thanks.

-- 
tejun
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for DRM management via cgroups (rev2)

[Patchwork]

== Series Details ==

Series: DRM management via cgroups (rev2)
URL   : https://patchwork.freedesktop.org/series/36837/
State : success

== Summary ==

Series 36837v2 DRM management via cgroups
https://patchwork.freedesktop.org/api/1.0/series/36837/revisions/2/mbox/

Test gem_exec_reloc:
        Subgroup basic-cpu-gtt-active:
                incomplete -> PASS       (fi-byt-j1900)

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:421s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:420s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:370s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:483s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:282s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:484s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:479s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:465s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:451s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:568s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:413s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:275s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:508s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:387s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:396s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:409s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:451s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:412s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:455s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:496s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:452s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:500s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:579s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:422s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:506s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:527s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:484s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:475s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:413s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:430s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
Blacklisted hosts:
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:470s

abd8913d5ab0f40061166da741a2969f6fc8206a drm-tip: 2018y-02m-01d-19h-16m-13s UTC integration manifest
efbe51d8915b drm/i915: Add context priority & priority offset to debugfs
2ad672fc3337 drm/i915: Introduce 'priority offset' for GPU contexts
955860954ef5 drm/i915: cgroup integration
86fff2c23f8a drm: Add helper to obtain cgroup of drm_file's owning process
33b5f92f9c44 cgroup: Add interface to allow drivers to lookup process cgroup membership
1b617cd2d0ec kernfs: Export kernfs_get_inode
09c9a771a688 cgroup: Allow drivers to store data associated with a cgroup

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7856/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for tools: Introduce intel_cgroup tool

[Patchwork]

== Series Details ==

Series: tools: Introduce intel_cgroup tool
URL   : https://patchwork.freedesktop.org/series/37505/
State : failure

== Summary ==

IGT patchset build failed on latest successful build
a20a69e25a18ec63236633b804d89cc0c0cea259 overlay: fix invalid pointer access

make  all-recursive
Making all in lib
make  all-recursive
Making all in .
Making all in tests
make[4]: Nothing to be done for 'all'.
Making all in man
make[2]: Nothing to be done for 'all'.
Making all in tools
Making all in null_state_gen
make[3]: Nothing to be done for 'all'.
Making all in registers
make[3]: Nothing to be done for 'all'.
  CCLD     intel_aubdump.la
  CCLD     intel_audio_dump
  CCLD     intel_reg
  CC       intel_cgroup.o
intel_cgroup.c: In function ‘main’:
intel_cgroup.c:52:31: error: storage size of ‘req’ isn’t known
  struct drm_i915_cgroup_param req;
                               ^~~
intel_cgroup.c:94:26: error: ‘DRM_IOCTL_I915_CGROUP_SETPARAM’ undeclared (first use in this function)
   ret = drmIoctl(drm_fd, DRM_IOCTL_I915_CGROUP_SETPARAM, &req);
                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
intel_cgroup.c:94:26: note: each undeclared identifier is reported only once for each function it appears in
intel_cgroup.c:52:31: warning: unused variable ‘req’ [-Wunused-variable]
  struct drm_i915_cgroup_param req;
                               ^~~
Makefile:1170: recipe for target 'intel_cgroup.o' failed
make[3]: *** [intel_cgroup.o] Error 1
Makefile:1234: recipe for target 'all-recursive' failed
make[2]: *** [all-recursive] Error 1
Makefile:532: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
Makefile:464: recipe for target 'all' failed
make: *** [all] Error 2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for DRM management via cgroups (rev2)

[Patchwork]

== Series Details ==

Series: DRM management via cgroups (rev2)
URL   : https://patchwork.freedesktop.org/series/36837/
State : failure

== Summary ==

Test perf_pmu:
        Subgroup busy-check-all-vecs0:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup busy-start-rcs0:
                pass       -> DMESG-WARN (shard-snb)
        Subgroup other-read-0:
                pass       -> DMESG-WARN (shard-snb)
                pass       -> DMESG-WARN (shard-hsw)
                pass       -> DMESG-WARN (shard-apl)
        Subgroup idle-vcs0:
                pass       -> DMESG-WARN (shard-snb)
        Subgroup busy-no-semaphores-vcs0:
                pass       -> DMESG-WARN (shard-hsw)
                pass       -> DMESG-WARN (shard-apl)
        Subgroup render-node-busy-rcs0:
                pass       -> DMESG-WARN (shard-apl)
        Subgroup busy-no-semaphores-rcs0:
                pass       -> DMESG-WARN (shard-snb)
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup most-busy-check-all-vcs0:
                pass       -> DMESG-WARN (shard-snb)
                pass       -> DMESG-WARN (shard-hsw)
                pass       -> DMESG-WARN (shard-apl)
        Subgroup semaphore-wait-vecs0:
                pass       -> DMESG-WARN (shard-apl)
        Subgroup interrupts:
                pass       -> DMESG-WARN (shard-snb)
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup busy-vecs0:
                pass       -> DMESG-WARN (shard-hsw)
                pass       -> DMESG-WARN (shard-apl)
        Subgroup rc6:
                pass       -> DMESG-WARN (shard-snb)
                pass       -> DMESG-WARN (shard-apl)
        Subgroup busy-no-semaphores-vecs0:
                pass       -> DMESG-WARN (shard-hsw)
        Subgroup semaphore-wait-vcs0:
                pass       -> DMESG-WARN (shard-apl)
Test kms_atomic_transition:
        Subgroup 1x-modeset-transitions-nonblocking:
                fail       -> PASS       (shard-apl) fdo#103336
        Subgroup plane-toggle-modeset-transition:
                skip       -> PASS       (shard-snb)
Test kms_vblank:
        Subgroup pipe-a-ts-continuation-suspend:
                pass       -> FAIL       (shard-hsw)
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                skip       -> PASS       (shard-snb)
Test perf:
        Subgroup buffer-fill:
                fail       -> PASS       (shard-apl) fdo#103755

fdo#103336 https://bugs.freedesktop.org/show_bug.cgi?id=103336
fdo#103755 https://bugs.freedesktop.org/show_bug.cgi?id=103755

shard-apl        total:2836 pass:1742 dwarn:9   dfail:0   fail:21  skip:1064 time:11404s
shard-hsw        total:2836 pass:1724 dwarn:9   dfail:0   fail:12  skip:1090 time:11212s
shard-snb        total:2836 pass:1321 dwarn:8   dfail:0   fail:10  skip:1497 time:6157s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7856/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx