All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4.5 0/8] cgroup private data and DRM/i915 integration
@ 2018-03-21 23:23 Matt Roper
  2018-03-21 23:23 ` [PATCH v4.5 1/8] cgroup: Allow registration and lookup of cgroup private data (v3) Matt Roper
                   ` (10 more replies)
  0 siblings, 11 replies; 14+ messages in thread
From: Matt Roper @ 2018-03-21 23:23 UTC (permalink / raw)
  To: dri-devel, intel-gfx, cgroups

This version of the patch series just contains some minor updates to
address checkpatch and sparse warnings.  There are no serious design or
implementation changes since v4.

You can find the previous versions of this series (and more detailed
cover letters) here:
 (v1) https://lists.freedesktop.org/archives/intel-gfx/2018-January/153156.html
 (v2) https://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg208170.html
 (v3) https://lists.freedesktop.org/archives/intel-gfx/2018-March/157928.html
 (v4) https://lists.freedesktop.org/archives/intel-gfx/2018-March/159222.html

Matt Roper (8):
  cgroup: Allow registration and lookup of cgroup private data (v3)
  cgroup: Introduce task_get_dfl_cgroup() (v2)
  cgroup: Introduce cgroup_priv_get_current
  drm/i915: Adjust internal priority definitions (v2)
  drm/i915: cgroup integration (v4)
  drm/i915: Introduce 'priority offset' for GPU contexts (v4)
  drm/i915: Introduce per-cgroup display boost setting
  drm/i915: Add context priority & priority offset to debugfs (v2)

 drivers/gpu/drm/i915/Makefile           |   1 +
 drivers/gpu/drm/i915/i915_cgroup.c      | 207 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_debugfs.c     |   3 +
 drivers/gpu/drm/i915/i915_drv.c         |   8 ++
 drivers/gpu/drm/i915/i915_drv.h         |  46 ++++++-
 drivers/gpu/drm/i915/i915_gem_context.c |  12 +-
 drivers/gpu/drm/i915/i915_gem_context.h |   9 ++
 drivers/gpu/drm/i915/i915_request.h     |  18 ++-
 drivers/gpu/drm/i915/intel_display.c    |   5 +-
 include/linux/cgroup-defs.h             |   8 ++
 include/linux/cgroup.h                  |  37 ++++++
 include/uapi/drm/i915_drm.h             |  14 +++
 kernel/cgroup/cgroup.c                  | 206 ++++++++++++++++++++++++++++++-
 13 files changed, 561 insertions(+), 13 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_cgroup.c

-- 
2.14.3

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4.5 1/8] cgroup: Allow registration and lookup of cgroup private data (v3)
  2018-03-21 23:23 [PATCH v4.5 0/8] cgroup private data and DRM/i915 integration Matt Roper
