Re: [PATCH v3 1/6] cgroup: Allow registration and lookup of cgroup private data

2018-03-13 Thread Tejun Heo
Hello, Matt.

cc'ing Roman and Alexei.

On Tue, Mar 06, 2018 at 03:46:55PM -0800, Matt Roper wrote:
> 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 will do so by subclassing the 'struct cgroup_priv'
> structure to describe the necessary data to store.  Before registering a
> private data structure to a cgroup, the caller should fill in the 'key'
> and 'free' fields of the base cgroup_priv structure.
> 
>  * 'key' should be a unique void* that will act as a key for future
>privdata lookups/removals.  Note that this allows drivers to store
>per-device private data for a cgroup by using a device pointer as a key.
> 
>  * 'free' should be a function pointer to a function that may be used
>to destroy the private data.  This function will be called
>automatically if the underlying cgroup is destroyed.

This feature turned out to have more users than I originally
anticipated and bpf also wants something like this to track network
states.  The requirements are pretty similar but not quite the same.
The extra requirements are...

* Lookup must be really cheap.  Probably using pointer hash or walking
  list isn't great, so maybe idr based lookup + RCU protected index
  table per cgroup?

* It should support both regular memory and percpu pointers.  Given
  that what cgroup does is pretty much cgroup:key -> pointer lookup,
  it's mostly about getting the interface right so that it's not too
  error-prone.

Sorry about moving the goalpost.

Thanks.

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


[PATCH v3 1/6] cgroup: Allow registration and lookup of cgroup private data

2018-03-06 Thread Matt Roper
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 will do so by subclassing the 'struct cgroup_priv'
structure to describe the necessary data to store.  Before registering a
private data structure to a cgroup, the caller should fill in the 'key'
and 'free' fields of the base cgroup_priv structure.

 * 'key' should be a unique void* that will act as a key for future
   privdata lookups/removals.  Note that this allows drivers to store
   per-device private data for a cgroup by using a device pointer as a key.

 * 'free' should be a function pointer to a function that may be used
   to destroy the private data.  This function will be called
   automatically if the underlying cgroup is destroyed.

Cc: Tejun Heo 
Cc: cgro...@vger.kernel.org
Signed-off-by: Matt Roper 
---
 include/linux/cgroup-defs.h | 38 ++
 include/linux/cgroup.h  | 78 +
 kernel/cgroup/cgroup.c  | 14 
 3 files changed, 130 insertions(+)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 9f242b876fde..17c679a7b5de 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -8,6 +8,7 @@
 #ifndef _LINUX_CGROUP_DEFS_H
 #define _LINUX_CGROUP_DEFS_H
 
+#include 
 #include 
 #include 
 #include 
@@ -307,6 +308,36 @@ struct cgroup_stat {
struct prev_cputime prev_cputime;
 };
 
+/*
+ * Private data associated with a cgroup by an indpendent (non-controller) part
+ * of the kernel.  This is useful for things like drivers that may wish to 
track
+ * their own cgroup-specific data.
+ *
+ * If an individual cgroup is destroyed, the cgroups framework will
+ * automatically free all associated private data.  If cgroup private data is
+ * registered by a kernel module, then it is the module's responsibility to
+ * manually free its own private data upon unload.
+ */
+struct cgroup_priv {
+   /* cgroup this private data is associated with */
+   struct cgroup *cgroup;
+
+   /*
+* Lookup key that defines the in-kernel consumer of this private
+* data.
+*/
+   const void *key;
+
+   /*
+* Function to release private data.  This will be automatically called
+* if/when the cgroup is destroyed.
+*/
+   void (*free)(struct cgroup_priv *priv);
+
+   /* Hashlist node in cgroup's privdata hashtable */
+   struct hlist_node hnode;
+};
+
 struct cgroup {
/* self css with NULL ->ss, points back to this cgroup */
struct cgroup_subsys_state self;
@@ -427,6 +458,13 @@ struct cgroup {
/* used to store eBPF programs */
struct cgroup_bpf bpf;
 
+   /*
+* cgroup private data registered by other non-controller parts of the
+* kernel
+*/
+   DECLARE_HASHTABLE(privdata, 4);
+   struct mutex privdata_mutex;
+
/* 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..a3604b005417 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -833,4 +833,82 @@ static inline void put_cgroup_ns(struct cgroup_namespace 
*ns)
free_cgroup_ns(ns);
 }
 
+/**
+ * cgroup_priv_install - install new cgroup private data
+ * @key: Key uniquely identifying kernel owner of private data
+ *
+ * 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.
+ *
+ * Callers should ensure that no existing private data exists for the given key
+ * before adding new private data.  If two sets of private data are registered
+ * with the same key, it is undefined which will be returned by future calls
+ * to cgroup_priv_lookup.
+ *
+ * Kernel modules that register private data with this function should take
+ * care to free their private data when unloaded to prevent leaks.
+ */
+static inline void
+cgroup_priv_install(struct cgroup *cgrp,
+   struct cgroup_priv *priv)
+{
+   WARN_ON(!mutex_is_locked(&cgrp->privdata_mutex));
+   WARN_ON(!priv->key);
+   WARN_ON(!priv->free);
+   WARN_ON(priv->cgroup);
+
+   priv->cgroup = cgrp;
+   hash_add(cgrp->privdata, &priv->hnode,
+(unsigned long)priv->key);
+}
+
+/**
+ * cgroup_priv_lookup - looks up cgroup private data
+ * @key: Key uniquely identifying owner of private data to lookup
+ *
+ * Looks up the private data associated with a key.
+ *
+ * Returns:
+ * Previously registered cgroup private data associated with the given key, or
+ * NULL if no p