linux-kernel - Re: [RFC PATCH 1/3] misc_cgroup: introduce misc.events and misc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 9 Sep 2021 10:08:39 -0700
From:   Vipin Sharma <vipinsh@...gle.com>
To:     Michal Koutný <mkoutny@...e.com>
Cc:     brookxu <brookxu.cn@...il.com>, tj@...nel.org,
        lizefan.x@...edance.com, hannes@...xchg.org,
        linux-kernel@...r.kernel.org, cgroups@...r.kernel.org
Subject: Re: [RFC PATCH 1/3] misc_cgroup: introduce misc.events and misc_events.local

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