@ 2018-03-21 23:23 ` Matt Roper
  2018-03-22 18:04   ` Chris Wilson
  2018-03-21 23:23 ` [PATCH v4.5 2/8] cgroup: Introduce task_get_dfl_cgroup() (v2) Matt Roper
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 14+ messages in thread
From: Matt Roper @ 2018-03-21 23:23 UTC (permalink / raw)
  To: dri-devel, intel-gfx, cgroups
  Cc: Tejun Heo, Roman Gushchin, Alexei Starovoitov

There are cases where other parts of the kernel may wish to store data
associated with individual cgroups without building a full cgroup
controller.  Let's add interfaces to allow them to register and lookup
this private data for individual cgroups.

A kernel system (e.g., a driver) that wishes to register private data
for a cgroup should start by obtaining a unique private data key by
calling cgroup_priv_getkey().  It may then associate private data
with a cgroup by calling cgroup_priv_install(cgrp, key, ref) where 'ref'
is a pointer to a kref field inside the private data structure.  The
private data may later be looked up by calling cgroup_priv_get(cgrp,
key) to obtain a new reference to the private data.  Private data may be
unregistered via cgroup_priv_release(cgrp, key).

If a cgroup is removed, the reference count for all private data objects
will be decremented.

v2:  Significant overhaul suggested by Tejun, Alexei, and Roman
 - Rework interface to make consumers obtain an ida-based key rather
   than supplying their own arbitrary void*
 - Internal implementation now uses per-cgroup radixtrees which should
   allow much faster lookup than the previous hashtable approach
 - Private data is registered via kref, allowing a single private data
   structure to potentially be assigned to multiple cgroups.

v3:
 - Spare warning fixes (kbuild test robot)

Cc: Tejun Heo <tj@kernel.org>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Roman Gushchin <guro@fb.com>
Cc: cgroups@vger.kernel.org
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>

fixup! cgroup: Allow registration and lookup of cgroup private data (v2)
---
 include/linux/cgroup-defs.h |  10 +++
 include/linux/cgroup.h      |   7 ++
 kernel/cgroup/cgroup.c      | 183 +++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 198 insertions(+), 2 deletions(-)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 9f242b876fde..9086d963cc0a 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -427,6 +427,16 @@ struct cgroup {
 	/* used to store eBPF programs */
 	struct cgroup_bpf bpf;
 
+	/*
+	 * cgroup private data registered by other non-controller parts of the
+	 * kernel.  Insertions are protected by privdata_lock, lookups by
+	 * rcu_read_lock().
+	 */
+	struct radix_tree_root privdata;
+
+	/* Protect against concurrent insertions/deletions to privdata */
+	spinlock_t privdata_lock;
+
 	/* ids of the ancestors at each level including self */
 	int ancestor_ids[];
 };
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 473e0c0abb86..63d22dfa00bd 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -833,4 +833,11 @@ static inline void put_cgroup_ns(struct cgroup_namespace *ns)
 		free_cgroup_ns(ns);
 }
 
+/* cgroup private data handling */
+int cgroup_priv_getkey(void (*free)(struct kref *));
+void cgroup_priv_destroykey(int key);
+int cgroup_priv_install(struct cgroup *cgrp, int key, struct kref *ref);
+struct kref *cgroup_priv_get(struct cgroup *cgrp, int key);
+void cgroup_priv_release(struct cgroup *cgrp, int key);
+
 #endif /* _LINUX_CGROUP_H */
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 8cda3bc3ae22..3268a21e8158 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -81,8 +81,15 @@ EXPORT_SYMBOL_GPL(css_set_lock);
 #endif
 
 /*
- * Protects cgroup_idr and css_idr so that IDs can be released without
- * grabbing cgroup_mutex.
+ * ID allocator for cgroup private data keys; the ID's allocated here will
+ * be used to index all per-cgroup radix trees.  The radix tree built into
+ * the IDR itself will store a key-specific function to be passed to kref_put.
+ */
+static DEFINE_IDR(cgroup_priv_idr);
+
+/*
+ * Protects cgroup_idr, css_idr, and cgroup_priv_idr so that IDs can be
+ * released without grabbing cgroup_mutex.
  */
 static DEFINE_SPINLOCK(cgroup_idr_lock);
 
@@ -1839,6 +1846,8 @@ static void init_cgroup_housekeeping(struct cgroup *cgrp)
 	INIT_LIST_HEAD(&cgrp->cset_links);
 	INIT_LIST_HEAD(&cgrp->pidlists);
 	mutex_init(&cgrp->pidlist_mutex);
+	INIT_RADIX_TREE(&cgrp->privdata, GFP_NOWAIT);
+	spin_lock_init(&cgrp->privdata_lock);
 	cgrp->self.cgroup = cgrp;
 	cgrp->self.flags |= CSS_ONLINE;
 	cgrp->dom_cgrp = cgrp;
@@ -4578,6 +4587,8 @@ static void css_release_work_fn(struct work_struct *work)
 		container_of(work, struct cgroup_subsys_state, destroy_work);
 	struct cgroup_subsys *ss = css->ss;
 	struct cgroup *cgrp = css->cgroup;
+	struct radix_tree_iter iter;
+	void __rcu **slot;
 
 	mutex_lock(&cgroup_mutex);
 
@@ -4617,6 +4628,12 @@ static void css_release_work_fn(struct work_struct *work)
 					 NULL);
 
 		cgroup_bpf_put(cgrp);
+
+		/* Drop reference on any private data */
+		rcu_read_lock();
+		radix_tree_for_each_slot(slot, &cgrp->privdata, &iter, 0)
+			cgroup_priv_release(cgrp, iter.index);
+		rcu_read_unlock();
 	}
 
 	mutex_unlock(&cgroup_mutex);
@@ -5932,3 +5949,165 @@ static int __init cgroup_sysfs_init(void)
 }
 subsys_initcall(cgroup_sysfs_init);
 #endif /* CONFIG_SYSFS */
+
+/**
+ * cgroup_priv_getkey - obtain a new cgroup_priv lookup key
+ * @free: Function to release private data associated with this key
+ *
+ * Allows non-controller kernel subsystems to register a new key that will
+ * be used to insert/lookup private data associated with individual cgroups.
+ * Private data lookup tables are implemented as per-cgroup radix trees.
+ *
+ * Returns:
+ * A positive integer lookup key if successful, or a negative error code
+ * on failure (e.g., if ID allocation fails).
+ */
+int
+cgroup_priv_getkey(void (*free)(struct kref *r))
+{
+	int ret;
+
+	WARN_ON(!free);
+
+	idr_preload(GFP_KERNEL);
+	spin_lock_bh(&cgroup_idr_lock);
+	ret = idr_alloc(&cgroup_priv_idr, free, 1, 0, GFP_NOWAIT);
+	spin_unlock_bh(&cgroup_idr_lock);
+	idr_preload_end();
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cgroup_priv_getkey);
+
+/**
+ * cgroup_priv_destroykey - release a cgroup_priv key
+ * @key: Private data key to be released
+ *
+ * Removes a cgroup private data key and all private data associated with this
+ * key in any cgroup.  This is a heavy operation that will take cgroup_mutex.
+ */
+void
+cgroup_priv_destroykey(int key)
+{
+	struct cgroup *cgrp;
+
+	WARN_ON(key == 0);
+
+	mutex_lock(&cgroup_mutex);
+	cgroup_for_each_live_child(cgrp, &cgrp_dfl_root.cgrp)
+		cgroup_priv_release(cgrp, key);
+	idr_remove(&cgroup_priv_idr, key);
+	mutex_unlock(&cgroup_mutex);
+}
+EXPORT_SYMBOL_GPL(cgroup_priv_destroykey);
+
+/**
+ * cgroup_priv_install - install new cgroup private data
+ * @cgrp: cgroup to store private data for
+ * @key: key uniquely identifying kernel owner of private data
+ * @data: pointer to kref field of private data structure
+ *
+ * Allows non-controller kernel subsystems to register their own private data
+ * associated with a cgroup.  This will often be used by drivers which wish to
+ * track their own per-cgroup data without building a full cgroup controller.
+ *
+ * The caller is responsible for ensuring that no private data already exists
+ * for the given key.
+ *
+ * Registering cgroup private data with this function will increment the
+ * reference count for the private data structure.  If the cgroup is removed,
+ * the reference count will be decremented, allowing the private data to
+ * be freed if there are no other outstanding references.
+ *
+ * Returns:
+ * 0 on success, otherwise a negative error code on failure.
+ */
+int
+cgroup_priv_install(struct cgroup *cgrp, int key, struct kref *ref)
+{
+	int ret;
+
+	WARN_ON(cgrp->root != &cgrp_dfl_root);
+	WARN_ON(key == 0);
+
+	kref_get(ref);
+
+	ret = radix_tree_preload(GFP_KERNEL);
+	if (ret)
+		return ret;
+
+	spin_lock_bh(&cgrp->privdata_lock);
+	ret = radix_tree_insert(&cgrp->privdata, key, ref);
+	spin_unlock_bh(&cgrp->privdata_lock);
+	radix_tree_preload_end();
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cgroup_priv_install);
+
+/**
+ * cgroup_priv_get - looks up cgroup private data
+ * @cgrp: cgroup to lookup private data for
+ * @key: key uniquely identifying owner of private data to lookup
+ *
+ * Looks up the cgroup private data associated with a key.  The private
+ * data's reference count is incremented and a pointer to its kref field
+ * is returned to the caller (which can use container_of()) to obtain
+ * the private data itself.
+ *
+ * Returns:
+ * A pointer to the private data's kref field, or NULL if no private data has
+ * been registered.
+ */
+struct kref *
+cgroup_priv_get(struct cgroup *cgrp, int key)
+{
+	struct kref *ref;
+
+	WARN_ON(cgrp->root != &cgrp_dfl_root);
+	WARN_ON(key == 0);
+
+	rcu_read_lock();
+
+	ref = radix_tree_lookup(&cgrp->privdata, key);
+	if (ref)
+		kref_get(ref);
+
+	rcu_read_unlock();
+
+	return ref;
+}
+EXPORT_SYMBOL_GPL(cgroup_priv_get);
+
+/**
+ * cgroup_priv_free - free cgroup private data
+ * @cgrp: cgroup to release private data for
+ * @key: key uniquely identifying owner of private data to free
+ *
+ * Removes private data associated with the given key from a cgroup's internal
+ * table and decrements the reference count for the private data removed,
+ * allowing it to freed if no other references exist.
+ */
+void
+cgroup_priv_release(struct cgroup *cgrp, int key)
+{
+	struct kref *ref;
+	void (*free)(struct kref *r);
+
+	WARN_ON(cgrp->root != &cgrp_dfl_root);
+	WARN_ON(key == 0);
+
+	rcu_read_lock();
+
+	free = idr_find(&cgroup_priv_idr, key);
+	WARN_ON(!free);
+
+	spin_lock_bh(&cgrp->privdata_lock);
+	ref = radix_tree_delete(&cgrp->privdata, key);
+	spin_unlock_bh(&cgrp->privdata_lock);
+	if (ref)
+		kref_put(ref, free);
+
+	rcu_read_unlock();
+}
+EXPORT_SYMBOL_GPL(cgroup_priv_release);
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4.5 2/8] cgroup: Introduce task_get_dfl_cgroup() (v2)
  2018-03-21 23:23 [PATCH v4.5 0/8] cgroup private data and DRM/i915 integration Matt Roper
  2018-03-21 23:23 ` [PATCH v4.5 1/8] cgroup: Allow registration and lookup of cgroup private data (v3) Matt Roper
@ 2018-03-21 23:23 ` Matt Roper
  2018-03-21 23:23 ` [PATCH v4.5 3/8] cgroup: Introduce cgroup_priv_get_current Matt Roper
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2018-03-21 23:23 UTC (permalink / raw)
  To: dri-devel, intel-gfx, cgroups; +Cc: Tejun Heo

Wraps task_dfl_cgroup() to also take a reference to the cgroup.

v2:
 - Eliminate cgroup_mutex and make lighter-weight (Tejun)

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 | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 63d22dfa00bd..74b435f913c1 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -527,6 +527,35 @@ static inline struct cgroup *task_dfl_cgroup(struct task_struct *task)
 	return task_css_set(task)->dfl_cgrp;
 }
 
