[17/26] cgroup/misc: Add notifier block list support for css events

Message ID 20221111183532.3676646-18-kristen@linux.intel.com
State New
Headers
Series Add Cgroup support for SGX EPC memory |

Commit Message

Kristen Carlson Accardi Nov. 11, 2022, 6:35 p.m. UTC
  Consumers of the misc cgroup controller might need to perform separate actions
in the event of a cgroup alloc, free or release call. In addition,
writes to the max value may also need separate action. Add the ability
to allow code to register for these notifications, and
call the notifier block chain list when appropriate.

This code will be utilized by the SGX driver in a future patch.

Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com>
---
 include/linux/misc_cgroup.h | 17 ++++++++++++
 kernel/cgroup/misc.c        | 52 +++++++++++++++++++++++++++++++++++--
 2 files changed, 67 insertions(+), 2 deletions(-)
  

Comments

Tejun Heo Nov. 14, 2022, 10:42 p.m. UTC | #1
Hello,

On Fri, Nov 11, 2022 at 10:35:22AM -0800, Kristen Carlson Accardi wrote:
> +/**
> + * register_misc_cg_notifier() - Register for css callback events
> + * @nb: notifier_block to register
> + *
> + * Context: Any context.
> + */
> +int register_misc_cg_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(&misc_cg_notify_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(register_misc_cg_notifier);
> +
> +/**
> + * unregister_misc_cg_notifier() - unregister for css callback events
> + * @nb: notifier_block to unregister
> + *
> + * Context: Any context.
> + */
> +int unregister_misc_cg_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_unregister(&misc_cg_notify_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(unregister_misc_cg_notifier);

So, I'm not necessarily against this but wonder whether it'd be more
straightforward to add sth like struct misc_res_ops which contains the
optional callbacks and then have an array of pointers to the structs which
are initialized / registered somehow. What do you think?

Thanks.
  
Kristen Carlson Accardi Nov. 14, 2022, 11:10 p.m. UTC | #2
On Mon, 2022-11-14 at 12:42 -1000, Tejun Heo wrote:
> Hello,
> 
> On Fri, Nov 11, 2022 at 10:35:22AM -0800, Kristen Carlson Accardi
> wrote:
> > +/**
> > + * register_misc_cg_notifier() - Register for css callback events
> > + * @nb: notifier_block to register
> > + *
> > + * Context: Any context.
> > + */
> > +int register_misc_cg_notifier(struct notifier_block *nb)
> > +{
> > +       return
> > blocking_notifier_chain_register(&misc_cg_notify_list, nb);
> > +}
> > +EXPORT_SYMBOL_GPL(register_misc_cg_notifier);
> > +
> > +/**
> > + * unregister_misc_cg_notifier() - unregister for css callback
> > events
> > + * @nb: notifier_block to unregister
> > + *
> > + * Context: Any context.
> > + */
> > +int unregister_misc_cg_notifier(struct notifier_block *nb)
> > +{
> > +       return
> > blocking_notifier_chain_unregister(&misc_cg_notify_list, nb);
> > +}
> > +EXPORT_SYMBOL_GPL(unregister_misc_cg_notifier);
> 
> So, I'm not necessarily against this but wonder whether it'd be more
> straightforward to add sth like struct misc_res_ops which contains
> the
> optional callbacks and then have an array of pointers to the structs
> which
> are initialized / registered somehow. What do you think?
> 
> Thanks.
> 

Makes no difference to me TBH - I believe they will be functionally
equivalent and from a downstream user perspective equally as easy to
use, so whatever you think is easiest for you to maintain.
  
Tejun Heo Nov. 14, 2022, 11:11 p.m. UTC | #3
Hello,

On Mon, Nov 14, 2022 at 03:10:05PM -0800, Kristen Carlson Accardi wrote:
> Makes no difference to me TBH - I believe they will be functionally
> equivalent and from a downstream user perspective equally as easy to
> use, so whatever you think is easiest for you to maintain.

Yeah, functionally they should be equivalent. Hmm... Let's go with the ops
table so that it's more explicit.

Thanks.
  
Kristen Carlson Accardi Nov. 14, 2022, 11:17 p.m. UTC | #4
On Mon, 2022-11-14 at 13:11 -1000, Tejun Heo wrote:
> Hello,
> 
> On Mon, Nov 14, 2022 at 03:10:05PM -0800, Kristen Carlson Accardi
> wrote:
> > Makes no difference to me TBH - I believe they will be functionally
> > equivalent and from a downstream user perspective equally as easy
> > to
> > use, so whatever you think is easiest for you to maintain.
> 
> Yeah, functionally they should be equivalent. Hmm... Let's go with
> the ops
> table so that it's more explicit.
> 
> Thanks.
> 

OK, in the next version I will make this change, consolidate everything
for the misc controller into 1 or 2 patches as you requested, and also
get rid of the helpers and just access the struct directly. Thanks for
your review.

Kristen
  

Patch

diff --git a/include/linux/misc_cgroup.h b/include/linux/misc_cgroup.h
index c238207d1615..8f1b7b6cb81d 100644
--- a/include/linux/misc_cgroup.h
+++ b/include/linux/misc_cgroup.h
@@ -21,6 +21,12 @@  enum misc_res_type {
 	MISC_CG_RES_TYPES
 };
 
+enum misc_cg_events {
+	MISC_CG_ALLOC,		/* a misc_cg was allocated */
+	MISC_CG_FREE,		/* a misc_cg was freed */
+	MISC_CG_RELEASED,	/* a misc_cg is being freed */
+	MISC_CG_CHANGE,		/* the misc_cg max value was changed */
+};
 struct misc_cg;
 
 #ifdef CONFIG_CGROUP_MISC
@@ -59,6 +65,8 @@  int misc_cg_try_charge(enum misc_res_type type, struct misc_cg *cg,
 		       unsigned long amount);
 void misc_cg_uncharge(enum misc_res_type type, struct misc_cg *cg,
 		      unsigned long amount);
+int register_misc_cg_notifier(struct notifier_block *nb);
+int unregister_misc_cg_notifier(struct notifier_block *nb);
 
 /**
  * css_misc() - Get misc cgroup from the css.
@@ -132,5 +140,14 @@  static inline void put_misc_cg(struct misc_cg *cg)
 {
 }
 
+static inline int register_misc_cg_notifier(struct notifier_block *nb)
+{
+	return 0;
+}
+
+static inline int unregister_misc_cg_notifier(struct notifier_block *nb)
+{
+	return 0;
+}
 #endif /* CONFIG_CGROUP_MISC */
 #endif /* _MISC_CGROUP_H_ */
diff --git a/kernel/cgroup/misc.c b/kernel/cgroup/misc.c
index fe3e8a0eb7ed..1e93e1d20347 100644
--- a/kernel/cgroup/misc.c
+++ b/kernel/cgroup/misc.c
@@ -11,6 +11,7 @@ 
 #include <linux/errno.h>
 #include <linux/atomic.h>
 #include <linux/slab.h>
+#include <linux/notifier.h>
 #include <linux/misc_cgroup.h>
 
 #define MAX_STR "max"
@@ -39,6 +40,11 @@  static struct misc_cg root_cg;
  */
 static unsigned long misc_res_capacity[MISC_CG_RES_TYPES];
 
+/*
+ * Notifier list for misc_cg cgroup callback events.
+ */
+static BLOCKING_NOTIFIER_HEAD(misc_cg_notify_list);
+
 /**
  * parent_misc() - Get the parent of the passed misc cgroup.
  * @cgroup: cgroup whose parent needs to be fetched.
@@ -278,10 +284,12 @@  static ssize_t misc_cg_max_write(struct kernfs_open_file *of, char *buf,
 
 	cg = css_misc(of_css(of));
 
-	if (READ_ONCE(misc_res_capacity[type]))
+	if (READ_ONCE(misc_res_capacity[type])) {
 		WRITE_ONCE(cg->res[type].max, max);
-	else
+		blocking_notifier_call_chain(&misc_cg_notify_list, MISC_CG_CHANGE, cg);
+	} else {
 		ret = -EINVAL;
+	}
 
 	return ret ? ret : nbytes;
 }
@@ -400,6 +408,7 @@  misc_cg_alloc(struct cgroup_subsys_state *parent_css)
 		WRITE_ONCE(cg->res[i].max, MAX_NUM);
 		atomic_long_set(&cg->res[i].usage, 0);
 	}
+	blocking_notifier_call_chain(&misc_cg_notify_list, MISC_CG_ALLOC, cg);
 
 	return &cg->css;
 }
@@ -412,13 +421,52 @@  misc_cg_alloc(struct cgroup_subsys_state *parent_css)
  */
 static void misc_cg_free(struct cgroup_subsys_state *css)
 {
+	blocking_notifier_call_chain(&misc_cg_notify_list, MISC_CG_FREE, css_misc(css));
 	kfree(css_misc(css));
 }
 
+/**
+ * misc_cg_released() - Release the misc cgroup
+ * @css: cgroup subsys object.
+ *
+ * Call the notifier chain to notify about the event.
+ *
+ * Context: Any context.
+ */
+static void misc_cg_released(struct cgroup_subsys_state *css)
+{
+	blocking_notifier_call_chain(&misc_cg_notify_list, MISC_CG_RELEASED, css_misc(css));
+}
+
 /* Cgroup controller callbacks */
 struct cgroup_subsys misc_cgrp_subsys = {
 	.css_alloc = misc_cg_alloc,
 	.css_free = misc_cg_free,
+	.css_released = misc_cg_released,
 	.legacy_cftypes = misc_cg_files,
 	.dfl_cftypes = misc_cg_files,
 };
+
+/**
+ * register_misc_cg_notifier() - Register for css callback events
+ * @nb: notifier_block to register
+ *
+ * Context: Any context.
+ */
+int register_misc_cg_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&misc_cg_notify_list, nb);
+}
+EXPORT_SYMBOL_GPL(register_misc_cg_notifier);
+
+/**
+ * unregister_misc_cg_notifier() - unregister for css callback events
+ * @nb: notifier_block to unregister
+ *
+ * Context: Any context.
+ */
+int unregister_misc_cg_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&misc_cg_notify_list, nb);
+}
+EXPORT_SYMBOL_GPL(unregister_misc_cg_notifier);