| Message ID | 20221111183532.3676646-18-kristen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Cgroup support for SGX EPC memory | expand |
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.
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.
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.
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
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);
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(-)