+/**
+ * task_get_dfl_cgroup() - find and get the cgroup for a task
+ * @task: the target task
+ *
+ * Find the cgroup in the v2 hierarchy that a task belongs to, increment its
+ * reference count, and return it.
+ *
+ * Returns:
+ * The appropriate cgroup from the default hierarchy.
+ */
+static inline struct cgroup *
+task_get_dfl_cgroup(struct task_struct *task)
+{
+	struct cgroup *cgrp;
+
+	rcu_read_lock();
+	while (true) {
+		cgrp = task_dfl_cgroup(task);
+
+		if (likely(cgroup_tryget(cgrp)))
+			break;
+
+		cpu_relax();
+	}
+	rcu_read_unlock();
+
+	return cgrp;
+}
+
 static inline struct cgroup *cgroup_parent(struct cgroup *cgrp)
 {
 	struct cgroup_subsys_state *parent_css = cgrp->self.parent;
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4.5 3/8] cgroup: Introduce cgroup_priv_get_current
  2018-03-21 23:23 [PATCH v4.5 0/8] cgroup private data and DRM/i915 integration Matt Roper
  2018-03-21 23:23 ` [PATCH v4.5 1/8] cgroup: Allow registration and lookup of cgroup private data (v3) Matt Roper
  2018-03-21 23:23 ` [PATCH v4.5 2/8] cgroup: Introduce task_get_dfl_cgroup() (v2) Matt Roper
@ 2018-03-21 23:23 ` Matt Roper
  2018-03-21 23:23 ` [PATCH v4.5 4/8] drm/i915: Adjust internal priority definitions (v2) Matt Roper
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2018-03-21 23:23 UTC (permalink / raw)
  To: dri-devel, intel-gfx, cgroups

Getting cgroup private data for the current process' cgroup is such a
common pattern that we should add a convenience wrapper for it.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 include/linux/cgroup.h |  1 +
 kernel/cgroup/cgroup.c | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 74b435f913c1..64d3dc45efd0 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -867,6 +867,7 @@ int cgroup_priv_getkey(void (*free)(struct kref *));
 void cgroup_priv_destroykey(int key);
 int cgroup_priv_install(struct cgroup *cgrp, int key, struct kref *ref);
 struct kref *cgroup_priv_get(struct cgroup *cgrp, int key);
+struct kref *cgroup_priv_get_current(int key);
 void cgroup_priv_release(struct cgroup *cgrp, int key);
 
 #endif /* _LINUX_CGROUP_H */
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 3268a21e8158..f1637d9c83d5 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -6079,6 +6079,29 @@ cgroup_priv_get(struct cgroup *cgrp, int key)
 }
 EXPORT_SYMBOL_GPL(cgroup_priv_get);
 
+/**
+ * cgroup_priv_get_current - looks up cgroup private data for current task
+ * @key: key uniquely identifying owner of private data to lookup
+ *
+ * Convenience function that performs cgroup_priv_get() on the cgroup that owns
+ * %current.
+ *
+ * Returns:
+ * A pointer to the private data's kref field, or NULL if no private data has
+ * been registered.
+ */
+struct kref *
+cgroup_priv_get_current(int key)
+{
+	struct cgroup *cgrp = task_get_dfl_cgroup(current);
+	struct kref *ref = cgroup_priv_get(cgrp, key);
+
+	cgroup_put(cgrp);
+
+	return ref;
+}
+EXPORT_SYMBOL_GPL(cgroup_priv_get_current);
+
 /**
  * cgroup_priv_free - free cgroup private data
  * @cgrp: cgroup to release private data for
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4.5 4/8] drm/i915: Adjust internal priority definitions (v2)
  2018-03-21 23:23 [PATCH v4.5 0/8] cgroup private data and DRM/i915 integration Matt Roper
                   ` (2 preceding siblings ...)
  2018-03-21 23:23 ` [PATCH v4.5 3/8] cgroup: Introduce cgroup_priv_get_current Matt Roper
@ 2018-03-21 23:23 ` Matt Roper
  2018-03-21 23:23 ` [PATCH v4.5 5/8] drm/i915: cgroup integration (v4) Matt Roper
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2018-03-21 23:23 UTC (permalink / raw)
  To: dri-devel, intel-gfx, cgroups

In preparation for adding cgroup-based priority adjustments, let's
define the driver's priority values a little more clearly.

v2:
 - checkpatch warning fix (Intel CI)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  1 -
 drivers/gpu/drm/i915/i915_gem_context.c |  5 +++--
 drivers/gpu/drm/i915/i915_request.h     | 13 ++++++++++---
 drivers/gpu/drm/i915/intel_display.c    |  2 +-
 4 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c9c3b2ba6a86..2d7a89fcc0dc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3150,7 +3150,6 @@ int i915_gem_object_wait(struct drm_i915_gem_object *obj,
 int i915_gem_object_wait_priority(struct drm_i915_gem_object *obj,
 				  unsigned int flags,
 				  int priority);
-#define I915_PRIORITY_DISPLAY I915_PRIORITY_MAX
 
 int __must_check
 i915_gem_object_set_to_wc_domain(struct drm_i915_gem_object *obj, bool write);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 5cfac0255758..4bae1be52294 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -474,7 +474,7 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
 	ida_init(&dev_priv->contexts.hw_ida);
 
 	/* lowest priority; idle task */
-	ctx = i915_gem_context_create_kernel(dev_priv, I915_PRIORITY_MIN);
+	ctx = i915_gem_context_create_kernel(dev_priv, I915_PRIORITY_IDLE);
 	if (IS_ERR(ctx)) {
 		DRM_ERROR("Failed to create default global context\n");
 		return PTR_ERR(ctx);
@@ -488,7 +488,8 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
 
 	/* highest priority; preempting task */
 	if (needs_preempt_context(dev_priv)) {
-		ctx = i915_gem_context_create_kernel(dev_priv, INT_MAX);
+		ctx = i915_gem_context_create_kernel(dev_priv,
+						     I915_PRIORITY_PREEMPT);
 		if (!IS_ERR(ctx))
 			dev_priv->preempt_context = ctx;
 		else
diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
index 7d6eb82eeb91..72b13fc2b72b 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -78,12 +78,19 @@ struct i915_priotree {
 	int priority;
 };
 
+/*
+ * Userspace can only assign priority values of [-1023,1023] via context param,
+ * but the effective priority value can fall in a larger range once we add in
+ * a cgroup-provided offset.
+ */
 enum {
-	I915_PRIORITY_MIN = I915_CONTEXT_MIN_USER_PRIORITY - 1,
 	I915_PRIORITY_NORMAL = I915_CONTEXT_DEFAULT_PRIORITY,
-	I915_PRIORITY_MAX = I915_CONTEXT_MAX_USER_PRIORITY + 1,
+	I915_PRIORITY_DEFAULT_DISPBOOST = I915_CONTEXT_MAX_USER_PRIORITY + 1,
 
-	I915_PRIORITY_INVALID = INT_MIN
+	/* Special case priority values */
+	I915_PRIORITY_INVALID = INT_MIN,
+	I915_PRIORITY_IDLE = INT_MIN + 1,
+	I915_PRIORITY_PREEMPT = INT_MAX,
 };
 
 struct i915_capture_list {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3e7ab75e1b41..b053a21f682b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12783,7 +12783,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 
 	ret = intel_plane_pin_fb(to_intel_plane_state(new_state));
 
-	i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DISPLAY);
+	i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DEFAULT_DISPBOOST);
 
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 	i915_gem_object_unpin_pages(obj);
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4.5 5/8] drm/i915: cgroup integration (v4)
  2018-03-21 23:23 [PATCH v4.5 0/8] cgroup private data and DRM/i915 integration Matt Roper
                   ` (3 preceding siblings ...)
  2018-03-21 23:23 ` [PATCH v4.5 4/8] drm/i915: Adjust internal priority definitions (v2) Matt Roper
