* [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* 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
* [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