Re: [RFC PATCH 1/3] misc_cgroup: introduce misc.events and misc_events.local

[Vipin Sharma]

On Thu, Sep 9, 2021 at 7:37 AM Michal Koutný <mkoutny@xxxxxxxx> wrote:
>
> 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).

I agree with Michal. We can have the same interface for v1 otherwise
there will not be any form of feedback in v1 for failures.

>
> On Wed, Sep 08, 2021 at 01:24:34PM +0800, brookxu <brookxu.cn@xxxxxxxxx> wrote:
> > +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.)
>
I am inclined more towards "%s.max", it looks nice to see the resource
name before its corresponding events.