@ 2018-03-21 23:23 ` Matt Roper
  2018-03-21 23:23 ` [PATCH v4.5 6/8] drm/i915: Introduce 'priority offset' for GPU contexts (v4) Matt Roper
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2018-03-21 23:23 UTC (permalink / raw)
  To: dri-devel, intel-gfx, cgroups

Introduce a new DRM_IOCTL_I915_CGROUP_SETPARAM ioctl that will allow
userspace to set i915-specific parameters for individual cgroups.  i915
cgroup data will be registered and later looked up via the new
cgroup_priv infrastructure.

v2:
 - Large rebase/rewrite for new cgroup_priv interface
v3:
 - Another rebase/rewrite for ida-based keys and kref-based storage
 - Access control no longer follows cgroup filesystem permissions;
   instead we restrict access to either DRM master or
   capable(CAP_SYS_RESOURCE).
v4:
 - Fix checkpatch warnings (Intel CI)

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/Makefile      |   1 +
 drivers/gpu/drm/i915/i915_cgroup.c | 140 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.c    |   8 +++
 drivers/gpu/drm/i915/i915_drv.h    |  32 +++++++++
 include/uapi/drm/i915_drm.h        |  12 ++++
 5 files changed, 193 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/i915_cgroup.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 552e43e9663f..26031185cf0e 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -48,6 +48,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..efe76c2a8915
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_cgroup.c
@@ -0,0 +1,140 @@
+/* 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 kref ref;
+};
+
+static inline struct i915_cgroup_data *
+cgrp_ref_to_i915(struct kref *ref)
+{
+	return container_of(ref, struct i915_cgroup_data, ref);
+}
+
+static void
+i915_cgroup_free(struct kref *ref)
+{
+	struct i915_cgroup_data *priv;
+
+	priv = cgrp_ref_to_i915(ref);
+	kfree(priv);
+}
+
+int
+i915_cgroup_init(struct drm_i915_private *dev_priv)
+{
+	int ret = 0;
+
+	dev_priv->cgroup_priv_key = cgroup_priv_getkey(i915_cgroup_free);
+	if (dev_priv->cgroup_priv_key < 0) {
+		DRM_DEBUG_DRIVER("Failed to get a cgroup private data key\n");
+		ret = dev_priv->cgroup_priv_key;
+	}
+
+	mutex_init(&dev_priv->cgroup_lock);
+
+	return ret;
+}
+
+void
+i915_cgroup_shutdown(struct drm_i915_private *dev_priv)
+{
+	cgroup_priv_destroykey(dev_priv->cgroup_priv_key);
+}
+
+/*
+ * Return i915 cgroup private data, creating and registering it if one doesn't
+ * already exist for this cgroup.
+ */
+__maybe_unused
+static struct i915_cgroup_data *
+get_or_create_cgroup_data(struct drm_i915_private *dev_priv,
+			  struct cgroup *cgrp)
+{
+	struct kref *ref;
+	struct i915_cgroup_data *priv;
+
+	mutex_lock(&dev_priv->cgroup_lock);
+
+	ref = cgroup_priv_get(cgrp, dev_priv->cgroup_priv_key);
+	if (ref) {
+		priv = cgrp_ref_to_i915(ref);
+	} else {
+		priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+		if (!priv) {
+			priv = ERR_PTR(-ENOMEM);
+			goto out;
+		}
+
+		kref_init(&priv->ref);
+		cgroup_priv_install(cgrp, dev_priv->cgroup_priv_key,
+				    &priv->ref);
+	}
+
+out:
+	mutex_unlock(&dev_priv->cgroup_lock);
+
+	return priv;
+}
+
+/**
+ * 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_cgroup_param *req = data;
+	struct cgroup *cgrp;
+	int ret;
+
+	/* 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:  For now we grant access via CAP_SYS_RESOURCE _or_
+	 * DRM master status.
+	 */
+	if (!capable(CAP_SYS_RESOURCE) && !drm_is_current_master(file)) {
+		DRM_DEBUG_DRIVER("Insufficient permissions to adjust i915 cgroup settings\n");
+		goto out;
+	}
+
+	switch (req->param) {
+	default:
+		DRM_DEBUG_DRIVER("Invalid cgroup parameter %lld\n", req->param);
+		ret = -EINVAL;
+	}
+
+out:
+	cgroup_put(cgrp);
+
+	return ret;
+}
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a7d3275f45d2..ad10fb2a2907 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1407,6 +1407,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;
@@ -1433,6 +1437,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))
@@ -2833,6 +2839,8 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
 	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_QUERY, i915_query_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 2d7a89fcc0dc..ed365fae1073 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1738,6 +1738,15 @@ struct drm_i915_private {
 
 	struct intel_ppat ppat;
 
+	/* cgroup private data */
+	int cgroup_priv_key;
+
+	/*
+	 * protects against concurrent attempts to create private data for a
+	 * cgroup
+	 */
+	struct mutex cgroup_lock;
+
 	/* Kernel Modesetting */
 
 	struct intel_crtc *plane_to_crtc_mapping[I915_MAX_PIPES];
@@ -2678,6 +2687,29 @@ 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 7f5634ce8e88..735128fa61de 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -319,6 +319,7 @@ typedef struct _drm_i915_sarea {
 #define DRM_I915_PERF_ADD_CONFIG	0x37
 #define DRM_I915_PERF_REMOVE_CONFIG	0x38
 #define DRM_I915_QUERY			0x39
+#define DRM_I915_CGROUP_SETPARAM	0x3a
 
 #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)
@@ -377,6 +378,7 @@ typedef struct _drm_i915_sarea {
 #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_QUERY			DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_QUERY, struct drm_i915_query)
+#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.
@@ -1717,6 +1719,16 @@ struct drm_i915_query_topology_info {
 	__u8 data[];
 };
 
+/**
+ * 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

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4.5 6/8] drm/i915: Introduce 'priority offset' for GPU contexts (v4)
  2018-03-21 23:23 [PATCH v4.5 0/8] cgroup private data and DRM/i915 integration Matt Roper
                   ` (4 preceding siblings ...)
  2018-03-21 23:23 ` [PATCH v4.5 5/8] drm/i915: cgroup integration (v4) Matt Roper
@ 2018-03-21 23:23 ` Matt Roper
  2018-03-21 23:23 ` [PATCH v4.5 7/8] drm/i915: Introduce per-cgroup display boost setting Matt Roper
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2018-03-21 23:23 UTC (permalink / raw)
  To: dri-devel, intel-gfx, cgroups

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.

v2:
 - Rebase onto new cgroup_priv API
 - Use current instead of drm_file->pid to determine which process to
   lookup priority for. (Chris)
 - Don't forget to subtract priority offset in context_getparam ioctl to
   make it match setparam behavior. (Chris)

v3:
 - Rebase again onto new idr/kref-based cgroup_priv API
 - Bound priority offset such that effective priority from settings
   on context + cgroup fall within [-0x7fffff, 0x7fffff]. (Chris)

v4:
 - checkpatch warning fixes (Intel CI)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/i915_cgroup.c      | 54 +++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_drv.h         |  7 +++++
 drivers/gpu/drm/i915/i915_gem_context.c |  7 +++--
 drivers/gpu/drm/i915/i915_gem_context.h |  9 ++++++
 drivers/gpu/drm/i915/i915_request.h     |  4 +++
 include/uapi/drm/i915_drm.h             |  1 +
 6 files changed, 77 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cgroup.c b/drivers/gpu/drm/i915/i915_cgroup.c
index efe76c2a8915..0921d624f840 100644
--- a/drivers/gpu/drm/i915/i915_cgroup.c
+++ b/drivers/gpu/drm/i915/i915_cgroup.c
@@ -10,6 +10,8 @@
 #include "i915_drv.h"
 
 struct i915_cgroup_data {
+	int priority_offset;
+
 	struct kref ref;
 };
 
@@ -54,7 +56,6 @@ i915_cgroup_shutdown(struct drm_i915_private *dev_priv)
  * Return i915 cgroup private data, creating and registering it if one doesn't
  * already exist for this cgroup.
  */
-__maybe_unused
 static struct i915_cgroup_data *
 get_or_create_cgroup_data(struct drm_i915_private *dev_priv,
 			  struct cgroup *cgrp)
@@ -98,9 +99,11 @@ 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;
-	int ret;
+	struct i915_cgroup_data *cgrpdata;
+	int top, bot, ret = 0;
 
 	/* We don't actually support any flags yet. */
 	if (req->flags) {
@@ -127,14 +130,61 @@ i915_cgroup_setparam_ioctl(struct drm_device *dev,
 		goto out;
 	}
 
+	cgrpdata = get_or_create_cgroup_data(dev_priv, cgrp);
+	if (IS_ERR(cgrpdata)) {
+		ret = PTR_ERR(cgrpdata);
+		goto out;
+	}
+
 	switch (req->param) {
+	case I915_CGROUP_PARAM_PRIORITY_OFFSET:
+		top = req->value + I915_CONTEXT_MAX_USER_PRIORITY;
+		bot = req->value - I915_CONTEXT_MIN_USER_PRIORITY;
+		if (top <= I915_PRIORITY_MAX && bot >= I915_PRIORITY_MIN) {
+			DRM_DEBUG_DRIVER("Setting cgroup priority offset to %lld\n",
+					 req->value);
+			cgrpdata->priority_offset = req->value;
+		} else {
+			DRM_DEBUG_DRIVER("Invalid cgroup priority offset %lld\n",
+					 req->value);
+			ret = -EINVAL;
+		}
+		break;
+
 	default:
 		DRM_DEBUG_DRIVER("Invalid cgroup parameter %lld\n", req->param);
 		ret = -EINVAL;
 	}
 
+	kref_put(&cgrpdata->ref, i915_cgroup_free);
+
 out:
 	cgroup_put(cgrp);
 
 	return ret;
 }
+
+/*
+ * Generator for simple getter functions that look up a cgroup private data
+ * field for the current task's cgroup.  It's safe to call these before
+ * a cgroup private data key has been registered; they'll just return the
+ * default value in that case.
+ */
+#define CGROUP_GET(name, field, def) \
+int i915_cgroup_get_current_##name(struct drm_i915_private *dev_priv)	\
+{									\
+	struct kref *ref;						\
+	int val = def;							\
+	if (!dev_priv->cgroup_priv_key)					\
+		return def;						\
+	ref = cgroup_priv_get_current(dev_priv->cgroup_priv_key);	\
+	if (ref) {							\
+		val = cgrp_ref_to_i915(ref)->field;			\
+		kref_put(ref, i915_cgroup_free);			\
+	}								\
+	return val;							\
+}
+
+CGROUP_GET(prio_offset, priority_offset, 0)
+
+#undef CGROUP_GET
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ed365fae1073..72e6cd7bfbae 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2693,6 +2693,7 @@ 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_current_prio_offset(struct drm_i915_private *dev_priv);
 #else
 static inline int
 i915_cgroup_init(struct drm_i915_private *dev_priv)
@@ -2708,6 +2709,12 @@ i915_cgroup_setparam_ioctl(struct drm_device *dev, void *data,
 {
 	return -EINVAL;
 }
+
+static inline int
+i915_cgroup_get_current_prio_offset(struct drm_i915_private *dev_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 4bae1be52294..f9b6fe0fd3ee 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -280,7 +280,8 @@ __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_current_prio_offset(dev_priv);
+	ctx->priority = I915_PRIORITY_NORMAL + ctx->priority_offset;
 
 	INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL);
 	INIT_LIST_HEAD(&ctx->handles_list);
@@ -748,7 +749,7 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
 		args->value = i915_gem_context_is_bannable(ctx);
 		break;
 	case I915_CONTEXT_PARAM_PRIORITY:
-		args->value = ctx->priority;
+		args->value = ctx->priority - ctx->priority_offset;
 		break;
 	default:
 		ret = -EINVAL;
@@ -821,7 +822,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 7854262ddfd9..c3b4fb54fbb6 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -150,6 +150,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/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
index 72b13fc2b72b..cf7a7147daf3 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -87,6 +87,10 @@ enum {
 	I915_PRIORITY_NORMAL = I915_CONTEXT_DEFAULT_PRIORITY,
 	I915_PRIORITY_DEFAULT_DISPBOOST = I915_CONTEXT_MAX_USER_PRIORITY + 1,
 
+	/* Range reachable by combining user priority + cgroup offset */
+	I915_PRIORITY_MAX = 0x7fffff,
+	I915_PRIORITY_MIN = -I915_PRIORITY_MAX,
+
 	/* Special case priority values */
 	I915_PRIORITY_INVALID = INT_MIN,
 	I915_PRIORITY_IDLE = INT_MIN + 1,
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 735128fa61de..6b70f46d224e 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1726,6 +1726,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

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4.5 7/8] drm/i915: Introduce per-cgroup display boost setting
  2018-03-21 23:23 [PATCH v4.5 0/8] cgroup private data and DRM/i915 integration Matt Roper
                   ` (5 preceding siblings ...)
  2018-03-21 23:23 ` [PATCH v4.5 6/8] drm/i915: Introduce 'priority offset' for GPU contexts (v4) Matt Roper
@ 2018-03-21 23:23 ` Matt Roper
  2018-03-21 23:23 ` [PATCH v4.5 8/8] drm/i915: Add context priority & priority offset to debugfs (v2) Matt Roper
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2018-03-21 23:23 UTC (permalink / raw)
  To: dri-devel, intel-gfx, cgroups

Usually display-boosted contexts get treated as
I915_CONTEXT_MAX_USER_PRIORITY+1, which prioritizes them above regular
GPU contexts.  Now that we allow a much larger range of effective
priority values via per-cgroup priority offsets, a system administrator
may want more detailed control over how much a process should be boosted
by display operations (i.e., can a context from a cgroup with a low
priority offset still be display boosted above contexts from a cgroup
with a much higher priority offset?  or are come cgroups more important
than maintaining display rate?).

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/i915_cgroup.c   | 17 +++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h      |  6 ++++++
 drivers/gpu/drm/i915/i915_request.h  |  1 +
 drivers/gpu/drm/i915/intel_display.c |  5 +++--
 include/uapi/drm/i915_drm.h          |  1 +
 5 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cgroup.c b/drivers/gpu/drm/i915/i915_cgroup.c
index 0921d624f840..2bef8a5cac93 100644
--- a/drivers/gpu/drm/i915/i915_cgroup.c
+++ b/drivers/gpu/drm/i915/i915_cgroup.c
@@ -11,6 +11,7 @@
 
 struct i915_cgroup_data {
 	int priority_offset;
+	int display_boost;
 
 	struct kref ref;
 };
@@ -75,6 +76,8 @@ get_or_create_cgroup_data(struct drm_i915_private *dev_priv,
 			goto out;
 		}
 
+		priv->display_boost = I915_PRIORITY_DEFAULT_DISPBOOST;
+
 		kref_init(&priv->ref);
 		cgroup_priv_install(cgrp, dev_priv->cgroup_priv_key,
 				    &priv->ref);
@@ -151,6 +154,19 @@ i915_cgroup_setparam_ioctl(struct drm_device *dev,
 		}
 		break;
 
+	case I915_CGROUP_PARAM_DISPBOOST_PRIORITY:
+		if (req->value < I915_PRIORITY_MAX_DISPBOOST &&
+		    req->value > I915_PRIORITY_MIN) {
+			DRM_DEBUG_DRIVER("Setting cgroup display boost priority to %lld\n",
+					 req->value);
+			cgrpdata->display_boost = req->value;
+		} else {
+			DRM_DEBUG_DRIVER("Invalid cgroup display boost priority %lld\n",
+					 req->value);
+			ret = -EINVAL;
+		}
+		break;
+
 	default:
 		DRM_DEBUG_DRIVER("Invalid cgroup parameter %lld\n", req->param);
 		ret = -EINVAL;
@@ -186,5 +202,6 @@ int i915_cgroup_get_current_##name(struct drm_i915_private *dev_priv)	\
 }
 
 CGROUP_GET(prio_offset, priority_offset, 0)
+CGROUP_GET(dispboost, display_boost, I915_PRIORITY_DEFAULT_DISPBOOST);
 
 #undef CGROUP_GET
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 72e6cd7bfbae..bde58327b892 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2694,6 +2694,7 @@ 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_current_prio_offset(struct drm_i915_private *dev_priv);
+int i915_cgroup_get_current_dispboost(struct drm_i915_private *dev_priv);
 #else
 static inline int
 i915_cgroup_init(struct drm_i915_private *dev_priv)
@@ -2715,6 +2716,11 @@ i915_cgroup_get_current_prio_offset(struct drm_i915_private *dev_priv)
 {
 	return 0;
 }
+static inline int
+i915_cgroup_get_current_dispboost(struct drm_i915_private *dev_priv)
+{
+	return I915_PRIORITY_DEFAULT_DISPBOOST;
+}
 #endif
 
 /* i915_drv.c */
diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
index cf7a7147daf3..db300d93fd08 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -90,6 +90,7 @@ enum {
 	/* Range reachable by combining user priority + cgroup offset */
 	I915_PRIORITY_MAX = 0x7fffff,
 	I915_PRIORITY_MIN = -I915_PRIORITY_MAX,
+	I915_PRIORITY_MAX_DISPBOOST = I915_PRIORITY_MAX + 1,
 
 	/* Special case priority values */
 	I915_PRIORITY_INVALID = INT_MIN,
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b053a21f682b..1d0245e2fd75 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12731,7 +12731,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 	struct drm_framebuffer *fb = new_state->fb;
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb);
-	int ret;
+	int boost, ret;
 
 	if (old_obj) {
 		struct drm_crtc_state *crtc_state =
@@ -12783,7 +12783,8 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 
 	ret = intel_plane_pin_fb(to_intel_plane_state(new_state));
 
-	i915_gem_object_wait_priority(obj, 0, I915_PRIORITY_DEFAULT_DISPBOOST);
+	boost = i915_cgroup_get_current_dispboost(dev_priv);
+	i915_gem_object_wait_priority(obj, 0, boost);
 
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 	i915_gem_object_unpin_pages(obj);
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 6b70f46d224e..ad454f121884 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1727,6 +1727,7 @@ struct drm_i915_cgroup_param {
 	__u32 flags;
 	__u64 param;
 #define I915_CGROUP_PARAM_PRIORITY_OFFSET	0x1
+#define I915_CGROUP_PARAM_DISPBOOST_PRIORITY    0x2
 	__s64 value;
 };
 
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v4.5 8/8] drm/i915: Add context priority & priority offset to debugfs (v2)
  2018-03-21 23:23 [PATCH v4.5 0/8] cgroup private data and DRM/i915 integration Matt Roper
                   ` (6 preceding siblings ...)
  2018-03-21 23:23 ` [PATCH v4.5 7/8] drm/i915: Introduce per-cgroup display boost setting Matt Roper
@ 2018-03-21 23:23 ` Matt Roper
  2018-03-21 23:30 ` ✗ Fi.CI.CHECKPATCH: warning for cgroup private data and DRM/i915 integration (rev3) Patchwork
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2018-03-21 23:23 UTC (permalink / raw)
  To: dri-devel, intel-gfx, cgroups

Update i915_context_status to include priority information.

v2:
 - Clarify that the offset is based on cgroup (Chris)

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 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 7816cd53100a..ac2dad38dee1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1959,6 +1959,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 (cgroup 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

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for cgroup private data and DRM/i915 integration (rev3)
  2018-03-21 23:23 [PATCH v4.5 0/8] cgroup private data and DRM/i915 integration Matt Roper
                   ` (7 preceding siblings ...)
  2018-03-21 23:23 ` [PATCH v4.5 8/8] drm/i915: Add context priority & priority offset to debugfs (v2) Matt Roper
@ 2018-03-21 23:30 ` Patchwork
  2018-03-22 10:12   ` Jani Nikula
  2018-03-21 23:48 ` ✓ Fi.CI.BAT: success " Patchwork
  2018-03-22  7:14 ` ✓ Fi.CI.IGT: " Patchwork
  10 siblings, 1 reply; 14+ messages in thread
From: Patchwork @ 2018-03-21 23:30 UTC (permalink / raw)
  To: kbuild test robot; +Cc: intel-gfx

== Series Details ==

Series: cgroup private data and DRM/i915 integration (rev3)
URL   : https://patchwork.freedesktop.org/series/40142/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
8c23fa59eef7 cgroup: Allow registration and lookup of cgroup private data (v3)
95f12a7ecd5d cgroup: Introduce task_get_dfl_cgroup() (v2)
b6569483024f cgroup: Introduce cgroup_priv_get_current
f8a545376030 drm/i915: Adjust internal priority definitions (v2)
8e4b9162691c drm/i915: cgroup integration (v4)
-:36: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#36: 
new file mode 100644

-:280: WARNING:LONG_LINE: line over 100 characters
#280: FILE: include/uapi/drm/i915_drm.h:381:
+#define DRM_IOCTL_I915_CGROUP_SETPARAM		DRM_IOW(DRM_COMMAND_BASE + DRM_I915_CGROUP_SETPARAM, struct drm_i915_cgroup_param)

total: 0 errors, 2 warnings, 0 checks, 247 lines checked
44858a8e8b6f drm/i915: Introduce 'priority offset' for GPU contexts (v4)
-:125: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'def' - possible side-effects?
#125: FILE: drivers/gpu/drm/i915/i915_cgroup.c:173:
+#define CGROUP_GET(name, field, def) \
+int i915_cgroup_get_current_##name(struct drm_i915_private *dev_priv)	\
+{									\
+	struct kref *ref;						\
+	int val = def;							\
+	if (!dev_priv->cgroup_priv_key)					\
+		return def;						\
+	ref = cgroup_priv_get_current(dev_priv->cgroup_priv_key);	\
+	if (ref) {							\
+		val = cgrp_ref_to_i915(ref)->field;			\
+		kref_put(ref, i915_cgroup_free);			\
+	}								\
+	return val;							\
+}

total: 0 errors, 0 warnings, 1 checks, 164 lines checked
5909f05aaca5 drm/i915: Introduce per-cgroup display boost setting
-:83: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
#83: FILE: drivers/gpu/drm/i915/i915_drv.h:2719:
 }
+static inline int

total: 0 errors, 0 warnings, 1 checks, 89 lines checked
5b1782be5720 drm/i915: Add context priority & priority offset to debugfs (v2)

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* ✓ Fi.CI.BAT: success for cgroup private data and DRM/i915 integration (rev3)
  2018-03-21 23:23 [PATCH v4.5 0/8] cgroup private data and DRM/i915 integration Matt Roper
                   ` (8 preceding siblings ...)
  2018-03-21 23:30 ` ✗ Fi.CI.CHECKPATCH: warning for cgroup private data and DRM/i915 integration (rev3) Patchwork
@ 2018-03-21 23:48 ` Patchwork
  2018-03-22  7:14 ` ✓ Fi.CI.IGT: " Patchwork
  10 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-03-21 23:48 UTC (permalink / raw)
  To: kbuild test robot; +Cc: intel-gfx

== Series Details ==

Series: cgroup private data and DRM/i915 integration (rev3)
URL   : https://patchwork.freedesktop.org/series/40142/
State : success

== Summary ==

Series 40142v3 cgroup private data and DRM/i915 integration
https://patchwork.freedesktop.org/api/1.0/series/40142/revisions/3/mbox/

---- Known issues:

Test debugfs_test:
        Subgroup read_all_entries:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (fi-skl-6770hq) fdo#100368
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                pass       -> INCOMPLETE (fi-bxt-dsi) fdo#103927

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:432s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:448s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:379s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:540s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:296s
fi-bxt-dsi       total:243  pass:216  dwarn:0   dfail:0   fail:0   skip:26 
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:514s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:514s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:505s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:411s
fi-cfl-s2        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:567s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:511s
fi-cnl-drrs      total:285  pass:254  dwarn:3   dfail:0   fail:0   skip:28  time:548s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:583s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:426s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:314s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:532s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:408s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:422s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:469s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:430s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:475s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:467s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:513s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:656s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:440s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:531s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:497s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:487s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:426s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:449s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:589s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:404s

dff9ece60048108782aab6d6123822c1d34b0e5a drm-tip: 2018y-03m-21d-20h-44m-14s UTC integration manifest
5b1782be5720 drm/i915: Add context priority & priority offset to debugfs (v2)
5909f05aaca5 drm/i915: Introduce per-cgroup display boost setting
44858a8e8b6f drm/i915: Introduce 'priority offset' for GPU contexts (v4)
8e4b9162691c drm/i915: cgroup integration (v4)
f8a545376030 drm/i915: Adjust internal priority definitions (v2)
b6569483024f cgroup: Introduce cgroup_priv_get_current
95f12a7ecd5d cgroup: Introduce task_get_dfl_cgroup() (v2)
8c23fa59eef7 cgroup: Allow registration and lookup of cgroup private data (v3)

== Logs ==

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* ✓ Fi.CI.IGT: success for cgroup private data and DRM/i915 integration (rev3)
  2018-03-21 23:23 [PATCH v4.5 0/8] cgroup private data and DRM/i915 integration Matt Roper
                   ` (9 preceding siblings ...)
  2018-03-21 23:48 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-03-22  7:14 ` Patchwork
  10 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2018-03-22  7:14 UTC (permalink / raw)
  To: kbuild test robot; +Cc: intel-gfx

== Series Details ==

Series: cgroup private data and DRM/i915 integration (rev3)
URL   : https://patchwork.freedesktop.org/series/40142/
State : success

== Summary ==

---- Known issues:

Test kms_flip:
        Subgroup 2x-flip-vs-expired-vblank:
                pass       -> FAIL       (shard-hsw) fdo#102887
        Subgroup 2x-plain-flip-ts-check-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#100368 +2
        Subgroup dpms-vs-vblank-race-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#103060
Test kms_frontbuffer_tracking:
        Subgroup fbc-rgb101010-draw-pwrite:
                pass       -> FAIL       (shard-apl) fdo#101623
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-apl) fdo#99912
Test kms_sysfs_edid_timing:
                pass       -> WARN       (shard-apl) fdo#100047

fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047

shard-apl        total:3478 pass:1813 dwarn:1   dfail:0   fail:8   skip:1655 time:13047s
shard-hsw        total:3478 pass:1764 dwarn:1   dfail:0   fail:5   skip:1707 time:11801s
shard-snb        total:3478 pass:1357 dwarn:1   dfail:0   fail:3   skip:2117 time:7275s
Blacklisted hosts:
shard-kbl        total:3478 pass:1933 dwarn:7   dfail:0   fail:10  skip:1528 time:9963s

== Logs ==

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: ✗ Fi.CI.CHECKPATCH: warning for cgroup private data and DRM/i915 integration (rev3)
  2018-03-21 23:30 ` ✗ Fi.CI.CHECKPATCH: warning for cgroup private data and DRM/i915 integration (rev3) Patchwork
@ 2018-03-22 10:12   ` Jani Nikula
  0 siblings, 0 replies; 14+ messages in thread
From: Jani Nikula @ 2018-03-22 10:12 UTC (permalink / raw)
  To: Patchwork, kbuild test robot; +Cc: intel-gfx

On Wed, 21 Mar 2018, Patchwork <patchwork@emeril.freedesktop.org> wrote:
> == Series Details ==
>
> Series: cgroup private data and DRM/i915 integration (rev3)
> URL   : https://patchwork.freedesktop.org/series/40142/
> State : warning
>
> == Summary ==
>
> $ dim checkpatch origin/drm-tip
> 8c23fa59eef7 cgroup: Allow registration and lookup of cgroup private data (v3)
> 95f12a7ecd5d cgroup: Introduce task_get_dfl_cgroup() (v2)
> b6569483024f cgroup: Introduce cgroup_priv_get_current
> f8a545376030 drm/i915: Adjust internal priority definitions (v2)
> 8e4b9162691c drm/i915: cgroup integration (v4)
> -:36: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
> #36: 
> new file mode 100644
>
> -:280: WARNING:LONG_LINE: line over 100 characters
> #280: FILE: include/uapi/drm/i915_drm.h:381:
> +#define DRM_IOCTL_I915_CGROUP_SETPARAM		DRM_IOW(DRM_COMMAND_BASE + DRM_I915_CGROUP_SETPARAM, struct drm_i915_cgroup_param)
>
> total: 0 errors, 2 warnings, 0 checks, 247 lines checked
> 44858a8e8b6f drm/i915: Introduce 'priority offset' for GPU contexts (v4)
> -:125: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'def' - possible side-effects?
> #125: FILE: drivers/gpu/drm/i915/i915_cgroup.c:173:
> +#define CGROUP_GET(name, field, def) \
> +int i915_cgroup_get_current_##name(struct drm_i915_private *dev_priv)	\
> +{									\
> +	struct kref *ref;						\
> +	int val = def;							\
> +	if (!dev_priv->cgroup_priv_key)					\
> +		return def;						\

Could return val here to only use def once. Even though none of the
macro instantiations use side-effects. No biggie.

BR,
Jani.

> +	ref = cgroup_priv_get_current(dev_priv->cgroup_priv_key);	\
> +	if (ref) {							\
> +		val = cgrp_ref_to_i915(ref)->field;			\
> +		kref_put(ref, i915_cgroup_free);			\
> +	}								\
> +	return val;							\
> +}
>
> total: 0 errors, 0 warnings, 1 checks, 164 lines checked
> 5909f05aaca5 drm/i915: Introduce per-cgroup display boost setting
> -:83: CHECK:LINE_SPACING: Please use a blank line after function/struct/union/enum declarations
> #83: FILE: drivers/gpu/drm/i915/i915_drv.h:2719:
>  }
> +static inline int
>
> total: 0 errors, 0 warnings, 1 checks, 89 lines checked
> 5b1782be5720 drm/i915: Add context priority & priority offset to debugfs (v2)
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4.5 1/8] cgroup: Allow registration and lookup of cgroup private data (v3)
  2018-03-21 23:23 ` [PATCH v4.5 1/8] cgroup: Allow registration and lookup of cgroup private data (v3) Matt Roper
@ 2018-03-22 18:04   ` Chris Wilson
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2018-03-22 18:04 UTC (permalink / raw)
  To: Matt Roper, dri-devel, intel-gfx, cgroups
  Cc: Tejun Heo, Roman Gushchin, Alexei Starovoitov

