Messages in this thread | | | Date | Thu, 9 Sep 2021 16:37:02 +0200 | From | Michal Koutný <> | Subject | Re: [RFC PATCH 1/3] misc_cgroup: introduce misc.events and misc_events.local |
| |
Hello Chunguang.
The new version looks like a good step generally.
My main remark is that I wouldn't make a distinct v1 and v2 interface, it's a new controller so I think the v2 could be exposed in both cases (or in other words, don't create new v1-specific features).
On Wed, Sep 08, 2021 at 01:24:34PM +0800, brookxu <brookxu.cn@gmail.com> wrote: > Introduce misc.events and misc.events.local to make it easier for > us to understand the pressure of resources. The main idea comes > from mem_cgroup.
BTW what are the events you really are interesed in? (See also the proposal in my reply to 1/3.)
> @@ -171,6 +171,16 @@ int misc_cg_try_charge(enum misc_res_type type, struct misc_cg *cg, > return 0; > > err_charge: > + if (cgroup_subsys_on_dfl(misc_cgrp_subsys)) { > + atomic_long_inc(&i->events_local[type]); > + cgroup_file_notify(&i->events_local_file); > + > + for (k = i; k; k = parent_misc(k)) { > + atomic_long_inc(&k->events[type]); > + cgroup_file_notify(&k->events_file); > + } > + } > +
No reason to wrap this for the default hierarchy only.
> +static int misc_events_show(struct seq_file *sf, void *v) > +{ > + struct misc_cg *cg = css_misc(seq_css(sf)); > + unsigned long count, i; > + > + for (i = 0; i < MISC_CG_RES_TYPES; i++) { > + count = atomic_long_read(&cg->events[i]); > + if (READ_ONCE(misc_res_capacity[i]) || count) > + seq_printf(sf, "%s %lu\n", misc_res_name[i], count);
More future-proof key would be seq_printf(sf, "%s.max %lu\n", misc_res_name[i], count); or seq_printf(sf, "max.%s %lu\n", misc_res_name[i], count);
(Which one is a judgement call but I'd include the "name" of event type too.)
> +static int misc_events_local_show(struct seq_file *sf, void *v) > +{ > + struct misc_cg *cg = css_misc(seq_css(sf)); > + unsigned long count, i; > + > + for (i = 0; i < MISC_CG_RES_TYPES; i++) { > + count = atomic_long_read(&cg->events_local[i]); > + if (READ_ONCE(misc_res_capacity[i]) || count) > + seq_printf(sf, "%s %lu\n", misc_res_name[i], count);
Ditto.
Thanks, Michal
| |