dri-devel.lists.freedesktop.org archive mirror
 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
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ 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] 10+ 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
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 10+ 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] 10+ 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
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ 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] 10+ 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
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ 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] 10+ 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
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ 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] 10+ 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
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ 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] 10+ 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
  2018-03-21 23:23 ` [PATCH v4.5 8/8] drm/i915: Add context priority & priority offset to debugfs (v2) Matt Roper
  7 siblings, 0 replies; 10+ 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] 10+ 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
  7 siblings, 0 replies; 10+ 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] 10+ 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
  7 siblings, 0 replies; 10+ 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] 10+ 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; 10+ 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] 10+ messages in thread

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

Thread overview: 10+ 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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).