Quoting Matt Roper (2018-03-21 23:23:37)
> There are cases where other parts of the kernel may wish to store data
> associated with individual cgroups without building a full cgroup
> controller.  Let's add interfaces to allow them to register and lookup
> this private data for individual cgroups.
> 
> A kernel system (e.g., a driver) that wishes to register private data
> for a cgroup should start by obtaining a unique private data key by
> calling cgroup_priv_getkey().  It may then associate private data
> with a cgroup by calling cgroup_priv_install(cgrp, key, ref) where 'ref'
> is a pointer to a kref field inside the private data structure.  The
> private data may later be looked up by calling cgroup_priv_get(cgrp,
> key) to obtain a new reference to the private data.  Private data may be
> unregistered via cgroup_priv_release(cgrp, key).
> 
> If a cgroup is removed, the reference count for all private data objects
> will be decremented.
> 
> v2:  Significant overhaul suggested by Tejun, Alexei, and Roman
>  - Rework interface to make consumers obtain an ida-based key rather
>    than supplying their own arbitrary void*
>  - Internal implementation now uses per-cgroup radixtrees which should
>    allow much faster lookup than the previous hashtable approach
>  - Private data is registered via kref, allowing a single private data
>    structure to potentially be assigned to multiple cgroups.
> 
> v3:
>  - Spare warning fixes (kbuild test robot)
> 
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Alexei Starovoitov <ast@fb.com>
> Cc: Roman Gushchin <guro@fb.com>
> Cc: cgroups@vger.kernel.org
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> 
> fixup! cgroup: Allow registration and lookup of cgroup private data (v2)
> ---
>  include/linux/cgroup-defs.h |  10 +++
>  include/linux/cgroup.h      |   7 ++
>  kernel/cgroup/cgroup.c      | 183 +++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 198 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
> index 9f242b876fde..9086d963cc0a 100644
> --- a/include/linux/cgroup-defs.h
> +++ b/include/linux/cgroup-defs.h
> @@ -427,6 +427,16 @@ struct cgroup {
>         /* used to store eBPF programs */
>         struct cgroup_bpf bpf;
>  
> +       /*
> +        * cgroup private data registered by other non-controller parts of the
> +        * kernel.  Insertions are protected by privdata_lock, lookups by
> +        * rcu_read_lock().
> +        */
> +       struct radix_tree_root privdata;
> +
> +       /* Protect against concurrent insertions/deletions to privdata */
> +       spinlock_t privdata_lock;
> +
>         /* ids of the ancestors at each level including self */
>         int ancestor_ids[];
>  };
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index 473e0c0abb86..63d22dfa00bd 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -833,4 +833,11 @@ static inline void put_cgroup_ns(struct cgroup_namespace *ns)
>                 free_cgroup_ns(ns);
>  }
>  
> +/* cgroup private data handling */
> +int cgroup_priv_getkey(void (*free)(struct kref *));
> +void cgroup_priv_destroykey(int key);
> +int cgroup_priv_install(struct cgroup *cgrp, int key, struct kref *ref);
> +struct kref *cgroup_priv_get(struct cgroup *cgrp, int key);
> +void cgroup_priv_release(struct cgroup *cgrp, int key);
> +
>  #endif /* _LINUX_CGROUP_H */
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index 8cda3bc3ae22..3268a21e8158 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -81,8 +81,15 @@ EXPORT_SYMBOL_GPL(css_set_lock);
>  #endif
>  
>  /*
> - * Protects cgroup_idr and css_idr so that IDs can be released without
> - * grabbing cgroup_mutex.
> + * ID allocator for cgroup private data keys; the ID's allocated here will
> + * be used to index all per-cgroup radix trees.  The radix tree built into
> + * the IDR itself will store a key-specific function to be passed to kref_put.
> + */
> +static DEFINE_IDR(cgroup_priv_idr);

Fun two radixtrees, one to hold the (*free)() and the other the void*.

Would not just a
struct cgroup_privdata {
	struct rcu_head rcu;
	void (*free)(struct kref *);
}
suffice for subclassing? (Also this is a prime candidate for switching
to XArray.)

> +struct kref *
> +cgroup_priv_get(struct cgroup *cgrp, int key)
> +{
> +       struct kref *ref;
> +
> +       WARN_ON(cgrp->root != &cgrp_dfl_root);
> +       WARN_ON(key == 0);
> +
> +       rcu_read_lock();
> +
> +       ref = radix_tree_lookup(&cgrp->privdata, key);
> +       if (ref)
> +               kref_get(ref);

This is not safe, you need

if (ref && !kref_get_unless_zero(ref))
	ref = NULL;

otherwise the cgroup_priv_release() may drop the last reference prior to
you obtaining yours. Also requires the privdata to be RCU protected.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-03-22 18:04 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-21 23:23 [PATCH v4.5 0/8] cgroup private data and DRM/i915 integration Matt Roper
2018-03-21 23:23 ` [PATCH v4.5 1/8] cgroup: Allow registration and lookup of cgroup private data (v3) Matt Roper
2018-03-22 18:04   ` Chris Wilson
2018-03-21 23:23 ` [PATCH v4.5 2/8] cgroup: Introduce task_get_dfl_cgroup() (v2) Matt Roper
2018-03-21 23:23 ` [PATCH v4.5 3/8] cgroup: Introduce cgroup_priv_get_current Matt Roper
2018-03-21 23:23 ` [PATCH v4.5 4/8] drm/i915: Adjust internal priority definitions (v2) Matt Roper
2018-03-21 23:23 ` [PATCH v4.5 5/8] drm/i915: cgroup integration (v4) Matt Roper
2018-03-21 23:23 ` [PATCH v4.5 6/8] drm/i915: Introduce 'priority offset' for GPU contexts (v4) Matt Roper
2018-03-21 23:23 ` [PATCH v4.5 7/8] drm/i915: Introduce per-cgroup display boost setting Matt Roper
2018-03-21 23:23 ` [PATCH v4.5 8/8] drm/i915: Add context priority & priority offset to debugfs (v2) Matt Roper
2018-03-21 23:30 ` ✗ Fi.CI.CHECKPATCH: warning for cgroup private data and DRM/i915 integration (rev3) Patchwork
2018-03-22 10:12   ` Jani Nikula
2018-03-21 23:48 ` ✓ Fi.CI.BAT: success " Patchwork
2018-03-22  7:14 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.